From f61db909dfb94f3411f8719916601a11a905b95e Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Fri, 4 Jun 2021 20:07:08 -0400 Subject: [PATCH] Fix postgres_fdw failure with whole-row Vars of type RECORD. Commit 86dc90056 expects that FDWs can cope with whole-row Vars for their tables, even if the Vars are marked with vartype RECORDOID. Previously, whole-row Vars generated by the planner had vartype equal to the relevant table's rowtype OID. (The point behind this change is to enable sharing of resjunk columns across inheritance child tables.) It turns out that postgres_fdw fails to cope with this, though through bad fortune none of its test cases exposed that. Things mostly work, but when we try to read back a value of such a Var, the expected rowtype is not available to record_in(). Fortunately, it's not difficult to hack up the tupdesc that controls this process to substitute the foreign table's rowtype for RECORDOID. Thus we can solve the runtime problem while still sharing the resjunk column with other tables. Per report from Alexander Pyhalov. Discussion: https://postgr.es/m/7817fb9ebd6661cdf9b67dec6e129a78@postgrespro.ru --- .../postgres_fdw/expected/postgres_fdw.out | 25 +++++++++ contrib/postgres_fdw/postgres_fdw.c | 55 ++++++++++++++++++- contrib/postgres_fdw/sql/postgres_fdw.sql | 8 +++ 3 files changed, 86 insertions(+), 2 deletions(-) diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out index f320a7578d..0659656b6b 100644 --- a/contrib/postgres_fdw/expected/postgres_fdw.out +++ b/contrib/postgres_fdw/expected/postgres_fdw.out @@ -5531,6 +5531,31 @@ UPDATE ft2 AS target SET (c2) = ( FROM ft2 AS src WHERE target.c1 = src.c1 ) WHERE c1 > 1100; +-- Test UPDATE involving a join that can be pushed down, +-- but a SET clause that can't be +EXPLAIN (VERBOSE, COSTS OFF) +UPDATE ft2 d SET c2 = CASE WHEN random() >= 0 THEN d.c2 ELSE 0 END + FROM ft2 AS t WHERE d.c1 = t.c1 AND d.c1 > 1100; + QUERY PLAN +----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- + Update on public.ft2 d + Remote SQL: UPDATE "S 1"."T 1" SET c2 = $2 WHERE ctid = $1 + -> Foreign Scan + Output: CASE WHEN (random() >= '0'::double precision) THEN d.c2 ELSE 0 END, d.ctid, d.*, t.* + Relations: (public.ft2 d) INNER JOIN (public.ft2 t) + Remote SQL: SELECT r1.c2, r1.ctid, CASE WHEN (r1.*)::text IS NOT NULL THEN ROW(r1."C 1", r1.c2, r1.c3, r1.c4, r1.c5, r1.c6, r1.c7, r1.c8) END, CASE WHEN (r2.*)::text IS NOT NULL THEN ROW(r2."C 1", r2.c2, r2.c3, r2.c4, r2.c5, r2.c6, r2.c7, r2.c8) END FROM ("S 1"."T 1" r1 INNER JOIN "S 1"."T 1" r2 ON (((r1."C 1" = r2."C 1")) AND ((r1."C 1" > 1100)))) FOR UPDATE OF r1 + -> Nested Loop + Output: d.c2, d.ctid, d.*, t.* + -> Foreign Scan on public.ft2 d + Output: d.c2, d.ctid, d.*, d.c1 + Remote SQL: SELECT "C 1", c2, c3, c4, c5, c6, c7, c8, ctid FROM "S 1"."T 1" WHERE (("C 1" > 1100)) ORDER BY "C 1" ASC NULLS LAST FOR UPDATE + -> Foreign Scan on public.ft2 t + Output: t.*, t.c1 + Remote SQL: SELECT "C 1", c2, c3, c4, c5, c6, c7, c8 FROM "S 1"."T 1" WHERE (($1::integer = "C 1")) +(14 rows) + +UPDATE ft2 d SET c2 = CASE WHEN random() >= 0 THEN d.c2 ELSE 0 END + FROM ft2 AS t WHERE d.c1 = t.c1 AND d.c1 > 1100; -- Test UPDATE/DELETE with WHERE or JOIN/ON conditions containing -- user-defined operators/functions ALTER SERVER loopback OPTIONS (DROP extensions); diff --git a/contrib/postgres_fdw/postgres_fdw.c b/contrib/postgres_fdw/postgres_fdw.c index c48a421e88..0843ed9dba 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) || var->varattno != 0) + 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 17dba77d7e..79295e996d 100644 --- a/contrib/postgres_fdw/sql/postgres_fdw.sql +++ b/contrib/postgres_fdw/sql/postgres_fdw.sql @@ -1255,6 +1255,14 @@ UPDATE ft2 AS target SET (c2) = ( WHERE target.c1 = src.c1 ) WHERE c1 > 1100; +-- Test UPDATE involving a join that can be pushed down, +-- but a SET clause that can't be +EXPLAIN (VERBOSE, COSTS OFF) +UPDATE ft2 d SET c2 = CASE WHEN random() >= 0 THEN d.c2 ELSE 0 END + FROM ft2 AS t WHERE d.c1 = t.c1 AND d.c1 > 1100; +UPDATE ft2 d SET c2 = CASE WHEN random() >= 0 THEN d.c2 ELSE 0 END + FROM ft2 AS t WHERE d.c1 = t.c1 AND d.c1 > 1100; + -- Test UPDATE/DELETE with WHERE or JOIN/ON conditions containing -- user-defined operators/functions ALTER SERVER loopback OPTIONS (DROP extensions);