Thread

[PATCH 1/1] Fix segfault during EvalPlanQual with mix of local and foreign partitions.

Heikki Linnakangas <heikki.linnakangas@iki.fi> — 2021-08-09T20:17:48Z

It's not sensible to re-evaluate a direct-modify Foreign Update or Delete
during EvalPlanQual. However, ExecInitForeignScan() can still get called
if a table mixes local and foreign partitions. Starting with commit
1375422c782, EvalPlanQualStart() left the es_result_relations array
uninitialized in the child EPQ EState, but ExecInitForeignScan() still
expected to find it. That caused a segfault.

Fix by skipping the es_result_relations lookup during EvalPlanQual
processing. To make things a bit more robust, also skip the
BeginDirectModify calls, and add a runtime check that ExecForeignScan()
is not called on direct-modify foreign scans during EvalPlanQual
processing.

Report and diagnosis by Andrey Lepikhov.

Discussion: https://www.postgresql.org/message-id/cb2b808d-cbaa-4772-76ee-c8809bafcf3d%40postgrespro.ru
---
 src/backend/executor/nodeForeignscan.c | 43 +++++++++++++++++++++++---
 1 file changed, 38 insertions(+), 5 deletions(-)

diff --git a/src/backend/executor/nodeForeignscan.c b/src/backend/executor/nodeForeignscan.c
index 9dc38d47ea..4114c7d855 100644
--- a/src/backend/executor/nodeForeignscan.c
+++ b/src/backend/executor/nodeForeignscan.c
@@ -44,12 +44,19 @@ ForeignNext(ForeignScanState *node)
 	TupleTableSlot *slot;
 	ForeignScan *plan = (ForeignScan *) node->ss.ps.plan;
 	ExprContext *econtext = node->ss.ps.ps_ExprContext;
+	EState	   *estate = node->ss.ps.state;
 	MemoryContext oldcontext;
 
 	/* Call the Iterate function in short-lived context */
 	oldcontext = MemoryContextSwitchTo(econtext->ecxt_per_tuple_memory);
 	if (plan->operation != CMD_SELECT)
+	{
+		/* direct modifications cannot be re-evaluated by EvalPlanQual */
+		if (estate->es_epq_active != NULL)
+			elog(ERROR, "cannot re-evaluate a Foreign Update or Delete during EvalPlanQual");
+
 		slot = node->fdwroutine->IterateDirectModify(node);
+	}
 	else
 		slot = node->fdwroutine->IterateForeignScan(node);
 	MemoryContextSwitchTo(oldcontext);
@@ -223,11 +230,25 @@ ExecInitForeignScan(ForeignScan *node, EState *estate, int eflags)
 	scanstate->fdw_state = NULL;
 
 	/*
-	 * For the FDW's convenience, look up the modification target relation's.
-	 * ResultRelInfo.
+	 * For the FDW's convenience, look up the modification target relation's
+	 * ResultRelInfo.  The ModifyTable node should have initialized it for us,
+	 * see ExecInitModifyTable.
+	 *
+	 * Don't try to look up the ResultRelInfo when EvalPlanQual is active,
+	 * though.  Direct modififications cannot be re-evaluated as part of
+	 * EvalPlanQual.  The lookup wouldn't work anyway because during
+	 * EvalPlanQual processing, because EvalPlanQual only initializes the
+	 * subtree under the ModifyTable, and doesn't run ExecInitModifyTable.
 	 */
-	if (node->resultRelation > 0)
+	if (node->resultRelation > 0 && estate->es_epq_active == NULL)
+	{
+		if (estate->es_result_relations == NULL ||
+			estate->es_result_relations[node->resultRelation - 1] == NULL)
+		{
+			elog(ERROR, "result relation not initialized");
+		}
 		scanstate->resultRelInfo = estate->es_result_relations[node->resultRelation - 1];
+	}
 
 	/* Initialize any outer plan. */
 	if (outerPlan(node))
@@ -238,7 +259,15 @@ ExecInitForeignScan(ForeignScan *node, EState *estate, int eflags)
 	 * Tell the FDW to initialize the scan.
 	 */
 	if (node->operation != CMD_SELECT)
-		fdwroutine->BeginDirectModify(scanstate, eflags);
+	{
+		/*
+		 * Direct modifications cannot be re-evaluated by EvalPlanQual, so
+		 * don't bother preparing the FDW.  There can ForeignScan nodes in the
+		 * EvalPlanQual subtree, but ExecForeignScan should never be called.
+		 */
+		if (estate->es_epq_active == NULL)
+			fdwroutine->BeginDirectModify(scanstate, eflags);
+	}
 	else
 		fdwroutine->BeginForeignScan(scanstate, eflags);
 
@@ -255,10 +284,14 @@ void
 ExecEndForeignScan(ForeignScanState *node)
 {
 	ForeignScan *plan = (ForeignScan *) node->ss.ps.plan;
+	EState	   *estate = node->ss.ps.state;
 
 	/* Let the FDW shut down */
 	if (plan->operation != CMD_SELECT)
-		node->fdwroutine->EndDirectModify(node);
+	{
+		if (estate->es_epq_active == NULL)
+			node->fdwroutine->EndDirectModify(node);
+	}
 	else
 		node->fdwroutine->EndForeignScan(node);
 
-- 
2.30.2


--------------CDE573369EDE9BBEEE65B90B--