Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit46e3442

Browse files
committed
Fix failures in validateForeignKeyConstraint's slow path.
The foreign-key-checking loop in ATRewriteTables failed to ignorerelations without storage (e.g., partitioned tables), unlike theinitial loop. This accidentally worked as long as RI_Initial_Checksucceeded, which it does in most practical cases (including all theones exercised in the existing regression tests :-(). However, ifthat 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 failif it lacks storage.A second problem, recently introduced in HEAD, was that this loophad been broken by sloppy refactoring for the tableam API changes.Repair both issues, and add a regression test case so we have somecoverage 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 FreundDiscussion:https://postgr.es/m/CAK=1=WrnNmBbe5D9sm3t0a6dnAq3cdbF1vXY816j1wsMqzC8bw@mail.gmail.comDiscussion:https://postgr.es/m/19030.1554574075@sss.pgh.pa.usDiscussion:https://postgr.es/m/20190325180405.jytoehuzkeozggxx%40alap3.anarazel.de
1 parent249d649 commit46e3442

File tree

3 files changed

+58
-8
lines changed

3 files changed

+58
-8
lines changed

‎src/backend/commands/tablecmds.c

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -4459,13 +4459,8 @@ ATRewriteTables(AlterTableStmt *parsetree, List **wqueue, LOCKMODE lockmode)
44594459
{
44604460
AlteredTableInfo *tab = (AlteredTableInfo *) lfirst(ltab);
44614461

4462-
/*
4463-
* Foreign tables have no storage, nor do partitioned tables and
4464-
* indexes.
4465-
*/
4466-
if (tab->relkind == RELKIND_FOREIGN_TABLE ||
4467-
tab->relkind == RELKIND_PARTITIONED_TABLE ||
4468-
tab->relkind == RELKIND_PARTITIONED_INDEX)
4462+
/* Relations without storage may be ignored here */
4463+
if (!RELKIND_HAS_STORAGE(tab->relkind))
44694464
continue;
44704465

44714466
/*
@@ -4645,6 +4640,10 @@ ATRewriteTables(AlterTableStmt *parsetree, List **wqueue, LOCKMODE lockmode)
46454640
Relationrel = NULL;
46464641
ListCell *lcon;
46474642

4643+
/* Relations without storage may be ignored here too */
4644+
if (!RELKIND_HAS_STORAGE(tab->relkind))
4645+
continue;
4646+
46484647
foreach(lcon, tab->constraints)
46494648
{
46504649
NewConstraint *con = lfirst(lcon);
@@ -9647,7 +9646,7 @@ validateForeignKeyConstraint(char *conname,
96479646
trigdata.type = T_TriggerData;
96489647
trigdata.tg_event = TRIGGER_EVENT_INSERT | TRIGGER_EVENT_ROW;
96499648
trigdata.tg_relation = rel;
9650-
trigdata.tg_trigtuple = ExecFetchSlotHeapTuple(slot,true, NULL);
9649+
trigdata.tg_trigtuple = ExecFetchSlotHeapTuple(slot,false, NULL);
96519650
trigdata.tg_trigslot = slot;
96529651
trigdata.tg_newtuple = NULL;
96539652
trigdata.tg_trigger = &trig;

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

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1895,6 +1895,32 @@ INSERT INTO fk_notpartitioned_pk VALUES (1600, 601), (1600, 1601);
18951895
ALTER TABLE fk_partitioned_fk ATTACH PARTITION fk_partitioned_fk_2
18961896
FOR VALUES IN (1600);
18971897
-- leave these tables around intentionally
1898+
-- test the case when the referenced table is owned by a different user
1899+
create role regress_other_partitioned_fk_owner;
1900+
grant references on fk_notpartitioned_pk to regress_other_partitioned_fk_owner;
1901+
set role regress_other_partitioned_fk_owner;
1902+
create table other_partitioned_fk(a int, b int) partition by list (a);
1903+
create table other_partitioned_fk_1 partition of other_partitioned_fk
1904+
for values in (2048);
1905+
insert into other_partitioned_fk
1906+
select 2048, x from generate_series(1,10) x;
1907+
-- this should fail
1908+
alter table other_partitioned_fk add foreign key (a, b)
1909+
references fk_notpartitioned_pk(a, b);
1910+
ERROR: insert or update on table "other_partitioned_fk_1" violates foreign key constraint "other_partitioned_fk_a_b_fkey"
1911+
DETAIL: Key (a, b)=(2048, 1) is not present in table "fk_notpartitioned_pk".
1912+
-- add the missing keys and retry
1913+
reset role;
1914+
insert into fk_notpartitioned_pk (a, b)
1915+
select 2048, x from generate_series(1,10) x;
1916+
set role regress_other_partitioned_fk_owner;
1917+
alter table other_partitioned_fk add foreign key (a, b)
1918+
references fk_notpartitioned_pk(a, b);
1919+
-- clean up
1920+
drop table other_partitioned_fk;
1921+
reset role;
1922+
revoke all on fk_notpartitioned_pk from regress_other_partitioned_fk_owner;
1923+
drop role regress_other_partitioned_fk_owner;
18981924
-- Test creating a constraint at the parent that already exists in partitions.
18991925
-- There should be no duplicated constraints, and attempts to drop the
19001926
-- 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
@@ -1365,6 +1365,31 @@ ALTER TABLE fk_partitioned_fk ATTACH PARTITION fk_partitioned_fk_2
13651365

13661366
-- leave these tables around intentionally
13671367

1368+
-- test the case when the referenced table is owned by a different user
1369+
create role regress_other_partitioned_fk_owner;
1370+
grantreferenceson fk_notpartitioned_pk to regress_other_partitioned_fk_owner;
1371+
set role regress_other_partitioned_fk_owner;
1372+
createtableother_partitioned_fk(aint, bint) partition by list (a);
1373+
createtableother_partitioned_fk_1 partition of other_partitioned_fk
1374+
forvaluesin (2048);
1375+
insert into other_partitioned_fk
1376+
select2048, xfrom generate_series(1,10) x;
1377+
-- this should fail
1378+
altertable other_partitioned_fk addforeign key (a, b)
1379+
references fk_notpartitioned_pk(a, b);
1380+
-- add the missing keys and retry
1381+
reset role;
1382+
insert into fk_notpartitioned_pk (a, b)
1383+
select2048, xfrom generate_series(1,10) x;
1384+
set role regress_other_partitioned_fk_owner;
1385+
altertable other_partitioned_fk addforeign key (a, b)
1386+
references fk_notpartitioned_pk(a, b);
1387+
-- clean up
1388+
droptable other_partitioned_fk;
1389+
reset role;
1390+
revoke allon fk_notpartitioned_pkfrom regress_other_partitioned_fk_owner;
1391+
drop role regress_other_partitioned_fk_owner;
1392+
13681393
-- Test creating a constraint at the parent that already exists in partitions.
13691394
-- There should be no duplicated constraints, and attempts to drop the
13701395
-- constraint in partitions should raise appropriate errors.

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp