From 62ed7026523f19af95595e3cdbe51720b1bb266d Mon Sep 17 00:00:00 2001 From: amit Date: Wed, 7 Aug 2019 09:46:27 +0900 Subject: [PATCH v7 1/4] Revise BeginDirectModify API to pass ResultRelInfo directly For ExecInitForeignScan() to efficiently get the ResultRelInfo to pass to BeginDirectModify(), add a field to ForeignScan that gives the index of a given result relation in the query's list of result relations. --- contrib/postgres_fdw/postgres_fdw.c | 27 +++++++++++++++++++++------ doc/src/sgml/fdwhandler.sgml | 12 ++++++++---- src/backend/executor/nodeForeignscan.c | 13 ++++++++++--- 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/foreign/fdwapi.h | 1 + src/include/nodes/plannodes.h | 4 ++++ 10 files changed, 64 insertions(+), 13 deletions(-) diff --git a/contrib/postgres_fdw/postgres_fdw.c b/contrib/postgres_fdw/postgres_fdw.c index 06a205877d..a61c2d8b5f 100644 --- a/contrib/postgres_fdw/postgres_fdw.c +++ b/contrib/postgres_fdw/postgres_fdw.c @@ -205,6 +205,9 @@ typedef struct PgFdwDirectModifyState List *retrieved_attrs; /* attr numbers retrieved by RETURNING */ bool set_processed; /* do we set the command es_processed? */ + /* Information about the relation being modified */ + ResultRelInfo *resultRelInfo; + /* for remote query execution */ PGconn *conn; /* connection for the update */ int numParams; /* number of parameters passed to query */ @@ -360,7 +363,9 @@ static bool postgresPlanDirectModify(PlannerInfo *root, ModifyTable *plan, Index resultRelation, int subplan_index); -static void postgresBeginDirectModify(ForeignScanState *node, int eflags); +static void postgresBeginDirectModify(ForeignScanState *node, + ResultRelInfo *rinfo, + int eflags); static TupleTableSlot *postgresIterateDirectModify(ForeignScanState *node); static void postgresEndDirectModify(ForeignScanState *node); static void postgresExplainForeignScan(ForeignScanState *node, @@ -2331,6 +2336,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; } @@ -2340,7 +2350,9 @@ postgresPlanDirectModify(PlannerInfo *root, * Prepare a direct foreign table modification */ static void -postgresBeginDirectModify(ForeignScanState *node, int eflags) +postgresBeginDirectModify(ForeignScanState *node, + ResultRelInfo *rinfo, + int eflags) { ForeignScan *fsplan = (ForeignScan *) node->ss.ps.plan; EState *estate = node->ss.ps.state; @@ -2368,7 +2380,7 @@ 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; + rtindex = rinfo->ri_RangeTableIndex; rte = exec_rt_fetch(rtindex, estate); userid = rte->checkAsUser ? rte->checkAsUser : GetUserId(); @@ -2414,6 +2426,9 @@ postgresBeginDirectModify(ForeignScanState *node, int eflags) dmstate->set_processed = intVal(list_nth(fsplan->fdw_private, FdwDirectModifyPrivateSetProcessed)); + /* Save the ResultRelInfo of the relation being modified. */ + dmstate->resultRelInfo = rinfo; + /* Create context for per-tuple temp workspace. */ dmstate->temp_cxt = AllocSetContextCreate(estate->es_query_cxt, "postgres_fdw temporary data", @@ -2463,7 +2478,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. @@ -4033,7 +4048,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; @@ -4180,7 +4195,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 27b94fb611..907ce46ee6 100644 --- a/doc/src/sgml/fdwhandler.sgml +++ b/doc/src/sgml/fdwhandler.sgml @@ -871,6 +871,7 @@ PlanDirectModify(PlannerInfo *root, void BeginDirectModify(ForeignScanState *node, + ResultRelInfo *rinfo, int eflags); @@ -883,7 +884,9 @@ BeginDirectModify(ForeignScanState *node, 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). + information provided by PlanDirectModify). In + addition, rinfo also contains information describing + the target foreign table. eflags contains flag bits describing the executor's operating mode for this plan node. @@ -915,9 +918,10 @@ 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. - Return NULL if no more rows are available. + 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 BeginDirectModify if you need longer-lived storage, or use diff --git a/src/backend/executor/nodeForeignscan.c b/src/backend/executor/nodeForeignscan.c index 52af1dac5c..9824c16e09 100644 --- a/src/backend/executor/nodeForeignscan.c +++ b/src/backend/executor/nodeForeignscan.c @@ -223,10 +223,17 @@ ExecInitForeignScan(ForeignScan *node, EState *estate, int eflags) /* * Tell the FDW to initialize the scan. */ - if (node->operation != CMD_SELECT) - fdwroutine->BeginDirectModify(scanstate, eflags); - else + if (node->operation == CMD_SELECT) fdwroutine->BeginForeignScan(scanstate, eflags); + else + { + /* Perform initializations for a direct modification. */ + ResultRelInfo *resultRelInfo; + + Assert(node->resultRelIndex >= 0); + resultRelInfo = &estate->es_result_relations[node->resultRelIndex]; + fdwroutine->BeginDirectModify(scanstate, resultRelInfo, eflags); + } return scanstate; } diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c index a2617c7cfd..e981298a75 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(resultRelIndex); return newnode; } diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c index e6ce8e2110..80eedc4a24 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(resultRelIndex); } static void diff --git a/src/backend/nodes/readfuncs.c b/src/backend/nodes/readfuncs.c index 764e3bb90c..92cc90c0f0 100644 --- a/src/backend/nodes/readfuncs.c +++ b/src/backend/nodes/readfuncs.c @@ -1983,6 +1983,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 f2325694c5..ff78167f79 100644 --- a/src/backend/optimizer/plan/createplan.c +++ b/src/backend/optimizer/plan/createplan.c @@ -5455,6 +5455,8 @@ make_foreignscan(List *qptlist, node->fs_relids = NULL; /* fsSystemCol will be filled in by create_foreignscan_plan */ node->fsSystemCol = false; + /* this might be filled to a >= 0 value by set_plan_refs() */ + node->resultRelIndex = -1; return node; } diff --git a/src/backend/optimizer/plan/setrefs.c b/src/backend/optimizer/plan/setrefs.c index 329ebd5f28..f18e94a879 100644 --- a/src/backend/optimizer/plan/setrefs.c +++ b/src/backend/optimizer/plan/setrefs.c @@ -877,6 +877,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, @@ -1225,6 +1232,14 @@ set_foreignscan_references(PlannerInfo *root, tempset = bms_add_member(tempset, x + rtoffset); fscan->fs_relids = tempset; } + + /* + * 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/foreign/fdwapi.h b/src/include/foreign/fdwapi.h index 822686033e..adf39bc618 100644 --- a/src/include/foreign/fdwapi.h +++ b/src/include/foreign/fdwapi.h @@ -112,6 +112,7 @@ typedef bool (*PlanDirectModify_function) (PlannerInfo *root, int subplan_index); typedef void (*BeginDirectModify_function) (ForeignScanState *node, + ResultRelInfo *rinfo, int eflags); typedef TupleTableSlot *(*IterateDirectModify_function) (ForeignScanState *node); diff --git a/src/include/nodes/plannodes.h b/src/include/nodes/plannodes.h index 8e6594e355..dc2061cf89 100644 --- a/src/include/nodes/plannodes.h +++ b/src/include/nodes/plannodes.h @@ -616,6 +616,10 @@ 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; /* For "direct modification" operations, this + * contains the offset of result relation in + * query-global list of result relations; -1 + * otherwise */ } ForeignScan; /* ---------------- -- 2.11.0