agora inbox for pgsql-hackers@postgresql.org
help / color / mirror / Atom feedFrom: Heikki Linnakangas <heikki.linnakangas@iki.fi>
Subject: [PATCH v17 1/2] Include result relation index in ForeignScan for direct modify plans.
Date: Tue, 13 Oct 2020 19:28:00 +0300
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%40ma...
---
contrib/postgres_fdw/postgres_fdw.c | 16 +++++++++-------
doc/src/sgml/fdwhandler.sgml | 15 +++++++++------
src/backend/executor/nodeForeignscan.c | 7 +++++++
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/execnodes.h | 2 ++
src/include/nodes/plannodes.h | 8 ++++++++
10 files changed, 46 insertions(+), 13 deletions(-)
diff --git a/contrib/postgres_fdw/postgres_fdw.c b/contrib/postgres_fdw/postgres_fdw.c
index a31abce7c9..78facb8ebf 100644
--- a/contrib/postgres_fdw/postgres_fdw.c
+++ b/contrib/postgres_fdw/postgres_fdw.c
@@ -451,6 +451,7 @@ static void init_returning_filter(PgFdwDirectModifyState *dmstate,
List *fdw_scan_tlist,
Index rtindex);
static TupleTableSlot *apply_returning_filter(PgFdwDirectModifyState *dmstate,
+ ResultRelInfo *resultRelInfo,
TupleTableSlot *slot,
EState *estate);
static void prepare_query_params(PlanState *node,
@@ -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.
@@ -2355,7 +2357,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 = node->resultRelInfo->ri_RangeTableIndex;
rte = exec_rt_fetch(rtindex, estate);
userid = rte->checkAsUser ? rte->checkAsUser : GetUserId();
@@ -2450,7 +2452,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 = node->resultRelInfo;
/*
* If this is the first call after Begin, execute the statement.
@@ -4086,7 +4088,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 = node->resultRelInfo;
TupleTableSlot *slot = node->ss.ss_ScanTupleSlot;
TupleTableSlot *resultSlot;
@@ -4141,7 +4143,7 @@ get_returning_data(ForeignScanState *node)
if (dmstate->rel)
resultSlot = slot;
else
- resultSlot = apply_returning_filter(dmstate, slot, estate);
+ resultSlot = apply_returning_filter(dmstate, resultRelInfo, slot, estate);
}
dmstate->next_tuple++;
@@ -4230,10 +4232,10 @@ init_returning_filter(PgFdwDirectModifyState *dmstate,
*/
static TupleTableSlot *
apply_returning_filter(PgFdwDirectModifyState *dmstate,
+ ResultRelInfo *resultRelInfo,
TupleTableSlot *slot,
EState *estate)
{
- ResultRelInfo *relInfo = estate->es_result_relation_info;
TupleDesc resultTupType = RelationGetDescr(dmstate->resultRel);
TupleTableSlot *resultSlot;
Datum *values;
@@ -4245,7 +4247,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..9c9293414c 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 <structname>ForeignScan</structname> plan
node that executes the direct modification on the remote server. The
- <structfield>operation</structfield> field of the <structname>ForeignScan</structname> must
- be set to the <literal>CmdType</literal> enumeration appropriately; that is,
+ <structfield>operation</structfield> and <structfield>resultRelation</structfield> fields
+ of the <structname>ForeignScan</structname> must be set appropriately.
+ <structfield>operation</structfield> must be set to the <literal>CmdType</literal>
+ enumeration corresponding to the statement kind (that is,
<literal>CMD_UPDATE</literal> for <command>UPDATE</command>,
<literal>CMD_INSERT</literal> for <command>INSERT</command>, and
- <literal>CMD_DELETE</literal> for <command>DELETE</command>.
+ <literal>CMD_DELETE</literal> for <command>DELETE</command>), and the
+ <literal>resultRelation</literal> argument must be copied to the
+ <structfield>resultRelation</structfield> field.
</para>
<para>
@@ -925,9 +929,8 @@ IterateDirectModify(ForeignScanState *node);
needed for the <literal>RETURNING</literal> calculation, returning it in a
tuple table slot (the node's <structfield>ScanTupleSlot</structfield> should be
used for this purpose). The data that was actually inserted, updated
- or deleted must be stored in the
- <literal>es_result_relation_info->ri_projectReturning->pi_exprContext->ecxt_scantuple</literal>
- of the node's <structname>EState</structname>.
+ or deleted must be stored in
+ <literal>node->resultRelInfo->ri_projectReturning->pi_exprContext->ecxt_scantuple</literal>.
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 513471ab9b..0b20f94035 100644
--- a/src/backend/executor/nodeForeignscan.c
+++ b/src/backend/executor/nodeForeignscan.c
@@ -215,6 +215,13 @@ ExecInitForeignScan(ForeignScan *node, EState *estate, int eflags)
scanstate->fdwroutine = fdwroutine;
scanstate->fdw_state = NULL;
+ /*
+ * For the FDW's convenience, look up the modification target relation's.
+ * ResultRelInfo.
+ */
+ if (node->resultRelation > 0)
+ scanstate->resultRelInfo = estate->es_result_relations[node->resultRelation - 1];
+
/* Initialize any outer plan. */
if (outerPlan(node))
outerPlanState(scanstate) =
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/execnodes.h b/src/include/nodes/execnodes.h
index a926ff1711..da1f3f269a 100644
--- a/src/include/nodes/execnodes.h
+++ b/src/include/nodes/execnodes.h
@@ -1780,6 +1780,8 @@ typedef struct ForeignScanState
/* use struct pointer to avoid including fdwapi.h here */
struct FdwRoutine *fdwroutine;
void *fdw_state; /* foreign-data wrapper can keep state here */
+
+ ResultRelInfo *resultRelInfo; /* result rel info, if UPDATE or DELETE */
} ForeignScanState;
/* ----------------
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
--------------870B35EF02674058C81E0BA1
Content-Type: text/x-patch; charset=UTF-8;
name="v17-0002-Remove-es_result_relation_info.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
filename="v17-0002-Remove-es_result_relation_info.patch"
view thread (4+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: heikki.linnakangas@iki.fi
Subject: Re: [PATCH v17 1/2] Include result relation index in ForeignScan for direct modify plans.
In-Reply-To: <no-message-id-690319@localhost>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox