Skip to content

Commit 7fc7dac

Browse files
robertmhaasrobertmhaas
committed
Pass the correct PlannerInfo to PlanForeignModify/PlanDirectModify.
Previously, we passed the toplevel PlannerInfo, but we actually want to pass the relevant subroot. One problem with passing the toplevel PlannerInfo is that the FDW which wants to push down an UPDATE or DELETE against a join won't find the relevant joinrel there. As of commit 1bc0100, postgres_fdw tries to do exactly this and can be made to fail an assertion as a result. It's possible that this should be regarded as a bug fix and back-patched to earlier releases, but for lack of a test case that fails in earlier releases, no back-patch for now. Etsuro Fujita, reviewed by Amit Langote. Discussion: http://postgr.es/m/5AF43E02.30000@lab.ntt.co.jp
1 parent 09b12d5 commit 7fc7dac

File tree

3 files changed

+121
-10
lines changed

3 files changed

+121
-10
lines changed

‎contrib/postgres_fdw/expected/postgres_fdw.out

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7370,6 +7370,81 @@ drop table bar cascade;
73707370
NOTICE: drop cascades to foreign table bar2
73717371
drop table loct1;
73727372
drop table loct2;
7373+
-- Test pushing down UPDATE/DELETE joins to the remote server
7374+
create table parent (a int, b text);
7375+
create table loct1 (a int, b text);
7376+
create table loct2 (a int, b text);
7377+
create foreign table remt1 (a int, b text)
7378+
server loopback options (table_name 'loct1');
7379+
create foreign table remt2 (a int, b text)
7380+
server loopback options (table_name 'loct2');
7381+
alter foreign table remt1 inherit parent;
7382+
insert into remt1 values (1, 'foo');
7383+
insert into remt1 values (2, 'bar');
7384+
insert into remt2 values (1, 'foo');
7385+
insert into remt2 values (2, 'bar');
7386+
analyze remt1;
7387+
analyze remt2;
7388+
explain (verbose, costs off)
7389+
update parent set b = parent.b || remt2.b from remt2 where parent.a = remt2.a returning *;
7390+
QUERY PLAN
7391+
-----------------------------------------------------------------------------------------------------------------------------------------------
7392+
Update on public.parent
7393+
Output: parent.a, parent.b, remt2.a, remt2.b
7394+
Update on public.parent
7395+
Foreign Update on public.remt1
7396+
-> Nested Loop
7397+
Output: parent.a, (parent.b || remt2.b), parent.ctid, remt2.*, remt2.a, remt2.b
7398+
Join Filter: (parent.a = remt2.a)
7399+
-> Seq Scan on public.parent
7400+
Output: parent.a, parent.b, parent.ctid
7401+
-> Foreign Scan on public.remt2
7402+
Output: remt2.b, remt2.*, remt2.a
7403+
Remote SQL: SELECT a, b FROM public.loct2
7404+
-> Foreign Update
7405+
Remote SQL: UPDATE public.loct1 r4 SET b = (r4.b || r2.b) FROM public.loct2 r2 WHERE ((r4.a = r2.a)) RETURNING r4.a, r4.b, r2.a, r2.b
7406+
(14 rows)
7407+
7408+
update parent set b = parent.b || remt2.b from remt2 where parent.a = remt2.a returning *;
7409+
a | b | a | b
7410+
---+--------+---+-----
7411+
1 | foofoo | 1 | foo
7412+
2 | barbar | 2 | bar
7413+
(2 rows)
7414+
7415+
explain (verbose, costs off)
7416+
delete from parent using remt2 where parent.a = remt2.a returning parent;
7417+
QUERY PLAN
7418+
------------------------------------------------------------------------------------------------------------------
7419+
Delete on public.parent
7420+
Output: parent.*
7421+
Delete on public.parent
7422+
Foreign Delete on public.remt1
7423+
-> Nested Loop
7424+
Output: parent.ctid, remt2.*
7425+
Join Filter: (parent.a = remt2.a)
7426+
-> Seq Scan on public.parent
7427+
Output: parent.ctid, parent.a
7428+
-> Foreign Scan on public.remt2
7429+
Output: remt2.*, remt2.a
7430+
Remote SQL: SELECT a, b FROM public.loct2
7431+
-> Foreign Delete
7432+
Remote SQL: DELETE FROM public.loct1 r4 USING public.loct2 r2 WHERE ((r4.a = r2.a)) RETURNING r4.a, r4.b
7433+
(14 rows)
7434+
7435+
delete from parent using remt2 where parent.a = remt2.a returning parent;
7436+
parent
7437+
------------
7438+
(1,foofoo)
7439+
(2,barbar)
7440+
(2 rows)
7441+
7442+
-- cleanup
7443+
drop foreign table remt1;
7444+
drop foreign table remt2;
7445+
drop table loct1;
7446+
drop table loct2;
7447+
drop table parent;
73737448
-- ===================================================================
73747449
-- test tuple routing for foreign-table partitions
73757450
-- ===================================================================

‎contrib/postgres_fdw/sql/postgres_fdw.sql

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1767,6 +1767,38 @@ drop table bar cascade;
17671767
drop table loct1;
17681768
drop table loct2;
17691769

1770+
-- Test pushing down UPDATE/DELETE joins to the remote server
1771+
create table parent (a int, b text);
1772+
create table loct1 (a int, b text);
1773+
create table loct2 (a int, b text);
1774+
create foreign table remt1 (a int, b text)
1775+
server loopback options (table_name 'loct1');
1776+
create foreign table remt2 (a int, b text)
1777+
server loopback options (table_name 'loct2');
1778+
alter foreign table remt1 inherit parent;
1779+
1780+
insert into remt1 values (1, 'foo');
1781+
insert into remt1 values (2, 'bar');
1782+
insert into remt2 values (1, 'foo');
1783+
insert into remt2 values (2, 'bar');
1784+
1785+
analyze remt1;
1786+
analyze remt2;
1787+
1788+
explain (verbose, costs off)
1789+
update parent set b = parent.b || remt2.b from remt2 where parent.a = remt2.a returning *;
1790+
update parent set b = parent.b || remt2.b from remt2 where parent.a = remt2.a returning *;
1791+
explain (verbose, costs off)
1792+
delete from parent using remt2 where parent.a = remt2.a returning parent;
1793+
delete from parent using remt2 where parent.a = remt2.a returning parent;
1794+
1795+
-- cleanup
1796+
drop foreign table remt1;
1797+
drop foreign table remt2;
1798+
drop table loct1;
1799+
drop table loct2;
1800+
drop table parent;
1801+
17701802
-- ===================================================================
17711803
-- test tuple routing for foreign-table partitions
17721804
-- ===================================================================

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

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -289,7 +289,7 @@ static ModifyTable *make_modifytable(PlannerInfo *root,
289289
CmdType operation, bool canSetTag,
290290
Index nominalRelation, List *partitioned_rels,
291291
bool partColsUpdated,
292-
List *resultRelations, List *subplans,
292+
List *resultRelations, List *subplans, List *subroots,
293293
List *withCheckOptionLists, List *returningLists,
294294
List *rowMarks, OnConflictExpr *onconflict, int epqParam);
295295
static GatherMerge *create_gather_merge_plan(PlannerInfo *root,
@@ -2484,6 +2484,7 @@ create_modifytable_plan(PlannerInfo *root, ModifyTablePath *best_path)
24842484
best_path->partColsUpdated,
24852485
best_path->resultRelations,
24862486
subplans,
2487+
best_path->subroots,
24872488
best_path->withCheckOptionLists,
24882489
best_path->returningLists,
24892490
best_path->rowMarks,
@@ -6558,17 +6559,19 @@ make_modifytable(PlannerInfo *root,
65586559
CmdType operation, bool canSetTag,
65596560
Index nominalRelation, List *partitioned_rels,
65606561
bool partColsUpdated,
6561-
List *resultRelations, List *subplans,
6562+
List *resultRelations, List *subplans, List *subroots,
65626563
List *withCheckOptionLists, List *returningLists,
65636564
List *rowMarks, OnConflictExpr *onconflict, int epqParam)
65646565
{
65656566
ModifyTable *node = makeNode(ModifyTable);
65666567
List *fdw_private_list;
65676568
Bitmapset *direct_modify_plans;
65686569
ListCell *lc;
6570+
ListCell *lc2;
65696571
int i;
65706572

65716573
Assert(list_length(resultRelations) == list_length(subplans));
6574+
Assert(list_length(resultRelations) == list_length(subroots));
65726575
Assert(withCheckOptionLists == NIL ||
65736576
list_length(resultRelations) == list_length(withCheckOptionLists));
65746577
Assert(returningLists == NIL ||
@@ -6627,9 +6630,10 @@ make_modifytable(PlannerInfo *root,
66276630
fdw_private_list = NIL;
66286631
direct_modify_plans = NULL;
66296632
i = 0;
6630-
foreach(lc, resultRelations)
6633+
forboth(lc, resultRelations, lc2, subroots)
66316634
{
66326635
Index rti = lfirst_int(lc);
6636+
PlannerInfo *subroot = lfirst_node(PlannerInfo, lc2);
66336637
FdwRoutine *fdwroutine;
66346638
List *fdw_private;
66356639
bool direct_modify;
@@ -6641,16 +6645,16 @@ make_modifytable(PlannerInfo *root,
66416645
* so it's not a baserel; and there are also corner cases for
66426646
* updatable views where the target rel isn't a baserel.)
66436647
*/
6644-
if (rti < root->simple_rel_array_size &&
6645-
root->simple_rel_array[rti] != NULL)
6648+
if (rti < subroot->simple_rel_array_size &&
6649+
subroot->simple_rel_array[rti] != NULL)
66466650
{
6647-
RelOptInfo *resultRel = root->simple_rel_array[rti];
6651+
RelOptInfo *resultRel = subroot->simple_rel_array[rti];
66486652

66496653
fdwroutine = resultRel->fdwroutine;
66506654
}
66516655
else
66526656
{
6653-
RangeTblEntry *rte = planner_rt_fetch(rti, root);
6657+
RangeTblEntry *rte = planner_rt_fetch(rti, subroot);
66546658

66556659
Assert(rte->rtekind == RTE_RELATION);
66566660
if (rte->relkind == RELKIND_FOREIGN_TABLE)
@@ -6672,15 +6676,15 @@ make_modifytable(PlannerInfo *root,
66726676
fdwroutine->IterateDirectModify != NULL &&
66736677
fdwroutine->EndDirectModify != NULL &&
66746678
withCheckOptionLists == NIL &&
6675-
!has_row_triggers(root, rti, operation))
6676-
direct_modify = fdwroutine->PlanDirectModify(root, node, rti, i);
6679+
!has_row_triggers(subroot, rti, operation))
6680+
direct_modify = fdwroutine->PlanDirectModify(subroot, node, rti, i);
66776681
if (direct_modify)
66786682
direct_modify_plans = bms_add_member(direct_modify_plans, i);
66796683

66806684
if (!direct_modify &&
66816685
fdwroutine != NULL &&
66826686
fdwroutine->PlanForeignModify != NULL)
6683-
fdw_private = fdwroutine->PlanForeignModify(root, node, rti, i);
6687+
fdw_private = fdwroutine->PlanForeignModify(subroot, node, rti, i);
66846688
else
66856689
fdw_private = NIL;
66866690
fdw_private_list = lappend(fdw_private_list, fdw_private);

0 commit comments

Comments
 (0)