From 17b7c302b5fc92bd0241c452599019e18df074dc Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Tue, 4 Sep 2018 13:45:35 -0400 Subject: [PATCH] Fully enforce uniqueness of constraint names. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It's been true for a long time that we expect names of table and domain constraints to be unique among the constraints of that table or domain. However, the enforcement of that has been pretty haphazard, and it missed some corner cases such as creating a CHECK constraint and then an index constraint of the same name (as per recent report from André Hänsel). Also, due to the lack of an actual unique index enforcing this, duplicates could be created through race conditions. Moreover, the code that searches pg_constraint has been quite inconsistent about how to handle duplicate names if one did occur: some places checked and threw errors if there was more than one match, while others just processed the first match they came to. To fix, create a unique index on (conrelid, contypid, conname). Since either conrelid or contypid is zero, this will separately enforce uniqueness of constraint names among constraints of any one table and any one domain. (If we ever implement SQL assertions, and put them into this catalog, more thought might be needed. But it'd be at least as reasonable to put them into a new catalog; having overloaded this one catalog with two kinds of constraints was a mistake already IMO.) This index can replace the existing non-unique index on conrelid, though we need to keep the one on contypid for query performance reasons. Having done that, we can simplify the logic in various places that either coped with duplicates or neglected to, as well as potentially improve lookup performance when searching for a constraint by name. Also, as per our usual practice, install a preliminary check so that you get something more friendly than a unique-index violation report in the case complained of by André. And teach ChooseIndexName to avoid choosing autogenerated names that would draw such a failure. While it's not possible to make such a change in the back branches, it doesn't seem quite too late to put this into v11, so do so. Discussion: https://postgr.es/m/0c1001d4428f$0942b430$1bc81c90$@webkr.de --- doc/src/sgml/ref/alter_index.sgml | 3 + doc/src/sgml/ref/alter_table.sgml | 41 ++-- doc/src/sgml/ref/create_table.sgml | 24 ++ src/backend/catalog/heap.c | 208 +++++++++--------- src/backend/catalog/index.c | 20 ++ src/backend/catalog/pg_constraint.c | 254 +++++++++++----------- src/backend/commands/indexcmds.c | 27 ++- src/backend/commands/tablecmds.c | 136 ++++++------ src/backend/commands/typecmds.c | 85 ++++---- src/backend/parser/parse_utilcmd.c | 3 +- src/backend/utils/cache/relcache.c | 10 +- src/include/catalog/catversion.h | 2 +- src/include/catalog/indexing.h | 4 +- src/include/catalog/pg_constraint.h | 7 +- src/include/commands/defrem.h | 3 +- src/test/regress/expected/alter_table.out | 35 +++ src/test/regress/sql/alter_table.sql | 18 ++ 17 files changed, 504 insertions(+), 376 deletions(-) diff --git a/doc/src/sgml/ref/alter_index.sgml b/doc/src/sgml/ref/alter_index.sgml index 7290d9a5bd..d0a6212358 100644 --- a/doc/src/sgml/ref/alter_index.sgml +++ b/doc/src/sgml/ref/alter_index.sgml @@ -48,6 +48,9 @@ ALTER INDEX ALL IN TABLESPACE name The RENAME form changes the name of the index. + If the index is associated with a table constraint (either + UNIQUE, PRIMARY KEY, + or EXCLUDE), the constraint is renamed as well. There is no effect on the stored data. diff --git a/doc/src/sgml/ref/alter_table.sgml b/doc/src/sgml/ref/alter_table.sgml index 1cce00eaf9..ec6b4c3311 100644 --- a/doc/src/sgml/ref/alter_table.sgml +++ b/doc/src/sgml/ref/alter_table.sgml @@ -474,7 +474,8 @@ WITH ( MODULUS numeric_literal, REM DROP CONSTRAINT [ IF EXISTS ] - This form drops the specified constraint on a table. + This form drops the specified constraint on a table, along with + any index underlying the constraint. If IF EXISTS is specified and the constraint does not exist, no error is thrown. In this case a notice is issued instead. @@ -822,8 +823,10 @@ WITH ( MODULUS numeric_literal, REM The RENAME forms change the name of a table - (or an index, sequence, view, materialized view, or foreign table), the name - of an individual column in a table, or the name of a constraint of the table. + (or an index, sequence, view, materialized view, or foreign table), the + name of an individual column in a table, or the name of a constraint of + the table. When renaming a constraint that has an underlying index, + the index is renamed as well. There is no effect on the stored data. @@ -1270,10 +1273,12 @@ WITH ( MODULUS numeric_literal, REM If a table has any descendant tables, it is not permitted to add, rename, or change the type of a column in the parent table without doing - same to the descendants. This ensures that the descendants always have - columns matching the parent. Similarly, a constraint cannot be renamed - in the parent without also renaming it in all descendants, so that - constraints also match between the parent and its descendants. + the same to the descendants. This ensures that the descendants always + have columns matching the parent. Similarly, a CHECK + constraint cannot be renamed in the parent without also renaming it in + all descendants, so that CHECK constraints also match + between the parent and its descendants. (That restriction does not apply + to index-based constraints, however.) Also, because selecting from the parent also selects from its descendants, a constraint on the parent cannot be marked valid unless it is also marked valid for those descendants. In all of these cases, ALTER TABLE @@ -1481,35 +1486,35 @@ ALTER TABLE distributors DROP CONSTRAINT distributors_pkey, - Attach a partition to range partitioned table: + To attach a partition to a range-partitioned table: ALTER TABLE measurement ATTACH PARTITION measurement_y2016m07 FOR VALUES FROM ('2016-07-01') TO ('2016-08-01'); - Attach a partition to list partitioned table: + To attach a partition to a list-partitioned table: ALTER TABLE cities ATTACH PARTITION cities_ab FOR VALUES IN ('a', 'b'); - Attach a default partition to a partitioned table: - -ALTER TABLE cities - ATTACH PARTITION cities_partdef DEFAULT; - - - - Attach a partition to hash partitioned table: + To attach a partition to a hash-partitioned table: ALTER TABLE orders ATTACH PARTITION orders_p4 FOR VALUES WITH (MODULUS 4, REMAINDER 3); - Detach a partition from partitioned table: + To attach a default partition to a partitioned table: + +ALTER TABLE cities + ATTACH PARTITION cities_partdef DEFAULT; + + + + To detach a partition from a partitioned table: ALTER TABLE measurement DETACH PARTITION measurement_y2015m12; diff --git a/doc/src/sgml/ref/create_table.sgml b/doc/src/sgml/ref/create_table.sgml index d936de3f23..5a19f94ce9 100644 --- a/doc/src/sgml/ref/create_table.sgml +++ b/doc/src/sgml/ref/create_table.sgml @@ -2004,6 +2004,30 @@ CREATE TABLE cities_partdef + + Constraint Naming + + + The SQL standard says that table and domain constraints must have names + that are unique across the schema containing the table or domain. + PostgreSQL is laxer: it only requires + constraint names to be unique across the constraints attached to a + particular table or domain. However, this extra freedom does not exist + for index-based constraints (UNIQUE, + PRIMARY KEY, and EXCLUDE + constraints), because the associated index is named the same as the + constraint, and index names must be unique across all relations within + the same schema. + + + + Currently, PostgreSQL does not record names + for NOT NULL constraints at all, so they are not + subject to the uniqueness restriction. This might change in a future + release. + + + Inheritance diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c index 910d3db063..5206d5e516 100644 --- a/src/backend/catalog/heap.c +++ b/src/backend/catalog/heap.c @@ -2708,7 +2708,7 @@ MergeWithExistingConstraint(Relation rel, const char *ccname, Node *expr, bool found; Relation conDesc; SysScanDesc conscan; - ScanKeyData skey[2]; + ScanKeyData skey[3]; HeapTuple tup; /* Search for a pg_constraint entry with same name and relation */ @@ -2717,120 +2717,120 @@ MergeWithExistingConstraint(Relation rel, const char *ccname, Node *expr, found = false; ScanKeyInit(&skey[0], + Anum_pg_constraint_conrelid, + BTEqualStrategyNumber, F_OIDEQ, + ObjectIdGetDatum(RelationGetRelid(rel))); + ScanKeyInit(&skey[1], + Anum_pg_constraint_contypid, + BTEqualStrategyNumber, F_OIDEQ, + ObjectIdGetDatum(InvalidOid)); + ScanKeyInit(&skey[2], Anum_pg_constraint_conname, BTEqualStrategyNumber, F_NAMEEQ, CStringGetDatum(ccname)); - ScanKeyInit(&skey[1], - Anum_pg_constraint_connamespace, - BTEqualStrategyNumber, F_OIDEQ, - ObjectIdGetDatum(RelationGetNamespace(rel))); + conscan = systable_beginscan(conDesc, ConstraintRelidTypidNameIndexId, true, + NULL, 3, skey); - conscan = systable_beginscan(conDesc, ConstraintNameNspIndexId, true, - NULL, 2, skey); - - while (HeapTupleIsValid(tup = systable_getnext(conscan))) + /* There can be at most one matching row */ + if (HeapTupleIsValid(tup = systable_getnext(conscan))) { Form_pg_constraint con = (Form_pg_constraint) GETSTRUCT(tup); - if (con->conrelid == RelationGetRelid(rel)) + /* Found it. Conflicts if not identical check constraint */ + if (con->contype == CONSTRAINT_CHECK) { - /* Found it. Conflicts if not identical check constraint */ - if (con->contype == CONSTRAINT_CHECK) - { - Datum val; - bool isnull; + Datum val; + bool isnull; - val = fastgetattr(tup, - Anum_pg_constraint_conbin, - conDesc->rd_att, &isnull); - if (isnull) - elog(ERROR, "null conbin for rel %s", - RelationGetRelationName(rel)); - if (equal(expr, stringToNode(TextDatumGetCString(val)))) - found = true; - } - - /* - * If the existing constraint is purely inherited (no local - * definition) then interpret addition of a local constraint as a - * legal merge. This allows ALTER ADD CONSTRAINT on parent and - * child tables to be given in either order with same end state. - * However if the relation is a partition, all inherited - * constraints are always non-local, including those that were - * merged. - */ - if (is_local && !con->conislocal && !rel->rd_rel->relispartition) - allow_merge = true; - - if (!found || !allow_merge) - ereport(ERROR, - (errcode(ERRCODE_DUPLICATE_OBJECT), - errmsg("constraint \"%s\" for relation \"%s\" already exists", - ccname, RelationGetRelationName(rel)))); - - /* If the child constraint is "no inherit" then cannot merge */ - if (con->connoinherit) - ereport(ERROR, - (errcode(ERRCODE_INVALID_OBJECT_DEFINITION), - errmsg("constraint \"%s\" conflicts with non-inherited constraint on relation \"%s\"", - ccname, RelationGetRelationName(rel)))); - - /* - * Must not change an existing inherited constraint to "no - * inherit" status. That's because inherited constraints should - * be able to propagate to lower-level children. - */ - if (con->coninhcount > 0 && is_no_inherit) - ereport(ERROR, - (errcode(ERRCODE_INVALID_OBJECT_DEFINITION), - errmsg("constraint \"%s\" conflicts with inherited constraint on relation \"%s\"", - ccname, RelationGetRelationName(rel)))); - - /* - * If the child constraint is "not valid" then cannot merge with a - * valid parent constraint - */ - if (is_initially_valid && !con->convalidated) - ereport(ERROR, - (errcode(ERRCODE_INVALID_OBJECT_DEFINITION), - errmsg("constraint \"%s\" conflicts with NOT VALID constraint on relation \"%s\"", - ccname, RelationGetRelationName(rel)))); - - /* OK to update the tuple */ - ereport(NOTICE, - (errmsg("merging constraint \"%s\" with inherited definition", - ccname))); - - tup = heap_copytuple(tup); - con = (Form_pg_constraint) GETSTRUCT(tup); - - /* - * In case of partitions, an inherited constraint must be - * inherited only once since it cannot have multiple parents and - * it is never considered local. - */ - if (rel->rd_rel->relispartition) - { - con->coninhcount = 1; - con->conislocal = false; - } - else - { - if (is_local) - con->conislocal = true; - else - con->coninhcount++; - } - - if (is_no_inherit) - { - Assert(is_local); - con->connoinherit = true; - } - CatalogTupleUpdate(conDesc, &tup->t_self, tup); - break; + val = fastgetattr(tup, + Anum_pg_constraint_conbin, + conDesc->rd_att, &isnull); + if (isnull) + elog(ERROR, "null conbin for rel %s", + RelationGetRelationName(rel)); + if (equal(expr, stringToNode(TextDatumGetCString(val)))) + found = true; } + + /* + * If the existing constraint is purely inherited (no local + * definition) then interpret addition of a local constraint as a + * legal merge. This allows ALTER ADD CONSTRAINT on parent and child + * tables to be given in either order with same end state. However if + * the relation is a partition, all inherited constraints are always + * non-local, including those that were merged. + */ + if (is_local && !con->conislocal && !rel->rd_rel->relispartition) + allow_merge = true; + + if (!found || !allow_merge) + ereport(ERROR, + (errcode(ERRCODE_DUPLICATE_OBJECT), + errmsg("constraint \"%s\" for relation \"%s\" already exists", + ccname, RelationGetRelationName(rel)))); + + /* If the child constraint is "no inherit" then cannot merge */ + if (con->connoinherit) + ereport(ERROR, + (errcode(ERRCODE_INVALID_OBJECT_DEFINITION), + errmsg("constraint \"%s\" conflicts with non-inherited constraint on relation \"%s\"", + ccname, RelationGetRelationName(rel)))); + + /* + * Must not change an existing inherited constraint to "no inherit" + * status. That's because inherited constraints should be able to + * propagate to lower-level children. + */ + if (con->coninhcount > 0 && is_no_inherit) + ereport(ERROR, + (errcode(ERRCODE_INVALID_OBJECT_DEFINITION), + errmsg("constraint \"%s\" conflicts with inherited constraint on relation \"%s\"", + ccname, RelationGetRelationName(rel)))); + + /* + * If the child constraint is "not valid" then cannot merge with a + * valid parent constraint. + */ + if (is_initially_valid && !con->convalidated) + ereport(ERROR, + (errcode(ERRCODE_INVALID_OBJECT_DEFINITION), + errmsg("constraint \"%s\" conflicts with NOT VALID constraint on relation \"%s\"", + ccname, RelationGetRelationName(rel)))); + + /* OK to update the tuple */ + ereport(NOTICE, + (errmsg("merging constraint \"%s\" with inherited definition", + ccname))); + + tup = heap_copytuple(tup); + con = (Form_pg_constraint) GETSTRUCT(tup); + + /* + * In case of partitions, an inherited constraint must be inherited + * only once since it cannot have multiple parents and it is never + * considered local. + */ + if (rel->rd_rel->relispartition) + { + con->coninhcount = 1; + con->conislocal = false; + } + else + { + if (is_local) + con->conislocal = true; + else + con->coninhcount++; + } + + if (is_no_inherit) + { + Assert(is_local); + con->connoinherit = true; + } + + CatalogTupleUpdate(conDesc, &tup->t_self, tup); } systable_endscan(conscan); diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c index b256054908..4b29120f19 100644 --- a/src/backend/catalog/index.c +++ b/src/backend/catalog/index.c @@ -842,6 +842,12 @@ index_create(Relation heapRelation, if (shared_relation && tableSpaceId != GLOBALTABLESPACE_OID) elog(ERROR, "shared relations must be placed in pg_global tablespace"); + /* + * Check for duplicate name (both as to the index, and as to the + * associated constraint if any). Such cases would fail on the relevant + * catalogs' unique indexes anyway, but we prefer to give a friendlier + * error message. + */ if (get_relname_relid(indexRelationName, namespaceId)) { if ((flags & INDEX_CREATE_IF_NOT_EXISTS) != 0) @@ -860,6 +866,20 @@ index_create(Relation heapRelation, indexRelationName))); } + if ((flags & INDEX_CREATE_ADD_CONSTRAINT) != 0 && + ConstraintNameIsUsed(CONSTRAINT_RELATION, heapRelationId, + indexRelationName)) + { + /* + * INDEX_CREATE_IF_NOT_EXISTS does not apply here, since the + * conflicting constraint is not an index. + */ + ereport(ERROR, + (errcode(ERRCODE_DUPLICATE_OBJECT), + errmsg("constraint \"%s\" for relation \"%s\" already exists", + indexRelationName, RelationGetRelationName(heapRelation)))); + } + /* * construct tuple descriptor for index tuples */ diff --git a/src/backend/catalog/pg_constraint.c b/src/backend/catalog/pg_constraint.c index ea84441360..6781b00c6e 100644 --- a/src/backend/catalog/pg_constraint.c +++ b/src/backend/catalog/pg_constraint.c @@ -422,7 +422,7 @@ CloneForeignKeyConstraints(Oid parentId, Oid relationId, List **cloned) ScanKeyInit(&key, Anum_pg_constraint_conrelid, BTEqualStrategyNumber, F_OIDEQ, ObjectIdGetDatum(parentId)); - scan = systable_beginscan(pg_constraint, ConstraintRelidIndexId, true, + scan = systable_beginscan(pg_constraint, ConstraintRelidTypidNameIndexId, true, NULL, 1, &key); while ((tuple = systable_getnext(scan)) != NULL) @@ -632,18 +632,59 @@ CloneForeignKeyConstraints(Oid parentId, Oid relationId, List **cloned) */ bool ConstraintNameIsUsed(ConstraintCategory conCat, Oid objId, - Oid objNamespace, const char *conname) + const char *conname) +{ + bool found; + Relation conDesc; + SysScanDesc conscan; + ScanKeyData skey[3]; + + conDesc = heap_open(ConstraintRelationId, AccessShareLock); + + ScanKeyInit(&skey[0], + Anum_pg_constraint_conrelid, + BTEqualStrategyNumber, F_OIDEQ, + ObjectIdGetDatum((conCat == CONSTRAINT_RELATION) + ? objId : InvalidOid)); + ScanKeyInit(&skey[1], + Anum_pg_constraint_contypid, + BTEqualStrategyNumber, F_OIDEQ, + ObjectIdGetDatum((conCat == CONSTRAINT_DOMAIN) + ? objId : InvalidOid)); + ScanKeyInit(&skey[2], + Anum_pg_constraint_conname, + BTEqualStrategyNumber, F_NAMEEQ, + CStringGetDatum(conname)); + + conscan = systable_beginscan(conDesc, ConstraintRelidTypidNameIndexId, + true, NULL, 3, skey); + + /* There can be at most one matching row */ + found = (HeapTupleIsValid(systable_getnext(conscan))); + + systable_endscan(conscan); + heap_close(conDesc, AccessShareLock); + + return found; +} + +/* + * Does any constraint of the given name exist in the given namespace? + * + * This is used for code that wants to match ChooseConstraintName's rule + * that we should avoid autogenerating duplicate constraint names within a + * namespace. + */ +bool +ConstraintNameExists(const char *conname, Oid namespaceid) { bool found; Relation conDesc; SysScanDesc conscan; ScanKeyData skey[2]; - HeapTuple tup; conDesc = heap_open(ConstraintRelationId, AccessShareLock); - found = false; - ScanKeyInit(&skey[0], Anum_pg_constraint_conname, BTEqualStrategyNumber, F_NAMEEQ, @@ -652,26 +693,12 @@ ConstraintNameIsUsed(ConstraintCategory conCat, Oid objId, ScanKeyInit(&skey[1], Anum_pg_constraint_connamespace, BTEqualStrategyNumber, F_OIDEQ, - ObjectIdGetDatum(objNamespace)); + ObjectIdGetDatum(namespaceid)); conscan = systable_beginscan(conDesc, ConstraintNameNspIndexId, true, NULL, 2, skey); - while (HeapTupleIsValid(tup = systable_getnext(conscan))) - { - Form_pg_constraint con = (Form_pg_constraint) GETSTRUCT(tup); - - if (conCat == CONSTRAINT_RELATION && con->conrelid == objId) - { - found = true; - break; - } - else if (conCat == CONSTRAINT_DOMAIN && con->contypid == objId) - { - found = true; - break; - } - } + found = (HeapTupleIsValid(systable_getnext(conscan))); systable_endscan(conscan); heap_close(conDesc, AccessShareLock); @@ -878,13 +905,11 @@ RenameConstraintById(Oid conId, const char *newname) con = (Form_pg_constraint) GETSTRUCT(tuple); /* - * We need to check whether the name is already in use --- note that there - * currently is not a unique index that would catch this. + * For user-friendliness, check whether the name is already in use. */ if (OidIsValid(con->conrelid) && ConstraintNameIsUsed(CONSTRAINT_RELATION, con->conrelid, - con->connamespace, newname)) ereport(ERROR, (errcode(ERRCODE_DUPLICATE_OBJECT), @@ -893,7 +918,6 @@ RenameConstraintById(Oid conId, const char *newname) if (OidIsValid(con->contypid) && ConstraintNameIsUsed(CONSTRAINT_DOMAIN, con->contypid, - con->connamespace, newname)) ereport(ERROR, (errcode(ERRCODE_DUPLICATE_OBJECT), @@ -923,32 +947,23 @@ AlterConstraintNamespaces(Oid ownerId, Oid oldNspId, Oid newNspId, bool isType, ObjectAddresses *objsMoved) { Relation conRel; - ScanKeyData key[1]; + ScanKeyData key[2]; SysScanDesc scan; HeapTuple tup; conRel = heap_open(ConstraintRelationId, RowExclusiveLock); - if (isType) - { - ScanKeyInit(&key[0], - Anum_pg_constraint_contypid, - BTEqualStrategyNumber, F_OIDEQ, - ObjectIdGetDatum(ownerId)); + ScanKeyInit(&key[0], + Anum_pg_constraint_conrelid, + BTEqualStrategyNumber, F_OIDEQ, + ObjectIdGetDatum(isType ? InvalidOid : ownerId)); + ScanKeyInit(&key[1], + Anum_pg_constraint_contypid, + BTEqualStrategyNumber, F_OIDEQ, + ObjectIdGetDatum(isType ? ownerId : InvalidOid)); - scan = systable_beginscan(conRel, ConstraintTypidIndexId, true, - NULL, 1, key); - } - else - { - ScanKeyInit(&key[0], - Anum_pg_constraint_conrelid, - BTEqualStrategyNumber, F_OIDEQ, - ObjectIdGetDatum(ownerId)); - - scan = systable_beginscan(conRel, ConstraintRelidIndexId, true, - NULL, 1, key); - } + scan = systable_beginscan(conRel, ConstraintRelidTypidNameIndexId, true, + NULL, 2, key); while (HeapTupleIsValid((tup = systable_getnext(scan)))) { @@ -1038,38 +1053,30 @@ get_relation_constraint_oid(Oid relid, const char *conname, bool missing_ok) Relation pg_constraint; HeapTuple tuple; SysScanDesc scan; - ScanKeyData skey[1]; + ScanKeyData skey[3]; Oid conOid = InvalidOid; - /* - * Fetch the constraint tuple from pg_constraint. There may be more than - * one match, because constraints are not required to have unique names; - * if so, error out. - */ pg_constraint = heap_open(ConstraintRelationId, AccessShareLock); ScanKeyInit(&skey[0], Anum_pg_constraint_conrelid, BTEqualStrategyNumber, F_OIDEQ, ObjectIdGetDatum(relid)); + ScanKeyInit(&skey[1], + Anum_pg_constraint_contypid, + BTEqualStrategyNumber, F_OIDEQ, + ObjectIdGetDatum(InvalidOid)); + ScanKeyInit(&skey[2], + Anum_pg_constraint_conname, + BTEqualStrategyNumber, F_NAMEEQ, + CStringGetDatum(conname)); - scan = systable_beginscan(pg_constraint, ConstraintRelidIndexId, true, - NULL, 1, skey); + scan = systable_beginscan(pg_constraint, ConstraintRelidTypidNameIndexId, true, + NULL, 3, skey); - while (HeapTupleIsValid(tuple = systable_getnext(scan))) - { - Form_pg_constraint con = (Form_pg_constraint) GETSTRUCT(tuple); - - if (strcmp(NameStr(con->conname), conname) == 0) - { - if (OidIsValid(conOid)) - ereport(ERROR, - (errcode(ERRCODE_DUPLICATE_OBJECT), - errmsg("table \"%s\" has multiple constraints named \"%s\"", - get_rel_name(relid), conname))); - conOid = HeapTupleGetOid(tuple); - } - } + /* There can be at most one matching row */ + if (HeapTupleIsValid(tuple = systable_getnext(scan))) + conOid = HeapTupleGetOid(tuple); systable_endscan(scan); @@ -1105,67 +1112,62 @@ get_relation_constraint_attnos(Oid relid, const char *conname, Relation pg_constraint; HeapTuple tuple; SysScanDesc scan; - ScanKeyData skey[1]; + ScanKeyData skey[3]; /* Set *constraintOid, to avoid complaints about uninitialized vars */ *constraintOid = InvalidOid; - /* - * Fetch the constraint tuple from pg_constraint. There may be more than - * one match, because constraints are not required to have unique names; - * if so, error out. - */ pg_constraint = heap_open(ConstraintRelationId, AccessShareLock); ScanKeyInit(&skey[0], Anum_pg_constraint_conrelid, BTEqualStrategyNumber, F_OIDEQ, ObjectIdGetDatum(relid)); + ScanKeyInit(&skey[1], + Anum_pg_constraint_contypid, + BTEqualStrategyNumber, F_OIDEQ, + ObjectIdGetDatum(InvalidOid)); + ScanKeyInit(&skey[2], + Anum_pg_constraint_conname, + BTEqualStrategyNumber, F_NAMEEQ, + CStringGetDatum(conname)); - scan = systable_beginscan(pg_constraint, ConstraintRelidIndexId, true, - NULL, 1, skey); + scan = systable_beginscan(pg_constraint, ConstraintRelidTypidNameIndexId, true, + NULL, 3, skey); - while (HeapTupleIsValid(tuple = systable_getnext(scan))) + /* There can be at most one matching row */ + if (HeapTupleIsValid(tuple = systable_getnext(scan))) { - Form_pg_constraint con = (Form_pg_constraint) GETSTRUCT(tuple); Datum adatum; bool isNull; - ArrayType *arr; - int16 *attnums; - int numcols; - int i; - - /* Check the constraint name */ - if (strcmp(NameStr(con->conname), conname) != 0) - continue; - if (OidIsValid(*constraintOid)) - ereport(ERROR, - (errcode(ERRCODE_DUPLICATE_OBJECT), - errmsg("table \"%s\" has multiple constraints named \"%s\"", - get_rel_name(relid), conname))); *constraintOid = HeapTupleGetOid(tuple); /* Extract the conkey array, ie, attnums of constrained columns */ adatum = heap_getattr(tuple, Anum_pg_constraint_conkey, RelationGetDescr(pg_constraint), &isNull); - if (isNull) - continue; /* no constrained columns */ - - arr = DatumGetArrayTypeP(adatum); /* ensure not toasted */ - numcols = ARR_DIMS(arr)[0]; - if (ARR_NDIM(arr) != 1 || - numcols < 0 || - ARR_HASNULL(arr) || - ARR_ELEMTYPE(arr) != INT2OID) - elog(ERROR, "conkey is not a 1-D smallint array"); - attnums = (int16 *) ARR_DATA_PTR(arr); - - /* Construct the result value */ - for (i = 0; i < numcols; i++) + if (!isNull) { - conattnos = bms_add_member(conattnos, - attnums[i] - FirstLowInvalidHeapAttributeNumber); + ArrayType *arr; + int numcols; + int16 *attnums; + int i; + + arr = DatumGetArrayTypeP(adatum); /* ensure not toasted */ + numcols = ARR_DIMS(arr)[0]; + if (ARR_NDIM(arr) != 1 || + numcols < 0 || + ARR_HASNULL(arr) || + ARR_ELEMTYPE(arr) != INT2OID) + elog(ERROR, "conkey is not a 1-D smallint array"); + attnums = (int16 *) ARR_DATA_PTR(arr); + + /* Construct the result value */ + for (i = 0; i < numcols; i++) + { + conattnos = bms_add_member(conattnos, + attnums[i] - FirstLowInvalidHeapAttributeNumber); + } } } @@ -1203,7 +1205,7 @@ get_relation_idx_constraint_oid(Oid relationId, Oid indexId) BTEqualStrategyNumber, F_OIDEQ, ObjectIdGetDatum(relationId)); - scan = systable_beginscan(pg_constraint, ConstraintRelidIndexId, + scan = systable_beginscan(pg_constraint, ConstraintRelidTypidNameIndexId, true, NULL, 1, &key); while ((tuple = systable_getnext(scan)) != NULL) { @@ -1233,38 +1235,30 @@ get_domain_constraint_oid(Oid typid, const char *conname, bool missing_ok) Relation pg_constraint; HeapTuple tuple; SysScanDesc scan; - ScanKeyData skey[1]; + ScanKeyData skey[3]; Oid conOid = InvalidOid; - /* - * Fetch the constraint tuple from pg_constraint. There may be more than - * one match, because constraints are not required to have unique names; - * if so, error out. - */ pg_constraint = heap_open(ConstraintRelationId, AccessShareLock); ScanKeyInit(&skey[0], + Anum_pg_constraint_conrelid, + BTEqualStrategyNumber, F_OIDEQ, + ObjectIdGetDatum(InvalidOid)); + ScanKeyInit(&skey[1], Anum_pg_constraint_contypid, BTEqualStrategyNumber, F_OIDEQ, ObjectIdGetDatum(typid)); + ScanKeyInit(&skey[2], + Anum_pg_constraint_conname, + BTEqualStrategyNumber, F_NAMEEQ, + CStringGetDatum(conname)); - scan = systable_beginscan(pg_constraint, ConstraintTypidIndexId, true, - NULL, 1, skey); + scan = systable_beginscan(pg_constraint, ConstraintRelidTypidNameIndexId, true, + NULL, 3, skey); - while (HeapTupleIsValid(tuple = systable_getnext(scan))) - { - Form_pg_constraint con = (Form_pg_constraint) GETSTRUCT(tuple); - - if (strcmp(NameStr(con->conname), conname) == 0) - { - if (OidIsValid(conOid)) - ereport(ERROR, - (errcode(ERRCODE_DUPLICATE_OBJECT), - errmsg("domain %s has multiple constraints named \"%s\"", - format_type_be(typid), conname))); - conOid = HeapTupleGetOid(tuple); - } - } + /* There can be at most one matching row */ + if (HeapTupleIsValid(tuple = systable_getnext(scan))) + conOid = HeapTupleGetOid(tuple); systable_endscan(scan); @@ -1314,7 +1308,7 @@ get_primary_key_attnos(Oid relid, bool deferrableOk, Oid *constraintOid) BTEqualStrategyNumber, F_OIDEQ, ObjectIdGetDatum(relid)); - scan = systable_beginscan(pg_constraint, ConstraintRelidIndexId, true, + scan = systable_beginscan(pg_constraint, ConstraintRelidTypidNameIndexId, true, NULL, 1, skey); while (HeapTupleIsValid(tuple = systable_getnext(scan))) diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c index d54c78c352..ab3d9a0a48 100644 --- a/src/backend/commands/indexcmds.c +++ b/src/backend/commands/indexcmds.c @@ -1995,6 +1995,12 @@ makeObjectName(const char *name1, const char *name2, const char *label) * except that the label can't be NULL; digits will be appended to the label * if needed to create a name that is unique within the specified namespace. * + * If isconstraint is true, we also avoid choosing a name matching any + * existing constraint in the same namespace. (This is stricter than what + * Postgres itself requires, but the SQL standard says that constraint names + * should be unique within schemas, so we follow that for autogenerated + * constraint names.) + * * Note: it is theoretically possible to get a collision anyway, if someone * else chooses the same name concurrently. This is fairly unlikely to be * a problem in practice, especially if one is holding an exclusive lock on @@ -2006,7 +2012,8 @@ makeObjectName(const char *name1, const char *name2, const char *label) */ char * ChooseRelationName(const char *name1, const char *name2, - const char *label, Oid namespaceid) + const char *label, Oid namespaceid, + bool isconstraint) { int pass = 0; char *relname = NULL; @@ -2020,7 +2027,11 @@ ChooseRelationName(const char *name1, const char *name2, relname = makeObjectName(name1, name2, modlabel); if (!OidIsValid(get_relname_relid(relname, namespaceid))) - break; + { + if (!isconstraint || + !ConstraintNameExists(relname, namespaceid)) + break; + } /* found a conflict, so try a new name component */ pfree(relname); @@ -2048,28 +2059,32 @@ ChooseIndexName(const char *tabname, Oid namespaceId, indexname = ChooseRelationName(tabname, NULL, "pkey", - namespaceId); + namespaceId, + true); } else if (exclusionOpNames != NIL) { indexname = ChooseRelationName(tabname, ChooseIndexNameAddition(colnames), "excl", - namespaceId); + namespaceId, + true); } else if (isconstraint) { indexname = ChooseRelationName(tabname, ChooseIndexNameAddition(colnames), "key", - namespaceId); + namespaceId, + true); } else { indexname = ChooseRelationName(tabname, ChooseIndexNameAddition(colnames), "idx", - namespaceId); + namespaceId, + false); } return indexname; diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index f46af41b56..f9e83c2456 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -7145,7 +7145,6 @@ ATExecAddConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel, { if (ConstraintNameIsUsed(CONSTRAINT_RELATION, RelationGetRelid(rel), - RelationGetNamespace(rel), newConstraint->conname)) ereport(ERROR, (errcode(ERRCODE_DUPLICATE_OBJECT), @@ -7801,10 +7800,9 @@ ATExecAlterConstraint(Relation rel, AlterTableCmd *cmd, Constraint *cmdcon; Relation conrel; SysScanDesc scan; - ScanKeyData key; + ScanKeyData skey[3]; HeapTuple contuple; - Form_pg_constraint currcon = NULL; - bool found = false; + Form_pg_constraint currcon; ObjectAddress address; cmdcon = castNode(Constraint, cmd->def); @@ -7814,29 +7812,29 @@ ATExecAlterConstraint(Relation rel, AlterTableCmd *cmd, /* * Find and check the target constraint */ - ScanKeyInit(&key, + ScanKeyInit(&skey[0], Anum_pg_constraint_conrelid, BTEqualStrategyNumber, F_OIDEQ, ObjectIdGetDatum(RelationGetRelid(rel))); - scan = systable_beginscan(conrel, ConstraintRelidIndexId, - true, NULL, 1, &key); + ScanKeyInit(&skey[1], + Anum_pg_constraint_contypid, + BTEqualStrategyNumber, F_OIDEQ, + ObjectIdGetDatum(InvalidOid)); + ScanKeyInit(&skey[2], + Anum_pg_constraint_conname, + BTEqualStrategyNumber, F_NAMEEQ, + CStringGetDatum(cmdcon->conname)); + scan = systable_beginscan(conrel, ConstraintRelidTypidNameIndexId, + true, NULL, 3, skey); - while (HeapTupleIsValid(contuple = systable_getnext(scan))) - { - currcon = (Form_pg_constraint) GETSTRUCT(contuple); - if (strcmp(NameStr(currcon->conname), cmdcon->conname) == 0) - { - found = true; - break; - } - } - - if (!found) + /* There can be at most one matching row */ + if (!HeapTupleIsValid(contuple = systable_getnext(scan))) ereport(ERROR, (errcode(ERRCODE_UNDEFINED_OBJECT), errmsg("constraint \"%s\" of relation \"%s\" does not exist", cmdcon->conname, RelationGetRelationName(rel)))); + currcon = (Form_pg_constraint) GETSTRUCT(contuple); if (currcon->contype != CONSTRAINT_FOREIGN) ereport(ERROR, (errcode(ERRCODE_WRONG_OBJECT_TYPE), @@ -7969,10 +7967,9 @@ ATExecValidateConstraint(Relation rel, char *constrName, bool recurse, { Relation conrel; SysScanDesc scan; - ScanKeyData key; + ScanKeyData skey[3]; HeapTuple tuple; - Form_pg_constraint con = NULL; - bool found = false; + Form_pg_constraint con; ObjectAddress address; conrel = heap_open(ConstraintRelationId, RowExclusiveLock); @@ -7980,29 +7977,29 @@ ATExecValidateConstraint(Relation rel, char *constrName, bool recurse, /* * Find and check the target constraint */ - ScanKeyInit(&key, + ScanKeyInit(&skey[0], Anum_pg_constraint_conrelid, BTEqualStrategyNumber, F_OIDEQ, ObjectIdGetDatum(RelationGetRelid(rel))); - scan = systable_beginscan(conrel, ConstraintRelidIndexId, - true, NULL, 1, &key); + ScanKeyInit(&skey[1], + Anum_pg_constraint_contypid, + BTEqualStrategyNumber, F_OIDEQ, + ObjectIdGetDatum(InvalidOid)); + ScanKeyInit(&skey[2], + Anum_pg_constraint_conname, + BTEqualStrategyNumber, F_NAMEEQ, + CStringGetDatum(constrName)); + scan = systable_beginscan(conrel, ConstraintRelidTypidNameIndexId, + true, NULL, 3, skey); - while (HeapTupleIsValid(tuple = systable_getnext(scan))) - { - con = (Form_pg_constraint) GETSTRUCT(tuple); - if (strcmp(NameStr(con->conname), constrName) == 0) - { - found = true; - break; - } - } - - if (!found) + /* There can be at most one matching row */ + if (!HeapTupleIsValid(tuple = systable_getnext(scan))) ereport(ERROR, (errcode(ERRCODE_UNDEFINED_OBJECT), errmsg("constraint \"%s\" of relation \"%s\" does not exist", constrName, RelationGetRelationName(rel)))); + con = (Form_pg_constraint) GETSTRUCT(tuple); if (con->contype != CONSTRAINT_FOREIGN && con->contype != CONSTRAINT_CHECK) ereport(ERROR, @@ -8865,7 +8862,7 @@ ATExecDropConstraint(Relation rel, const char *constrName, Relation conrel; Form_pg_constraint con; SysScanDesc scan; - ScanKeyData key; + ScanKeyData skey[3]; HeapTuple tuple; bool found = false; bool is_no_inherit_constraint = false; @@ -8879,22 +8876,28 @@ ATExecDropConstraint(Relation rel, const char *constrName, /* * Find and drop the target constraint */ - ScanKeyInit(&key, + ScanKeyInit(&skey[0], Anum_pg_constraint_conrelid, BTEqualStrategyNumber, F_OIDEQ, ObjectIdGetDatum(RelationGetRelid(rel))); - scan = systable_beginscan(conrel, ConstraintRelidIndexId, - true, NULL, 1, &key); + ScanKeyInit(&skey[1], + Anum_pg_constraint_contypid, + BTEqualStrategyNumber, F_OIDEQ, + ObjectIdGetDatum(InvalidOid)); + ScanKeyInit(&skey[2], + Anum_pg_constraint_conname, + BTEqualStrategyNumber, F_NAMEEQ, + CStringGetDatum(constrName)); + scan = systable_beginscan(conrel, ConstraintRelidTypidNameIndexId, + true, NULL, 3, skey); - while (HeapTupleIsValid(tuple = systable_getnext(scan))) + /* There can be at most one matching row */ + if (HeapTupleIsValid(tuple = systable_getnext(scan))) { ObjectAddress conobj; con = (Form_pg_constraint) GETSTRUCT(tuple); - if (strcmp(NameStr(con->conname), constrName) != 0) - continue; - /* Don't drop inherited constraints */ if (con->coninhcount > 0 && !recursing) ereport(ERROR, @@ -8932,9 +8935,6 @@ ATExecDropConstraint(Relation rel, const char *constrName, performDeletion(&conobj, behavior, 0); found = true; - - /* constraint found and dropped -- no need to keep looping */ - break; } systable_endscan(scan); @@ -8990,27 +8990,23 @@ ATExecDropConstraint(Relation rel, const char *constrName, childrel = heap_open(childrelid, NoLock); CheckTableNotInUse(childrel, "ALTER TABLE"); - ScanKeyInit(&key, + ScanKeyInit(&skey[0], Anum_pg_constraint_conrelid, BTEqualStrategyNumber, F_OIDEQ, ObjectIdGetDatum(childrelid)); - scan = systable_beginscan(conrel, ConstraintRelidIndexId, - true, NULL, 1, &key); + ScanKeyInit(&skey[1], + Anum_pg_constraint_contypid, + BTEqualStrategyNumber, F_OIDEQ, + ObjectIdGetDatum(InvalidOid)); + ScanKeyInit(&skey[2], + Anum_pg_constraint_conname, + BTEqualStrategyNumber, F_NAMEEQ, + CStringGetDatum(constrName)); + scan = systable_beginscan(conrel, ConstraintRelidTypidNameIndexId, + true, NULL, 3, skey); - /* scan for matching tuple - there should only be one */ - while (HeapTupleIsValid(tuple = systable_getnext(scan))) - { - con = (Form_pg_constraint) GETSTRUCT(tuple); - - /* Right now only CHECK constraints can be inherited */ - if (con->contype != CONSTRAINT_CHECK) - continue; - - if (strcmp(NameStr(con->conname), constrName) == 0) - break; - } - - if (!HeapTupleIsValid(tuple)) + /* There can be at most one matching row */ + if (!HeapTupleIsValid(tuple = systable_getnext(scan))) ereport(ERROR, (errcode(ERRCODE_UNDEFINED_OBJECT), errmsg("constraint \"%s\" of relation \"%s\" does not exist", @@ -9023,6 +9019,10 @@ ATExecDropConstraint(Relation rel, const char *constrName, con = (Form_pg_constraint) GETSTRUCT(copy_tuple); + /* Right now only CHECK constraints can be inherited */ + if (con->contype != CONSTRAINT_CHECK) + elog(ERROR, "inherited constraint is not a CHECK constraint"); + if (con->coninhcount <= 0) /* shouldn't happen */ elog(ERROR, "relation %u has non-inherited constraint \"%s\"", childrelid, constrName); @@ -11824,7 +11824,7 @@ MergeConstraintsIntoExisting(Relation child_rel, Relation parent_rel) Anum_pg_constraint_conrelid, BTEqualStrategyNumber, F_OIDEQ, ObjectIdGetDatum(RelationGetRelid(parent_rel))); - parent_scan = systable_beginscan(catalog_relation, ConstraintRelidIndexId, + parent_scan = systable_beginscan(catalog_relation, ConstraintRelidTypidNameIndexId, true, NULL, 1, &parent_key); while (HeapTupleIsValid(parent_tuple = systable_getnext(parent_scan))) @@ -11847,7 +11847,7 @@ MergeConstraintsIntoExisting(Relation child_rel, Relation parent_rel) Anum_pg_constraint_conrelid, BTEqualStrategyNumber, F_OIDEQ, ObjectIdGetDatum(RelationGetRelid(child_rel))); - child_scan = systable_beginscan(catalog_relation, ConstraintRelidIndexId, + child_scan = systable_beginscan(catalog_relation, ConstraintRelidTypidNameIndexId, true, NULL, 1, &child_key); while (HeapTupleIsValid(child_tuple = systable_getnext(child_scan))) @@ -12068,7 +12068,7 @@ RemoveInheritance(Relation child_rel, Relation parent_rel) Anum_pg_constraint_conrelid, BTEqualStrategyNumber, F_OIDEQ, ObjectIdGetDatum(RelationGetRelid(parent_rel))); - scan = systable_beginscan(catalogRelation, ConstraintRelidIndexId, + scan = systable_beginscan(catalogRelation, ConstraintRelidTypidNameIndexId, true, NULL, 1, key); connames = NIL; @@ -12088,7 +12088,7 @@ RemoveInheritance(Relation child_rel, Relation parent_rel) Anum_pg_constraint_conrelid, BTEqualStrategyNumber, F_OIDEQ, ObjectIdGetDatum(RelationGetRelid(child_rel))); - scan = systable_beginscan(catalogRelation, ConstraintRelidIndexId, + scan = systable_beginscan(catalogRelation, ConstraintRelidTypidNameIndexId, true, NULL, 1, key); while (HeapTupleIsValid(constraintTuple = systable_getnext(scan))) @@ -12829,7 +12829,7 @@ ATPrepChangePersistence(Relation rel, bool toLogged) BTEqualStrategyNumber, F_OIDEQ, ObjectIdGetDatum(RelationGetRelid(rel))); scan = systable_beginscan(pg_constraint, - toLogged ? ConstraintRelidIndexId : InvalidOid, + toLogged ? ConstraintRelidTypidNameIndexId : InvalidOid, true, NULL, 1, skey); while (HeapTupleIsValid(tuple = systable_getnext(scan))) diff --git a/src/backend/commands/typecmds.c b/src/backend/commands/typecmds.c index 175ecc8b48..b018585aef 100644 --- a/src/backend/commands/typecmds.c +++ b/src/backend/commands/typecmds.c @@ -2444,6 +2444,8 @@ AlterDomainNotNull(List *names, bool notNull) * AlterDomainDropConstraint * * Implements the ALTER DOMAIN DROP CONSTRAINT statement + * + * Returns ObjectAddress of the modified domain. */ ObjectAddress AlterDomainDropConstraint(List *names, const char *constrName, @@ -2455,10 +2457,10 @@ AlterDomainDropConstraint(List *names, const char *constrName, Relation rel; Relation conrel; SysScanDesc conscan; - ScanKeyData key[1]; + ScanKeyData skey[3]; HeapTuple contup; bool found = false; - ObjectAddress address = InvalidObjectAddress; + ObjectAddress address; /* Make a TypeName so we can use standard type lookup machinery */ typename = makeTypeNameFromNameList(names); @@ -2477,37 +2479,36 @@ AlterDomainDropConstraint(List *names, const char *constrName, /* Grab an appropriate lock on the pg_constraint relation */ conrel = heap_open(ConstraintRelationId, RowExclusiveLock); - /* Use the index to scan only constraints of the target relation */ - ScanKeyInit(&key[0], + /* Find and remove the target constraint */ + ScanKeyInit(&skey[0], + Anum_pg_constraint_conrelid, + BTEqualStrategyNumber, F_OIDEQ, + ObjectIdGetDatum(InvalidOid)); + ScanKeyInit(&skey[1], Anum_pg_constraint_contypid, BTEqualStrategyNumber, F_OIDEQ, - ObjectIdGetDatum(HeapTupleGetOid(tup))); + ObjectIdGetDatum(domainoid)); + ScanKeyInit(&skey[2], + Anum_pg_constraint_conname, + BTEqualStrategyNumber, F_NAMEEQ, + CStringGetDatum(constrName)); - conscan = systable_beginscan(conrel, ConstraintTypidIndexId, true, - NULL, 1, key); + conscan = systable_beginscan(conrel, ConstraintRelidTypidNameIndexId, true, + NULL, 3, skey); - /* - * Scan over the result set, removing any matching entries. - */ - while ((contup = systable_getnext(conscan)) != NULL) + /* There can be at most one matching row */ + if ((contup = systable_getnext(conscan)) != NULL) { - Form_pg_constraint con = (Form_pg_constraint) GETSTRUCT(contup); + ObjectAddress conobj; - if (strcmp(NameStr(con->conname), constrName) == 0) - { - ObjectAddress conobj; + conobj.classId = ConstraintRelationId; + conobj.objectId = HeapTupleGetOid(contup); + conobj.objectSubId = 0; - conobj.classId = ConstraintRelationId; - conobj.objectId = HeapTupleGetOid(contup); - conobj.objectSubId = 0; - - performDeletion(&conobj, behavior, 0); - found = true; - } + performDeletion(&conobj, behavior, 0); + found = true; } - ObjectAddressSet(address, TypeRelationId, domainoid); - /* Clean up after the scan */ systable_endscan(conscan); heap_close(conrel, RowExclusiveLock); @@ -2527,6 +2528,8 @@ AlterDomainDropConstraint(List *names, const char *constrName, constrName, TypeNameToString(typename)))); } + ObjectAddressSet(address, TypeRelationId, domainoid); + return address; } @@ -2652,16 +2655,15 @@ AlterDomainValidateConstraint(List *names, const char *constrName) Relation typrel; Relation conrel; HeapTuple tup; - Form_pg_constraint con = NULL; + Form_pg_constraint con; Form_pg_constraint copy_con; char *conbin; SysScanDesc scan; Datum val; - bool found = false; bool isnull; HeapTuple tuple; HeapTuple copyTuple; - ScanKeyData key; + ScanKeyData skey[3]; ObjectAddress address; /* Make a TypeName so we can use standard type lookup machinery */ @@ -2682,29 +2684,31 @@ AlterDomainValidateConstraint(List *names, const char *constrName) * Find and check the target constraint */ conrel = heap_open(ConstraintRelationId, RowExclusiveLock); - ScanKeyInit(&key, + + ScanKeyInit(&skey[0], + Anum_pg_constraint_conrelid, + BTEqualStrategyNumber, F_OIDEQ, + ObjectIdGetDatum(InvalidOid)); + ScanKeyInit(&skey[1], Anum_pg_constraint_contypid, BTEqualStrategyNumber, F_OIDEQ, ObjectIdGetDatum(domainoid)); - scan = systable_beginscan(conrel, ConstraintTypidIndexId, - true, NULL, 1, &key); + ScanKeyInit(&skey[2], + Anum_pg_constraint_conname, + BTEqualStrategyNumber, F_NAMEEQ, + CStringGetDatum(constrName)); - while (HeapTupleIsValid(tuple = systable_getnext(scan))) - { - con = (Form_pg_constraint) GETSTRUCT(tuple); - if (strcmp(NameStr(con->conname), constrName) == 0) - { - found = true; - break; - } - } + scan = systable_beginscan(conrel, ConstraintRelidTypidNameIndexId, true, + NULL, 3, skey); - if (!found) + /* There can be at most one matching row */ + if (!HeapTupleIsValid(tuple = systable_getnext(scan))) ereport(ERROR, (errcode(ERRCODE_UNDEFINED_OBJECT), errmsg("constraint \"%s\" of domain \"%s\" does not exist", constrName, TypeNameToString(typename)))); + con = (Form_pg_constraint) GETSTRUCT(tuple); if (con->contype != CONSTRAINT_CHECK) ereport(ERROR, (errcode(ERRCODE_WRONG_OBJECT_TYPE), @@ -3072,7 +3076,6 @@ domainAddConstraint(Oid domainOid, Oid domainNamespace, Oid baseTypeOid, { if (ConstraintNameIsUsed(CONSTRAINT_DOMAIN, domainOid, - domainNamespace, constr->conname)) ereport(ERROR, (errcode(ERRCODE_DUPLICATE_OBJECT), diff --git a/src/backend/parser/parse_utilcmd.c b/src/backend/parser/parse_utilcmd.c index 656b1b5f1b..f5c1e2a0d7 100644 --- a/src/backend/parser/parse_utilcmd.c +++ b/src/backend/parser/parse_utilcmd.c @@ -440,7 +440,8 @@ generateSerialExtraStmts(CreateStmtContext *cxt, ColumnDef *column, sname = ChooseRelationName(cxt->relation->relname, column->colname, "seq", - snamespaceid); + snamespaceid, + false); } ereport(DEBUG1, diff --git a/src/backend/utils/cache/relcache.c b/src/backend/utils/cache/relcache.c index 6125421d39..a4fc001103 100644 --- a/src/backend/utils/cache/relcache.c +++ b/src/backend/utils/cache/relcache.c @@ -4016,7 +4016,7 @@ CheckConstraintFetch(Relation relation) ObjectIdGetDatum(RelationGetRelid(relation))); conrel = heap_open(ConstraintRelationId, AccessShareLock); - conscan = systable_beginscan(conrel, ConstraintRelidIndexId, true, + conscan = systable_beginscan(conrel, ConstraintRelidTypidNameIndexId, true, NULL, 1, skey); while (HeapTupleIsValid(htup = systable_getnext(conscan))) @@ -4127,7 +4127,7 @@ RelationGetFKeyList(Relation relation) ObjectIdGetDatum(RelationGetRelid(relation))); conrel = heap_open(ConstraintRelationId, AccessShareLock); - conscan = systable_beginscan(conrel, ConstraintRelidIndexId, true, + conscan = systable_beginscan(conrel, ConstraintRelidTypidNameIndexId, true, NULL, 1, &skey); while (HeapTupleIsValid(htup = systable_getnext(conscan))) @@ -5105,6 +5105,10 @@ RelationGetExclusionInfo(Relation indexRelation, * Search pg_constraint for the constraint associated with the index. To * make this not too painfully slow, we use the index on conrelid; that * will hold the parent relation's OID not the index's own OID. + * + * Note: if we wanted to rely on the constraint name matching the index's + * name, we could just do a direct lookup using pg_constraint's unique + * index. For the moment it doesn't seem worth requiring that. */ ScanKeyInit(&skey[0], Anum_pg_constraint_conrelid, @@ -5112,7 +5116,7 @@ RelationGetExclusionInfo(Relation indexRelation, ObjectIdGetDatum(indexRelation->rd_index->indrelid)); conrel = heap_open(ConstraintRelationId, AccessShareLock); - conscan = systable_beginscan(conrel, ConstraintRelidIndexId, true, + conscan = systable_beginscan(conrel, ConstraintRelidTypidNameIndexId, true, NULL, 1, skey); found = false; diff --git a/src/include/catalog/catversion.h b/src/include/catalog/catversion.h index 0585e9b3ba..a365807dab 100644 --- a/src/include/catalog/catversion.h +++ b/src/include/catalog/catversion.h @@ -53,6 +53,6 @@ */ /* yyyymmddN */ -#define CATALOG_VERSION_NO 201809031 +#define CATALOG_VERSION_NO 201809042 #endif diff --git a/src/include/catalog/indexing.h b/src/include/catalog/indexing.h index 24915824ca..254fbef1f7 100644 --- a/src/include/catalog/indexing.h +++ b/src/include/catalog/indexing.h @@ -121,8 +121,8 @@ DECLARE_UNIQUE_INDEX(pg_collation_oid_index, 3085, on pg_collation using btree(o DECLARE_INDEX(pg_constraint_conname_nsp_index, 2664, on pg_constraint using btree(conname name_ops, connamespace oid_ops)); #define ConstraintNameNspIndexId 2664 -DECLARE_INDEX(pg_constraint_conrelid_index, 2665, on pg_constraint using btree(conrelid oid_ops)); -#define ConstraintRelidIndexId 2665 +DECLARE_UNIQUE_INDEX(pg_constraint_conrelid_contypid_conname_index, 2665, on pg_constraint using btree(conrelid oid_ops, contypid oid_ops, conname name_ops)); +#define ConstraintRelidTypidNameIndexId 2665 DECLARE_INDEX(pg_constraint_contypid_index, 2666, on pg_constraint using btree(contypid oid_ops)); #define ConstraintTypidIndexId 2666 DECLARE_UNIQUE_INDEX(pg_constraint_oid_index, 2667, on pg_constraint using btree(oid oid_ops)); diff --git a/src/include/catalog/pg_constraint.h b/src/include/catalog/pg_constraint.h index 9d209c9d19..66b3f13f74 100644 --- a/src/include/catalog/pg_constraint.h +++ b/src/include/catalog/pg_constraint.h @@ -39,6 +39,10 @@ CATALOG(pg_constraint,2606,ConstraintRelationId) * global lock to generate a globally unique name for a nameless * constraint. We associate a namespace with constraint names only for * SQL-spec compatibility. + * + * However, we do require conname to be unique among the constraints of a + * single relation or domain. This is enforced by a unique index on + * conrelid + contypid + conname. */ NameData conname; /* name of this constraint */ Oid connamespace; /* OID of namespace containing constraint */ @@ -233,7 +237,8 @@ extern void RemoveConstraintById(Oid conId); extern void RenameConstraintById(Oid conId, const char *newname); extern bool ConstraintNameIsUsed(ConstraintCategory conCat, Oid objId, - Oid objNamespace, const char *conname); + const char *conname); +extern bool ConstraintNameExists(const char *conname, Oid namespaceid); extern char *ChooseConstraintName(const char *name1, const char *name2, const char *label, Oid namespaceid, List *others); diff --git a/src/include/commands/defrem.h b/src/include/commands/defrem.h index 6b837236d4..1d05a4bcdc 100644 --- a/src/include/commands/defrem.h +++ b/src/include/commands/defrem.h @@ -41,7 +41,8 @@ extern void ReindexMultipleTables(const char *objectName, ReindexObjectType obje extern char *makeObjectName(const char *name1, const char *name2, const char *label); extern char *ChooseRelationName(const char *name1, const char *name2, - const char *label, Oid namespaceid); + const char *label, Oid namespaceid, + bool isconstraint); extern bool CheckIndexCompatible(Oid oldId, const char *accessMethodName, List *attributeList, diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out index 0218c2c362..dccc9b27c5 100644 --- a/src/test/regress/expected/alter_table.out +++ b/src/test/regress/expected/alter_table.out @@ -2994,6 +2994,41 @@ Check constraints: DROP TABLE alter2.tt8; DROP SCHEMA alter2; +-- +-- Check conflicts between index and CHECK constraint names +-- +CREATE TABLE tt9(c integer); +ALTER TABLE tt9 ADD CHECK(c > 1); +ALTER TABLE tt9 ADD CHECK(c > 2); -- picks nonconflicting name +ALTER TABLE tt9 ADD CONSTRAINT foo CHECK(c > 3); +ALTER TABLE tt9 ADD CONSTRAINT foo CHECK(c > 4); -- fail, dup name +ERROR: constraint "foo" for relation "tt9" already exists +ALTER TABLE tt9 ADD UNIQUE(c); +ALTER TABLE tt9 ADD UNIQUE(c); -- picks nonconflicting name +ALTER TABLE tt9 ADD CONSTRAINT tt9_c_key UNIQUE(c); -- fail, dup name +ERROR: relation "tt9_c_key" already exists +ALTER TABLE tt9 ADD CONSTRAINT foo UNIQUE(c); -- fail, dup name +ERROR: constraint "foo" for relation "tt9" already exists +ALTER TABLE tt9 ADD CONSTRAINT tt9_c_key CHECK(c > 5); -- fail, dup name +ERROR: constraint "tt9_c_key" for relation "tt9" already exists +ALTER TABLE tt9 ADD CONSTRAINT tt9_c_key2 CHECK(c > 6); +ALTER TABLE tt9 ADD UNIQUE(c); -- picks nonconflicting name +\d tt9 + Table "public.tt9" + Column | Type | Collation | Nullable | Default +--------+---------+-----------+----------+--------- + c | integer | | | +Indexes: + "tt9_c_key" UNIQUE CONSTRAINT, btree (c) + "tt9_c_key1" UNIQUE CONSTRAINT, btree (c) + "tt9_c_key3" UNIQUE CONSTRAINT, btree (c) +Check constraints: + "foo" CHECK (c > 3) + "tt9_c_check" CHECK (c > 1) + "tt9_c_check1" CHECK (c > 2) + "tt9_c_key2" CHECK (c > 6) + +DROP TABLE tt9; -- Check that comments on constraints and indexes are not lost at ALTER TABLE. CREATE TABLE comment_test ( id int, diff --git a/src/test/regress/sql/alter_table.sql b/src/test/regress/sql/alter_table.sql index 22cf4ef0a7..b90497804b 100644 --- a/src/test/regress/sql/alter_table.sql +++ b/src/test/regress/sql/alter_table.sql @@ -1865,6 +1865,24 @@ ALTER TABLE IF EXISTS tt8 SET SCHEMA alter2; DROP TABLE alter2.tt8; DROP SCHEMA alter2; +-- +-- Check conflicts between index and CHECK constraint names +-- +CREATE TABLE tt9(c integer); +ALTER TABLE tt9 ADD CHECK(c > 1); +ALTER TABLE tt9 ADD CHECK(c > 2); -- picks nonconflicting name +ALTER TABLE tt9 ADD CONSTRAINT foo CHECK(c > 3); +ALTER TABLE tt9 ADD CONSTRAINT foo CHECK(c > 4); -- fail, dup name +ALTER TABLE tt9 ADD UNIQUE(c); +ALTER TABLE tt9 ADD UNIQUE(c); -- picks nonconflicting name +ALTER TABLE tt9 ADD CONSTRAINT tt9_c_key UNIQUE(c); -- fail, dup name +ALTER TABLE tt9 ADD CONSTRAINT foo UNIQUE(c); -- fail, dup name +ALTER TABLE tt9 ADD CONSTRAINT tt9_c_key CHECK(c > 5); -- fail, dup name +ALTER TABLE tt9 ADD CONSTRAINT tt9_c_key2 CHECK(c > 6); +ALTER TABLE tt9 ADD UNIQUE(c); -- picks nonconflicting name +\d tt9 +DROP TABLE tt9; + -- Check that comments on constraints and indexes are not lost at ALTER TABLE. CREATE TABLE comment_test (