Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 5128b98

Browse files
EDB-AmulSulCommitfest Bot
authored and
Commitfest Bot
committed
Allow NOT VALID foreign key constraints on partitioned tables.
When merging an existing foreign key during the addition or attachment of a partition, consider the following: 1. If the parent foreign key constraint being added is NOT VALID, the validity flag of the matching child constraint is ignored. It is harmless for the child constraint to remain valid. 2. If the parent foreign key constraint being added is VALID and the matching child constraint is NOT VALID, the we implicitly validates the child constraint, marking it as valid. This behavior is consistent with creating a new constraint on the child table rather than attaching it to an existing one, as it ensures the child data is validated.
1 parent 991974b commit 5128b98

File tree

4 files changed

+259
-48
lines changed

4 files changed

+259
-48
lines changed

doc/src/sgml/ref/alter_table.sgml

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -486,8 +486,6 @@ WITH ( MODULUS <replaceable class="parameter">numeric_literal</replaceable>, REM
486486
<para>
487487
Additional restrictions apply when unique or primary key constraints
488488
are added to partitioned tables; see <link linkend="sql-createtable"><command>CREATE TABLE</command></link>.
489-
Also, foreign key constraints on partitioned
490-
tables may not be declared <literal>NOT VALID</literal> at present.
491489
</para>
492490

493491
</listitem>

src/backend/commands/tablecmds.c

Lines changed: 100 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -574,8 +574,9 @@ static void createForeignKeyActionTriggers(Relation rel, Oid refRelOid,
574574
Oid indexOid,
575575
Oid parentDelTrigger, Oid parentUpdTrigger,
576576
Oid *deleteTrigOid, Oid *updateTrigOid);
577-
static bool tryAttachPartitionForeignKey(ForeignKeyCacheInfo *fk,
578-
Oid partRelid,
577+
static bool tryAttachPartitionForeignKey(List **wqueue,
578+
ForeignKeyCacheInfo *fk,
579+
Relation partition,
579580
Oid parentConstrOid, int numfks,
580581
AttrNumber *mapped_conkey, AttrNumber *confkey,
581582
Oid *conpfeqop,
@@ -9772,22 +9773,12 @@ ATAddForeignKeyConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel,
97729773
* Validity checks (permission checks wait till we have the column
97739774
* numbers)
97749775
*/
9775-
if (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
9776-
{
9777-
if (!recurse)
9778-
ereport(ERROR,
9779-
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
9780-
errmsg("cannot use ONLY for foreign key on partitioned table \"%s\" referencing relation \"%s\"",
9781-
RelationGetRelationName(rel),
9782-
RelationGetRelationName(pkrel))));
9783-
if (fkconstraint->skip_validation && !fkconstraint->initially_valid)
9784-
ereport(ERROR,
9785-
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
9786-
errmsg("cannot add NOT VALID foreign key on partitioned table \"%s\" referencing relation \"%s\"",
9787-
RelationGetRelationName(rel),
9788-
RelationGetRelationName(pkrel)),
9789-
errdetail("This feature is not yet supported on partitioned tables.")));
9790-
}
9776+
if (!recurse && rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
9777+
ereport(ERROR,
9778+
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
9779+
errmsg("cannot use ONLY for foreign key on partitioned table \"%s\" referencing relation \"%s\"",
9780+
RelationGetRelationName(rel),
9781+
RelationGetRelationName(pkrel))));
97919782

97929783
if (pkrel->rd_rel->relkind != RELKIND_RELATION &&
97939784
pkrel->rd_rel->relkind != RELKIND_PARTITIONED_TABLE)
@@ -10782,8 +10773,7 @@ addFkRecurseReferencing(List **wqueue, Constraint *fkconstraint, Relation rel,
1078210773
*/
1078310774
for (int i = 0; i < pd->nparts; i++)
1078410775
{
10785-
Oid partitionId = pd->oids[i];
10786-
Relation partition = table_open(partitionId, lockmode);
10776+
Relation partition = table_open(pd->oids[i], lockmode);
1078710777
List *partFKs;
1078810778
AttrMap *attmap;
1078910779
AttrNumber mapped_fkattnum[INDEX_MAX_KEYS];
@@ -10807,8 +10797,9 @@ addFkRecurseReferencing(List **wqueue, Constraint *fkconstraint, Relation rel,
1080710797
ForeignKeyCacheInfo *fk;
1080810798

1080910799
fk = lfirst_node(ForeignKeyCacheInfo, cell);
10810-
if (tryAttachPartitionForeignKey(fk,
10811-
partitionId,
10800+
if (tryAttachPartitionForeignKey(wqueue,
10801+
fk,
10802+
partition,
1081210803
parentConstr,
1081310804
numfks,
1081410805
mapped_fkattnum,
@@ -11260,8 +11251,9 @@ CloneFkReferencing(List **wqueue, Relation parentRel, Relation partRel)
1126011251
{
1126111252
ForeignKeyCacheInfo *fk = lfirst_node(ForeignKeyCacheInfo, lc);
1126211253

11263-
if (tryAttachPartitionForeignKey(fk,
11264-
RelationGetRelid(partRel),
11254+
if (tryAttachPartitionForeignKey(wqueue,
11255+
fk,
11256+
partRel,
1126511257
parentConstrOid,
1126611258
numfks,
1126711259
mapped_conkey,
@@ -11364,8 +11356,9 @@ CloneFkReferencing(List **wqueue, Relation parentRel, Relation partRel)
1136411356
* return false.
1136511357
*/
1136611358
static bool
11367-
tryAttachPartitionForeignKey(ForeignKeyCacheInfo *fk,
11368-
Oid partRelid,
11359+
tryAttachPartitionForeignKey(List **wqueue,
11360+
ForeignKeyCacheInfo *fk,
11361+
Relation partition,
1136911362
Oid parentConstrOid,
1137011363
int numfks,
1137111364
AttrNumber *mapped_conkey,
@@ -11384,12 +11377,15 @@ tryAttachPartitionForeignKey(ForeignKeyCacheInfo *fk,
1138411377
HeapTuple trigtup;
1138511378
Oid insertTriggerOid,
1138611379
updateTriggerOid;
11380+
bool parentConstrIsValid;
11381+
bool partConstrIsValid;
1138711382

1138811383
parentConstrTup = SearchSysCache1(CONSTROID,
1138911384
ObjectIdGetDatum(parentConstrOid));
1139011385
if (!HeapTupleIsValid(parentConstrTup))
1139111386
elog(ERROR, "cache lookup failed for constraint %u", parentConstrOid);
1139211387
parentConstr = (Form_pg_constraint) GETSTRUCT(parentConstrTup);
11388+
parentConstrIsValid = parentConstr->convalidated;
1139311389

1139411390
/*
1139511391
* Do some quick & easy initial checks. If any of these fail, we cannot
@@ -11412,17 +11408,18 @@ tryAttachPartitionForeignKey(ForeignKeyCacheInfo *fk,
1141211408
}
1141311409

1141411410
/*
11415-
* Looks good so far; do some more extensive checks. Presumably the check
11416-
* for 'convalidated' could be dropped, since we don't really care about
11417-
* that, but let's be careful for now.
11411+
* Looks good so far; perform more extensive checks except for
11412+
* convalidated. There's no need to worry about it because attaching a
11413+
* valid parent constraint to an invalid child constraint will eventually
11414+
* trigger implicit data validation for the child.
1141811415
*/
1141911416
partcontup = SearchSysCache1(CONSTROID,
1142011417
ObjectIdGetDatum(fk->conoid));
1142111418
if (!HeapTupleIsValid(partcontup))
1142211419
elog(ERROR, "cache lookup failed for constraint %u", fk->conoid);
1142311420
partConstr = (Form_pg_constraint) GETSTRUCT(partcontup);
11421+
partConstrIsValid = partConstr->convalidated;
1142411422
if (OidIsValid(partConstr->conparentid) ||
11425-
!partConstr->convalidated ||
1142611423
partConstr->condeferrable != parentConstr->condeferrable ||
1142711424
partConstr->condeferred != parentConstr->condeferred ||
1142811425
partConstr->confupdtype != parentConstr->confupdtype ||
@@ -11481,7 +11478,8 @@ tryAttachPartitionForeignKey(ForeignKeyCacheInfo *fk,
1148111478

1148211479
systable_endscan(scan);
1148311480

11484-
ConstraintSetParentConstraint(fk->conoid, parentConstrOid, partRelid);
11481+
ConstraintSetParentConstraint(fk->conoid, parentConstrOid,
11482+
RelationGetRelid(partition));
1148511483

1148611484
/*
1148711485
* Like the constraint, attach partition's "check" triggers to the
@@ -11492,10 +11490,10 @@ tryAttachPartitionForeignKey(ForeignKeyCacheInfo *fk,
1149211490
&insertTriggerOid, &updateTriggerOid);
1149311491
Assert(OidIsValid(insertTriggerOid) && OidIsValid(parentInsTrigger));
1149411492
TriggerSetParentTrigger(trigrel, insertTriggerOid, parentInsTrigger,
11495-
partRelid);
11493+
RelationGetRelid(partition));
1149611494
Assert(OidIsValid(updateTriggerOid) && OidIsValid(parentUpdTrigger));
1149711495
TriggerSetParentTrigger(trigrel, updateTriggerOid, parentUpdTrigger,
11498-
partRelid);
11496+
RelationGetRelid(partition));
1149911497

1150011498
/*
1150111499
* If the referenced table is partitioned, then the partition we're
@@ -11573,6 +11571,32 @@ tryAttachPartitionForeignKey(ForeignKeyCacheInfo *fk,
1157311571
}
1157411572

1157511573
CommandCounterIncrement();
11574+
11575+
/*
11576+
* If a valid parent constraint is attached to a NOT VALID child
11577+
* constraint, implicitly trigger validation for the child constraint.
11578+
* This behavior aligns with creating a new constraint on the child table
11579+
* rather than attaching it to the existing one, as it would ultimately
11580+
* validate the child data. Conversely, having an invalid parent
11581+
* constraint while the child constraint is valid doesn't cause any harm.
11582+
*/
11583+
if (parentConstrIsValid && !partConstrIsValid)
11584+
{
11585+
Relation conrel;
11586+
11587+
conrel = table_open(ConstraintRelationId, RowExclusiveLock);
11588+
11589+
partcontup = SearchSysCache1(CONSTROID, ObjectIdGetDatum(fk->conoid));
11590+
if (!HeapTupleIsValid(partcontup))
11591+
elog(ERROR, "cache lookup failed for constraint %u", fk->conoid);
11592+
11593+
/* Using the same lock as used in AT_ValidateConstraint */
11594+
QueueFKConstraintValidation(wqueue, conrel, partition, partcontup,
11595+
ShareUpdateExclusiveLock);
11596+
ReleaseSysCache(partcontup);
11597+
table_close(conrel, RowExclusiveLock);
11598+
}
11599+
1157611600
return true;
1157711601
}
1157811602

@@ -12113,7 +12137,7 @@ ATExecValidateConstraint(List **wqueue, Relation rel, char *constrName,
1211312137
*
1211412138
* Add an entry to the wqueue to validate the given foreign key constraint in
1211512139
* Phase 3 and update the convalidated field in the pg_constraint catalog
12116-
* for the specified relation.
12140+
* for the specified relation and all its children.
1211712141
*/
1211812142
static void
1211912143
QueueFKConstraintValidation(List **wqueue, Relation conrel, Relation rel,
@@ -12126,6 +12150,7 @@ QueueFKConstraintValidation(List **wqueue, Relation conrel, Relation rel,
1212612150

1212712151
con = (Form_pg_constraint) GETSTRUCT(contuple);
1212812152
Assert(con->contype == CONSTRAINT_FOREIGN);
12153+
Assert(!con->convalidated);
1212912154

1213012155
if (rel->rd_rel->relkind == RELKIND_RELATION)
1213112156
{
@@ -12151,9 +12176,48 @@ QueueFKConstraintValidation(List **wqueue, Relation conrel, Relation rel,
1215112176
}
1215212177

1215312178
/*
12154-
* We disallow creating invalid foreign keys to or from partitioned
12155-
* tables, so ignoring the recursion bit is okay.
12179+
* If the table at either end of the constraint is partitioned, we need to
12180+
* recurse and handle every constraint that is a child of this constraint.
1215612181
*/
12182+
if (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE ||
12183+
get_rel_relkind(con->confrelid) == RELKIND_PARTITIONED_TABLE)
12184+
{
12185+
ScanKeyData pkey;
12186+
SysScanDesc pscan;
12187+
HeapTuple childtup;
12188+
12189+
ScanKeyInit(&pkey,
12190+
Anum_pg_constraint_conparentid,
12191+
BTEqualStrategyNumber, F_OIDEQ,
12192+
ObjectIdGetDatum(con->oid));
12193+
12194+
pscan = systable_beginscan(conrel, ConstraintParentIndexId,
12195+
true, NULL, 1, &pkey);
12196+
12197+
while (HeapTupleIsValid(childtup = systable_getnext(pscan)))
12198+
{
12199+
Form_pg_constraint childcon;
12200+
Relation childrel;
12201+
12202+
childcon = (Form_pg_constraint) GETSTRUCT(childtup);
12203+
12204+
/*
12205+
* If the child constraint has already been validated, no further
12206+
* action is required for it or its descendants, as they are all
12207+
* valid.
12208+
*/
12209+
if (childcon->convalidated)
12210+
continue;
12211+
12212+
childrel = table_open(childcon->conrelid, lockmode);
12213+
12214+
QueueFKConstraintValidation(wqueue, conrel, childrel, childtup,
12215+
lockmode);
12216+
table_close(childrel, NoLock);
12217+
}
12218+
12219+
systable_endscan(pscan);
12220+
}
1215712221

1215812222
/*
1215912223
* Now update the catalog, while we have the door open.

src/test/regress/expected/foreign_key.out

Lines changed: 91 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1571,6 +1571,97 @@ drop table pktable2, fktable2;
15711571
--
15721572
-- Foreign keys and partitioned tables
15731573
--
1574+
-- NOT VALID foreign key on partitioned table
1575+
CREATE TABLE fk_notpartitioned_pk (a int, b int, PRIMARY KEY (a, b));
1576+
CREATE TABLE fk_partitioned_fk (b int, a int) PARTITION BY RANGE (a, b);
1577+
ALTER TABLE fk_partitioned_fk ADD FOREIGN KEY (a, b) REFERENCES fk_notpartitioned_pk NOT VALID;
1578+
-- Attaching a child table with the same valid foreign key constraint.
1579+
CREATE TABLE fk_partitioned_fk_1 (a int, b int);
1580+
ALTER TABLE fk_partitioned_fk_1 ADD FOREIGN KEY (a, b) REFERENCES fk_notpartitioned_pk;
1581+
ALTER TABLE fk_partitioned_fk ATTACH PARTITION fk_partitioned_fk_1 FOR VALUES FROM (0,0) TO (1000,1000);
1582+
-- Child constraint will remain valid.
1583+
SELECT conname, convalidated, conrelid::regclass FROM pg_constraint
1584+
WHERE conrelid::regclass::text like 'fk_partitioned_fk%' ORDER BY oid;
1585+
conname | convalidated | conrelid
1586+
------------------------------+--------------+---------------------
1587+
fk_partitioned_fk_a_b_fkey | f | fk_partitioned_fk
1588+
fk_partitioned_fk_1_a_b_fkey | t | fk_partitioned_fk_1
1589+
(2 rows)
1590+
1591+
-- Validate the constraint
1592+
ALTER TABLE fk_partitioned_fk VALIDATE CONSTRAINT fk_partitioned_fk_a_b_fkey;
1593+
-- All constraints are now valid.
1594+
SELECT conname, convalidated, conrelid::regclass FROM pg_constraint
1595+
WHERE conrelid::regclass::text like 'fk_partitioned_fk%' ORDER BY oid;
1596+
conname | convalidated | conrelid
1597+
------------------------------+--------------+---------------------
1598+
fk_partitioned_fk_a_b_fkey | t | fk_partitioned_fk
1599+
fk_partitioned_fk_1_a_b_fkey | t | fk_partitioned_fk_1
1600+
(2 rows)
1601+
1602+
-- Attaching a child with a NOT VALID constraint.
1603+
CREATE TABLE fk_partitioned_fk_2 (a int, b int);
1604+
INSERT INTO fk_partitioned_fk_2 VALUES(1000, 1000); -- doesn't exist in referenced table
1605+
ALTER TABLE fk_partitioned_fk_2 ADD FOREIGN KEY (a, b) REFERENCES fk_notpartitioned_pk NOT VALID;
1606+
-- It will fail because the attach operation implicitly validates the data.
1607+
ALTER TABLE fk_partitioned_fk ATTACH PARTITION fk_partitioned_fk_2 FOR VALUES FROM (1000,1000) TO (2000,2000);
1608+
ERROR: insert or update on table "fk_partitioned_fk_2" violates foreign key constraint "fk_partitioned_fk_2_a_b_fkey"
1609+
DETAIL: Key (a, b)=(1000, 1000) is not present in table "fk_notpartitioned_pk".
1610+
-- Remove the invalid data and try again.
1611+
TRUNCATE fk_partitioned_fk_2;
1612+
ALTER TABLE fk_partitioned_fk ATTACH PARTITION fk_partitioned_fk_2 FOR VALUES FROM (1000,1000) TO (2000,2000);
1613+
-- The child constraint will also be valid.
1614+
SELECT conname, convalidated FROM pg_constraint WHERE conrelid = 'fk_partitioned_fk_2'::regclass;
1615+
conname | convalidated
1616+
------------------------------+--------------
1617+
fk_partitioned_fk_2_a_b_fkey | t
1618+
(1 row)
1619+
1620+
-- Test case where the child constraint is invalid, the grandchild constraint
1621+
-- is valid, and the validation for the grandchild should be skipped when a
1622+
-- valid constraint is applied to the top parent.
1623+
CREATE TABLE fk_partitioned_fk_3 (a int, b int) PARTITION BY RANGE (a, b);
1624+
ALTER TABLE fk_partitioned_fk_3 ADD FOREIGN KEY (a, b) REFERENCES fk_notpartitioned_pk NOT VALID;
1625+
CREATE TABLE fk_partitioned_fk_3_1 (a int, b int);
1626+
ALTER TABLE fk_partitioned_fk_3_1 ADD FOREIGN KEY (a, b) REFERENCES fk_notpartitioned_pk;
1627+
ALTER TABLE fk_partitioned_fk_3 ATTACH PARTITION fk_partitioned_fk_3_1 FOR VALUES FROM (2000,2000) TO (3000,3000);
1628+
ALTER TABLE fk_partitioned_fk ATTACH PARTITION fk_partitioned_fk_3 FOR VALUES FROM (2000,2000) TO (3000,3000);
1629+
-- All constraints are now valid.
1630+
SELECT conname, convalidated, conrelid::regclass FROM pg_constraint
1631+
WHERE conrelid::regclass::text like 'fk_partitioned_fk%' ORDER BY oid;
1632+
conname | convalidated | conrelid
1633+
--------------------------------+--------------+-----------------------
1634+
fk_partitioned_fk_a_b_fkey | t | fk_partitioned_fk
1635+
fk_partitioned_fk_1_a_b_fkey | t | fk_partitioned_fk_1
1636+
fk_partitioned_fk_2_a_b_fkey | t | fk_partitioned_fk_2
1637+
fk_partitioned_fk_3_a_b_fkey | t | fk_partitioned_fk_3
1638+
fk_partitioned_fk_3_1_a_b_fkey | t | fk_partitioned_fk_3_1
1639+
(5 rows)
1640+
1641+
DROP TABLE fk_partitioned_fk, fk_notpartitioned_pk;
1642+
-- NOT VALID foreign key on a non-partitioned table referencing a partitioned table
1643+
CREATE TABLE fk_partitioned_pk (a int, b int, PRIMARY KEY (a, b)) PARTITION BY RANGE (a, b);
1644+
CREATE TABLE fk_partitioned_pk_1 PARTITION OF fk_partitioned_pk FOR VALUES FROM (0,0) TO (1000,1000);
1645+
CREATE TABLE fk_notpartitioned_fk (b int, a int);
1646+
ALTER TABLE fk_notpartitioned_fk ADD FOREIGN KEY (a, b) REFERENCES fk_partitioned_pk NOT VALID;
1647+
-- Constraint will be invalid.
1648+
SELECT conname, convalidated FROM pg_constraint WHERE conrelid = 'fk_notpartitioned_fk'::regclass;
1649+
conname | convalidated
1650+
--------------------------------+--------------
1651+
fk_notpartitioned_fk_a_b_fkey | f
1652+
fk_notpartitioned_fk_a_b_fkey1 | f
1653+
(2 rows)
1654+
1655+
ALTER TABLE fk_notpartitioned_fk VALIDATE CONSTRAINT fk_notpartitioned_fk_a_b_fkey;
1656+
-- All constraints are now valid.
1657+
SELECT conname, convalidated FROM pg_constraint WHERE conrelid = 'fk_notpartitioned_fk'::regclass;
1658+
conname | convalidated
1659+
--------------------------------+--------------
1660+
fk_notpartitioned_fk_a_b_fkey | t
1661+
fk_notpartitioned_fk_a_b_fkey1 | t
1662+
(2 rows)
1663+
1664+
DROP TABLE fk_notpartitioned_fk, fk_partitioned_pk;
15741665
-- Creation of a partitioned hierarchy with irregular definitions
15751666
CREATE TABLE fk_notpartitioned_pk (fdrop1 int, a int, fdrop2 int, b int,
15761667
PRIMARY KEY (a, b));
@@ -1597,12 +1688,6 @@ ALTER TABLE fk_partitioned_fk ATTACH PARTITION fk_partitioned_fk_3
15971688
ALTER TABLE ONLY fk_partitioned_fk ADD FOREIGN KEY (a, b)
15981689
REFERENCES fk_notpartitioned_pk;
15991690
ERROR: cannot use ONLY for foreign key on partitioned table "fk_partitioned_fk" referencing relation "fk_notpartitioned_pk"
1600-
-- Adding a NOT VALID foreign key on a partitioned table referencing
1601-
-- a non-partitioned table fails.
1602-
ALTER TABLE fk_partitioned_fk ADD FOREIGN KEY (a, b)
1603-
REFERENCES fk_notpartitioned_pk NOT VALID;
1604-
ERROR: cannot add NOT VALID foreign key on partitioned table "fk_partitioned_fk" referencing relation "fk_notpartitioned_pk"
1605-
DETAIL: This feature is not yet supported on partitioned tables.
16061691
-- these inserts, targeting both the partition directly as well as the
16071692
-- partitioned table, should all fail
16081693
INSERT INTO fk_partitioned_fk (a,b) VALUES (500, 501);

0 commit comments

Comments
 (0)