database/postgres
1
0
Fork 0
mirror of https://github.com/postgres/postgres.git synced 2025-06-17 17:02:08 +03:00
Code Activity

Fully enforce uniqueness of constraint names.

Browse Source 
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
This commit is contained in:
Tom Lane
2018-09-04 13:45:35 -04:00
parent f30c6f523f
commit 17b7c302b5
17 changed files with 504 additions and 376 deletions
Download Patch File Download Diff File Expand all files Collapse all files

136
src/backend/commands/tablecmds.c
View File

@ -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)))