Re: join pushdown and issue with foreign update - Mailing list pgsql-hackers

I wrote:
> I think a preferable fix involves making sure that the correct
> record-type typmod is propagated to record_in in this context.
> Alternatively, maybe we could insert the foreign table's rowtype
> during execution of the input operation, without touching the
> plan as such.

Here's a draft-quality patch based on that idea.  It resolves
the offered test case, but I haven't beat on it beyond that.

            regards, tom lane

diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out
index 7df30010f2..79bc08efb4 100644
--- a/contrib/postgres_fdw/expected/postgres_fdw.out
+++ b/contrib/postgres_fdw/expected/postgres_fdw.out
@@ -10231,3 +10231,50 @@ DROP TABLE result_tbl;
 DROP TABLE join_tbl;
 ALTER SERVER loopback OPTIONS (DROP async_capable);
 ALTER SERVER loopback2 OPTIONS (DROP async_capable);
+CREATE TABLE base_tbl (a int, b int);
+CREATE FOREIGN TABLE remote_tbl (a int, b int)
+  SERVER loopback OPTIONS (table_name 'base_tbl');
+INSERT INTO base_tbl SELECT a, a+1 FROM generate_series(1,10) a;
+ANALYZE base_tbl;
+ANALYZE remote_tbl;
+EXPLAIN (VERBOSE, COSTS OFF)
+UPDATE remote_tbl d SET a = CASE WHEN random() >= 0 THEN 5 ELSE 6 END
+  FROM remote_tbl AS t WHERE d.a = t.a;
+
 QUERY PLAN
            

+------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
+ Update on public.remote_tbl d
+   Remote SQL: UPDATE public.base_tbl SET a = $2 WHERE ctid = $1
+   ->  Foreign Scan
+         Output: CASE WHEN (random() >= '0'::double precision) THEN 5 ELSE 6 END, d.ctid, d.*, t.*
+         Relations: (public.remote_tbl d) INNER JOIN (public.remote_tbl t)
+         Remote SQL: SELECT r1.ctid, CASE WHEN (r1.*)::text IS NOT NULL THEN ROW(r1.a, r1.b) END, CASE WHEN
(r2.*)::textIS NOT NULL THEN ROW(r2.a, r2.b) END FROM (public.base_tbl r1 INNER JOIN public.base_tbl r2 ON (((r1.a =
r2.a))))FOR UPDATE OF r1 
+         ->  Hash Join
+               Output: d.ctid, d.*, t.*
+               Hash Cond: (d.a = t.a)
+               ->  Foreign Scan on public.remote_tbl d
+                     Output: d.ctid, d.*, d.a
+                     Remote SQL: SELECT a, b, ctid FROM public.base_tbl FOR UPDATE
+               ->  Hash
+                     Output: t.*, t.a
+                     ->  Foreign Scan on public.remote_tbl t
+                           Output: t.*, t.a
+                           Remote SQL: SELECT a, b FROM public.base_tbl
+(17 rows)
+
+UPDATE remote_tbl d SET a = CASE WHEN random() >= 0 THEN 5 ELSE 6 END
+  FROM remote_tbl AS t WHERE d.a = t.a;
+SELECT * FROM base_tbl ORDER BY b;
+ a | b
+---+----
+ 5 |  2
+ 5 |  3
+ 5 |  4
+ 5 |  5
+ 5 |  6
+ 5 |  7
+ 5 |  8
+ 5 |  9
+ 5 | 10
+ 5 | 11
+(10 rows)
+
diff --git a/contrib/postgres_fdw/postgres_fdw.c b/contrib/postgres_fdw/postgres_fdw.c
index c48a421e88..24ba60e00a 100644
--- a/contrib/postgres_fdw/postgres_fdw.c
+++ b/contrib/postgres_fdw/postgres_fdw.c
@@ -1439,6 +1439,57 @@ postgresGetForeignPlan(PlannerInfo *root,
                             outer_plan);
 }

+/*
+ * Construct a tuple descriptor for the scan tuples handled by a foreign join.
+ */
+static TupleDesc
+get_tupdesc_for_join_scan_tuples(ForeignScanState *node)
+{
+    ForeignScan *fsplan = (ForeignScan *) node->ss.ps.plan;
+    EState       *estate = node->ss.ps.state;
+    TupleDesc    tupdesc;
+
+    /*
+     * The core code has already set up a scan tuple slot based on
+     * fsplan->fdw_scan_tlist, and this slot's tupdesc is mostly good enough,
+     * but there's one case where it isn't.  If we have any whole-row row
+     * identifier Vars, they may have vartype RECORD, and we need to replace
+     * that with the associated table's actual composite type.  This ensures
+     * that when we read those ROW() expression values from the remote server,
+     * we can convert them to a composite type the local server knows.
+     */
+    tupdesc = CreateTupleDescCopy(node->ss.ss_ScanTupleSlot->tts_tupleDescriptor);
+    for (int i = 0; i < tupdesc->natts; i++)
+    {
+        Form_pg_attribute att = TupleDescAttr(tupdesc, i);
+        Var           *var;
+        RangeTblEntry *rte;
+        Oid            reltype;
+
+        /* Nothing to do if it's not a generic RECORD attribute */
+        if (att->atttypid != RECORDOID || att->atttypmod >= 0)
+            continue;
+
+        /*
+         * If we can't identify the referenced table, do nothing.  This'll
+         * likely lead to failure later, but perhaps we can muddle through.
+         */
+        var = (Var *) list_nth_node(TargetEntry, fsplan->fdw_scan_tlist,
+                                    i)->expr;
+        if (!IsA(var, Var))
+            continue;
+        rte = list_nth(estate->es_range_table, var->varno - 1);
+        if (rte->rtekind != RTE_RELATION)
+            continue;
+        reltype = get_rel_type_id(rte->relid);
+        if (!OidIsValid(reltype))
+            continue;
+        att->atttypid = reltype;
+        /* shouldn't need to change anything else */
+    }
+    return tupdesc;
+}
+
 /*
  * postgresBeginForeignScan
  *        Initiate an executor scan of a foreign PostgreSQL table.
@@ -1523,7 +1574,7 @@ postgresBeginForeignScan(ForeignScanState *node, int eflags)
     else
     {
         fsstate->rel = NULL;
-        fsstate->tupdesc = node->ss.ss_ScanTupleSlot->tts_tupleDescriptor;
+        fsstate->tupdesc = get_tupdesc_for_join_scan_tuples(node);
     }

     fsstate->attinmeta = TupleDescGetAttInMetadata(fsstate->tupdesc);
@@ -2631,7 +2682,7 @@ postgresBeginDirectModify(ForeignScanState *node, int eflags)
         TupleDesc    tupdesc;

         if (fsplan->scan.scanrelid == 0)
-            tupdesc = node->ss.ss_ScanTupleSlot->tts_tupleDescriptor;
+            tupdesc = get_tupdesc_for_join_scan_tuples(node);
         else
             tupdesc = RelationGetDescr(dmstate->rel);

diff --git a/contrib/postgres_fdw/sql/postgres_fdw.sql b/contrib/postgres_fdw/sql/postgres_fdw.sql
index 78379bdea5..a7070870c7 100644
--- a/contrib/postgres_fdw/sql/postgres_fdw.sql
+++ b/contrib/postgres_fdw/sql/postgres_fdw.sql
@@ -3262,3 +3262,20 @@ DROP TABLE join_tbl;

 ALTER SERVER loopback OPTIONS (DROP async_capable);
 ALTER SERVER loopback2 OPTIONS (DROP async_capable);
+
+CREATE TABLE base_tbl (a int, b int);
+CREATE FOREIGN TABLE remote_tbl (a int, b int)
+  SERVER loopback OPTIONS (table_name 'base_tbl');
+
+INSERT INTO base_tbl SELECT a, a+1 FROM generate_series(1,10) a;
+
+ANALYZE base_tbl;
+ANALYZE remote_tbl;
+
+EXPLAIN (VERBOSE, COSTS OFF)
+UPDATE remote_tbl d SET a = CASE WHEN random() >= 0 THEN 5 ELSE 6 END
+  FROM remote_tbl AS t WHERE d.a = t.a;
+UPDATE remote_tbl d SET a = CASE WHEN random() >= 0 THEN 5 ELSE 6 END
+  FROM remote_tbl AS t WHERE d.a = t.a;
+
+SELECT * FROM base_tbl ORDER BY b;