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

Commit 4dff893

Browse files
alvherrealvherre
committed
Fix creation of duplicate foreign keys on partitions
When creating a foreign key in a partitioned table, if some partitions already have equivalent constraints, we wastefully create duplicates of the constraints instead of attaching to the existing ones. That's inconsistent with the de-duplication that is applied when a table is attached as a partition. To fix, reuse the FK-cloning code instead of having a separate code path. Backpatch to Postgres 11. This is a subtle behavior change, but surely a welcome one since there's no use in having duplicate foreign keys. Discovered by Álvaro Herrera while thinking about a different problem reported by Jesper Pedersen (bug #15587). Author: Álvaro Herrera Discussion: https://postgr.es/m/201901151935.zfadrzvyof4k@alvherre.pgsql
1 parent fca6cab commit 4dff893

File tree

3 files changed

+150
-10
lines changed

3 files changed

+150
-10
lines changed

‎src/backend/commands/tablecmds.c

Lines changed: 29 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -7751,30 +7751,49 @@ ATAddForeignKeyConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel,
77517751
if (recurse && rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
77527752
{
77537753
PartitionDesc partdesc;
7754+
Relation pg_constraint;
7755+
List *cloned = NIL;
7756+
ListCell *cell;
77547757

7758+
pg_constraint = heap_open(ConstraintRelationId, RowExclusiveLock);
77557759
partdesc = RelationGetPartitionDesc(rel);
77567760

77577761
for (i = 0; i < partdesc->nparts; i++)
77587762
{
77597763
Oid partitionId = partdesc->oids[i];
77607764
Relation partition = heap_open(partitionId, lockmode);
7761-
AlteredTableInfo *childtab;
7762-
ObjectAddress childAddr;
77637765

77647766
CheckTableNotInUse(partition, "ALTER TABLE");
77657767

7766-
/* Find or create work queue entry for this table */
7768+
CloneFkReferencing(pg_constraint, rel, partition,
7769+
list_make1_oid(constrOid),
7770+
&cloned);
7771+
7772+
heap_close(partition, NoLock);
7773+
}
7774+
heap_close(pg_constraint, RowExclusiveLock);
7775+
7776+
foreach(cell, cloned)
7777+
{
7778+
ClonedConstraint *cc = (ClonedConstraint *) lfirst(cell);
7779+
Relation partition = heap_open(cc->relid, lockmode);
7780+
AlteredTableInfo *childtab;
7781+
NewConstraint *newcon;
7782+
7783+
/* Find or create work queue entry for this partition */
77677784
childtab = ATGetQueueEntry(wqueue, partition);
77687785

7769-
childAddr =
7770-
ATAddForeignKeyConstraint(wqueue, childtab, partition,
7771-
fkconstraint, constrOid,
7772-
recurse, true, lockmode);
7786+
newcon = (NewConstraint *) palloc0(sizeof(NewConstraint));
7787+
newcon->name = cc->constraint->conname;
7788+
newcon->contype = CONSTR_FOREIGN;
7789+
newcon->refrelid = cc->refrelid;
7790+
newcon->refindid = cc->conindid;
7791+
newcon->conid = cc->conid;
7792+
newcon->qual = (Node *) fkconstraint;
77737793

7774-
/* Record this constraint as dependent on the parent one */
7775-
recordDependencyOn(&childAddr, &address, DEPENDENCY_INTERNAL_AUTO);
7794+
childtab->constraints = lappend(childtab->constraints, newcon);
77767795

7777-
heap_close(partition, NoLock);
7796+
heap_close(partition, lockmode);
77787797
}
77797798
}
77807799

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

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1781,3 +1781,86 @@ INSERT INTO fk_notpartitioned_pk VALUES (1600, 601), (1600, 1601);
17811781
ALTER TABLE fk_partitioned_fk ATTACH PARTITION fk_partitioned_fk_2
17821782
FOR VALUES IN (1600);
17831783
-- leave these tables around intentionally
1784+
-- Test creating a constraint at the parent that already exists in partitions.
1785+
-- There should be no duplicated constraints, and attempts to drop the
1786+
-- constraint in partitions should raise appropriate errors.
1787+
create schema fkpart0
1788+
create table pkey (a int primary key)
1789+
create table fk_part (a int) partition by list (a)
1790+
create table fk_part_1 partition of fk_part
1791+
(foreign key (a) references fkpart0.pkey) for values in (1)
1792+
create table fk_part_23 partition of fk_part
1793+
(foreign key (a) references fkpart0.pkey) for values in (2, 3)
1794+
partition by list (a)
1795+
create table fk_part_23_2 partition of fk_part_23 for values in (2);
1796+
alter table fkpart0.fk_part add foreign key (a) references fkpart0.pkey;
1797+
\d fkpart0.fk_part_1 \\ -- should have only one FK
1798+
Table "fkpart0.fk_part_1"
1799+
Column | Type | Collation | Nullable | Default
1800+
--------+---------+-----------+----------+---------
1801+
a | integer | | |
1802+
Partition of: fkpart0.fk_part FOR VALUES IN (1)
1803+
Foreign-key constraints:
1804+
"fk_part_1_a_fkey" FOREIGN KEY (a) REFERENCES fkpart0.pkey(a)
1805+
1806+
alter table fkpart0.fk_part_1 drop constraint fk_part_1_a_fkey;
1807+
ERROR: cannot drop inherited constraint "fk_part_1_a_fkey" of relation "fk_part_1"
1808+
\d fkpart0.fk_part_23 \\ -- should have only one FK
1809+
Table "fkpart0.fk_part_23"
1810+
Column | Type | Collation | Nullable | Default
1811+
--------+---------+-----------+----------+---------
1812+
a | integer | | |
1813+
Partition of: fkpart0.fk_part FOR VALUES IN (2, 3)
1814+
Partition key: LIST (a)
1815+
Foreign-key constraints:
1816+
"fk_part_23_a_fkey" FOREIGN KEY (a) REFERENCES fkpart0.pkey(a)
1817+
Number of partitions: 1 (Use \d+ to list them.)
1818+
1819+
\d fkpart0.fk_part_23_2 \\ -- should have only one FK
1820+
Table "fkpart0.fk_part_23_2"
1821+
Column | Type | Collation | Nullable | Default
1822+
--------+---------+-----------+----------+---------
1823+
a | integer | | |
1824+
Partition of: fkpart0.fk_part_23 FOR VALUES IN (2)
1825+
Foreign-key constraints:
1826+
"fk_part_23_a_fkey" FOREIGN KEY (a) REFERENCES fkpart0.pkey(a)
1827+
1828+
alter table fkpart0.fk_part_23 drop constraint fk_part_23_a_fkey;
1829+
ERROR: cannot drop inherited constraint "fk_part_23_a_fkey" of relation "fk_part_23"
1830+
alter table fkpart0.fk_part_23_2 drop constraint fk_part_23_a_fkey;
1831+
ERROR: cannot drop inherited constraint "fk_part_23_a_fkey" of relation "fk_part_23_2"
1832+
create table fkpart0.fk_part_4 partition of fkpart0.fk_part for values in (4);
1833+
\d fkpart0.fk_part_4
1834+
Table "fkpart0.fk_part_4"
1835+
Column | Type | Collation | Nullable | Default
1836+
--------+---------+-----------+----------+---------
1837+
a | integer | | |
1838+
Partition of: fkpart0.fk_part FOR VALUES IN (4)
1839+
Foreign-key constraints:
1840+
"fk_part_a_fkey" FOREIGN KEY (a) REFERENCES fkpart0.pkey(a)
1841+
1842+
alter table fkpart0.fk_part_4 drop constraint fk_part_a_fkey;
1843+
ERROR: cannot drop inherited constraint "fk_part_a_fkey" of relation "fk_part_4"
1844+
create table fkpart0.fk_part_56 partition of fkpart0.fk_part
1845+
for values in (5,6) partition by list (a);
1846+
create table fkpart0.fk_part_56_5 partition of fkpart0.fk_part_56
1847+
for values in (5);
1848+
\d fkpart0.fk_part_56
1849+
Table "fkpart0.fk_part_56"
1850+
Column | Type | Collation | Nullable | Default
1851+
--------+---------+-----------+----------+---------
1852+
a | integer | | |
1853+
Partition of: fkpart0.fk_part FOR VALUES IN (5, 6)
1854+
Partition key: LIST (a)
1855+
Foreign-key constraints:
1856+
"fk_part_a_fkey" FOREIGN KEY (a) REFERENCES fkpart0.pkey(a)
1857+
Number of partitions: 1 (Use \d+ to list them.)
1858+
1859+
alter table fkpart0.fk_part_56 drop constraint fk_part_a_fkey;
1860+
ERROR: cannot drop inherited constraint "fk_part_a_fkey" of relation "fk_part_56"
1861+
alter table fkpart0.fk_part_56_5 drop constraint fk_part_a_fkey;
1862+
ERROR: cannot drop inherited constraint "fk_part_a_fkey" of relation "fk_part_56_5"
1863+
\set VERBOSITY terse \\ -- suppress cascade details
1864+
drop schema fkpart0 cascade;
1865+
NOTICE: drop cascades to 2 other objects
1866+
\set VERBOSITY default

‎src/test/regress/sql/foreign_key.sql

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1289,3 +1289,41 @@ ALTER TABLE fk_partitioned_fk ATTACH PARTITION fk_partitioned_fk_2
12891289
FOR VALUES IN (1600);
12901290

12911291
-- leave these tables around intentionally
1292+
1293+
-- Test creating a constraint at the parent that already exists in partitions.
1294+
-- There should be no duplicated constraints, and attempts to drop the
1295+
-- constraint in partitions should raise appropriate errors.
1296+
create schema fkpart0
1297+
create table pkey (a int primary key)
1298+
create table fk_part (a int) partition by list (a)
1299+
create table fk_part_1 partition of fk_part
1300+
(foreign key (a) references fkpart0.pkey) for values in (1)
1301+
create table fk_part_23 partition of fk_part
1302+
(foreign key (a) references fkpart0.pkey) for values in (2, 3)
1303+
partition by list (a)
1304+
create table fk_part_23_2 partition of fk_part_23 for values in (2);
1305+
1306+
alter table fkpart0.fk_part add foreign key (a) references fkpart0.pkey;
1307+
\d fkpart0.fk_part_1 \\ -- should have only one FK
1308+
alter table fkpart0.fk_part_1 drop constraint fk_part_1_a_fkey;
1309+
1310+
\d fkpart0.fk_part_23 \\ -- should have only one FK
1311+
\d fkpart0.fk_part_23_2 \\ -- should have only one FK
1312+
alter table fkpart0.fk_part_23 drop constraint fk_part_23_a_fkey;
1313+
alter table fkpart0.fk_part_23_2 drop constraint fk_part_23_a_fkey;
1314+
1315+
create table fkpart0.fk_part_4 partition of fkpart0.fk_part for values in (4);
1316+
\d fkpart0.fk_part_4
1317+
alter table fkpart0.fk_part_4 drop constraint fk_part_a_fkey;
1318+
1319+
create table fkpart0.fk_part_56 partition of fkpart0.fk_part
1320+
for values in (5,6) partition by list (a);
1321+
create table fkpart0.fk_part_56_5 partition of fkpart0.fk_part_56
1322+
for values in (5);
1323+
\d fkpart0.fk_part_56
1324+
alter table fkpart0.fk_part_56 drop constraint fk_part_a_fkey;
1325+
alter table fkpart0.fk_part_56_5 drop constraint fk_part_a_fkey;
1326+
1327+
\set VERBOSITY terse \\ -- suppress cascade details
1328+
drop schema fkpart0 cascade;
1329+
\set VERBOSITY default

0 commit comments

Comments
 (0)