WIP: generalized index constraints
in [PgSql]
|
Prev: corrupted double-linked list
Next: Timestamp to time_t
From: Joshua Tolley on 15 Sep 2009 10:08 On Tue, Sep 15, 2009 at 11:21:14PM +1000, Brendan Jurd wrote: > 2009/9/15 Jeff Davis <pgsql(a)j-davis.com>: > > Attached is the latest version. > > > > The new error message for a conflict is: > > ERROR: index constraint violation detected > DETAIL: tuple conflicts with existing data > > How about also including the name of the constraint (or index) that > was violated? I could imagine this error message being frustrating > for someone who had a table with multiple index constraints, as they > wouldn't know which one had raised the conflict. Perhaps the tuple that caused the violation as well, like UNIQUE index violations already do? Even if we know what constraint has been tripped, we might not know what value did it. josh(a)josh# create table a (a integer); josh(a)josh*# create unique index a_unique on a (a); josh(a)josh*# insert into a values (1), (2), (3); josh(a)josh*# insert into a values (8), (3), (4); ERROR: duplicate key value violates unique constraint "a_unique" DETAIL: Key (a)=(3) already exists. -- Joshua Tolley / eggyknap End Point Corporation http://www.endpoint.com
From: Jeff Davis on 15 Sep 2009 12:18 On Tue, 2009-09-15 at 22:52 +1000, Brendan Jurd wrote: > I'm just getting started reviewing this version now. I noticed that > your patch seems to have been generated by git. Are you hosting this > work on a public repo somewhere that I can pull from? I just requested a public repo. I will publish there as soon as its approved. > Also I think > the committers generally prefer context diffs (pipe it through > "filterdiff --format=context --strip=1") in submissions. Thanks, I will do that for my future patch submissions. > Regarding the documentation updates, I think you might want to add > some commentary to Chapter 11: Indexes -- perhaps add a new section > after 11.6 Unique Indexes to talk about general index constraints, > and/or update the wording of 11.6 to reflect your changes. Will do. > My eyes started to cross in the second sentence. "Detect conflicts > symmetrically"? I have actually *used* this feature successfully in > testing the patch, and I still don't know quite what to make of that > phrase. You might need to dumb it down. Will do. > It might also be good to be a bit more explicit about the way the > choice of operators works. It is the inverse of the logic used to > express an ordinary value constraint. E.g., when you use the equality > operator in an index constraint you are in effect saying that new rows > MUST NOT satisfy this operator for any existing rows. I'll include that, thanks. I appreciate the quick feedback; I'll make these changes tonight. Regards, Jeff Davis -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
From: Jeff Davis on 15 Sep 2009 12:26 On Tue, 2009-09-15 at 23:21 +1000, Brendan Jurd wrote: > How about also including the name of the constraint (or index) that > was violated? I could imagine this error message being frustrating > for someone who had a table with multiple index constraints, as they > wouldn't know which one had raised the conflict. Yes, that makes sense. As Joshua Tolley mentions, I'll also include the tuples that caused the conflict. > Also, the DETAIL part should be written as a full sentence with > leading capital and full stop [1], see Oh, I haven't seen that document before. Thanks. > postgres=# alter table circles add constraint circles_overlap (c <->) > using index circle_idx; > ERROR: no strategy found for operator 1520 in operator family 2595 > > The error message is pretty unfriendly, but I'm ambivalent about > whether it's worth doing anything about this particular case. I think I could make that error a little better by providing a detail message explaining what the operator should be able to do. > One of the comments I made in my original review [2] was that "\d" in > psql should show the constraint. I don't think you've addressed this > in the current version. I have psql on my list along with pg_dump, but unfortunately I haven't done either yet. I don't think it will take too much work, so I'll fix this as soon as I can. Regards, Jeff Davis -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
From: Jeff Davis on 15 Sep 2009 12:28 On Tue, 2009-09-15 at 08:08 -0600, Joshua Tolley wrote: > Perhaps the tuple that caused the violation as well, like UNIQUE index > violations already do? Even if we know what constraint has been tripped, we > might not know what value did it. Or, even better, include both tuples. With these new constraints the conflicting tuples aren't necessarily equal. Regards, Jeff Davis -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
From: Robert Haas on 15 Sep 2009 12:37
On Tue, Sep 15, 2009 at 12:18 PM, Jeff Davis <pgsql(a)j-davis.com> wrote: > On Tue, 2009-09-15 at 22:52 +1000, Brendan Jurd wrote: >> I'm just getting started reviewing this version now. I noticed that >> your patch seems to have been generated by git. Are you hosting this >> work on a public repo somewhere that I can pull from? > > I just requested a public repo. I will publish there as soon as its > approved. > >> Also I think >> the committers generally prefer context diffs (pipe it through >> "filterdiff --format=context --strip=1") in submissions. > > Thanks, I will do that for my future patch submissions. > >> Regarding the documentation updates, I think you might want to add >> some commentary to Chapter 11: Indexes -- perhaps add a new section >> after 11.6 Unique Indexes to talk about general index constraints, >> and/or update the wording of 11.6 to reflect your changes. > > Will do. > >> My eyes started to cross in the second sentence. "Detect conflicts >> symmetrically"? I have actually *used* this feature successfully in >> testing the patch, and I still don't know quite what to make of that >> phrase. You might need to dumb it down. > > Will do. > >> It might also be good to be a bit more explicit about the way the >> choice of operators works. It is the inverse of the logic used to >> express an ordinary value constraint. E.g., when you use the equality >> operator in an index constraint you are in effect saying that new rows >> MUST NOT satisfy this operator for any existing rows. > > I'll include that, thanks. > > I appreciate the quick feedback; I'll make these changes tonight. Instead of calling these generalized index constraints, I wonder if we oughtn't to be calling them something like "don't-overlap constraints" (that's a bad name, but something along those lines). They're not really general at all, except compared to uniqueness constraints (and they aren't called generalized unique-index constraints, just generalized index constraints). I didn't realize understand what this was all for until I read Brendan's review. ....Robert -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers |