Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commitc0f03aa

Browse files
committed
Fix ALTER TABLE ONLY .. DROP CONSTRAINT.
When I consolidated two copies of the HOT-chain search logic in commit4da99ea, I introduced a behaviorchange: the old code wouldn't necessarily traverse the entire chain,if the most recently returned tuple were updated while the HOT chaintraversal is in progress. The new behavior seems more correct, butunfortunately, the code here relies on a scan with SnapshotNow failingto see its own updates. That seems pretty shaky even with the old HOTchain traversal behavior, since there's no guarantee that theseupdates will always be HOT, but it's trivial to broke a failure withthe new HOT search logic. Fix by updating just the first matchingpg_constraint tuple, rather than all of them, since there should beonly one anyway. But since nobody has reproduced this failure on olderversions, no back-patch for now.Report and test case by Alex Hunsaker; tablecmds.c changes by me.
1 parentc980426 commitc0f03aa

File tree

3 files changed

+65
-51
lines changed

3 files changed

+65
-51
lines changed

‎src/backend/commands/tablecmds.c

Lines changed: 48 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -6734,6 +6734,7 @@ ATExecDropConstraint(Relation rel, const char *constrName,
67346734
{
67356735
Oidchildrelid=lfirst_oid(child);
67366736
Relationchildrel;
6737+
HeapTuplecopy_tuple;
67376738

67386739
/* find_inheritance_children already got lock */
67396740
childrel=heap_open(childrelid,NoLock);
@@ -6746,83 +6747,79 @@ ATExecDropConstraint(Relation rel, const char *constrName,
67466747
scan=systable_beginscan(conrel,ConstraintRelidIndexId,
67476748
true,SnapshotNow,1,&key);
67486749

6749-
found= false;
6750-
6750+
/* scan for matching tuple - there should only be one */
67516751
while (HeapTupleIsValid(tuple=systable_getnext(scan)))
67526752
{
6753-
HeapTuplecopy_tuple;
6754-
67556753
con= (Form_pg_constraint)GETSTRUCT(tuple);
67566754

67576755
/* Right now only CHECK constraints can be inherited */
67586756
if (con->contype!=CONSTRAINT_CHECK)
67596757
continue;
67606758

6761-
if (strcmp(NameStr(con->conname),constrName)!=0)
6762-
continue;
6759+
if (strcmp(NameStr(con->conname),constrName)==0)
6760+
break;
6761+
}
67636762

6764-
found= true;
6763+
if (!HeapTupleIsValid(tuple))
6764+
ereport(ERROR,
6765+
(errcode(ERRCODE_UNDEFINED_OBJECT),
6766+
errmsg("constraint \"%s\" of relation \"%s\" does not exist",
6767+
constrName,
6768+
RelationGetRelationName(childrel))));
67656769

6766-
if (con->coninhcount <=0)/* shouldn't happen */
6767-
elog(ERROR,"relation %u has non-inherited constraint \"%s\"",
6768-
childrelid,constrName);
6770+
copy_tuple=heap_copytuple(tuple);
67696771

6770-
copy_tuple=heap_copytuple(tuple);
6771-
con= (Form_pg_constraint)GETSTRUCT(copy_tuple);
6772+
systable_endscan(scan);
67726773

6773-
if (recurse)
6774-
{
6775-
/*
6776-
* If the child constraint has other definition sources, just
6777-
* decrement its inheritance count; if not, recurse to delete
6778-
* it.
6779-
*/
6780-
if (con->coninhcount==1&& !con->conislocal)
6781-
{
6782-
/* Time to delete this child constraint, too */
6783-
ATExecDropConstraint(childrel,constrName,behavior,
6784-
true, true,
6785-
false,lockmode);
6786-
}
6787-
else
6788-
{
6789-
/* Child constraint must survive my deletion */
6790-
con->coninhcount--;
6791-
simple_heap_update(conrel,&copy_tuple->t_self,copy_tuple);
6792-
CatalogUpdateIndexes(conrel,copy_tuple);
6774+
con= (Form_pg_constraint)GETSTRUCT(copy_tuple);
67936775

6794-
/* Make update visible */
6795-
CommandCounterIncrement();
6796-
}
6776+
if (con->coninhcount <=0)/* shouldn't happen */
6777+
elog(ERROR,"relation %u has non-inherited constraint \"%s\"",
6778+
childrelid,constrName);
6779+
6780+
if (recurse)
6781+
{
6782+
/*
6783+
* If the child constraint has other definition sources, just
6784+
* decrement its inheritance count; if not, recurse to delete
6785+
* it.
6786+
*/
6787+
if (con->coninhcount==1&& !con->conislocal)
6788+
{
6789+
/* Time to delete this child constraint, too */
6790+
ATExecDropConstraint(childrel,constrName,behavior,
6791+
true, true,
6792+
false,lockmode);
67976793
}
67986794
else
67996795
{
6800-
/*
6801-
* If we were told to drop ONLY in this table (no recursion),
6802-
* we need to mark the inheritors' constraints as locally
6803-
* defined rather than inherited.
6804-
*/
6796+
/* Child constraint must survive my deletion */
68056797
con->coninhcount--;
6806-
con->conislocal= true;
6807-
68086798
simple_heap_update(conrel,&copy_tuple->t_self,copy_tuple);
68096799
CatalogUpdateIndexes(conrel,copy_tuple);
68106800

68116801
/* Make update visible */
68126802
CommandCounterIncrement();
68136803
}
6814-
6815-
heap_freetuple(copy_tuple);
68166804
}
6805+
else
6806+
{
6807+
/*
6808+
* If we were told to drop ONLY in this table (no recursion),
6809+
* we need to mark the inheritors' constraints as locally
6810+
* defined rather than inherited.
6811+
*/
6812+
con->coninhcount--;
6813+
con->conislocal= true;
68176814

6818-
systable_endscan(scan);
6815+
simple_heap_update(conrel,&copy_tuple->t_self,copy_tuple);
6816+
CatalogUpdateIndexes(conrel,copy_tuple);
68196817

6820-
if (!found)
6821-
ereport(ERROR,
6822-
(errcode(ERRCODE_UNDEFINED_OBJECT),
6823-
errmsg("constraint \"%s\" of relation \"%s\" does not exist",
6824-
constrName,
6825-
RelationGetRelationName(childrel))));
6818+
/* Make update visible */
6819+
CommandCounterIncrement();
6820+
}
6821+
6822+
heap_freetuple(copy_tuple);
68266823

68276824
heap_close(childrel,NoLock);
68286825
}

‎src/test/regress/expected/alter_table.out

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2103,3 +2103,12 @@ ALTER TABLE tt7 NOT OF;
21032103
x | integer |
21042104
y | numeric(8,2) |
21052105

2106+
-- make sure we can drop a constraint on the parent but it remains on the child
2107+
CREATE TABLE test_drop_constr_parent (c text CHECK (c IS NOT NULL));
2108+
CREATE TABLE test_drop_constr_child () INHERITS (test_drop_constr_parent);
2109+
ALTER TABLE ONLY test_drop_constr_parent DROP CONSTRAINT "test_drop_constr_parent_c_check";
2110+
-- should fail
2111+
INSERT INTO test_drop_constr_child (c) VALUES (NULL);
2112+
ERROR: new row for relation "test_drop_constr_child" violates check constraint "test_drop_constr_parent_c_check"
2113+
DROP TABLE test_drop_constr_parent CASCADE;
2114+
NOTICE: drop cascades to table test_drop_constr_child

‎src/test/regress/sql/alter_table.sql

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1456,3 +1456,11 @@ CREATE TYPE tt_t1 AS (x int, y numeric(8,2));
14561456
ALTERTABLE tt7 OF tt_t1;-- reassign an already-typed table
14571457
ALTERTABLE tt7 NOT OF;
14581458
\d tt7
1459+
1460+
-- make sure we can drop a constraint on the parent but it remains on the child
1461+
CREATETABLEtest_drop_constr_parent (ctextCHECK (cIS NOT NULL));
1462+
CREATETABLEtest_drop_constr_child () INHERITS (test_drop_constr_parent);
1463+
ALTERTABLE ONLY test_drop_constr_parent DROPCONSTRAINT"test_drop_constr_parent_c_check";
1464+
-- should fail
1465+
INSERT INTO test_drop_constr_child (c)VALUES (NULL);
1466+
DROPTABLE test_drop_constr_parent CASCADE;

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp