Two constraints with the same name not always allowed

Поиск
Список
Период
Сортировка
Искать

Two constraints with the same name not always allowed

От:
André Hänsel <andre@webkr.de>
Дата:
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é


Re: Two constraints with the same name not always allowed

От:
Tom Lane <tgl@sss.pgh.pa.us>
Дата:
Andrew Gierth  writes:
> "André" == André Hänsel  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

Re: Two constraints with the same name not always allowed

От:
Tom Lane <tgl@sss.pgh.pa.us>
Дата:
Alvaro Herrera  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

Re: Two constraints with the same name not always allowed

От:
Tom Lane <tgl@sss.pgh.pa.us>
Дата:
I wrote:
> Andrew Gierth  writes:
>> "André" == André Hänsel  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

Re: Two constraints with the same name not always allowed

От:
Alvaro Herrera <alvherre@2ndquadrant.com>
Дата:
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

Re: Two constraints with the same name not always allowed

От:
Tom Lane <tgl@sss.pgh.pa.us>
Дата:
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

diff --git a/src/backend/catalog/pg_constraint.c b/src/backend/catalog/pg_constraint.c
index 7a6d158..3d65464 100644
*** a/src/backend/catalog/pg_constraint.c
--- b/src/backend/catalog/pg_constraint.c
*************** CloneForeignKeyConstraints(Oid parentId,
*** 443,449 ****
  	ScanKeyInit(&key,
  				Anum_pg_constraint_conrelid, BTEqualStrategyNumber,
  				F_OIDEQ, ObjectIdGetDatum(parentId));
! 	scan = systable_beginscan(pg_constraint, ConstraintRelidIndexId, true,
  							  NULL, 1, &key);
  
  	while ((tuple = systable_getnext(scan)) != NULL)
--- 443,449 ----
  	ScanKeyInit(&key,
  				Anum_pg_constraint_conrelid, BTEqualStrategyNumber,
  				F_OIDEQ, ObjectIdGetDatum(parentId));
! 	scan = systable_beginscan(pg_constraint, ConstraintRelidTypidNameIndexId, true,
  							  NULL, 1, &key);
  
  	while ((tuple = systable_getnext(scan)) != NULL)
*************** AlterConstraintNamespaces(Oid ownerId, O
*** 944,975 ****
  						  Oid newNspId, bool isType, ObjectAddresses *objsMoved)
  {
  	Relation	conRel;
! 	ScanKeyData key[1];
  	SysScanDesc scan;
  	HeapTuple	tup;
  
  	conRel = heap_open(ConstraintRelationId, RowExclusiveLock);
  
! 	if (isType)
! 	{
! 		ScanKeyInit(&key[0],
! 					Anum_pg_constraint_contypid,
! 					BTEqualStrategyNumber, F_OIDEQ,
! 					ObjectIdGetDatum(ownerId));
! 
! 		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);
! 	}
  
  	while (HeapTupleIsValid((tup = systable_getnext(scan))))
  	{
--- 944,966 ----
  						  Oid newNspId, bool isType, ObjectAddresses *objsMoved)
  {
  	Relation	conRel;
! 	ScanKeyData key[2];
  	SysScanDesc scan;
  	HeapTuple	tup;
  
  	conRel = heap_open(ConstraintRelationId, RowExclusiveLock);
  
! 	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, ConstraintRelidTypidNameIndexId, true,
! 							  NULL, 2, key);
  
  	while (HeapTupleIsValid((tup = systable_getnext(scan))))
  	{
*************** get_relation_constraint_oid(Oid relid, c
*** 1059,1095 ****
  	Relation	pg_constraint;
  	HeapTuple	tuple;
  	SysScanDesc scan;
! 	ScanKeyData skey[1];
  	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));
  
! 	scan = systable_beginscan(pg_constraint, ConstraintRelidIndexId, true,
! 							  NULL, 1, 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);
! 		}
  	}
  
  	systable_endscan(scan);
--- 1050,1080 ----
  	Relation	pg_constraint;
  	HeapTuple	tuple;
  	SysScanDesc scan;
! 	ScanKeyData skey[3];
  	Oid			conOid = InvalidOid;
  
  	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, ConstraintRelidTypidNameIndexId, true,
! 							  NULL, 3, skey);
  
! 	/* There can be at most one matching row */
! 	if (HeapTupleIsValid(tuple = systable_getnext(scan)))
  	{
! 		conOid = HeapTupleGetOid(tuple);
  	}
  
  	systable_endscan(scan);
*************** get_relation_constraint_attnos(Oid relid
*** 1126,1192 ****
  	Relation	pg_constraint;
  	HeapTuple	tuple;
  	SysScanDesc scan;
! 	ScanKeyData skey[1];
  
  	/* 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));
  
! 	scan = systable_beginscan(pg_constraint, ConstraintRelidIndexId, true,
! 							  NULL, 1, skey);
  
! 	while (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++)
! 		{
! 			conattnos = bms_add_member(conattnos,
! 									   attnums[i] - FirstLowInvalidHeapAttributeNumber);
  		}
  	}
  
--- 1111,1172 ----
  	Relation	pg_constraint;
  	HeapTuple	tuple;
  	SysScanDesc scan;
! 	ScanKeyData skey[3];
  
  	/* Set *constraintOid, to avoid complaints about uninitialized vars */
  	*constraintOid = InvalidOid;
  
  	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, ConstraintRelidTypidNameIndexId, true,
! 							  NULL, 3, skey);
  
! 	/* There can be at most one matching row */
! 	if (HeapTupleIsValid(tuple = systable_getnext(scan)))
  	{
  		Datum		adatum;
  		bool		isNull;
  
  		*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)
! 		{
! 			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);
! 			}
  		}
  	}
  
*************** get_relation_idx_constraint_oid(Oid rela
*** 1224,1230 ****
  				BTEqualStrategyNumber,
  				F_OIDEQ,
  				ObjectIdGetDatum(relationId));
! 	scan = systable_beginscan(pg_constraint, ConstraintRelidIndexId,
  							  true, NULL, 1, &key);
  	while ((tuple = systable_getnext(scan)) != NULL)
  	{
--- 1204,1210 ----
  				BTEqualStrategyNumber,
  				F_OIDEQ,
  				ObjectIdGetDatum(relationId));
! 	scan = systable_beginscan(pg_constraint, ConstraintRelidTypidNameIndexId,
  							  true, NULL, 1, &key);
  	while ((tuple = systable_getnext(scan)) != NULL)
  	{
*************** get_domain_constraint_oid(Oid typid, con
*** 1254,1291 ****
  	Relation	pg_constraint;
  	HeapTuple	tuple;
  	SysScanDesc scan;
! 	ScanKeyData skey[1];
  	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_contypid,
  				BTEqualStrategyNumber, F_OIDEQ,
  				ObjectIdGetDatum(typid));
  
! 	scan = systable_beginscan(pg_constraint, ConstraintTypidIndexId, true,
! 							  NULL, 1, 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);
! 		}
! 	}
  
  	systable_endscan(scan);
  
--- 1234,1263 ----
  	Relation	pg_constraint;
  	HeapTuple	tuple;
  	SysScanDesc scan;
! 	ScanKeyData skey[3];
  	Oid			conOid = InvalidOid;
  
  	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, ConstraintRelidTypidNameIndexId, true,
! 							  NULL, 3, skey);
  
! 	/* There can be at most one matching row */
! 	if (HeapTupleIsValid(tuple = systable_getnext(scan)))
! 		conOid = HeapTupleGetOid(tuple);
  
  	systable_endscan(scan);
  
*************** get_primary_key_attnos(Oid relid, bool d
*** 1335,1341 ****
  				BTEqualStrategyNumber, F_OIDEQ,
  				ObjectIdGetDatum(relid));
  
! 	scan = systable_beginscan(pg_constraint, ConstraintRelidIndexId, true,
  							  NULL, 1, skey);
  
  	while (HeapTupleIsValid(tuple = systable_getnext(scan)))
--- 1307,1313 ----
  				BTEqualStrategyNumber, F_OIDEQ,
  				ObjectIdGetDatum(relid));
  
! 	scan = systable_beginscan(pg_constraint, ConstraintRelidTypidNameIndexId, true,
  							  NULL, 1, skey);
  
  	while (HeapTupleIsValid(tuple = systable_getnext(scan)))
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index f46af41..a3edef5 100644
*** a/src/backend/commands/tablecmds.c
--- b/src/backend/commands/tablecmds.c
*************** ATExecAlterConstraint(Relation rel, Alte
*** 7801,7810 ****
  	Constraint *cmdcon;
  	Relation	conrel;
  	SysScanDesc scan;
! 	ScanKeyData key;
  	HeapTuple	contuple;
! 	Form_pg_constraint currcon = NULL;
! 	bool		found = false;
  	ObjectAddress address;
  
  	cmdcon = castNode(Constraint, cmd->def);
--- 7801,7809 ----
  	Constraint *cmdcon;
  	Relation	conrel;
  	SysScanDesc scan;
! 	ScanKeyData skey[3];
  	HeapTuple	contuple;
! 	Form_pg_constraint currcon;
  	ObjectAddress address;
  
  	cmdcon = castNode(Constraint, cmd->def);
*************** ATExecAlterConstraint(Relation rel, Alte
*** 7814,7842 ****
  	/*
  	 * Find and check the target constraint
  	 */
! 	ScanKeyInit(&key,
  				Anum_pg_constraint_conrelid,
  				BTEqualStrategyNumber, F_OIDEQ,
  				ObjectIdGetDatum(RelationGetRelid(rel)));
! 	scan = systable_beginscan(conrel, ConstraintRelidIndexId,
! 							  true, NULL, 1, &key);
! 
! 	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)
  		ereport(ERROR,
  				(errcode(ERRCODE_UNDEFINED_OBJECT),
  				 errmsg("constraint \"%s\" of relation \"%s\" does not exist",
  						cmdcon->conname, RelationGetRelationName(rel))));
  
  	if (currcon->contype != CONSTRAINT_FOREIGN)
  		ereport(ERROR,
  				(errcode(ERRCODE_WRONG_OBJECT_TYPE),
--- 7813,7841 ----
  	/*
  	 * Find and check the target constraint
  	 */
! 	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(cmdcon->conname));
! 	scan = systable_beginscan(conrel, ConstraintRelidTypidNameIndexId,
! 							  true, NULL, 3, skey);
  
! 	/* 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),
*************** ATExecValidateConstraint(Relation rel, c
*** 7969,7978 ****
  {
  	Relation	conrel;
  	SysScanDesc scan;
! 	ScanKeyData key;
  	HeapTuple	tuple;
! 	Form_pg_constraint con = NULL;
! 	bool		found = false;
  	ObjectAddress address;
  
  	conrel = heap_open(ConstraintRelationId, RowExclusiveLock);
--- 7968,7976 ----
  {
  	Relation	conrel;
  	SysScanDesc scan;
! 	ScanKeyData skey[3];
  	HeapTuple	tuple;
! 	Form_pg_constraint con;
  	ObjectAddress address;
  
  	conrel = heap_open(ConstraintRelationId, RowExclusiveLock);
*************** ATExecValidateConstraint(Relation rel, c
*** 7980,8008 ****
  	/*
  	 * Find and check the target constraint
  	 */
! 	ScanKeyInit(&key,
  				Anum_pg_constraint_conrelid,
  				BTEqualStrategyNumber, F_OIDEQ,
  				ObjectIdGetDatum(RelationGetRelid(rel)));
! 	scan = systable_beginscan(conrel, ConstraintRelidIndexId,
! 							  true, NULL, 1, &key);
! 
! 	while (HeapTupleIsValid(tuple = systable_getnext(scan)))
! 	{
! 		con = (Form_pg_constraint) GETSTRUCT(tuple);
! 		if (strcmp(NameStr(con->conname), constrName) == 0)
! 		{
! 			found = true;
! 			break;
! 		}
! 	}
  
! 	if (!found)
  		ereport(ERROR,
  				(errcode(ERRCODE_UNDEFINED_OBJECT),
  				 errmsg("constraint \"%s\" of relation \"%s\" does not exist",
  						constrName, RelationGetRelationName(rel))));
  
  	if (con->contype != CONSTRAINT_FOREIGN &&
  		con->contype != CONSTRAINT_CHECK)
  		ereport(ERROR,
--- 7978,8006 ----
  	/*
  	 * Find and check the target constraint
  	 */
! 	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(constrName));
! 	scan = systable_beginscan(conrel, ConstraintRelidTypidNameIndexId,
! 							  true, NULL, 3, skey);
  
! 	/* 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,
*************** ATExecDropConstraint(Relation rel, const
*** 8865,8871 ****
  	Relation	conrel;
  	Form_pg_constraint con;
  	SysScanDesc scan;
! 	ScanKeyData key;
  	HeapTuple	tuple;
  	bool		found = false;
  	bool		is_no_inherit_constraint = false;
--- 8863,8869 ----
  	Relation	conrel;
  	Form_pg_constraint con;
  	SysScanDesc scan;
! 	ScanKeyData skey[3];
  	HeapTuple	tuple;
  	bool		found = false;
  	bool		is_no_inherit_constraint = false;
*************** ATExecDropConstraint(Relation rel, const
*** 8879,8900 ****
  	/*
  	 * Find and drop the target constraint
  	 */
! 	ScanKeyInit(&key,
  				Anum_pg_constraint_conrelid,
  				BTEqualStrategyNumber, F_OIDEQ,
  				ObjectIdGetDatum(RelationGetRelid(rel)));
! 	scan = systable_beginscan(conrel, ConstraintRelidIndexId,
! 							  true, NULL, 1, &key);
  
! 	while (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,
--- 8877,8904 ----
  	/*
  	 * Find and drop the target constraint
  	 */
! 	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(constrName));
! 	scan = systable_beginscan(conrel, ConstraintRelidTypidNameIndexId,
! 							  true, NULL, 3, skey);
  
! 	/* There can be at most one matching row */
! 	if (HeapTupleIsValid(tuple = systable_getnext(scan)))
  	{
  		ObjectAddress conobj;
  
  		con = (Form_pg_constraint) GETSTRUCT(tuple);
  
  		/* Don't drop inherited constraints */
  		if (con->coninhcount > 0 && !recursing)
  			ereport(ERROR,
*************** ATExecDropConstraint(Relation rel, const
*** 8932,8940 ****
  		performDeletion(&conobj, behavior, 0);
  
  		found = true;
- 
- 		/* constraint found and dropped -- no need to keep looping */
- 		break;
  	}
  
  	systable_endscan(scan);
--- 8936,8941 ----
*************** ATExecDropConstraint(Relation rel, const
*** 8990,9016 ****
  		childrel = heap_open(childrelid, NoLock);
  		CheckTableNotInUse(childrel, "ALTER TABLE");
  
! 		ScanKeyInit(&key,
  					Anum_pg_constraint_conrelid,
  					BTEqualStrategyNumber, F_OIDEQ,
  					ObjectIdGetDatum(childrelid));
! 		scan = systable_beginscan(conrel, ConstraintRelidIndexId,
! 								  true, NULL, 1, &key);
! 
! 		/* 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))
  			ereport(ERROR,
  					(errcode(ERRCODE_UNDEFINED_OBJECT),
  					 errmsg("constraint \"%s\" of relation \"%s\" does not exist",
--- 8991,9013 ----
  		childrel = heap_open(childrelid, NoLock);
  		CheckTableNotInUse(childrel, "ALTER TABLE");
  
! 		ScanKeyInit(&skey[0],
  					Anum_pg_constraint_conrelid,
  					BTEqualStrategyNumber, F_OIDEQ,
  					ObjectIdGetDatum(childrelid));
! 		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);
  
! 		/* 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",
*************** ATExecDropConstraint(Relation rel, const
*** 9023,9028 ****
--- 9020,9029 ----
  
  		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);
*************** MergeConstraintsIntoExisting(Relation ch
*** 11824,11830 ****
  				Anum_pg_constraint_conrelid,
  				BTEqualStrategyNumber, F_OIDEQ,
  				ObjectIdGetDatum(RelationGetRelid(parent_rel)));
! 	parent_scan = systable_beginscan(catalog_relation, ConstraintRelidIndexId,
  									 true, NULL, 1, &parent_key);
  
  	while (HeapTupleIsValid(parent_tuple = systable_getnext(parent_scan)))
--- 11825,11831 ----
  				Anum_pg_constraint_conrelid,
  				BTEqualStrategyNumber, F_OIDEQ,
  				ObjectIdGetDatum(RelationGetRelid(parent_rel)));
! 	parent_scan = systable_beginscan(catalog_relation, ConstraintRelidTypidNameIndexId,
  									 true, NULL, 1, &parent_key);
  
  	while (HeapTupleIsValid(parent_tuple = systable_getnext(parent_scan)))
*************** MergeConstraintsIntoExisting(Relation ch
*** 11847,11853 ****
  					Anum_pg_constraint_conrelid,
  					BTEqualStrategyNumber, F_OIDEQ,
  					ObjectIdGetDatum(RelationGetRelid(child_rel)));
! 		child_scan = systable_beginscan(catalog_relation, ConstraintRelidIndexId,
  										true, NULL, 1, &child_key);
  
  		while (HeapTupleIsValid(child_tuple = systable_getnext(child_scan)))
--- 11848,11854 ----
  					Anum_pg_constraint_conrelid,
  					BTEqualStrategyNumber, F_OIDEQ,
  					ObjectIdGetDatum(RelationGetRelid(child_rel)));
! 		child_scan = systable_beginscan(catalog_relation, ConstraintRelidTypidNameIndexId,
  										true, NULL, 1, &child_key);
  
  		while (HeapTupleIsValid(child_tuple = systable_getnext(child_scan)))
*************** RemoveInheritance(Relation child_rel, Re
*** 12068,12074 ****
  				Anum_pg_constraint_conrelid,
  				BTEqualStrategyNumber, F_OIDEQ,
  				ObjectIdGetDatum(RelationGetRelid(parent_rel)));
! 	scan = systable_beginscan(catalogRelation, ConstraintRelidIndexId,
  							  true, NULL, 1, key);
  
  	connames = NIL;
--- 12069,12075 ----
  				Anum_pg_constraint_conrelid,
  				BTEqualStrategyNumber, F_OIDEQ,
  				ObjectIdGetDatum(RelationGetRelid(parent_rel)));
! 	scan = systable_beginscan(catalogRelation, ConstraintRelidTypidNameIndexId,
  							  true, NULL, 1, key);
  
  	connames = NIL;
*************** RemoveInheritance(Relation child_rel, Re
*** 12088,12094 ****
  				Anum_pg_constraint_conrelid,
  				BTEqualStrategyNumber, F_OIDEQ,
  				ObjectIdGetDatum(RelationGetRelid(child_rel)));
! 	scan = systable_beginscan(catalogRelation, ConstraintRelidIndexId,
  							  true, NULL, 1, key);
  
  	while (HeapTupleIsValid(constraintTuple = systable_getnext(scan)))
--- 12089,12095 ----
  				Anum_pg_constraint_conrelid,
  				BTEqualStrategyNumber, F_OIDEQ,
  				ObjectIdGetDatum(RelationGetRelid(child_rel)));
! 	scan = systable_beginscan(catalogRelation, ConstraintRelidTypidNameIndexId,
  							  true, NULL, 1, key);
  
  	while (HeapTupleIsValid(constraintTuple = systable_getnext(scan)))
*************** ATPrepChangePersistence(Relation rel, bo
*** 12829,12835 ****
  				BTEqualStrategyNumber, F_OIDEQ,
  				ObjectIdGetDatum(RelationGetRelid(rel)));
  	scan = systable_beginscan(pg_constraint,
! 							  toLogged ? ConstraintRelidIndexId : InvalidOid,
  							  true, NULL, 1, skey);
  
  	while (HeapTupleIsValid(tuple = systable_getnext(scan)))
--- 12830,12836 ----
  				BTEqualStrategyNumber, F_OIDEQ,
  				ObjectIdGetDatum(RelationGetRelid(rel)));
  	scan = systable_beginscan(pg_constraint,
! 							  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 175ecc8..a67f9ce 100644
*** a/src/backend/commands/typecmds.c
--- b/src/backend/commands/typecmds.c
*************** AlterDomainNotNull(List *names, bool not
*** 2444,2449 ****
--- 2444,2451 ----
   * AlterDomainDropConstraint
   *
   * Implements the ALTER DOMAIN DROP CONSTRAINT statement
+  *
+  * Returns ObjectAddress of the modified domain.
   */
  ObjectAddress
  AlterDomainDropConstraint(List *names, const char *constrName,
*************** AlterDomainDropConstraint(List *names, c
*** 2455,2464 ****
  	Relation	rel;
  	Relation	conrel;
  	SysScanDesc conscan;
! 	ScanKeyData key[1];
  	HeapTuple	contup;
  	bool		found = false;
! 	ObjectAddress address = InvalidObjectAddress;
  
  	/* Make a TypeName so we can use standard type lookup machinery */
  	typename = makeTypeNameFromNameList(names);
--- 2457,2466 ----
  	Relation	rel;
  	Relation	conrel;
  	SysScanDesc conscan;
! 	ScanKeyData skey[3];
  	HeapTuple	contup;
  	bool		found = false;
! 	ObjectAddress address;
  
  	/* Make a TypeName so we can use standard type lookup machinery */
  	typename = makeTypeNameFromNameList(names);
*************** AlterDomainDropConstraint(List *names, c
*** 2477,2513 ****
  	/* 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],
  				Anum_pg_constraint_contypid,
  				BTEqualStrategyNumber, F_OIDEQ,
! 				ObjectIdGetDatum(HeapTupleGetOid(tup)));
  
! 	conscan = systable_beginscan(conrel, ConstraintTypidIndexId, true,
! 								 NULL, 1, key);
  
! 	/*
! 	 * Scan over the result set, removing any matching entries.
! 	 */
! 	while ((contup = systable_getnext(conscan)) != NULL)
  	{
! 		Form_pg_constraint con = (Form_pg_constraint) GETSTRUCT(contup);
! 
! 		if (strcmp(NameStr(con->conname), constrName) == 0)
! 		{
! 			ObjectAddress conobj;
  
! 			conobj.classId = ConstraintRelationId;
! 			conobj.objectId = HeapTupleGetOid(contup);
! 			conobj.objectSubId = 0;
  
! 			performDeletion(&conobj, behavior, 0);
! 			found = true;
! 		}
  	}
  
- 	ObjectAddressSet(address, TypeRelationId, domainoid);
- 
  	/* Clean up after the scan */
  	systable_endscan(conscan);
  	heap_close(conrel, RowExclusiveLock);
--- 2479,2514 ----
  	/* Grab an appropriate lock on the pg_constraint relation */
  	conrel = heap_open(ConstraintRelationId, RowExclusiveLock);
  
! 	/* 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(domainoid));
! 	ScanKeyInit(&skey[2],
! 				Anum_pg_constraint_conname,
! 				BTEqualStrategyNumber, F_NAMEEQ,
! 				CStringGetDatum(constrName));
  
! 	conscan = systable_beginscan(conrel, ConstraintRelidTypidNameIndexId, true,
! 								 NULL, 3, skey);
  
! 	/* There can be at most one matching row */
! 	if ((contup = systable_getnext(conscan)) != NULL)
  	{
! 		ObjectAddress conobj;
  
! 		conobj.classId = ConstraintRelationId;
! 		conobj.objectId = HeapTupleGetOid(contup);
! 		conobj.objectSubId = 0;
  
! 		performDeletion(&conobj, behavior, 0);
! 		found = true;
  	}
  
  	/* Clean up after the scan */
  	systable_endscan(conscan);
  	heap_close(conrel, RowExclusiveLock);
*************** AlterDomainDropConstraint(List *names, c
*** 2527,2532 ****
--- 2528,2535 ----
  							constrName, TypeNameToString(typename))));
  	}
  
+ 	ObjectAddressSet(address, TypeRelationId, domainoid);
+ 
  	return address;
  }
  
*************** AlterDomainValidateConstraint(List *name
*** 2652,2667 ****
  	Relation	typrel;
  	Relation	conrel;
  	HeapTuple	tup;
! 	Form_pg_constraint con = NULL;
  	Form_pg_constraint copy_con;
  	char	   *conbin;
  	SysScanDesc scan;
  	Datum		val;
- 	bool		found = false;
  	bool		isnull;
  	HeapTuple	tuple;
  	HeapTuple	copyTuple;
! 	ScanKeyData key;
  	ObjectAddress address;
  
  	/* Make a TypeName so we can use standard type lookup machinery */
--- 2655,2669 ----
  	Relation	typrel;
  	Relation	conrel;
  	HeapTuple	tup;
! 	Form_pg_constraint con;
  	Form_pg_constraint copy_con;
  	char	   *conbin;
  	SysScanDesc scan;
  	Datum		val;
  	bool		isnull;
  	HeapTuple	tuple;
  	HeapTuple	copyTuple;
! 	ScanKeyData skey[3];
  	ObjectAddress address;
  
  	/* Make a TypeName so we can use standard type lookup machinery */
*************** AlterDomainValidateConstraint(List *name
*** 2682,2710 ****
  	 * Find and check the target constraint
  	 */
  	conrel = heap_open(ConstraintRelationId, RowExclusiveLock);
! 	ScanKeyInit(&key,
  				Anum_pg_constraint_contypid,
  				BTEqualStrategyNumber, F_OIDEQ,
  				ObjectIdGetDatum(domainoid));
! 	scan = systable_beginscan(conrel, ConstraintTypidIndexId,
! 							  true, NULL, 1, &key);
  
! 	while (HeapTupleIsValid(tuple = systable_getnext(scan)))
! 	{
! 		con = (Form_pg_constraint) GETSTRUCT(tuple);
! 		if (strcmp(NameStr(con->conname), constrName) == 0)
! 		{
! 			found = true;
! 			break;
! 		}
! 	}
  
! 	if (!found)
  		ereport(ERROR,
  				(errcode(ERRCODE_UNDEFINED_OBJECT),
  				 errmsg("constraint \"%s\" of domain \"%s\" does not exist",
  						constrName, TypeNameToString(typename))));
  
  	if (con->contype != CONSTRAINT_CHECK)
  		ereport(ERROR,
  				(errcode(ERRCODE_WRONG_OBJECT_TYPE),
--- 2684,2714 ----
  	 * Find and check the target constraint
  	 */
  	conrel = heap_open(ConstraintRelationId, RowExclusiveLock);
! 
! 	ScanKeyInit(&skey[0],
! 				Anum_pg_constraint_conrelid,
! 				BTEqualStrategyNumber, F_OIDEQ,
! 				ObjectIdGetDatum(InvalidOid));
! 	ScanKeyInit(&skey[1],
  				Anum_pg_constraint_contypid,
  				BTEqualStrategyNumber, F_OIDEQ,
  				ObjectIdGetDatum(domainoid));
! 	ScanKeyInit(&skey[2],
! 				Anum_pg_constraint_conname,
! 				BTEqualStrategyNumber, F_NAMEEQ,
! 				CStringGetDatum(constrName));
  
! 	scan = systable_beginscan(conrel, ConstraintRelidTypidNameIndexId, true,
! 							  NULL, 3, skey);
  
! 	/* 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),
diff --git a/src/backend/utils/cache/relcache.c b/src/backend/utils/cache/relcache.c
index 6125421..a4fc001 100644
*** a/src/backend/utils/cache/relcache.c
--- b/src/backend/utils/cache/relcache.c
*************** CheckConstraintFetch(Relation relation)
*** 4016,4022 ****
  				ObjectIdGetDatum(RelationGetRelid(relation)));
  
  	conrel = heap_open(ConstraintRelationId, AccessShareLock);
! 	conscan = systable_beginscan(conrel, ConstraintRelidIndexId, true,
  								 NULL, 1, skey);
  
  	while (HeapTupleIsValid(htup = systable_getnext(conscan)))
--- 4016,4022 ----
  				ObjectIdGetDatum(RelationGetRelid(relation)));
  
  	conrel = heap_open(ConstraintRelationId, AccessShareLock);
! 	conscan = systable_beginscan(conrel, ConstraintRelidTypidNameIndexId, true,
  								 NULL, 1, skey);
  
  	while (HeapTupleIsValid(htup = systable_getnext(conscan)))
*************** RelationGetFKeyList(Relation relation)
*** 4127,4133 ****
  				ObjectIdGetDatum(RelationGetRelid(relation)));
  
  	conrel = heap_open(ConstraintRelationId, AccessShareLock);
! 	conscan = systable_beginscan(conrel, ConstraintRelidIndexId, true,
  								 NULL, 1, &skey);
  
  	while (HeapTupleIsValid(htup = systable_getnext(conscan)))
--- 4127,4133 ----
  				ObjectIdGetDatum(RelationGetRelid(relation)));
  
  	conrel = heap_open(ConstraintRelationId, AccessShareLock);
! 	conscan = systable_beginscan(conrel, ConstraintRelidTypidNameIndexId, true,
  								 NULL, 1, &skey);
  
  	while (HeapTupleIsValid(htup = systable_getnext(conscan)))
*************** RelationGetExclusionInfo(Relation indexR
*** 5105,5110 ****
--- 5105,5114 ----
  	 * 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,
*************** RelationGetExclusionInfo(Relation indexR
*** 5112,5118 ****
  				ObjectIdGetDatum(indexRelation->rd_index->indrelid));
  
  	conrel = heap_open(ConstraintRelationId, AccessShareLock);
! 	conscan = systable_beginscan(conrel, ConstraintRelidIndexId, true,
  								 NULL, 1, skey);
  	found = false;
  
--- 5116,5122 ----
  				ObjectIdGetDatum(indexRelation->rd_index->indrelid));
  
  	conrel = heap_open(ConstraintRelationId, AccessShareLock);
! 	conscan = systable_beginscan(conrel, ConstraintRelidTypidNameIndexId, true,
  								 NULL, 1, skey);
  	found = false;
  
diff --git a/src/include/catalog/indexing.h b/src/include/catalog/indexing.h
index 2491582..254fbef 100644
*** a/src/include/catalog/indexing.h
--- b/src/include/catalog/indexing.h
*************** DECLARE_UNIQUE_INDEX(pg_collation_oid_in
*** 121,128 ****
  
  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_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));
--- 121,128 ----
  
  DECLARE_INDEX(pg_constraint_conname_nsp_index, 2664, on pg_constraint using btree(conname name_ops, connamespace oid_ops));
  #define ConstraintNameNspIndexId  2664
! 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 7c1c0e1..4435310 100644
*** a/src/include/catalog/pg_constraint.h
--- b/src/include/catalog/pg_constraint.h
*************** CATALOG(pg_constraint,2606,ConstraintRel
*** 39,44 ****
--- 39,48 ----
  	 * 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 */
diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c
index b256054..70a1767 100644
*** a/src/backend/catalog/index.c
--- b/src/backend/catalog/index.c
*************** index_create(Relation heapRelation,
*** 842,847 ****
--- 842,853 ----
  	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)
*************** index_create(Relation heapRelation,
*** 860,865 ****
--- 866,885 ----
  						indexRelationName)));
  	}
  
+ 	if ((flags & INDEX_CREATE_ADD_CONSTRAINT) != 0 &&
+ 		ConstraintNameIsUsed(CONSTRAINT_RELATION, heapRelationId,
+ 							 namespaceId, 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 3d65464..51f16ce 100644
*** a/src/backend/catalog/pg_constraint.c
--- b/src/backend/catalog/pg_constraint.c
*************** ConstraintNameIsUsed(ConstraintCategory 
*** 701,706 ****
--- 701,744 ----
  }
  
  /*
+  * 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];
+ 
+ 	conDesc = heap_open(ConstraintRelationId, AccessShareLock);
+ 
+ 	ScanKeyInit(&skey[0],
+ 				Anum_pg_constraint_conname,
+ 				BTEqualStrategyNumber, F_NAMEEQ,
+ 				CStringGetDatum(conname));
+ 
+ 	ScanKeyInit(&skey[1],
+ 				Anum_pg_constraint_connamespace,
+ 				BTEqualStrategyNumber, F_OIDEQ,
+ 				ObjectIdGetDatum(namespaceid));
+ 
+ 	conscan = systable_beginscan(conDesc, ConstraintNameNspIndexId, true,
+ 								 NULL, 2, skey);
+ 
+ 	found = (HeapTupleIsValid(systable_getnext(conscan)));
+ 
+ 	systable_endscan(conscan);
+ 	heap_close(conDesc, AccessShareLock);
+ 
+ 	return found;
+ }
+ 
+ /*
   * Select a nonconflicting name for a new constraint.
   *
   * The objective here is to choose a name that is unique within the
*************** RenameConstraintById(Oid conId, const ch
*** 899,906 ****
  	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.
  	 */
  	if (OidIsValid(con->conrelid) &&
  		ConstraintNameIsUsed(CONSTRAINT_RELATION,
--- 937,943 ----
  	con = (Form_pg_constraint) GETSTRUCT(tuple);
  
  	/*
! 	 * For user-friendliness, check whether the name is already in use.
  	 */
  	if (OidIsValid(con->conrelid) &&
  		ConstraintNameIsUsed(CONSTRAINT_RELATION,
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index d54c78c..ab3d9a0 100644
*** a/src/backend/commands/indexcmds.c
--- b/src/backend/commands/indexcmds.c
*************** makeObjectName(const char *name1, const 
*** 1995,2000 ****
--- 1995,2006 ----
   * 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
*************** makeObjectName(const char *name1, const 
*** 2006,2012 ****
   */
  char *
  ChooseRelationName(const char *name1, const char *name2,
! 				   const char *label, Oid namespaceid)
  {
  	int			pass = 0;
  	char	   *relname = NULL;
--- 2012,2019 ----
   */
  char *
  ChooseRelationName(const char *name1, const char *name2,
! 				   const char *label, Oid namespaceid,
! 				   bool isconstraint)
  {
  	int			pass = 0;
  	char	   *relname = NULL;
*************** ChooseRelationName(const char *name1, co
*** 2020,2026 ****
  		relname = makeObjectName(name1, name2, modlabel);
  
  		if (!OidIsValid(get_relname_relid(relname, namespaceid)))
! 			break;
  
  		/* found a conflict, so try a new name component */
  		pfree(relname);
--- 2027,2037 ----
  		relname = makeObjectName(name1, name2, modlabel);
  
  		if (!OidIsValid(get_relname_relid(relname, namespaceid)))
! 		{
! 			if (!isconstraint ||
! 				!ConstraintNameExists(relname, namespaceid))
! 				break;
! 		}
  
  		/* found a conflict, so try a new name component */
  		pfree(relname);
*************** ChooseIndexName(const char *tabname, Oid
*** 2048,2075 ****
  		indexname = ChooseRelationName(tabname,
  									   NULL,
  									   "pkey",
! 									   namespaceId);
  	}
  	else if (exclusionOpNames != NIL)
  	{
  		indexname = ChooseRelationName(tabname,
  									   ChooseIndexNameAddition(colnames),
  									   "excl",
! 									   namespaceId);
  	}
  	else if (isconstraint)
  	{
  		indexname = ChooseRelationName(tabname,
  									   ChooseIndexNameAddition(colnames),
  									   "key",
! 									   namespaceId);
  	}
  	else
  	{
  		indexname = ChooseRelationName(tabname,
  									   ChooseIndexNameAddition(colnames),
  									   "idx",
! 									   namespaceId);
  	}
  
  	return indexname;
--- 2059,2090 ----
  		indexname = ChooseRelationName(tabname,
  									   NULL,
  									   "pkey",
! 									   namespaceId,
! 									   true);
  	}
  	else if (exclusionOpNames != NIL)
  	{
  		indexname = ChooseRelationName(tabname,
  									   ChooseIndexNameAddition(colnames),
  									   "excl",
! 									   namespaceId,
! 									   true);
  	}
  	else if (isconstraint)
  	{
  		indexname = ChooseRelationName(tabname,
  									   ChooseIndexNameAddition(colnames),
  									   "key",
! 									   namespaceId,
! 									   true);
  	}
  	else
  	{
  		indexname = ChooseRelationName(tabname,
  									   ChooseIndexNameAddition(colnames),
  									   "idx",
! 									   namespaceId,
! 									   false);
  	}
  
  	return indexname;
diff --git a/src/backend/parser/parse_utilcmd.c b/src/backend/parser/parse_utilcmd.c
index 656b1b5..f5c1e2a 100644
*** a/src/backend/parser/parse_utilcmd.c
--- b/src/backend/parser/parse_utilcmd.c
*************** generateSerialExtraStmts(CreateStmtConte
*** 440,446 ****
  		sname = ChooseRelationName(cxt->relation->relname,
  								   column->colname,
  								   "seq",
! 								   snamespaceid);
  	}
  
  	ereport(DEBUG1,
--- 440,447 ----
  		sname = ChooseRelationName(cxt->relation->relname,
  								   column->colname,
  								   "seq",
! 								   snamespaceid,
! 								   false);
  	}
  
  	ereport(DEBUG1,
diff --git a/src/include/catalog/pg_constraint.h b/src/include/catalog/pg_constraint.h
index 4435310..49df169 100644
*** a/src/include/catalog/pg_constraint.h
--- b/src/include/catalog/pg_constraint.h
*************** extern void RenameConstraintById(Oid con
*** 244,249 ****
--- 244,250 ----
  
  extern bool ConstraintNameIsUsed(ConstraintCategory conCat, Oid objId,
  					 Oid objNamespace, 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 6b83723..1d05a4b 100644
*** a/src/include/commands/defrem.h
--- b/src/include/commands/defrem.h
*************** extern void ReindexMultipleTables(const 
*** 41,47 ****
  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);
  extern bool CheckIndexCompatible(Oid oldId,
  					 const char *accessMethodName,
  					 List *attributeList,
--- 41,48 ----
  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,
! 				   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 0218c2c..dccc9b2 100644
*** a/src/test/regress/expected/alter_table.out
--- b/src/test/regress/expected/alter_table.out
*************** Check constraints:
*** 2994,2999 ****
--- 2994,3034 ----
  
  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 22cf4ef..b904978 100644
*** a/src/test/regress/sql/alter_table.sql
--- b/src/test/regress/sql/alter_table.sql
*************** ALTER TABLE IF EXISTS tt8 SET SCHEMA alt
*** 1865,1870 ****
--- 1865,1888 ----
  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 (

Re: Two constraints with the same name not always allowed

От:
Tom Lane <tgl@sss.pgh.pa.us>
Дата:
Peter Eisentraut  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

Re: Two constraints with the same name not always allowed

От:
Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
Дата:
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

Re: Two constraints with the same name not always allowed

От:
Andrew Gierth <andrew@tao11.riddles.org.uk>
Дата:
>>>>> "André" == André Hänsel  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)

FAQ