mirrors/postgres
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Fix failures in validateForeignKeyConstraint's slow path.

Browse Source 
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
This commit is contained in:
Tom Lane 2019-04-06 15:09:09 -04:00
parent 249d649996
commit 46e3442c9e
3 changed files with 58 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

15
src/backend/commands/tablecmds.c
View File

@ -4459,13 +4459,8 @@ ATRewriteTables(AlterTableStmt *parsetree, List **wqueue, LOCKMODE lockmode)
{
AlteredTableInfo *tab = (AlteredTableInfo *) lfirst(ltab);
/*
* Foreign tables have no storage, nor do partitioned tables and
* indexes.
*/
if (tab->relkind == RELKIND_FOREIGN_TABLE ||
tab->relkind == RELKIND_PARTITIONED_TABLE ||
tab->relkind == RELKIND_PARTITIONED_INDEX)
/* Relations without storage may be ignored here */
if (!RELKIND_HAS_STORAGE(tab->relkind))
continue;
/*
@ -4645,6 +4640,10 @@ ATRewriteTables(AlterTableStmt *parsetree, List **wqueue, LOCKMODE lockmode)
Relation rel = NULL;
ListCell *lcon;
/* Relations without storage may be ignored here too */
if (!RELKIND_HAS_STORAGE(tab->relkind))
continue;
foreach(lcon, tab->constraints)
{
NewConstraint *con = lfirst(lcon);
@ -9647,7 +9646,7 @@ validateForeignKeyConstraint(char *conname,
trigdata.type = T_TriggerData;
trigdata.tg_event = TRIGGER_EVENT_INSERT | TRIGGER_EVENT_ROW;
trigdata.tg_relation = rel;
trigdata.tg_trigtuple = ExecFetchSlotHeapTuple(slot, true, NULL);
trigdata.tg_trigtuple = ExecFetchSlotHeapTuple(slot, false, NULL);
trigdata.tg_trigslot = slot;
trigdata.tg_newtuple = NULL;
trigdata.tg_trigger = &trig;

26
src/test/regress/expected/foreign_key.out
View File

@ -1895,6 +1895,32 @@ INSERT INTO fk_notpartitioned_pk VALUES (1600, 601), (1600, 1601);
ALTER TABLE fk_partitioned_fk ATTACH PARTITION fk_partitioned_fk_2
FOR VALUES IN (1600);
-- leave these tables around intentionally
-- test the case when the referenced table is owned by a different user
create role regress_other_partitioned_fk_owner;
grant references on fk_notpartitioned_pk to regress_other_partitioned_fk_owner;
set role regress_other_partitioned_fk_owner;
create table other_partitioned_fk(a int, b int) partition by list (a);
create table other_partitioned_fk_1 partition of other_partitioned_fk
for values in (2048);
insert into other_partitioned_fk
select 2048, x from generate_series(1,10) x;
-- this should fail
alter table other_partitioned_fk add foreign key (a, b)
references fk_notpartitioned_pk(a, b);
ERROR: insert or update on table "other_partitioned_fk_1" violates foreign key constraint "other_partitioned_fk_a_b_fkey"
DETAIL: Key (a, b)=(2048, 1) is not present in table "fk_notpartitioned_pk".
-- add the missing keys and retry
reset role;
insert into fk_notpartitioned_pk (a, b)
select 2048, x from generate_series(1,10) x;
set role regress_other_partitioned_fk_owner;
alter table other_partitioned_fk add foreign key (a, b)
references fk_notpartitioned_pk(a, b);
-- clean up
drop table other_partitioned_fk;
reset role;
revoke all on fk_notpartitioned_pk from regress_other_partitioned_fk_owner;
drop role regress_other_partitioned_fk_owner;
-- Test creating a constraint at the parent that already exists in partitions.
-- There should be no duplicated constraints, and attempts to drop the
-- constraint in partitions should raise appropriate errors.

25
src/test/regress/sql/foreign_key.sql
View File

@ -1365,6 +1365,31 @@ ALTER TABLE fk_partitioned_fk ATTACH PARTITION fk_partitioned_fk_2
-- leave these tables around intentionally
-- test the case when the referenced table is owned by a different user
create role regress_other_partitioned_fk_owner;
grant references on fk_notpartitioned_pk to regress_other_partitioned_fk_owner;
set role regress_other_partitioned_fk_owner;
create table other_partitioned_fk(a int, b int) partition by list (a);
create table other_partitioned_fk_1 partition of other_partitioned_fk
for values in (2048);
insert into other_partitioned_fk
select 2048, x from generate_series(1,10) x;
-- this should fail
alter table other_partitioned_fk add foreign key (a, b)
references fk_notpartitioned_pk(a, b);
-- add the missing keys and retry
reset role;
insert into fk_notpartitioned_pk (a, b)
select 2048, x from generate_series(1,10) x;
set role regress_other_partitioned_fk_owner;
alter table other_partitioned_fk add foreign key (a, b)
references fk_notpartitioned_pk(a, b);
-- clean up
drop table other_partitioned_fk;
reset role;
revoke all on fk_notpartitioned_pk from regress_other_partitioned_fk_owner;
drop role regress_other_partitioned_fk_owner;
-- Test creating a constraint at the parent that already exists in partitions.
-- There should be no duplicated constraints, and attempts to drop the
-- constraint in partitions should raise appropriate errors.