From: Heikki Linnakangas Date: Tue, 13 Oct 2020 18:28:43 +0300 Subject: [PATCH v16 1/2] Include result relation index in ForeignScan for direct modify plans. FDWs that can perform an UPDATE/DELETE remotely using the "direct modify" set of APIs need in some cases to access the result relation properties for which they can currently look at EState.es_result_relation_info, which the core executor laboriously makes sure is set correctly. An upcoming patch will remove that field from EState. This commit adds a new resultRelation field in ForeignScan, to store the target relation's RT index. The FDW's PlanDirectModify callback is expected to set it along with 'operation'. The core code doesn't need it for anything, but the FDW's Begin- and IterateDirectModify callbacks can use it to get the target relation's ResultRelInfo. Amit Langote, Etsuro Fujita Discussion: https://www.postgresql.org/message-id/CA%2BHiwqGEmiib8FLiHMhKB%2BCH5dRgHSLc5N5wnvc4kym%2BZYpQEQ%40mail.gmail.com --- contrib/postgres_fdw/postgres_fdw.c | 41 +++++++++++++++++-------- doc/src/sgml/fdwhandler.sgml | 22 ++++++++----- src/backend/nodes/copyfuncs.c | 1 + src/backend/nodes/outfuncs.c | 1 + src/backend/nodes/readfuncs.c | 1 + src/backend/optimizer/plan/createplan.c | 4 +++ src/backend/optimizer/plan/setrefs.c | 4 +++ src/include/nodes/plannodes.h | 8 +++++ 8 files changed, 61 insertions(+), 21 deletions(-) diff --git a/contrib/postgres_fdw/postgres_fdw.c b/contrib/postgres_fdw/postgres_fdw.c index a31abce7c9..bfd73b40f2 100644 --- a/contrib/postgres_fdw/postgres_fdw.c +++ b/contrib/postgres_fdw/postgres_fdw.c @@ -218,6 +218,7 @@ typedef struct PgFdwDirectModifyState int num_tuples; /* # of result tuples */ int next_tuple; /* index of next one to return */ Relation resultRel; /* relcache entry for the target relation */ + ResultRelInfo *resultRelInfo; /* ResultRelInfo for the target relation */ AttrNumber *attnoMap; /* array of attnums of input user columns */ AttrNumber ctidAttno; /* attnum of input ctid column */ AttrNumber oidAttno; /* attnum of input oid column */ @@ -2287,9 +2288,10 @@ postgresPlanDirectModify(PlannerInfo *root, } /* - * Update the operation info. + * Update the operation and target relation info. */ fscan->operation = operation; + fscan->resultRelation = resultRelation; /* * Update the fdw_exprs list that will be available to the executor. @@ -2333,6 +2335,7 @@ postgresBeginDirectModify(ForeignScanState *node, int eflags) EState *estate = node->ss.ps.state; PgFdwDirectModifyState *dmstate; Index rtindex; + Relation rel; RangeTblEntry *rte; Oid userid; ForeignTable *table; @@ -2355,18 +2358,31 @@ postgresBeginDirectModify(ForeignScanState *node, int eflags) * Identify which user to do the remote access as. This should match what * ExecCheckRTEPerms() does. */ - rtindex = estate->es_result_relation_info->ri_RangeTableIndex; + Assert(fsplan->resultRelation > 0); + rtindex = fsplan->resultRelation; rte = exec_rt_fetch(rtindex, estate); userid = rte->checkAsUser ? rte->checkAsUser : GetUserId(); - /* Get info about foreign table. */ + /* + * Get info about target table. For a simple scan on a single foreign + * table, the target table is the table being scanned. For a join, it's + * one of the tables being joined. + */ if (fsplan->scan.scanrelid == 0) - dmstate->rel = ExecOpenScanRelation(estate, rtindex, eflags); + rel = ExecOpenScanRelation(estate, rtindex, eflags); else - dmstate->rel = node->ss.ss_currentRelation; - table = GetForeignTable(RelationGetRelid(dmstate->rel)); + { + Assert(rtindex == fsplan->scan.scanrelid); + rel = node->ss.ss_currentRelation; + } + table = GetForeignTable(RelationGetRelid(rel)); user = GetUserMapping(userid, table->serverid); + dmstate->resultRelInfo = estate->es_result_relations[rtindex - 1]; + /* the executor must have initialized the ResultRelInfo for us. */ + Assert(dmstate->resultRelInfo != NULL); + dmstate->resultRel = rel; + /* * Get connection to the foreign server. Connection manager will * establish new connection if necessary. @@ -2376,9 +2392,6 @@ postgresBeginDirectModify(ForeignScanState *node, int eflags) /* Update the foreign-join-related fields. */ if (fsplan->scan.scanrelid == 0) { - /* Save info about foreign table. */ - dmstate->resultRel = dmstate->rel; - /* * Set dmstate->rel to NULL to teach get_returning_data() and * make_tuple_from_result_row() that columns fetched from the remote @@ -2387,6 +2400,8 @@ postgresBeginDirectModify(ForeignScanState *node, int eflags) */ dmstate->rel = NULL; } + else + dmstate->rel = rel; /* Initialize state variable */ dmstate->num_tuples = -1; /* -1 means not set yet */ @@ -2450,7 +2465,7 @@ postgresIterateDirectModify(ForeignScanState *node) { PgFdwDirectModifyState *dmstate = (PgFdwDirectModifyState *) node->fdw_state; EState *estate = node->ss.ps.state; - ResultRelInfo *resultRelInfo = estate->es_result_relation_info; + ResultRelInfo *resultRelInfo = dmstate->resultRelInfo; /* * If this is the first call after Begin, execute the statement. @@ -4085,8 +4100,8 @@ static TupleTableSlot * get_returning_data(ForeignScanState *node) { PgFdwDirectModifyState *dmstate = (PgFdwDirectModifyState *) node->fdw_state; + ResultRelInfo *resultRelInfo = dmstate->resultRelInfo; EState *estate = node->ss.ps.state; - ResultRelInfo *resultRelInfo = estate->es_result_relation_info; TupleTableSlot *slot = node->ss.ss_ScanTupleSlot; TupleTableSlot *resultSlot; @@ -4233,7 +4248,7 @@ apply_returning_filter(PgFdwDirectModifyState *dmstate, TupleTableSlot *slot, EState *estate) { - ResultRelInfo *relInfo = estate->es_result_relation_info; + ResultRelInfo *resultRelInfo = dmstate->resultRelInfo; TupleDesc resultTupType = RelationGetDescr(dmstate->resultRel); TupleTableSlot *resultSlot; Datum *values; @@ -4245,7 +4260,7 @@ apply_returning_filter(PgFdwDirectModifyState *dmstate, /* * Use the return tuple slot as a place to store the result tuple. */ - resultSlot = ExecGetReturningSlot(estate, relInfo); + resultSlot = ExecGetReturningSlot(estate, resultRelInfo); /* * Extract all the values of the scan tuple. diff --git a/doc/src/sgml/fdwhandler.sgml b/doc/src/sgml/fdwhandler.sgml index 72fa127212..43f287a29c 100644 --- a/doc/src/sgml/fdwhandler.sgml +++ b/doc/src/sgml/fdwhandler.sgml @@ -861,11 +861,15 @@ PlanDirectModify(PlannerInfo *root, To execute the direct modification on the remote server, this function must rewrite the target subplan with a ForeignScan plan node that executes the direct modification on the remote server. The - operation field of the ForeignScan must - be set to the CmdType enumeration appropriately; that is, + operation and resultRelation fields + of the ForeignScan must be set appropriately. + operation must be set to the CmdType + enumeration corresponding to the statement kind (that is, CMD_UPDATE for UPDATE, CMD_INSERT for INSERT, and - CMD_DELETE for DELETE. + CMD_DELETE for DELETE), and the + resultRelation argument must be copied to the + resultRelation field. @@ -892,9 +896,10 @@ BeginDirectModify(ForeignScanState *node, The ForeignScanState node has already been created, but its fdw_state field is still NULL. Information about the table to modify is accessible through the - ForeignScanState node (in particular, from the underlying - ForeignScan plan node, which contains any FDW-private - information provided by PlanDirectModify). + ForeignScanState node (in particular, from the + underlying ForeignScan plan node, which contains + the target table's range table index and any FDW-private information + provided by PlanDirectModify). eflags contains flag bits describing the executor's operating mode for this plan node. @@ -926,8 +931,9 @@ IterateDirectModify(ForeignScanState *node); tuple table slot (the node's ScanTupleSlot should be used for this purpose). The data that was actually inserted, updated or deleted must be stored in the - es_result_relation_info->ri_projectReturning->pi_exprContext->ecxt_scantuple - of the node's EState. + ri_projectReturning->pi_exprContext->ecxt_scantuple + of the target foreign table's ResultRelInfo + obtained using the information passed to BeginDirectModify. Return NULL if no more rows are available. Note that this is called in a short-lived memory context that will be reset between invocations. Create a memory context in diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c index 4d79f70950..2b4d7654cc 100644 --- a/src/backend/nodes/copyfuncs.c +++ b/src/backend/nodes/copyfuncs.c @@ -758,6 +758,7 @@ _copyForeignScan(const ForeignScan *from) COPY_NODE_FIELD(fdw_recheck_quals); COPY_BITMAPSET_FIELD(fs_relids); COPY_SCALAR_FIELD(fsSystemCol); + COPY_SCALAR_FIELD(resultRelation); return newnode; } diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c index f441ae3c51..08a049232e 100644 --- a/src/backend/nodes/outfuncs.c +++ b/src/backend/nodes/outfuncs.c @@ -695,6 +695,7 @@ _outForeignScan(StringInfo str, const ForeignScan *node) WRITE_NODE_FIELD(fdw_recheck_quals); WRITE_BITMAPSET_FIELD(fs_relids); WRITE_BOOL_FIELD(fsSystemCol); + WRITE_INT_FIELD(resultRelation); } static void diff --git a/src/backend/nodes/readfuncs.c b/src/backend/nodes/readfuncs.c index 3a54765f5c..ab7b535caa 100644 --- a/src/backend/nodes/readfuncs.c +++ b/src/backend/nodes/readfuncs.c @@ -2014,6 +2014,7 @@ _readForeignScan(void) READ_NODE_FIELD(fdw_recheck_quals); READ_BITMAPSET_FIELD(fs_relids); READ_BOOL_FIELD(fsSystemCol); + READ_INT_FIELD(resultRelation); READ_DONE(); } diff --git a/src/backend/optimizer/plan/createplan.c b/src/backend/optimizer/plan/createplan.c index 881eaf4813..94280a730c 100644 --- a/src/backend/optimizer/plan/createplan.c +++ b/src/backend/optimizer/plan/createplan.c @@ -5530,7 +5530,11 @@ make_foreignscan(List *qptlist, plan->lefttree = outer_plan; plan->righttree = NULL; node->scan.scanrelid = scanrelid; + + /* these may be overridden by the FDW's PlanDirectModify callback. */ node->operation = CMD_SELECT; + node->resultRelation = 0; + /* fs_server will be filled in by create_foreignscan_plan */ node->fs_server = InvalidOid; node->fdw_exprs = fdw_exprs; diff --git a/src/backend/optimizer/plan/setrefs.c b/src/backend/optimizer/plan/setrefs.c index 6847ff6f44..8b43371425 100644 --- a/src/backend/optimizer/plan/setrefs.c +++ b/src/backend/optimizer/plan/setrefs.c @@ -1310,6 +1310,10 @@ set_foreignscan_references(PlannerInfo *root, } fscan->fs_relids = offset_relid_set(fscan->fs_relids, rtoffset); + + /* Adjust resultRelation if it's valid */ + if (fscan->resultRelation > 0) + fscan->resultRelation += rtoffset; } /* diff --git a/src/include/nodes/plannodes.h b/src/include/nodes/plannodes.h index a7bdf3497e..7e6b10f86b 100644 --- a/src/include/nodes/plannodes.h +++ b/src/include/nodes/plannodes.h @@ -599,12 +599,20 @@ typedef struct WorkTableScan * When the plan node represents a foreign join, scan.scanrelid is zero and * fs_relids must be consulted to identify the join relation. (fs_relids * is valid for simple scans as well, but will always match scan.scanrelid.) + * + * If the FDW's PlanDirectModify() callback decides to repurpose a ForeignScan + * node to perform the UPDATE or DELETE operation directly in the remote + * server, it sets 'operation' and 'resultRelation' to identify the operation + * type and target relation. Note that these fields are only set if the + * modification is performed *fully* remotely; otherwise, the modification is + * driven by a local ModifyTable node and 'operation' is left to CMD_SELECT. * ---------------- */ typedef struct ForeignScan { Scan scan; CmdType operation; /* SELECT/INSERT/UPDATE/DELETE */ + Index resultRelation; /* direct modification target's RT index */ Oid fs_server; /* OID of foreign server */ List *fdw_exprs; /* expressions that FDW may evaluate */ List *fdw_private; /* private data for FDW */ -- 2.20.1 --------------6149DBE54148CBD3985845BA Content-Type: text/x-patch; charset=UTF-8; name="v16-0002-Remove-es_result_relation_info.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="v16-0002-Remove-es_result_relation_info.patch"