Skip to content

Commit ce88170

Browse files
author
Etsuro Fujita
committed
Disallow collecting transition tuples from child foreign tables.
Commit 9e6104c disallowed transition tables on foreign tables, but failed to account for cases where a foreign table is a child table of a partitioned/inherited table on which transition tables exist, leading to incorrect transition tuples collected from such foreign tables for queries on the parent table triggering transition capture. This occurred not only for inherited UPDATE/DELETE but for partitioned INSERT later supported by commit 3d956d9, which should have handled it at least for the INSERT case, but didn't. To fix, modify ExecAR*Triggers to throw an error if the given relation is a foreign table requesting transition capture. Also, this commit fixes make_modifytable so that in case of an inherited UPDATE/DELETE triggering transition capture, FDWs choose normal operations to modify child foreign tables, not DirectModify; which is needed because they would otherwise skip the calls to ExecAR*Triggers at execution, causing unexpected behavior. Author: Etsuro Fujita <etsuro.fujita@gmail.com> Reviewed-by: Amit Langote <amitlangote09@gmail.com> Discussion: https://postgr.es/m/CAPmGK14QJYikKzBDCe3jMbpGENnQ7popFmbEgm-XTNuk55oyHg%40mail.gmail.com Backpatch-through: 13
1 parent ab74ce4 commit ce88170

File tree

6 files changed

+295
-4
lines changed

6 files changed

+295
-4
lines changed

‎contrib/postgres_fdw/expected/postgres_fdw.out

Lines changed: 113 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8212,6 +8212,119 @@ DELETE FROM rem1; -- can't be pushed down
82128212
(5 rows)
82138213

82148214
DROP TRIGGER trig_row_after_delete ON rem1;
8215+
-- We are allowed to create transition-table triggers on both kinds of
8216+
-- inheritance even if they contain foreign tables as children, but currently
8217+
-- collecting transition tuples from such foreign tables is not supported.
8218+
CREATE TABLE local_tbl (a text, b int);
8219+
CREATE FOREIGN TABLE foreign_tbl (a text, b int)
8220+
SERVER loopback OPTIONS (table_name 'local_tbl');
8221+
INSERT INTO foreign_tbl VALUES ('AAA', 42);
8222+
-- Test case for partition hierarchy
8223+
CREATE TABLE parent_tbl (a text, b int) PARTITION BY LIST (a);
8224+
ALTER TABLE parent_tbl ATTACH PARTITION foreign_tbl FOR VALUES IN ('AAA');
8225+
CREATE TRIGGER parent_tbl_insert_trig
8226+
AFTER INSERT ON parent_tbl REFERENCING NEW TABLE AS new_table
8227+
FOR EACH STATEMENT EXECUTE PROCEDURE trigger_func();
8228+
CREATE TRIGGER parent_tbl_update_trig
8229+
AFTER UPDATE ON parent_tbl REFERENCING OLD TABLE AS old_table NEW TABLE AS new_table
8230+
FOR EACH STATEMENT EXECUTE PROCEDURE trigger_func();
8231+
CREATE TRIGGER parent_tbl_delete_trig
8232+
AFTER DELETE ON parent_tbl REFERENCING OLD TABLE AS old_table
8233+
FOR EACH STATEMENT EXECUTE PROCEDURE trigger_func();
8234+
INSERT INTO parent_tbl VALUES ('AAA', 42);
8235+
ERROR: cannot collect transition tuples from child foreign tables
8236+
COPY parent_tbl (a, b) FROM stdin;
8237+
ERROR: cannot collect transition tuples from child foreign tables
8238+
CONTEXT: COPY parent_tbl, line 1: "AAA 42"
8239+
ALTER SERVER loopback OPTIONS (ADD batch_size '10');
8240+
INSERT INTO parent_tbl VALUES ('AAA', 42);
8241+
ERROR: cannot collect transition tuples from child foreign tables
8242+
COPY parent_tbl (a, b) FROM stdin;
8243+
ERROR: cannot collect transition tuples from child foreign tables
8244+
CONTEXT: COPY parent_tbl, line 1: "AAA 42"
8245+
ALTER SERVER loopback OPTIONS (DROP batch_size);
8246+
EXPLAIN (VERBOSE, COSTS OFF)
8247+
UPDATE parent_tbl SET b = b + 1;
8248+
QUERY PLAN
8249+
------------------------------------------------------------------------------------------------
8250+
Update on public.parent_tbl
8251+
Foreign Update on public.foreign_tbl parent_tbl_1
8252+
Remote SQL: UPDATE public.local_tbl SET b = $2 WHERE ctid = $1
8253+
-> Foreign Scan on public.foreign_tbl parent_tbl_1
8254+
Output: (parent_tbl_1.b + 1), parent_tbl_1.tableoid, parent_tbl_1.ctid, parent_tbl_1.*
8255+
Remote SQL: SELECT a, b, ctid FROM public.local_tbl FOR UPDATE
8256+
(6 rows)
8257+
8258+
UPDATE parent_tbl SET b = b + 1;
8259+
ERROR: cannot collect transition tuples from child foreign tables
8260+
EXPLAIN (VERBOSE, COSTS OFF)
8261+
DELETE FROM parent_tbl;
8262+
QUERY PLAN
8263+
------------------------------------------------------------------
8264+
Delete on public.parent_tbl
8265+
Foreign Delete on public.foreign_tbl parent_tbl_1
8266+
Remote SQL: DELETE FROM public.local_tbl WHERE ctid = $1
8267+
-> Foreign Scan on public.foreign_tbl parent_tbl_1
8268+
Output: parent_tbl_1.tableoid, parent_tbl_1.ctid
8269+
Remote SQL: SELECT ctid FROM public.local_tbl FOR UPDATE
8270+
(6 rows)
8271+
8272+
DELETE FROM parent_tbl;
8273+
ERROR: cannot collect transition tuples from child foreign tables
8274+
ALTER TABLE parent_tbl DETACH PARTITION foreign_tbl;
8275+
DROP TABLE parent_tbl;
8276+
-- Test case for non-partition hierarchy
8277+
CREATE TABLE parent_tbl (a text, b int);
8278+
ALTER FOREIGN TABLE foreign_tbl INHERIT parent_tbl;
8279+
CREATE TRIGGER parent_tbl_update_trig
8280+
AFTER UPDATE ON parent_tbl REFERENCING OLD TABLE AS old_table NEW TABLE AS new_table
8281+
FOR EACH STATEMENT EXECUTE PROCEDURE trigger_func();
8282+
CREATE TRIGGER parent_tbl_delete_trig
8283+
AFTER DELETE ON parent_tbl REFERENCING OLD TABLE AS old_table
8284+
FOR EACH STATEMENT EXECUTE PROCEDURE trigger_func();
8285+
EXPLAIN (VERBOSE, COSTS OFF)
8286+
UPDATE parent_tbl SET b = b + 1;
8287+
QUERY PLAN
8288+
------------------------------------------------------------------------------------------------------
8289+
Update on public.parent_tbl
8290+
Update on public.parent_tbl parent_tbl_1
8291+
Foreign Update on public.foreign_tbl parent_tbl_2
8292+
Remote SQL: UPDATE public.local_tbl SET b = $2 WHERE ctid = $1
8293+
-> Result
8294+
Output: (parent_tbl.b + 1), parent_tbl.tableoid, parent_tbl.ctid, (NULL::record)
8295+
-> Append
8296+
-> Seq Scan on public.parent_tbl parent_tbl_1
8297+
Output: parent_tbl_1.b, parent_tbl_1.tableoid, parent_tbl_1.ctid, NULL::record
8298+
-> Foreign Scan on public.foreign_tbl parent_tbl_2
8299+
Output: parent_tbl_2.b, parent_tbl_2.tableoid, parent_tbl_2.ctid, parent_tbl_2.*
8300+
Remote SQL: SELECT a, b, ctid FROM public.local_tbl FOR UPDATE
8301+
(12 rows)
8302+
8303+
UPDATE parent_tbl SET b = b + 1;
8304+
ERROR: cannot collect transition tuples from child foreign tables
8305+
EXPLAIN (VERBOSE, COSTS OFF)
8306+
DELETE FROM parent_tbl;
8307+
QUERY PLAN
8308+
------------------------------------------------------------------------
8309+
Delete on public.parent_tbl
8310+
Delete on public.parent_tbl parent_tbl_1
8311+
Foreign Delete on public.foreign_tbl parent_tbl_2
8312+
Remote SQL: DELETE FROM public.local_tbl WHERE ctid = $1
8313+
-> Append
8314+
-> Seq Scan on public.parent_tbl parent_tbl_1
8315+
Output: parent_tbl_1.tableoid, parent_tbl_1.ctid
8316+
-> Foreign Scan on public.foreign_tbl parent_tbl_2
8317+
Output: parent_tbl_2.tableoid, parent_tbl_2.ctid
8318+
Remote SQL: SELECT ctid FROM public.local_tbl FOR UPDATE
8319+
(10 rows)
8320+
8321+
DELETE FROM parent_tbl;
8322+
ERROR: cannot collect transition tuples from child foreign tables
8323+
ALTER FOREIGN TABLE foreign_tbl NO INHERIT parent_tbl;
8324+
DROP TABLE parent_tbl;
8325+
-- Cleanup
8326+
DROP FOREIGN TABLE foreign_tbl;
8327+
DROP TABLE local_tbl;
82158328
-- ===================================================================
82168329
-- test inheritance features
82178330
-- ===================================================================

‎contrib/postgres_fdw/sql/postgres_fdw.sql

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2272,6 +2272,84 @@ EXPLAIN (verbose, costs off)
22722272
DELETE FROM rem1; -- can't be pushed down
22732273
DROP TRIGGER trig_row_after_delete ON rem1;
22742274

2275+
2276+
-- We are allowed to create transition-table triggers on both kinds of
2277+
-- inheritance even if they contain foreign tables as children, but currently
2278+
-- collecting transition tuples from such foreign tables is not supported.
2279+
2280+
CREATE TABLE local_tbl (a text, b int);
2281+
CREATE FOREIGN TABLE foreign_tbl (a text, b int)
2282+
SERVER loopback OPTIONS (table_name 'local_tbl');
2283+
2284+
INSERT INTO foreign_tbl VALUES ('AAA', 42);
2285+
2286+
-- Test case for partition hierarchy
2287+
CREATE TABLE parent_tbl (a text, b int) PARTITION BY LIST (a);
2288+
ALTER TABLE parent_tbl ATTACH PARTITION foreign_tbl FOR VALUES IN ('AAA');
2289+
2290+
CREATE TRIGGER parent_tbl_insert_trig
2291+
AFTER INSERT ON parent_tbl REFERENCING NEW TABLE AS new_table
2292+
FOR EACH STATEMENT EXECUTE PROCEDURE trigger_func();
2293+
CREATE TRIGGER parent_tbl_update_trig
2294+
AFTER UPDATE ON parent_tbl REFERENCING OLD TABLE AS old_table NEW TABLE AS new_table
2295+
FOR EACH STATEMENT EXECUTE PROCEDURE trigger_func();
2296+
CREATE TRIGGER parent_tbl_delete_trig
2297+
AFTER DELETE ON parent_tbl REFERENCING OLD TABLE AS old_table
2298+
FOR EACH STATEMENT EXECUTE PROCEDURE trigger_func();
2299+
2300+
INSERT INTO parent_tbl VALUES ('AAA', 42);
2301+
2302+
COPY parent_tbl (a, b) FROM stdin;
2303+
AAA 42
2304+
\.
2305+
2306+
ALTER SERVER loopback OPTIONS (ADD batch_size '10');
2307+
2308+
INSERT INTO parent_tbl VALUES ('AAA', 42);
2309+
2310+
COPY parent_tbl (a, b) FROM stdin;
2311+
AAA 42
2312+
\.
2313+
2314+
ALTER SERVER loopback OPTIONS (DROP batch_size);
2315+
2316+
EXPLAIN (VERBOSE, COSTS OFF)
2317+
UPDATE parent_tbl SET b = b + 1;
2318+
UPDATE parent_tbl SET b = b + 1;
2319+
2320+
EXPLAIN (VERBOSE, COSTS OFF)
2321+
DELETE FROM parent_tbl;
2322+
DELETE FROM parent_tbl;
2323+
2324+
ALTER TABLE parent_tbl DETACH PARTITION foreign_tbl;
2325+
DROP TABLE parent_tbl;
2326+
2327+
-- Test case for non-partition hierarchy
2328+
CREATE TABLE parent_tbl (a text, b int);
2329+
ALTER FOREIGN TABLE foreign_tbl INHERIT parent_tbl;
2330+
2331+
CREATE TRIGGER parent_tbl_update_trig
2332+
AFTER UPDATE ON parent_tbl REFERENCING OLD TABLE AS old_table NEW TABLE AS new_table
2333+
FOR EACH STATEMENT EXECUTE PROCEDURE trigger_func();
2334+
CREATE TRIGGER parent_tbl_delete_trig
2335+
AFTER DELETE ON parent_tbl REFERENCING OLD TABLE AS old_table
2336+
FOR EACH STATEMENT EXECUTE PROCEDURE trigger_func();
2337+
2338+
EXPLAIN (VERBOSE, COSTS OFF)
2339+
UPDATE parent_tbl SET b = b + 1;
2340+
UPDATE parent_tbl SET b = b + 1;
2341+
2342+
EXPLAIN (VERBOSE, COSTS OFF)
2343+
DELETE FROM parent_tbl;
2344+
DELETE FROM parent_tbl;
2345+
2346+
ALTER FOREIGN TABLE foreign_tbl NO INHERIT parent_tbl;
2347+
DROP TABLE parent_tbl;
2348+
2349+
-- Cleanup
2350+
DROP FOREIGN TABLE foreign_tbl;
2351+
DROP TABLE local_tbl;
2352+
22752353
-- ===================================================================
22762354
-- test inheritance features
22772355
-- ===================================================================

‎src/backend/commands/trigger.c

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2545,6 +2545,15 @@ ExecARInsertTriggers(EState *estate, ResultRelInfo *relinfo,
25452545
{
25462546
TriggerDesc *trigdesc = relinfo->ri_TrigDesc;
25472547

2548+
if (relinfo->ri_FdwRoutine && transition_capture &&
2549+
transition_capture->tcs_insert_new_table)
2550+
{
2551+
Assert(relinfo->ri_RootResultRelInfo);
2552+
ereport(ERROR,
2553+
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
2554+
errmsg("cannot collect transition tuples from child foreign tables")));
2555+
}
2556+
25482557
if ((trigdesc && trigdesc->trig_insert_after_row) ||
25492558
(transition_capture && transition_capture->tcs_insert_new_table))
25502559
AfterTriggerSaveEvent(estate, relinfo, NULL, NULL,
@@ -2797,6 +2806,15 @@ ExecARDeleteTriggers(EState *estate,
27972806
{
27982807
TriggerDesc *trigdesc = relinfo->ri_TrigDesc;
27992808

2809+
if (relinfo->ri_FdwRoutine && transition_capture &&
2810+
transition_capture->tcs_delete_old_table)
2811+
{
2812+
Assert(relinfo->ri_RootResultRelInfo);
2813+
ereport(ERROR,
2814+
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
2815+
errmsg("cannot collect transition tuples from child foreign tables")));
2816+
}
2817+
28002818
if ((trigdesc && trigdesc->trig_delete_after_row) ||
28012819
(transition_capture && transition_capture->tcs_delete_old_table))
28022820
{
@@ -3134,6 +3152,16 @@ ExecARUpdateTriggers(EState *estate, ResultRelInfo *relinfo,
31343152
{
31353153
TriggerDesc *trigdesc = relinfo->ri_TrigDesc;
31363154

3155+
if (relinfo->ri_FdwRoutine && transition_capture &&
3156+
(transition_capture->tcs_update_old_table ||
3157+
transition_capture->tcs_update_new_table))
3158+
{
3159+
Assert(relinfo->ri_RootResultRelInfo);
3160+
ereport(ERROR,
3161+
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
3162+
errmsg("cannot collect transition tuples from child foreign tables")));
3163+
}
3164+
31373165
if ((trigdesc && trigdesc->trig_update_after_row) ||
31383166
(transition_capture &&
31393167
(transition_capture->tcs_update_old_table ||

‎src/backend/optimizer/plan/createplan.c

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7174,6 +7174,8 @@ make_modifytable(PlannerInfo *root, Plan *subplan,
71747174
ModifyTable *node = makeNode(ModifyTable);
71757175
bool returning_old_or_new = false;
71767176
bool returning_old_or_new_valid = false;
7177+
bool transition_tables = false;
7178+
bool transition_tables_valid = false;
71777179
List *fdw_private_list;
71787180
Bitmapset *direct_modify_plans;
71797181
ListCell *lc;
@@ -7320,8 +7322,8 @@ make_modifytable(PlannerInfo *root, Plan *subplan,
73207322
* callback functions needed for that and (2) there are no local
73217323
* structures that need to be run for each modified row: row-level
73227324
* triggers on the foreign table, stored generated columns, WITH CHECK
7323-
* OPTIONs from parent views, or Vars returning OLD/NEW in the
7324-
* RETURNING list.
7325+
* OPTIONs from parent views, Vars returning OLD/NEW in the RETURNING
7326+
* list, or transition tables on the named relation.
73257327
*/
73267328
direct_modify = false;
73277329
if (fdwroutine != NULL &&
@@ -7333,7 +7335,10 @@ make_modifytable(PlannerInfo *root, Plan *subplan,
73337335
!has_row_triggers(root, rti, operation) &&
73347336
!has_stored_generated_columns(root, rti))
73357337
{
7336-
/* returning_old_or_new is the same for all result relations */
7338+
/*
7339+
* returning_old_or_new and transition_tables are the same for all
7340+
* result relations, respectively
7341+
*/
73377342
if (!returning_old_or_new_valid)
73387343
{
73397344
returning_old_or_new =
@@ -7342,7 +7347,18 @@ make_modifytable(PlannerInfo *root, Plan *subplan,
73427347
returning_old_or_new_valid = true;
73437348
}
73447349
if (!returning_old_or_new)
7345-
direct_modify = fdwroutine->PlanDirectModify(root, node, rti, i);
7350+
{
7351+
if (!transition_tables_valid)
7352+
{
7353+
transition_tables = has_transition_tables(root,
7354+
nominalRelation,
7355+
operation);
7356+
transition_tables_valid = true;
7357+
}
7358+
if (!transition_tables)
7359+
direct_modify = fdwroutine->PlanDirectModify(root, node,
7360+
rti, i);
7361+
}
73467362
}
73477363
if (direct_modify)
73487364
direct_modify_plans = bms_add_member(direct_modify_plans, i);

‎src/backend/optimizer/util/plancat.c

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2303,6 +2303,60 @@ has_row_triggers(PlannerInfo *root, Index rti, CmdType event)
23032303
return result;
23042304
}
23052305

2306+
/*
2307+
* has_transition_tables
2308+
*
2309+
* Detect whether the specified relation has any transition tables for event.
2310+
*/
2311+
bool
2312+
has_transition_tables(PlannerInfo *root, Index rti, CmdType event)
2313+
{
2314+
RangeTblEntry *rte = planner_rt_fetch(rti, root);
2315+
Relation relation;
2316+
TriggerDesc *trigDesc;
2317+
bool result = false;
2318+
2319+
Assert(rte->rtekind == RTE_RELATION);
2320+
2321+
/* Currently foreign tables cannot have transition tables */
2322+
if (rte->relkind == RELKIND_FOREIGN_TABLE)
2323+
return result;
2324+
2325+
/* Assume we already have adequate lock */
2326+
relation = table_open(rte->relid, NoLock);
2327+
2328+
trigDesc = relation->trigdesc;
2329+
switch (event)
2330+
{
2331+
case CMD_INSERT:
2332+
if (trigDesc &&
2333+
trigDesc->trig_insert_new_table)
2334+
result = true;
2335+
break;
2336+
case CMD_UPDATE:
2337+
if (trigDesc &&
2338+
(trigDesc->trig_update_old_table ||
2339+
trigDesc->trig_update_new_table))
2340+
result = true;
2341+
break;
2342+
case CMD_DELETE:
2343+
if (trigDesc &&
2344+
trigDesc->trig_delete_old_table)
2345+
result = true;
2346+
break;
2347+
/* There is no separate event for MERGE, only INSERT/UPDATE/DELETE */
2348+
case CMD_MERGE:
2349+
result = false;
2350+
break;
2351+
default:
2352+
elog(ERROR, "unrecognized CmdType: %d", (int) event);
2353+
break;
2354+
}
2355+
2356+
table_close(relation, NoLock);
2357+
return result;
2358+
}
2359+
23062360
/*
23072361
* has_stored_generated_columns
23082362
*

‎src/include/optimizer/plancat.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,8 @@ extern double get_function_rows(PlannerInfo *root, Oid funcid, Node *node);
7272

7373
extern bool has_row_triggers(PlannerInfo *root, Index rti, CmdType event);
7474

75+
extern bool has_transition_tables(PlannerInfo *root, Index rti, CmdType event);
76+
7577
extern bool has_stored_generated_columns(PlannerInfo *root, Index rti);
7678

7779
extern Bitmapset *get_dependent_generated_columns(PlannerInfo *root, Index rti,

0 commit comments

Comments
 (0)