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

Fix ALTER TABLE ONLY .. DROP CONSTRAINT.

Browse Source 
When I consolidated two copies of the HOT-chain search logic in commit
4da99ea423, I introduced a behavior
change: the old code wouldn't necessarily traverse the entire chain,
if the most recently returned tuple were updated while the HOT chain
traversal is in progress.  The new behavior seems more correct, but
unfortunately, the code here relies on a scan with SnapshotNow failing
to see its own updates.  That seems pretty shaky even with the old HOT
chain traversal behavior, since there's no guarantee that these
updates will always be HOT, but it's trivial to broke a failure with
the new HOT search logic.  Fix by updating just the first matching
pg_constraint tuple, rather than all of them, since there should be
only one anyway.  But since nobody has reproduced this failure on older
versions, no back-patch for now.

Report and test case by Alex Hunsaker; tablecmds.c changes by me.
This commit is contained in:
Robert Haas
2011-10-09 23:39:52 -04:00
parent c980426c69
commit c0f03aae04
3 changed files with 65 additions and 51 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@ -6734,6 +6734,7 @@ ATExecDropConstraint(Relation rel, const char *constrName,
{
Oid childrelid = lfirst_oid(child);
Relation childrel;
HeapTuple copy_tuple;
/* find_inheritance_children already got lock */
childrel = heap_open(childrelid, NoLock);
@ -6746,83 +6747,79 @@ ATExecDropConstraint(Relation rel, const char *constrName,
scan = systable_beginscan(conrel, ConstraintRelidIndexId,
true, SnapshotNow, 1, &key);
found = false;
/* scan for matching tuple - there should only be one */
while (HeapTupleIsValid(tuple = systable_getnext(scan)))
{
HeapTuple copy_tuple;
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)
continue;
if (strcmp(NameStr(con->conname), constrName) == 0)
break;
}
found = true;
if (!HeapTupleIsValid(tuple))
ereport(ERROR,
(errcode(ERRCODE_UNDEFINED_OBJECT),
errmsg("constraint \"%s\" of relation \"%s\" does not exist",
constrName,
RelationGetRelationName(childrel))));
if (con->coninhcount <= 0) /* shouldn't happen */
elog(ERROR, "relation %u has non-inherited constraint \"%s\"",
childrelid, constrName);
copy_tuple = heap_copytuple(tuple);
copy_tuple = heap_copytuple(tuple);
con = (Form_pg_constraint) GETSTRUCT(copy_tuple);
systable_endscan(scan);
if (recurse)
con = (Form_pg_constraint) GETSTRUCT(copy_tuple);
if (con->coninhcount <= 0) /* shouldn't happen */
elog(ERROR, "relation %u has non-inherited constraint \"%s\"",
childrelid, constrName);
if (recurse)
{
/*
* If the child constraint has other definition sources, just
* decrement its inheritance count; if not, recurse to delete
* it.
*/
if (con->coninhcount == 1 && !con->conislocal)
{
/*
* If the child constraint has other definition sources, just
* decrement its inheritance count; if not, recurse to delete
* it.
*/
if (con->coninhcount == 1 && !con->conislocal)
{
/* Time to delete this child constraint, too */
ATExecDropConstraint(childrel, constrName, behavior,
true, true,
false, lockmode);
}
else
{
/* Child constraint must survive my deletion */
con->coninhcount--;
simple_heap_update(conrel, &copy_tuple->t_self, copy_tuple);
CatalogUpdateIndexes(conrel, copy_tuple);
/* Make update visible */
CommandCounterIncrement();
}
/* Time to delete this child constraint, too */
ATExecDropConstraint(childrel, constrName, behavior,
true, true,
false, lockmode);
}
else
{
/*
* If we were told to drop ONLY in this table (no recursion),
* we need to mark the inheritors' constraints as locally
* defined rather than inherited.
*/
/* Child constraint must survive my deletion */
con->coninhcount--;
con->conislocal = true;
simple_heap_update(conrel, &copy_tuple->t_self, copy_tuple);
CatalogUpdateIndexes(conrel, copy_tuple);
/* Make update visible */
CommandCounterIncrement();
}
}
else
{
/*
* If we were told to drop ONLY in this table (no recursion),
* we need to mark the inheritors' constraints as locally
* defined rather than inherited.
*/
con->coninhcount--;
con->conislocal = true;
heap_freetuple(copy_tuple);
simple_heap_update(conrel, &copy_tuple->t_self, copy_tuple);
CatalogUpdateIndexes(conrel, copy_tuple);
/* Make update visible */
CommandCounterIncrement();
}
systable_endscan(scan);
if (!found)
ereport(ERROR,
(errcode(ERRCODE_UNDEFINED_OBJECT),
errmsg("constraint \"%s\" of relation \"%s\" does not exist",
constrName,
RelationGetRelationName(childrel))));
heap_freetuple(copy_tuple);
heap_close(childrel, NoLock);
}