Two constraints with the same name not always allowed
Hi list,
calling this a bug might be pedantic, but I noticed this inconsistency:
Case 1:
CREATE TABLE t(c integer);
ALTER TABLE t ADD CONSTRAINT foo UNIQUE(c);
ALTER TABLE t ADD CONSTRAINT foo CHECK(c > 1);
-> ERROR: constraint "foo" for relation "t" already exists
Case 2:
CREATE TABLE t(c integer);
ALTER TABLE t ADD CONSTRAINT foo CHECK(c > 1);
ALTER TABLE t ADD CONSTRAINT foo UNIQUE(c);
-> Creates two constraints, both called "foo".
Case 3:
CREATE TABLE t(c integer);
ALTER TABLE t ADD CONSTRAINT t_c_check UNIQUE(c); -- add the UNIQUE with the
same name that the following CHECK will automatically choose
ALTER TABLE t ADD CHECK(c > 1);
-> Creates the UNIQUE constraint as "t_c_check" and the CHECK as
"t_c_check1"
Case 4:
CREATE TABLE t(c integer);
ALTER TABLE t ADD CONSTRAINT t_c_key CHECK(c > 1);
ALTER TABLE t ADD UNIQUE(c);
-> Creates two constraints, both called "t_c_key".
In cases where two constraints with the same name are created, an "ALTER
TABLE t DROP CONSTRAINT ..." drops the UNIQUE first. Issuing the ALTER TABLE
a second time then drops the CHECK.
Regards,
André
"André" == André Hänsel <andre@webkr.de> writes:
André> Case 2:
André> CREATE TABLE t(c integer);
André> ALTER TABLE t ADD CONSTRAINT foo CHECK(c > 1);
André> ALTER TABLE t ADD CONSTRAINT foo UNIQUE(c);
André> -> Creates two constraints, both called "foo".
I'd call _that_ a bug, myself - having two constraints on a table with
the same name potentially messes up a lot of automated maintenance
operations.
André> In cases where two constraints with the same name are created,
André> an "ALTER TABLE t DROP CONSTRAINT ..." drops the UNIQUE first.
André> Issuing the ALTER TABLE a second time then drops the CHECK.
I think that's purely an artifact of what order an index scan on
pg_constraint_conrelid_index finds the constraints.
--
Andrew (irc:RhodiumToad)
Andrew Gierth <andrew@tao11.riddles.org.uk> writes:
"André" == André Hänsel <andre@webkr.de> writes:
André> Case 2:
André> CREATE TABLE t(c integer);
André> ALTER TABLE t ADD CONSTRAINT foo CHECK(c > 1);
André> ALTER TABLE t ADD CONSTRAINT foo UNIQUE(c);
André> -> Creates two constraints, both called "foo".
I'd call _that_ a bug, myself - having two constraints on a table with
the same name potentially messes up a lot of automated maintenance
operations.
Agreed. We must have missed a check for constraint-exists someplace.
This also points up the lack of a suitable unique index on pg_constraint.
It's sort of difficult to figure out what that should look like given that
pg_constraint contains two quasi-independent collections of constraints,
but maybe UNIQUE(conrelid,contypid,conname) would serve given the
reasonable assumption that exactly one of conrelid and contypid is zero.
Potentially we could drop pg_constraint_conrelid_index and
pg_constraint_contypid_index, replacing scans on those with
scans on this new unique index.
regards, tom lane
On 2018-Sep-02, Tom Lane wrote:
This also points up the lack of a suitable unique index on pg_constraint.
It's sort of difficult to figure out what that should look like given that
pg_constraint contains two quasi-independent collections of constraints,
but maybe UNIQUE(conrelid,contypid,conname) would serve given the
reasonable assumption that exactly one of conrelid and contypid is zero.
Hmm ... c.f. 7eca575d1c28. Maybe we should split them out? Are there
reasons to have them together at all?
--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Alvaro Herrera <alvherre@2ndquadrant.com> writes:
On 2018-Sep-02, Tom Lane wrote:
This also points up the lack of a suitable unique index on pg_constraint.
It's sort of difficult to figure out what that should look like given that
pg_constraint contains two quasi-independent collections of constraints,
but maybe UNIQUE(conrelid,contypid,conname) would serve given the
reasonable assumption that exactly one of conrelid and contypid is zero.
Hmm ... c.f. 7eca575d1c28. Maybe we should split them out? Are there
reasons to have them together at all?
Yeah, I've occasionally thought about replacing pg_constraint with
two separate catalogs; we could keep pg_constraint as a union view
to avoid breaking clients that look at it. But that'd be kind of
a large project, whereas adjusting the set of indexes for a catalog
is a pretty simple finger exercise in most cases. (It's also unclear
how smart the planner would be about optimizing queries on such a
view.)
Thinking about the planner angle some more, it seems like probably
the most reasonable proposal is to add UNIQUE(conrelid,contypid,conname)
replacing pg_constraint_conrelid_index, but keep
pg_constraint_contypid_index. While we could teach relevant parts
of backend/catalog how to use such a unique index to search for the
constraints of a domain, the planner would not know how to optimize
SQL queries with "WHERE contypid = xxx" unless we keep that index.
It would figure out that "WHERE conrelid = xxx" works with the
unique index, though.
regards, tom lane
I wrote:
Andrew Gierth <andrew@tao11.riddles.org.uk> writes:
"André" == André Hänsel <andre@webkr.de> writes:
André> Case 2:
André> CREATE TABLE t(c integer);
André> ALTER TABLE t ADD CONSTRAINT foo CHECK(c > 1);
André> ALTER TABLE t ADD CONSTRAINT foo UNIQUE(c);
André> -> Creates two constraints, both called "foo".
I'd call _that_ a bug, myself - having two constraints on a table with
the same name potentially messes up a lot of automated maintenance
operations.
Agreed. We must have missed a check for constraint-exists someplace.
Note that if you repeat that last command, what you get is
regression=# ALTER TABLE t ADD CONSTRAINT foo UNIQUE(c);
ERROR: relation "foo" already exists
I think the code supposes that checking for duplicate relation name
is sufficient; but of course it is not if we want a table's constraints
to have distinct names, since they may not all correspond to indexes.
I do not think we can back-patch a change here --- it might break
databases that are working satisfactorily today. But it seems like
we could tighten this up in HEAD and maybe v11.
regards, tom lane
I wrote:
I think the code supposes that checking for duplicate relation name
is sufficient; but of course it is not if we want a table's constraints
to have distinct names, since they may not all correspond to indexes.
I do not think we can back-patch a change here --- it might break
databases that are working satisfactorily today. But it seems like
we could tighten this up in HEAD and maybe v11.
Attached is a draft patchset for this.
0001 replaces the existing index with a unique one and makes necessary
backend code adjustments. Said adjustments could have been as simple
as s/ConstraintRelidIndexId/ConstraintRelidTypidNameIndexId/g -- I tried
that, and it passed regression tests -- but I couldn't resist the
temptation to fix a few places that could make better use of the
redesigned index.
0002 adds user-friendliness by installing a nicer error message for
the complained-of case and by improving ChooseIndexName to avoid
autogenerating index names that will conflict with existing constraints.
I didn't look for possible documentation changes yet, but I think the
code changes are OK.
regards, tom lane
Attachments:
0001-enforce-constraint-name-uniqueness.patchtext/x-diff; charset=us-ascii; name=0001-enforce-constraint-name-uniqueness.patchDownload+377-372
0002-enforce-constraint-name-uniqueness-more.patchtext/x-diff; charset=us-ascii; name=0002-enforce-constraint-name-uniqueness-more.patchDownload+148-30
On 02/09/2018 19:00, Tom Lane wrote:
This also points up the lack of a suitable unique index on pg_constraint.
It's sort of difficult to figure out what that should look like given that
pg_constraint contains two quasi-independent collections of constraints,
but maybe UNIQUE(conrelid,contypid,conname) would serve given the
reasonable assumption that exactly one of conrelid and contypid is zero.
Sketches for assertions set both conrelid and contypid to zero. I think
the unique constraint would have to include connamespace to support that
properly.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:
On 02/09/2018 19:00, Tom Lane wrote:
This also points up the lack of a suitable unique index on pg_constraint.
It's sort of difficult to figure out what that should look like given that
pg_constraint contains two quasi-independent collections of constraints,
but maybe UNIQUE(conrelid,contypid,conname) would serve given the
reasonable assumption that exactly one of conrelid and contypid is zero.
Sketches for assertions set both conrelid and contypid to zero. I think
the unique constraint would have to include connamespace to support that
properly.
Well, as I said in the commit message, I'm now of the opinion that
assertions should go in some new catalog. It was a mistake to put
domain and relation constraints into the same catalog, and I don't
think we ought to double down on that mistake by confusing the
question of "what's this catalog's primary key?" still more.
But yes, *if* we bull ahead and do it the wrong way, that would be
a necessary change. I don't like it much, because it would fuzz up
the question of whether the unique index actually guarantees only
one instance of a constraint name per relid or typid.
regards, tom lane