From e0e60f50bea1cb7228b8d84c88c3aab533e67400 Mon Sep 17 00:00:00 2001 From: Etsuro Fujita Date: Thu, 8 Aug 2019 21:41:12 +0900 Subject: [PATCH v2 1/3] Revise how some FDW executor APIs obtain ResultRelInfo This adds a field to ForeignScan node to get the index of the target foreign table in the global list of query result relations (the es_result_relations array). So, BeginDirectModify() uses that index to get the RT index of the target relation from the global list of target relation RT indexes. Also, IterateDirectModify() uses that index to get the ResultRelInfo from es_result_relations. Amit Langote, Etsuro Fujita Discussion: https://postgr.es/m/20190718010911.l6xcdv6birtxiei4@alap3.anarazel.de --- contrib/postgres_fdw/postgres_fdw.c | 34 +++++++++++++++++++++++++++------ doc/src/sgml/fdwhandler.sgml | 5 +++-- src/backend/executor/nodeForeignscan.c | 11 +++++++---- src/backend/nodes/copyfuncs.c | 1 + src/backend/nodes/outfuncs.c | 1 + src/backend/nodes/readfuncs.c | 1 + src/backend/optimizer/plan/createplan.c | 2 ++ src/backend/optimizer/plan/setrefs.c | 15 +++++++++++++++ src/include/nodes/plannodes.h | 3 +++ 9 files changed, 61 insertions(+), 12 deletions(-) diff --git a/contrib/postgres_fdw/postgres_fdw.c b/contrib/postgres_fdw/postgres_fdw.c index 697ec68..4a495bf 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 */ @@ -361,7 +362,8 @@ static bool postgresPlanDirectModify(PlannerInfo *root, ModifyTable *plan, Index resultRelation, int subplan_index); -static void postgresBeginDirectModify(ForeignScanState *node, int eflags); +static void postgresBeginDirectModify(ForeignScanState *node, + int eflags); static TupleTableSlot *postgresIterateDirectModify(ForeignScanState *node); static void postgresEndDirectModify(ForeignScanState *node); static void postgresExplainForeignScan(ForeignScanState *node, @@ -2321,6 +2323,11 @@ postgresPlanDirectModify(PlannerInfo *root, rebuild_fdw_scan_tlist(fscan, returningList); } + /* + * Set the index of the subplan result rel. + */ + fscan->resultRelIndex = subplan_index; + table_close(rel, NoLock); return true; } @@ -2330,10 +2337,12 @@ postgresPlanDirectModify(PlannerInfo *root, * Prepare a direct foreign table modification */ static void -postgresBeginDirectModify(ForeignScanState *node, int eflags) +postgresBeginDirectModify(ForeignScanState *node, + int eflags) { ForeignScan *fsplan = (ForeignScan *) node->ss.ps.plan; EState *estate = node->ss.ps.state; + List *resultRelations = estate->es_plannedstmt->resultRelations; PgFdwDirectModifyState *dmstate; Index rtindex; RangeTblEntry *rte; @@ -2358,7 +2367,8 @@ 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->resultRelIndex >= 0); + rtindex = list_nth_int(resultRelations, fsplan->resultRelIndex); rte = exec_rt_fetch(rtindex, estate); userid = rte->checkAsUser ? rte->checkAsUser : GetUserId(); @@ -2391,6 +2401,9 @@ postgresBeginDirectModify(ForeignScanState *node, int eflags) dmstate->rel = NULL; } + /* ResultRelInfo is set when first needed by IterateDirectModify(). */ + dmstate->resultRelInfo = NULL; + /* Initialize state variable */ dmstate->num_tuples = -1; /* -1 means not set yet */ @@ -2453,7 +2466,16 @@ 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 (resultRelInfo == NULL) + { + ForeignScan *fscan = (ForeignScan *) node->ss.ps.plan; + + resultRelInfo = &estate->es_result_relations[fscan->resultRelIndex]; + Assert(resultRelInfo != NULL); + dmstate->resultRelInfo = resultRelInfo; + } /* * If this is the first call after Begin, execute the statement. @@ -4089,7 +4111,7 @@ get_returning_data(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; TupleTableSlot *slot = node->ss.ss_ScanTupleSlot; TupleTableSlot *resultSlot; @@ -4236,7 +4258,7 @@ apply_returning_filter(PgFdwDirectModifyState *dmstate, TupleTableSlot *slot, EState *estate) { - ResultRelInfo *relInfo = estate->es_result_relation_info; + ResultRelInfo *relInfo = dmstate->resultRelInfo; TupleDesc resultTupType = RelationGetDescr(dmstate->resultRel); TupleTableSlot *resultSlot; Datum *values; diff --git a/doc/src/sgml/fdwhandler.sgml b/doc/src/sgml/fdwhandler.sgml index 6587678..7439cd1 100644 --- a/doc/src/sgml/fdwhandler.sgml +++ b/doc/src/sgml/fdwhandler.sgml @@ -917,8 +917,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 + 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/executor/nodeForeignscan.c b/src/backend/executor/nodeForeignscan.c index 513471a..ec5f166 100644 --- a/src/backend/executor/nodeForeignscan.c +++ b/src/backend/executor/nodeForeignscan.c @@ -221,12 +221,15 @@ ExecInitForeignScan(ForeignScan *node, EState *estate, int eflags) ExecInitNode(outerPlan(node), estate, eflags); /* - * Tell the FDW to initialize the scan. + * Tell the FDW to initialize the scan or the direct modification. */ - if (node->operation != CMD_SELECT) - fdwroutine->BeginDirectModify(scanstate, eflags); - else + if (node->operation == CMD_SELECT) fdwroutine->BeginForeignScan(scanstate, eflags); + else + { + Assert(node->resultRelIndex >= 0); + fdwroutine->BeginDirectModify(scanstate, eflags); + } return scanstate; } diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c index c988c96..4d4b434 100644 --- a/src/backend/nodes/copyfuncs.c +++ b/src/backend/nodes/copyfuncs.c @@ -762,6 +762,7 @@ _copyForeignScan(const ForeignScan *from) COPY_NODE_FIELD(fdw_recheck_quals); COPY_BITMAPSET_FIELD(fs_relids); COPY_SCALAR_FIELD(fsSystemCol); + COPY_SCALAR_FIELD(resultRelIndex); return newnode; } diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c index d168ecc..0997e74 100644 --- a/src/backend/nodes/outfuncs.c +++ b/src/backend/nodes/outfuncs.c @@ -699,6 +699,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(resultRelIndex); } static void diff --git a/src/backend/nodes/readfuncs.c b/src/backend/nodes/readfuncs.c index 92bb7ad..872cf10 100644 --- a/src/backend/nodes/readfuncs.c +++ b/src/backend/nodes/readfuncs.c @@ -2018,6 +2018,7 @@ _readForeignScan(void) READ_NODE_FIELD(fdw_recheck_quals); READ_BITMAPSET_FIELD(fs_relids); READ_BOOL_FIELD(fsSystemCol); + READ_INT_FIELD(resultRelIndex); READ_DONE(); } diff --git a/src/backend/optimizer/plan/createplan.c b/src/backend/optimizer/plan/createplan.c index da4a804..1078704 100644 --- a/src/backend/optimizer/plan/createplan.c +++ b/src/backend/optimizer/plan/createplan.c @@ -5557,6 +5557,8 @@ make_foreignscan(List *qptlist, node->fs_relids = NULL; /* fsSystemCol will be filled in by create_foreignscan_plan */ node->fsSystemCol = false; + /* resultRelIndex will be set by PlanDirectModify/setrefs.c, if needed */ + node->resultRelIndex = -1; return node; } diff --git a/src/backend/optimizer/plan/setrefs.c b/src/backend/optimizer/plan/setrefs.c index 6a52c6c..3eb0fc1 100644 --- a/src/backend/optimizer/plan/setrefs.c +++ b/src/backend/optimizer/plan/setrefs.c @@ -906,6 +906,13 @@ set_plan_refs(PlannerInfo *root, Plan *plan, int rtoffset) rc->rti += rtoffset; rc->prti += rtoffset; } + /* + * Caution: Do not change the relative ordering of this loop + * and the statement below that adds the result relations to + * root->glob->resultRelations, because we need to use the + * current value of list_length(root->glob->resultRelations) + * in some plans. + */ foreach(l, splan->plans) { lfirst(l) = set_plan_refs(root, @@ -1245,6 +1252,14 @@ set_foreignscan_references(PlannerInfo *root, } fscan->fs_relids = offset_relid_set(fscan->fs_relids, rtoffset); + + /* + * Adjust resultRelIndex if it's valid (note that we are called before + * adding the RT indexes of ModifyTable result relations to the global + * list) + */ + if (fscan->resultRelIndex >= 0) + fscan->resultRelIndex += list_length(root->glob->resultRelations); } /* diff --git a/src/include/nodes/plannodes.h b/src/include/nodes/plannodes.h index 909820b..ffe1830 100644 --- a/src/include/nodes/plannodes.h +++ b/src/include/nodes/plannodes.h @@ -621,6 +621,9 @@ typedef struct ForeignScan List *fdw_recheck_quals; /* original quals not in scan.plan.qual */ Bitmapset *fs_relids; /* RTIs generated by this scan */ bool fsSystemCol; /* true if any "system column" is needed */ + int resultRelIndex; /* index of foreign table in the list of query + * result relations for INSERT/UPDATE/DELETE; + * -1 for SELECT */ } ForeignScan; /* ---------------- -- 1.8.3.1