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 1bc0100d270e5bcc980a0629b8726a32a497e788, 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

diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out
index bb6b1a8fdf69c4741b999d479d129b9b9602d900..e803c0b39cafe7b00936560f3e8edc0c7676cf53 100644 (file)
--- a/contrib/postgres_fdw/expected/postgres_fdw.out
+++ b/contrib/postgres_fdw/expected/postgres_fdw.out
@@ -7370,6 +7370,81 @@ drop table bar cascade;
 NOTICE:  drop cascades to foreign table bar2
 drop table loct1;
 drop table loct2;
+-- Test pushing down UPDATE/DELETE joins to the remote server
+create table parent (a int, b text);
+create table loct1 (a int, b text);
+create table loct2 (a int, b text);
+create foreign table remt1 (a int, b text)
+  server loopback options (table_name 'loct1');
+create foreign table remt2 (a int, b text)
+  server loopback options (table_name 'loct2');
+alter foreign table remt1 inherit parent;
+insert into remt1 values (1, 'foo');
+insert into remt1 values (2, 'bar');
+insert into remt2 values (1, 'foo');
+insert into remt2 values (2, 'bar');
+analyze remt1;
+analyze remt2;
+explain (verbose, costs off)
+update parent set b = parent.b || remt2.b from remt2 where parent.a = remt2.a returning *;
+                                                                  QUERY PLAN                                                                   
+-----------------------------------------------------------------------------------------------------------------------------------------------
+ Update on public.parent
+   Output: parent.a, parent.b, remt2.a, remt2.b
+   Update on public.parent
+   Foreign Update on public.remt1
+   ->  Nested Loop
+         Output: parent.a, (parent.b || remt2.b), parent.ctid, remt2.*, remt2.a, remt2.b
+         Join Filter: (parent.a = remt2.a)
+         ->  Seq Scan on public.parent
+               Output: parent.a, parent.b, parent.ctid
+         ->  Foreign Scan on public.remt2
+               Output: remt2.b, remt2.*, remt2.a
+               Remote SQL: SELECT a, b FROM public.loct2
+   ->  Foreign Update
+         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
+(14 rows)
+
+update parent set b = parent.b || remt2.b from remt2 where parent.a = remt2.a returning *;
+ a |   b    | a |  b  
+---+--------+---+-----
+ 1 | foofoo | 1 | foo
+ 2 | barbar | 2 | bar
+(2 rows)
+
+explain (verbose, costs off)
+delete from parent using remt2 where parent.a = remt2.a returning parent;
+                                                    QUERY PLAN                                                    
+------------------------------------------------------------------------------------------------------------------
+ Delete on public.parent
+   Output: parent.*
+   Delete on public.parent
+   Foreign Delete on public.remt1
+   ->  Nested Loop
+         Output: parent.ctid, remt2.*
+         Join Filter: (parent.a = remt2.a)
+         ->  Seq Scan on public.parent
+               Output: parent.ctid, parent.a
+         ->  Foreign Scan on public.remt2
+               Output: remt2.*, remt2.a
+               Remote SQL: SELECT a, b FROM public.loct2
+   ->  Foreign Delete
+         Remote SQL: DELETE FROM public.loct1 r4 USING public.loct2 r2 WHERE ((r4.a = r2.a)) RETURNING r4.a, r4.b
+(14 rows)
+
+delete from parent using remt2 where parent.a = remt2.a returning parent;
+   parent   
+------------
+ (1,foofoo)
+ (2,barbar)
+(2 rows)
+
+-- cleanup
+drop foreign table remt1;
+drop foreign table remt2;
+drop table loct1;
+drop table loct2;
+drop table parent;
 -- ===================================================================
 -- test tuple routing for foreign-table partitions
 -- ===================================================================

diff --git a/contrib/postgres_fdw/sql/postgres_fdw.sql b/contrib/postgres_fdw/sql/postgres_fdw.sql
index 231b1e01a5fc97febb987714411a00cfe0923f8c..cdfd9c960e50da975978f95760ed37ac988a9ef1 100644 (file)
--- a/contrib/postgres_fdw/sql/postgres_fdw.sql
+++ b/contrib/postgres_fdw/sql/postgres_fdw.sql
@@ -1767,6 +1767,38 @@ drop table bar cascade;
 drop table loct1;
 drop table loct2;
 
+-- Test pushing down UPDATE/DELETE joins to the remote server
+create table parent (a int, b text);
+create table loct1 (a int, b text);
+create table loct2 (a int, b text);
+create foreign table remt1 (a int, b text)
+  server loopback options (table_name 'loct1');
+create foreign table remt2 (a int, b text)
+  server loopback options (table_name 'loct2');
+alter foreign table remt1 inherit parent;
+
+insert into remt1 values (1, 'foo');
+insert into remt1 values (2, 'bar');
+insert into remt2 values (1, 'foo');
+insert into remt2 values (2, 'bar');
+
+analyze remt1;
+analyze remt2;
+
+explain (verbose, costs off)
+update parent set b = parent.b || remt2.b from remt2 where parent.a = remt2.a returning *;
+update parent set b = parent.b || remt2.b from remt2 where parent.a = remt2.a returning *;
+explain (verbose, costs off)
+delete from parent using remt2 where parent.a = remt2.a returning parent;
+delete from parent using remt2 where parent.a = remt2.a returning parent;
+
+-- cleanup
+drop foreign table remt1;
+drop foreign table remt2;
+drop table loct1;
+drop table loct2;
+drop table parent;
+
 -- ===================================================================
 -- test tuple routing for foreign-table partitions
 -- ===================================================================

diff --git a/src/backend/optimizer/plan/createplan.c b/src/backend/optimizer/plan/createplan.c
index 0317763f43c7a10313176c3d063ea620cbe855ae..ca2e0527dbce5411cda1a9b213f9d41343e48865 100644 (file)
--- a/src/backend/optimizer/plan/createplan.c
+++ b/src/backend/optimizer/plan/createplan.c
@@ -289,7 +289,7 @@ static ModifyTable *make_modifytable(PlannerInfo *root,
                 CmdType operation, bool canSetTag,
                 Index nominalRelation, List *partitioned_rels,
                 bool partColsUpdated,
-                List *resultRelations, List *subplans,
+                List *resultRelations, List *subplans, List *subroots,
                 List *withCheckOptionLists, List *returningLists,
                 List *rowMarks, OnConflictExpr *onconflict, int epqParam);
 static GatherMerge *create_gather_merge_plan(PlannerInfo *root,
@@ -2484,6 +2484,7 @@ create_modifytable_plan(PlannerInfo *root, ModifyTablePath *best_path)
                            best_path->partColsUpdated,
                            best_path->resultRelations,
                            subplans,
+                           best_path->subroots,
                            best_path->withCheckOptionLists,
                            best_path->returningLists,
                            best_path->rowMarks,
@@ -6558,7 +6559,7 @@ make_modifytable(PlannerInfo *root,
                 CmdType operation, bool canSetTag,
                 Index nominalRelation, List *partitioned_rels,
                 bool partColsUpdated,
-                List *resultRelations, List *subplans,
+                List *resultRelations, List *subplans, List *subroots,
                 List *withCheckOptionLists, List *returningLists,
                 List *rowMarks, OnConflictExpr *onconflict, int epqParam)
 {
@@ -6566,9 +6567,11 @@ make_modifytable(PlannerInfo *root,
    List       *fdw_private_list;
    Bitmapset  *direct_modify_plans;
    ListCell   *lc;
+   ListCell   *lc2;
    int         i;
 
    Assert(list_length(resultRelations) == list_length(subplans));
+   Assert(list_length(resultRelations) == list_length(subroots));
    Assert(withCheckOptionLists == NIL ||
           list_length(resultRelations) == list_length(withCheckOptionLists));
    Assert(returningLists == NIL ||
@@ -6627,9 +6630,10 @@ make_modifytable(PlannerInfo *root,
    fdw_private_list = NIL;
    direct_modify_plans = NULL;
    i = 0;
-   foreach(lc, resultRelations)
+   forboth(lc, resultRelations, lc2, subroots)
    {
        Index       rti = lfirst_int(lc);
+       PlannerInfo *subroot = lfirst_node(PlannerInfo, lc2);
        FdwRoutine *fdwroutine;
        List       *fdw_private;
        bool        direct_modify;
@@ -6641,16 +6645,16 @@ make_modifytable(PlannerInfo *root,
         * so it's not a baserel; and there are also corner cases for
         * updatable views where the target rel isn't a baserel.)
         */
-       if (rti < root->simple_rel_array_size &&
-           root->simple_rel_array[rti] != NULL)
+       if (rti < subroot->simple_rel_array_size &&
+           subroot->simple_rel_array[rti] != NULL)
        {
-           RelOptInfo *resultRel = root->simple_rel_array[rti];
+           RelOptInfo *resultRel = subroot->simple_rel_array[rti];
 
            fdwroutine = resultRel->fdwroutine;
        }
        else
        {
-           RangeTblEntry *rte = planner_rt_fetch(rti, root);
+           RangeTblEntry *rte = planner_rt_fetch(rti, subroot);
 
            Assert(rte->rtekind == RTE_RELATION);
            if (rte->relkind == RELKIND_FOREIGN_TABLE)
@@ -6672,15 +6676,15 @@ make_modifytable(PlannerInfo *root,
            fdwroutine->IterateDirectModify != NULL &&
            fdwroutine->EndDirectModify != NULL &&
            withCheckOptionLists == NIL &&
-           !has_row_triggers(root, rti, operation))
-           direct_modify = fdwroutine->PlanDirectModify(root, node, rti, i);
+           !has_row_triggers(subroot, rti, operation))
+           direct_modify = fdwroutine->PlanDirectModify(subroot, node, rti, i);
        if (direct_modify)
            direct_modify_plans = bms_add_member(direct_modify_plans, i);
 
        if (!direct_modify &&
            fdwroutine != NULL &&
            fdwroutine->PlanForeignModify != NULL)
-           fdw_private = fdwroutine->PlanForeignModify(root, node, rti, i);
+           fdw_private = fdwroutine->PlanForeignModify(subroot, node, rti, i);
        else
            fdw_private = NIL;
        fdw_private_list = lappend(fdw_private_list, fdw_private);