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

Commit c2a5fb3

Browse files
committed
Fix failures in validateForeignKeyConstraint's slow path.
The foreign-key-checking loop in ATRewriteTables failed to ignore relations without storage (e.g., partitioned tables), unlike the initial loop. This accidentally worked as long as RI_Initial_Check succeeded, which it does in most practical cases (including all the ones exercised in the existing regression tests :-(). However, if that failed, as for instance when there are permissions issues, then we entered the slow fire-the-trigger-on-each-tuple path. And that would try to read from the referencing relation, and fail if it lacks storage. A second problem, recently introduced in HEAD, was that this loop had been broken by sloppy refactoring for the tableam API changes. Repair both issues, and add a regression test case so we have some coverage on this code path. Back-patch as needed to v11. (It looks like this code could do with additional bulletproofing, but let's get a working test case in place first.) Hadi Moshayedi, Tom Lane, Andres Freund Discussion: https://postgr.es/m/CAK=1=WrnNmBbe5D9sm3t0a6dnAq3cdbF1vXY816j1wsMqzC8bw@mail.gmail.com Discussion: https://postgr.es/m/19030.1554574075@sss.pgh.pa.us Discussion: https://postgr.es/m/20190325180405.jytoehuzkeozggxx%40alap3.anarazel.de
1 parent 7338ed2 commit c2a5fb3

File tree

3 files changed

+61
-1
lines changed

3 files changed

+61
-1
lines changed

src/backend/commands/tablecmds.c

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4534,6 +4534,15 @@ ATRewriteTables(AlterTableStmt *parsetree, List **wqueue, LOCKMODE lockmode)
45344534
Relation rel = NULL;
45354535
ListCell *lcon;
45364536

4537+
/*
4538+
* Foreign tables have no storage, nor do partitioned tables and
4539+
* indexes.
4540+
*/
4541+
if (tab->relkind == RELKIND_FOREIGN_TABLE ||
4542+
tab->relkind == RELKIND_PARTITIONED_TABLE ||
4543+
tab->relkind == RELKIND_PARTITIONED_INDEX)
4544+
continue;
4545+
45374546
foreach(lcon, tab->constraints)
45384547
{
45394548
NewConstraint *con = lfirst(lcon);

src/test/regress/expected/foreign_key.out

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1775,12 +1775,38 @@ CREATE TABLE fk_partitioned_fk_2_2 PARTITION OF fk_partitioned_fk_2 FOR VALUES F
17751775
INSERT INTO fk_partitioned_fk_2 VALUES (1600, 601), (1600, 1601);
17761776
ALTER TABLE fk_partitioned_fk ATTACH PARTITION fk_partitioned_fk_2
17771777
FOR VALUES IN (1600);
1778-
ERROR: insert or update on table "fk_partitioned_fk_2" violates foreign key constraint "fk_partitioned_fk_a_fkey"
1778+
ERROR: insert or update on table "fk_partitioned_fk_2_1" violates foreign key constraint "fk_partitioned_fk_a_fkey"
17791779
DETAIL: Key (a, b)=(1600, 601) is not present in table "fk_notpartitioned_pk".
17801780
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 the case when the referenced table is owned by a different user
1785+
create role regress_other_partitioned_fk_owner;
1786+
grant references on fk_notpartitioned_pk to regress_other_partitioned_fk_owner;
1787+
set role regress_other_partitioned_fk_owner;
1788+
create table other_partitioned_fk(a int, b int) partition by list (a);
1789+
create table other_partitioned_fk_1 partition of other_partitioned_fk
1790+
for values in (2048);
1791+
insert into other_partitioned_fk
1792+
select 2048, x from generate_series(1,10) x;
1793+
-- this should fail
1794+
alter table other_partitioned_fk add foreign key (a, b)
1795+
references fk_notpartitioned_pk(a, b);
1796+
ERROR: insert or update on table "other_partitioned_fk_1" violates foreign key constraint "other_partitioned_fk_a_fkey"
1797+
DETAIL: Key (a, b)=(2048, 1) is not present in table "fk_notpartitioned_pk".
1798+
-- add the missing keys and retry
1799+
reset role;
1800+
insert into fk_notpartitioned_pk (a, b)
1801+
select 2048, x from generate_series(1,10) x;
1802+
set role regress_other_partitioned_fk_owner;
1803+
alter table other_partitioned_fk add foreign key (a, b)
1804+
references fk_notpartitioned_pk(a, b);
1805+
-- clean up
1806+
drop table other_partitioned_fk;
1807+
reset role;
1808+
revoke all on fk_notpartitioned_pk from regress_other_partitioned_fk_owner;
1809+
drop role regress_other_partitioned_fk_owner;
17841810
-- Test creating a constraint at the parent that already exists in partitions.
17851811
-- There should be no duplicated constraints, and attempts to drop the
17861812
-- constraint in partitions should raise appropriate errors.

src/test/regress/sql/foreign_key.sql

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1290,6 +1290,31 @@ ALTER TABLE fk_partitioned_fk ATTACH PARTITION fk_partitioned_fk_2
12901290

12911291
-- leave these tables around intentionally
12921292

1293+
-- test the case when the referenced table is owned by a different user
1294+
create role regress_other_partitioned_fk_owner;
1295+
grant references on fk_notpartitioned_pk to regress_other_partitioned_fk_owner;
1296+
set role regress_other_partitioned_fk_owner;
1297+
create table other_partitioned_fk(a int, b int) partition by list (a);
1298+
create table other_partitioned_fk_1 partition of other_partitioned_fk
1299+
for values in (2048);
1300+
insert into other_partitioned_fk
1301+
select 2048, x from generate_series(1,10) x;
1302+
-- this should fail
1303+
alter table other_partitioned_fk add foreign key (a, b)
1304+
references fk_notpartitioned_pk(a, b);
1305+
-- add the missing keys and retry
1306+
reset role;
1307+
insert into fk_notpartitioned_pk (a, b)
1308+
select 2048, x from generate_series(1,10) x;
1309+
set role regress_other_partitioned_fk_owner;
1310+
alter table other_partitioned_fk add foreign key (a, b)
1311+
references fk_notpartitioned_pk(a, b);
1312+
-- clean up
1313+
drop table other_partitioned_fk;
1314+
reset role;
1315+
revoke all on fk_notpartitioned_pk from regress_other_partitioned_fk_owner;
1316+
drop role regress_other_partitioned_fk_owner;
1317+
12931318
-- Test creating a constraint at the parent that already exists in partitions.
12941319
-- There should be no duplicated constraints, and attempts to drop the
12951320
-- constraint in partitions should raise appropriate errors.

0 commit comments

Comments
 (0)