Skip to content

Commit 0325d7a

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 03afae2 commit 0325d7a

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
@@ -7658,30 +7658,49 @@ ATAddForeignKeyConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel,
76587658
if (recurse && rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
76597659
{
76607660
PartitionDesc partdesc;
7661+
Relation pg_constraint;
7662+
List *cloned = NIL;
7663+
ListCell *cell;
76617664

7665+
pg_constraint = heap_open(ConstraintRelationId, RowExclusiveLock);
76627666
partdesc = RelationGetPartitionDesc(rel);
76637667

76647668
for (i = 0; i < partdesc->nparts; i++)
76657669
{
76667670
Oid partitionId = partdesc->oids[i];
76677671
Relation partition = heap_open(partitionId, lockmode);
7668-
AlteredTableInfo *childtab;
7669-
ObjectAddress childAddr;
76707672

76717673
CheckTableNotInUse(partition, "ALTER TABLE");
76727674

7673-
/* Find or create work queue entry for this table */
7675+
CloneFkReferencing(pg_constraint, rel, partition,
7676+
list_make1_oid(constrOid),
7677+
&cloned);
7678+
7679+
heap_close(partition, NoLock);
7680+
}
7681+
heap_close(pg_constraint, RowExclusiveLock);
7682+
7683+
foreach(cell, cloned)
7684+
{
7685+
ClonedConstraint *cc = (ClonedConstraint *) lfirst(cell);
7686+
Relation partition = heap_open(cc->relid, lockmode);
7687+
AlteredTableInfo *childtab;
7688+
NewConstraint *newcon;
7689+
7690+
/* Find or create work queue entry for this partition */
76747691
childtab = ATGetQueueEntry(wqueue, partition);
76757692

7676-
childAddr =
7677-
ATAddForeignKeyConstraint(wqueue, childtab, partition,
7678-
fkconstraint, constrOid,
7679-
recurse, true, lockmode);
7693+
newcon = (NewConstraint *) palloc0(sizeof(NewConstraint));
7694+
newcon->name = cc->constraint->conname;
7695+
newcon->contype = CONSTR_FOREIGN;
7696+
newcon->refrelid = cc->refrelid;
7697+
newcon->refindid = cc->conindid;
7698+
newcon->conid = cc->conid;
7699+
newcon->qual = (Node *) fkconstraint;
76807700

7681-
/* Record this constraint as dependent on the parent one */
7682-
recordDependencyOn(&childAddr, &address, DEPENDENCY_INTERNAL_AUTO);
7701+
childtab->constraints = lappend(childtab->constraints, newcon);
76837702

7684-
heap_close(partition, NoLock);
7703+
heap_close(partition, lockmode);
76857704
}
76867705
}
76877706

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

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1842,3 +1842,86 @@ INSERT INTO fk_notpartitioned_pk VALUES (1600, 601), (1600, 1601);
18421842
ALTER TABLE fk_partitioned_fk ATTACH PARTITION fk_partitioned_fk_2
18431843
FOR VALUES IN (1600);
18441844
-- leave these tables around intentionally
1845+
-- Test creating a constraint at the parent that already exists in partitions.
1846+
-- There should be no duplicated constraints, and attempts to drop the
1847+
-- constraint in partitions should raise appropriate errors.
1848+
create schema fkpart0
1849+
create table pkey (a int primary key)
1850+
create table fk_part (a int) partition by list (a)
1851+
create table fk_part_1 partition of fk_part
1852+
(foreign key (a) references fkpart0.pkey) for values in (1)
1853+
create table fk_part_23 partition of fk_part
1854+
(foreign key (a) references fkpart0.pkey) for values in (2, 3)
1855+
partition by list (a)
1856+
create table fk_part_23_2 partition of fk_part_23 for values in (2);
1857+
alter table fkpart0.fk_part add foreign key (a) references fkpart0.pkey;
1858+
\d fkpart0.fk_part_1 \\ -- should have only one FK
1859+
Table "fkpart0.fk_part_1"
1860+
Column | Type | Collation | Nullable | Default
1861+
--------+---------+-----------+----------+---------
1862+
a | integer | | |
1863+
Partition of: fkpart0.fk_part FOR VALUES IN (1)
1864+
Foreign-key constraints:
1865+
"fk_part_1_a_fkey" FOREIGN KEY (a) REFERENCES fkpart0.pkey(a)
1866+
1867+
alter table fkpart0.fk_part_1 drop constraint fk_part_1_a_fkey;
1868+
ERROR: cannot drop inherited constraint "fk_part_1_a_fkey" of relation "fk_part_1"
1869+
\d fkpart0.fk_part_23 \\ -- should have only one FK
1870+
Partitioned table "fkpart0.fk_part_23"
1871+
Column | Type | Collation | Nullable | Default
1872+
--------+---------+-----------+----------+---------
1873+
a | integer | | |
1874+
Partition of: fkpart0.fk_part FOR VALUES IN (2, 3)
1875+
Partition key: LIST (a)
1876+
Foreign-key constraints:
1877+
"fk_part_23_a_fkey" FOREIGN KEY (a) REFERENCES fkpart0.pkey(a)
1878+
Number of partitions: 1 (Use \d+ to list them.)
1879+
1880+
\d fkpart0.fk_part_23_2 \\ -- should have only one FK
1881+
Table "fkpart0.fk_part_23_2"
1882+
Column | Type | Collation | Nullable | Default
1883+
--------+---------+-----------+----------+---------
1884+
a | integer | | |
1885+
Partition of: fkpart0.fk_part_23 FOR VALUES IN (2)
1886+
Foreign-key constraints:
1887+
"fk_part_23_a_fkey" FOREIGN KEY (a) REFERENCES fkpart0.pkey(a)
1888+
1889+
alter table fkpart0.fk_part_23 drop constraint fk_part_23_a_fkey;
1890+
ERROR: cannot drop inherited constraint "fk_part_23_a_fkey" of relation "fk_part_23"
1891+
alter table fkpart0.fk_part_23_2 drop constraint fk_part_23_a_fkey;
1892+
ERROR: cannot drop inherited constraint "fk_part_23_a_fkey" of relation "fk_part_23_2"
1893+
create table fkpart0.fk_part_4 partition of fkpart0.fk_part for values in (4);
1894+
\d fkpart0.fk_part_4
1895+
Table "fkpart0.fk_part_4"
1896+
Column | Type | Collation | Nullable | Default
1897+
--------+---------+-----------+----------+---------
1898+
a | integer | | |
1899+
Partition of: fkpart0.fk_part FOR VALUES IN (4)
1900+
Foreign-key constraints:
1901+
"fk_part_a_fkey" FOREIGN KEY (a) REFERENCES fkpart0.pkey(a)
1902+
1903+
alter table fkpart0.fk_part_4 drop constraint fk_part_a_fkey;
1904+
ERROR: cannot drop inherited constraint "fk_part_a_fkey" of relation "fk_part_4"
1905+
create table fkpart0.fk_part_56 partition of fkpart0.fk_part
1906+
for values in (5,6) partition by list (a);
1907+
create table fkpart0.fk_part_56_5 partition of fkpart0.fk_part_56
1908+
for values in (5);
1909+
\d fkpart0.fk_part_56
1910+
Partitioned table "fkpart0.fk_part_56"
1911+
Column | Type | Collation | Nullable | Default
1912+
--------+---------+-----------+----------+---------
1913+
a | integer | | |
1914+
Partition of: fkpart0.fk_part FOR VALUES IN (5, 6)
1915+
Partition key: LIST (a)
1916+
Foreign-key constraints:
1917+
"fk_part_a_fkey" FOREIGN KEY (a) REFERENCES fkpart0.pkey(a)
1918+
Number of partitions: 1 (Use \d+ to list them.)
1919+
1920+
alter table fkpart0.fk_part_56 drop constraint fk_part_a_fkey;
1921+
ERROR: cannot drop inherited constraint "fk_part_a_fkey" of relation "fk_part_56"
1922+
alter table fkpart0.fk_part_56_5 drop constraint fk_part_a_fkey;
1923+
ERROR: cannot drop inherited constraint "fk_part_a_fkey" of relation "fk_part_56_5"
1924+
\set VERBOSITY terse \\ -- suppress cascade details
1925+
drop schema fkpart0 cascade;
1926+
NOTICE: drop cascades to 2 other objects
1927+
\set VERBOSITY default

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

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

13421342
-- leave these tables around intentionally
1343+
1344+
-- Test creating a constraint at the parent that already exists in partitions.
1345+
-- There should be no duplicated constraints, and attempts to drop the
1346+
-- constraint in partitions should raise appropriate errors.
1347+
create schema fkpart0
1348+
create table pkey (a int primary key)
1349+
create table fk_part (a int) partition by list (a)
1350+
create table fk_part_1 partition of fk_part
1351+
(foreign key (a) references fkpart0.pkey) for values in (1)
1352+
create table fk_part_23 partition of fk_part
1353+
(foreign key (a) references fkpart0.pkey) for values in (2, 3)
1354+
partition by list (a)
1355+
create table fk_part_23_2 partition of fk_part_23 for values in (2);
1356+
1357+
alter table fkpart0.fk_part add foreign key (a) references fkpart0.pkey;
1358+
\d fkpart0.fk_part_1 \\ -- should have only one FK
1359+
alter table fkpart0.fk_part_1 drop constraint fk_part_1_a_fkey;
1360+
1361+
\d fkpart0.fk_part_23 \\ -- should have only one FK
1362+
\d fkpart0.fk_part_23_2 \\ -- should have only one FK
1363+
alter table fkpart0.fk_part_23 drop constraint fk_part_23_a_fkey;
1364+
alter table fkpart0.fk_part_23_2 drop constraint fk_part_23_a_fkey;
1365+
1366+
create table fkpart0.fk_part_4 partition of fkpart0.fk_part for values in (4);
1367+
\d fkpart0.fk_part_4
1368+
alter table fkpart0.fk_part_4 drop constraint fk_part_a_fkey;
1369+
1370+
create table fkpart0.fk_part_56 partition of fkpart0.fk_part
1371+
for values in (5,6) partition by list (a);
1372+
create table fkpart0.fk_part_56_5 partition of fkpart0.fk_part_56
1373+
for values in (5);
1374+
\d fkpart0.fk_part_56
1375+
alter table fkpart0.fk_part_56 drop constraint fk_part_a_fkey;
1376+
alter table fkpart0.fk_part_56_5 drop constraint fk_part_a_fkey;
1377+
1378+
\set VERBOSITY terse \\ -- suppress cascade details
1379+
drop schema fkpart0 cascade;
1380+
\set VERBOSITY default

0 commit comments

Comments
 (0)