From a32d7916a4788d958a3dcd7e59a494916d78ddce Mon Sep 17 00:00:00 2001 From: Amit Langote Date: Fri, 22 Sep 2023 18:12:04 +0900 Subject: [PATCH v48 2/8] Prepare executor to support detecting CachedPlan invalidation This adds checks at various points during the executor's initialization of the plan tree to determine whether the originating CachedPlan has become invalid as a result of taking locks on the relations referenced in the plan. This includes addding the check after every call to ExecOpenScanRelation() and to ExecInitNode(), including the recursive ones to initialize child nodes. If a given ExecInit*() function detects that the plan has become invalid, it should return immediately even if the PlanState node it's building may only be partially valid. That is crucial for two reasons depending on where the check is: * The checks following ExecOpenScanRelation() may find the plan having become invalid because the requested relation was dropped or had its schema changed concurrently in a manner that risks unsafe operations in the code that follows. For example, it might try to dereference a NULL pointer when the check failed because the relation was dropped. * For the checks following ExecInitNode(), the returned child PlanState node might be only partially invalid. The code that follows may misbehave if it depends on inspecting the child PlanState. Note that this commit adds the check following all calls of ExecInitNode() that exist in the code base, even at sites where there is no code that might misbehave today, because it might misbehave in the future. It seems like a good idea to put the guards in place today rather than in the future when the need arises. To pass the CachedPlan that the executor will use for these checks, this adds a new field to QueryDesc and a new parameter to CreateQueryDesc(). No caller of CreateQueryDesc() is made to pass an actual CachedPlan though, so there is no functional change. Reviewed-by: Robert Haas --- contrib/postgres_fdw/postgres_fdw.c | 10 +++++- src/backend/commands/copyto.c | 3 +- src/backend/commands/createas.c | 2 +- src/backend/commands/explain.c | 2 +- src/backend/commands/extension.c | 1 + src/backend/commands/matview.c | 2 +- src/backend/executor/execMain.c | 39 ++++++++++++++++++---- src/backend/executor/execParallel.c | 9 ++++- src/backend/executor/execProcnode.c | 4 +++ src/backend/executor/functions.c | 1 + src/backend/executor/nodeAgg.c | 2 ++ src/backend/executor/nodeAppend.c | 10 +++--- src/backend/executor/nodeBitmapAnd.c | 2 ++ src/backend/executor/nodeBitmapHeapscan.c | 4 +++ src/backend/executor/nodeBitmapOr.c | 2 ++ src/backend/executor/nodeCustom.c | 2 ++ src/backend/executor/nodeForeignscan.c | 4 +++ src/backend/executor/nodeGather.c | 2 ++ src/backend/executor/nodeGatherMerge.c | 2 ++ src/backend/executor/nodeGroup.c | 2 ++ src/backend/executor/nodeHash.c | 2 ++ src/backend/executor/nodeHashjoin.c | 4 +++ src/backend/executor/nodeIncrementalSort.c | 2 ++ src/backend/executor/nodeIndexonlyscan.c | 2 ++ src/backend/executor/nodeIndexscan.c | 2 ++ src/backend/executor/nodeLimit.c | 2 ++ src/backend/executor/nodeLockRows.c | 2 ++ src/backend/executor/nodeMaterial.c | 2 ++ src/backend/executor/nodeMemoize.c | 2 ++ src/backend/executor/nodeMergeAppend.c | 4 ++- src/backend/executor/nodeMergejoin.c | 4 +++ src/backend/executor/nodeModifyTable.c | 13 ++++++++ src/backend/executor/nodeNestloop.c | 4 +++ src/backend/executor/nodeProjectSet.c | 2 ++ src/backend/executor/nodeRecursiveunion.c | 4 +++ src/backend/executor/nodeResult.c | 2 ++ src/backend/executor/nodeSamplescan.c | 2 ++ src/backend/executor/nodeSeqscan.c | 2 ++ src/backend/executor/nodeSetOp.c | 2 ++ src/backend/executor/nodeSort.c | 2 ++ src/backend/executor/nodeSubqueryscan.c | 2 ++ src/backend/executor/nodeTidrangescan.c | 2 ++ src/backend/executor/nodeTidscan.c | 2 ++ src/backend/executor/nodeUnique.c | 2 ++ src/backend/executor/nodeWindowAgg.c | 2 ++ src/backend/executor/spi.c | 1 + src/backend/tcop/pquery.c | 5 ++- src/include/executor/execdesc.h | 4 +++ src/include/executor/executor.h | 10 ++++++ src/include/nodes/execnodes.h | 2 ++ src/include/utils/plancache.h | 14 ++++++++ 51 files changed, 194 insertions(+), 18 deletions(-) diff --git a/contrib/postgres_fdw/postgres_fdw.c b/contrib/postgres_fdw/postgres_fdw.c index 1393716587..0af60463c2 100644 --- a/contrib/postgres_fdw/postgres_fdw.c +++ b/contrib/postgres_fdw/postgres_fdw.c @@ -2126,7 +2126,11 @@ postgresEndForeignModify(EState *estate, { PgFdwModifyState *fmstate = (PgFdwModifyState *) resultRelInfo->ri_FdwState; - /* If fmstate is NULL, we are in EXPLAIN; nothing to do */ + /* + * fmstate could be NULL under two conditions: during an EXPLAIN + * operation, or if BeginForeignModify() hasn't been invoked. + * In either case, no action is required. + */ if (fmstate == NULL) return; @@ -2660,7 +2664,11 @@ postgresBeginDirectModify(ForeignScanState *node, int eflags) /* Get info about foreign table. */ rtindex = node->resultRelInfo->ri_RangeTableIndex; if (fsplan->scan.scanrelid == 0) + { dmstate->rel = ExecOpenScanRelation(estate, rtindex, eflags); + if (unlikely(!ExecPlanStillValid(estate))) + return; + } else dmstate->rel = node->ss.ss_currentRelation; table = GetForeignTable(RelationGetRelid(dmstate->rel)); diff --git a/src/backend/commands/copyto.c b/src/backend/commands/copyto.c index eaa3172793..0e3547c35b 100644 --- a/src/backend/commands/copyto.c +++ b/src/backend/commands/copyto.c @@ -558,7 +558,8 @@ BeginCopyTo(ParseState *pstate, ((DR_copy *) dest)->cstate = cstate; /* Create a QueryDesc requesting no output */ - cstate->queryDesc = CreateQueryDesc(plan, pstate->p_sourcetext, + cstate->queryDesc = CreateQueryDesc(plan, NULL, + pstate->p_sourcetext, GetActiveSnapshot(), InvalidSnapshot, dest, NULL, NULL, 0); diff --git a/src/backend/commands/createas.c b/src/backend/commands/createas.c index e91920ca14..18b07c0200 100644 --- a/src/backend/commands/createas.c +++ b/src/backend/commands/createas.c @@ -325,7 +325,7 @@ ExecCreateTableAs(ParseState *pstate, CreateTableAsStmt *stmt, UpdateActiveSnapshotCommandId(); /* Create a QueryDesc, redirecting output to our tuple receiver */ - queryDesc = CreateQueryDesc(plan, pstate->p_sourcetext, + queryDesc = CreateQueryDesc(plan, NULL, pstate->p_sourcetext, GetActiveSnapshot(), InvalidSnapshot, dest, params, queryEnv, 0); diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.c index 13217807ee..281c47b2ee 100644 --- a/src/backend/commands/explain.c +++ b/src/backend/commands/explain.c @@ -572,7 +572,7 @@ ExplainOnePlan(PlannedStmt *plannedstmt, IntoClause *into, ExplainState *es, dest = None_Receiver; /* Create a QueryDesc for the query */ - queryDesc = CreateQueryDesc(plannedstmt, queryString, + queryDesc = CreateQueryDesc(plannedstmt, NULL, queryString, GetActiveSnapshot(), InvalidSnapshot, dest, params, queryEnv, instrument_option); diff --git a/src/backend/commands/extension.c b/src/backend/commands/extension.c index 535072d181..b287a2e84c 100644 --- a/src/backend/commands/extension.c +++ b/src/backend/commands/extension.c @@ -797,6 +797,7 @@ execute_sql_string(const char *sql) QueryDesc *qdesc; qdesc = CreateQueryDesc(stmt, + NULL, sql, GetActiveSnapshot(), NULL, dest, NULL, NULL, 0); diff --git a/src/backend/commands/matview.c b/src/backend/commands/matview.c index ac2e74fa3f..22b8b820c3 100644 --- a/src/backend/commands/matview.c +++ b/src/backend/commands/matview.c @@ -408,7 +408,7 @@ refresh_matview_datafill(DestReceiver *dest, Query *query, UpdateActiveSnapshotCommandId(); /* Create a QueryDesc, redirecting output to our tuple receiver */ - queryDesc = CreateQueryDesc(plan, queryString, + queryDesc = CreateQueryDesc(plan, NULL, queryString, GetActiveSnapshot(), InvalidSnapshot, dest, NULL, NULL, 0); diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c index f7f18d3054..de7bf7ca67 100644 --- a/src/backend/executor/execMain.c +++ b/src/backend/executor/execMain.c @@ -79,7 +79,7 @@ ExecutorEnd_hook_type ExecutorEnd_hook = NULL; ExecutorCheckPerms_hook_type ExecutorCheckPerms_hook = NULL; /* decls for local routines only used within this module */ -static void InitPlan(QueryDesc *queryDesc, int eflags); +static bool InitPlan(QueryDesc *queryDesc, int eflags); static void CheckValidRowMarkRel(Relation rel, RowMarkType markType); static void ExecPostprocessPlan(EState *estate); static void ExecEndPlan(PlanState *planstate, EState *estate); @@ -263,7 +263,7 @@ standard_ExecutorStart(QueryDesc *queryDesc, int eflags) /* * Initialize the plan state tree */ - InitPlan(queryDesc, eflags); + (void) InitPlan(queryDesc, eflags); MemoryContextSwitchTo(oldcontext); } @@ -829,9 +829,13 @@ ExecCheckXactReadOnly(PlannedStmt *plannedstmt) * * Initializes the query plan: open files, allocate storage * and start up the rule manager + * + * Returns true if the plan tree is successfully initialized for execution, + * false otherwise. The latter case may occur if the CachedPlan that provides + * the plan tree (queryDesc->cplan) got invalidated during the initialization. * ---------------------------------------------------------------- */ -static void +static bool InitPlan(QueryDesc *queryDesc, int eflags) { CmdType operation = queryDesc->operation; @@ -839,11 +843,14 @@ InitPlan(QueryDesc *queryDesc, int eflags) Plan *plan = plannedstmt->planTree; List *rangeTable = plannedstmt->rtable; EState *estate = queryDesc->estate; - PlanState *planstate; - TupleDesc tupType; + PlanState *planstate = NULL; + TupleDesc tupType = NULL; ListCell *l; int i; + Assert(queryDesc->planstate == NULL); + Assert(queryDesc->tupDesc == NULL); + /* * Do permissions checks */ @@ -855,6 +862,7 @@ InitPlan(QueryDesc *queryDesc, int eflags) ExecInitRangeTable(estate, rangeTable, plannedstmt->permInfos); estate->es_plannedstmt = plannedstmt; + estate->es_cachedplan = queryDesc->cplan; /* * Next, build the ExecRowMark array from the PlanRowMark(s), if any. @@ -886,6 +894,8 @@ InitPlan(QueryDesc *queryDesc, int eflags) case ROW_MARK_KEYSHARE: case ROW_MARK_REFERENCE: relation = ExecGetRangeTableRelation(estate, rc->rti); + if (unlikely(relation == NULL)) + return false; break; case ROW_MARK_COPY: /* no physical table access is required */ @@ -956,6 +966,8 @@ InitPlan(QueryDesc *queryDesc, int eflags) estate->es_subplanstates = lappend(estate->es_subplanstates, subplanstate); + if (unlikely(!ExecPlanStillValid(estate))) + return false; i++; } @@ -966,6 +978,8 @@ InitPlan(QueryDesc *queryDesc, int eflags) * processing tuples. */ planstate = ExecInitNode(plan, estate, eflags); + if (unlikely(!ExecPlanStillValid(estate))) + return false; /* * Get the tuple descriptor describing the type of tuples to return. @@ -1009,6 +1023,8 @@ InitPlan(QueryDesc *queryDesc, int eflags) queryDesc->tupDesc = tupType; queryDesc->planstate = planstate; + + return true; } /* @@ -2858,7 +2874,8 @@ EvalPlanQualStart(EPQState *epqstate, Plan *planTree) * Child EPQ EStates share the parent's copy of unchanging state such as * the snapshot, rangetable, and external Param info. They need their own * copies of local state, including a tuple table, es_param_exec_vals, - * result-rel info, etc. + * result-rel info, etc. Also, we don't pass the parent't copy of the + * CachedPlan, because no new locks will be taken for EvalPlanQual(). */ rcestate->es_direction = ForwardScanDirection; rcestate->es_snapshot = parentestate->es_snapshot; @@ -2947,6 +2964,13 @@ EvalPlanQualStart(EPQState *epqstate, Plan *planTree) subplanstate = ExecInitNode(subplan, rcestate, 0); rcestate->es_subplanstates = lappend(rcestate->es_subplanstates, subplanstate); + + /* + * All the necessary locks must already have been taken when + * initializing the parent's copy of subplanstate, so the CachedPlan, + * if any, should not have become invalid during ExecInitNode(). + */ + Assert(ExecPlanStillValid(rcestate)); } /* @@ -2988,6 +3012,9 @@ EvalPlanQualStart(EPQState *epqstate, Plan *planTree) */ epqstate->recheckplanstate = ExecInitNode(planTree, rcestate, 0); + /* See the comment above. */ + Assert(ExecPlanStillValid(rcestate)); + MemoryContextSwitchTo(oldcontext); } diff --git a/src/backend/executor/execParallel.c b/src/backend/executor/execParallel.c index cc2b8ccab7..457ee46faf 100644 --- a/src/backend/executor/execParallel.c +++ b/src/backend/executor/execParallel.c @@ -1248,8 +1248,15 @@ ExecParallelGetQueryDesc(shm_toc *toc, DestReceiver *receiver, paramspace = shm_toc_lookup(toc, PARALLEL_KEY_PARAMLISTINFO, false); paramLI = RestoreParamList(¶mspace); - /* Create a QueryDesc for the query. */ + /* + * Set up a QueryDesc for the query. While the leader might've sourced + * the plan tree from a CachedPlan, we don't have one here. This isn't + * an issue since the leader ensured the required locks, making our + * plan tree valid. Even as we get our own lock copies in + * ExecGetRangeTableRelation(), they're all already held by the leader. + */ return CreateQueryDesc(pstmt, + NULL, queryString, GetActiveSnapshot(), InvalidSnapshot, receiver, paramLI, NULL, instrument_options); diff --git a/src/backend/executor/execProcnode.c b/src/backend/executor/execProcnode.c index b4b5c562c0..febaa194c4 100644 --- a/src/backend/executor/execProcnode.c +++ b/src/backend/executor/execProcnode.c @@ -136,6 +136,10 @@ static bool ExecShutdownNode_walker(PlanState *node, void *context); * 'eflags' is a bitwise OR of flag bits described in executor.h * * Returns a PlanState node corresponding to the given Plan node. + * + * Callers should check upon returning that ExecPlanStillValid(estate) + * returns true before continuing further with its processing, because the + * returned PlanState might be only partially valid otherwise. * ------------------------------------------------------------------------ */ PlanState * diff --git a/src/backend/executor/functions.c b/src/backend/executor/functions.c index f55424eb5a..7e452ed743 100644 --- a/src/backend/executor/functions.c +++ b/src/backend/executor/functions.c @@ -838,6 +838,7 @@ postquel_start(execution_state *es, SQLFunctionCachePtr fcache) dest = None_Receiver; es->qd = CreateQueryDesc(es->stmt, + NULL, /* fmgr_sql() doesn't use CachedPlans */ fcache->src, GetActiveSnapshot(), InvalidSnapshot, diff --git a/src/backend/executor/nodeAgg.c b/src/backend/executor/nodeAgg.c index af22b1676f..597d68139e 100644 --- a/src/backend/executor/nodeAgg.c +++ b/src/backend/executor/nodeAgg.c @@ -3304,6 +3304,8 @@ ExecInitAgg(Agg *node, EState *estate, int eflags) eflags &= ~EXEC_FLAG_REWIND; outerPlan = outerPlan(node); outerPlanState(aggstate) = ExecInitNode(outerPlan, estate, eflags); + if (unlikely(!ExecPlanStillValid(estate))) + return aggstate; /* * initialize source tuple type. diff --git a/src/backend/executor/nodeAppend.c b/src/backend/executor/nodeAppend.c index a2af221e05..53ca9dc85d 100644 --- a/src/backend/executor/nodeAppend.c +++ b/src/backend/executor/nodeAppend.c @@ -185,8 +185,10 @@ ExecInitAppend(Append *node, EState *estate, int eflags) appendstate->ps.resultopsset = true; appendstate->ps.resultopsfixed = false; - appendplanstates = (PlanState **) palloc(nplans * - sizeof(PlanState *)); + appendplanstates = (PlanState **) palloc0(nplans * + sizeof(PlanState *)); + appendstate->appendplans = appendplanstates; + appendstate->as_nplans = nplans; /* * call ExecInitNode on each of the valid plans to be executed and save @@ -221,11 +223,11 @@ ExecInitAppend(Append *node, EState *estate, int eflags) firstvalid = j; appendplanstates[j++] = ExecInitNode(initNode, estate, eflags); + if (unlikely(!ExecPlanStillValid(estate))) + return appendstate; } appendstate->as_first_partial_plan = firstvalid; - appendstate->appendplans = appendplanstates; - appendstate->as_nplans = nplans; /* Initialize async state */ appendstate->as_asyncplans = asyncplans; diff --git a/src/backend/executor/nodeBitmapAnd.c b/src/backend/executor/nodeBitmapAnd.c index 4abb0609a0..7556be713c 100644 --- a/src/backend/executor/nodeBitmapAnd.c +++ b/src/backend/executor/nodeBitmapAnd.c @@ -89,6 +89,8 @@ ExecInitBitmapAnd(BitmapAnd *node, EState *estate, int eflags) { initNode = (Plan *) lfirst(l); bitmapplanstates[i] = ExecInitNode(initNode, estate, eflags); + if (unlikely(!ExecPlanStillValid(estate))) + return bitmapandstate; i++; } diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c index d3f58c22f9..f1f8e16b17 100644 --- a/src/backend/executor/nodeBitmapHeapscan.c +++ b/src/backend/executor/nodeBitmapHeapscan.c @@ -770,11 +770,15 @@ ExecInitBitmapHeapScan(BitmapHeapScan *node, EState *estate, int eflags) * open the scan relation */ currentRelation = ExecOpenScanRelation(estate, node->scan.scanrelid, eflags); + if (unlikely(!ExecPlanStillValid(estate))) + return scanstate; /* * initialize child nodes */ outerPlanState(scanstate) = ExecInitNode(outerPlan(node), estate, eflags); + if (unlikely(!ExecPlanStillValid(estate))) + return scanstate; /* * get the scan type from the relation descriptor. diff --git a/src/backend/executor/nodeBitmapOr.c b/src/backend/executor/nodeBitmapOr.c index ace18593aa..7d2bf45d9c 100644 --- a/src/backend/executor/nodeBitmapOr.c +++ b/src/backend/executor/nodeBitmapOr.c @@ -90,6 +90,8 @@ ExecInitBitmapOr(BitmapOr *node, EState *estate, int eflags) { initNode = (Plan *) lfirst(l); bitmapplanstates[i] = ExecInitNode(initNode, estate, eflags); + if (unlikely(!ExecPlanStillValid(estate))) + return bitmaporstate; i++; } diff --git a/src/backend/executor/nodeCustom.c b/src/backend/executor/nodeCustom.c index 28b5bb9353..a0befbd0c6 100644 --- a/src/backend/executor/nodeCustom.c +++ b/src/backend/executor/nodeCustom.c @@ -61,6 +61,8 @@ ExecInitCustomScan(CustomScan *cscan, EState *estate, int eflags) if (scanrelid > 0) { scan_rel = ExecOpenScanRelation(estate, scanrelid, eflags); + if (unlikely(!ExecPlanStillValid(estate))) + return css; css->ss.ss_currentRelation = scan_rel; } diff --git a/src/backend/executor/nodeForeignscan.c b/src/backend/executor/nodeForeignscan.c index 3aba28285a..336acff719 100644 --- a/src/backend/executor/nodeForeignscan.c +++ b/src/backend/executor/nodeForeignscan.c @@ -173,6 +173,8 @@ ExecInitForeignScan(ForeignScan *node, EState *estate, int eflags) if (scanrelid > 0) { currentRelation = ExecOpenScanRelation(estate, scanrelid, eflags); + if (unlikely(!ExecPlanStillValid(estate))) + return scanstate; scanstate->ss.ss_currentRelation = currentRelation; fdwroutine = GetFdwRoutineForRelation(currentRelation, true); } @@ -264,6 +266,8 @@ ExecInitForeignScan(ForeignScan *node, EState *estate, int eflags) if (outerPlan(node)) outerPlanState(scanstate) = ExecInitNode(outerPlan(node), estate, eflags); + if (unlikely(!ExecPlanStillValid(estate))) + return scanstate; /* * Tell the FDW to initialize the scan. diff --git a/src/backend/executor/nodeGather.c b/src/backend/executor/nodeGather.c index 1a3c8abdad..c524022c04 100644 --- a/src/backend/executor/nodeGather.c +++ b/src/backend/executor/nodeGather.c @@ -89,6 +89,8 @@ ExecInitGather(Gather *node, EState *estate, int eflags) */ outerNode = outerPlan(node); outerPlanState(gatherstate) = ExecInitNode(outerNode, estate, eflags); + if (unlikely(!ExecPlanStillValid(estate))) + return gatherstate; tupDesc = ExecGetResultType(outerPlanState(gatherstate)); /* diff --git a/src/backend/executor/nodeGatherMerge.c b/src/backend/executor/nodeGatherMerge.c index c6fb45fee0..676faabef5 100644 --- a/src/backend/executor/nodeGatherMerge.c +++ b/src/backend/executor/nodeGatherMerge.c @@ -108,6 +108,8 @@ ExecInitGatherMerge(GatherMerge *node, EState *estate, int eflags) */ outerNode = outerPlan(node); outerPlanState(gm_state) = ExecInitNode(outerNode, estate, eflags); + if (unlikely(!ExecPlanStillValid(estate))) + return gm_state; /* * Leader may access ExecProcNode result directly (if diff --git a/src/backend/executor/nodeGroup.c b/src/backend/executor/nodeGroup.c index 6dfe5a1d23..efa1c44ab4 100644 --- a/src/backend/executor/nodeGroup.c +++ b/src/backend/executor/nodeGroup.c @@ -185,6 +185,8 @@ ExecInitGroup(Group *node, EState *estate, int eflags) * initialize child nodes */ outerPlanState(grpstate) = ExecInitNode(outerPlan(node), estate, eflags); + if (unlikely(!ExecPlanStillValid(estate))) + return grpstate; /* * Initialize scan slot and type. diff --git a/src/backend/executor/nodeHash.c b/src/backend/executor/nodeHash.c index 88ba336882..1a4bd5504e 100644 --- a/src/backend/executor/nodeHash.c +++ b/src/backend/executor/nodeHash.c @@ -386,6 +386,8 @@ ExecInitHash(Hash *node, EState *estate, int eflags) * initialize child nodes */ outerPlanState(hashstate) = ExecInitNode(outerPlan(node), estate, eflags); + if (unlikely(!ExecPlanStillValid(estate))) + return hashstate; /* * initialize our result slot and type. No need to build projection diff --git a/src/backend/executor/nodeHashjoin.c b/src/backend/executor/nodeHashjoin.c index 6dc43b9ff2..c0919074b0 100644 --- a/src/backend/executor/nodeHashjoin.c +++ b/src/backend/executor/nodeHashjoin.c @@ -752,8 +752,12 @@ ExecInitHashJoin(HashJoin *node, EState *estate, int eflags) hashNode = (Hash *) innerPlan(node); outerPlanState(hjstate) = ExecInitNode(outerNode, estate, eflags); + if (unlikely(!ExecPlanStillValid(estate))) + return hjstate; outerDesc = ExecGetResultType(outerPlanState(hjstate)); innerPlanState(hjstate) = ExecInitNode((Plan *) hashNode, estate, eflags); + if (unlikely(!ExecPlanStillValid(estate))) + return hjstate; innerDesc = ExecGetResultType(innerPlanState(hjstate)); /* diff --git a/src/backend/executor/nodeIncrementalSort.c b/src/backend/executor/nodeIncrementalSort.c index 28a0e81cb3..621ffafe02 100644 --- a/src/backend/executor/nodeIncrementalSort.c +++ b/src/backend/executor/nodeIncrementalSort.c @@ -1041,6 +1041,8 @@ ExecInitIncrementalSort(IncrementalSort *node, EState *estate, int eflags) * nodes may be able to do something more useful. */ outerPlanState(incrsortstate) = ExecInitNode(outerPlan(node), estate, eflags); + if (unlikely(!ExecPlanStillValid(estate))) + return incrsortstate; /* * Initialize scan slot and type. diff --git a/src/backend/executor/nodeIndexonlyscan.c b/src/backend/executor/nodeIndexonlyscan.c index 1f3843abe9..c555c14888 100644 --- a/src/backend/executor/nodeIndexonlyscan.c +++ b/src/backend/executor/nodeIndexonlyscan.c @@ -495,6 +495,8 @@ ExecInitIndexOnlyScan(IndexOnlyScan *node, EState *estate, int eflags) * open the scan relation */ currentRelation = ExecOpenScanRelation(estate, node->scan.scanrelid, eflags); + if (unlikely(!ExecPlanStillValid(estate))) + return indexstate; indexstate->ss.ss_currentRelation = currentRelation; indexstate->ss.ss_currentScanDesc = NULL; /* no heap scan here */ diff --git a/src/backend/executor/nodeIndexscan.c b/src/backend/executor/nodeIndexscan.c index 32e1714f15..a3bd1f7fb0 100644 --- a/src/backend/executor/nodeIndexscan.c +++ b/src/backend/executor/nodeIndexscan.c @@ -908,6 +908,8 @@ ExecInitIndexScan(IndexScan *node, EState *estate, int eflags) * open the scan relation */ currentRelation = ExecOpenScanRelation(estate, node->scan.scanrelid, eflags); + if (unlikely(!ExecPlanStillValid(estate))) + return indexstate; indexstate->ss.ss_currentRelation = currentRelation; indexstate->ss.ss_currentScanDesc = NULL; /* no heap scan here */ diff --git a/src/backend/executor/nodeLimit.c b/src/backend/executor/nodeLimit.c index a97bac9f6d..ab133f1580 100644 --- a/src/backend/executor/nodeLimit.c +++ b/src/backend/executor/nodeLimit.c @@ -476,6 +476,8 @@ ExecInitLimit(Limit *node, EState *estate, int eflags) */ outerPlan = outerPlan(node); outerPlanState(limitstate) = ExecInitNode(outerPlan, estate, eflags); + if (unlikely(!ExecPlanStillValid(estate))) + return limitstate; /* * initialize child expressions diff --git a/src/backend/executor/nodeLockRows.c b/src/backend/executor/nodeLockRows.c index 26fbe95c57..e1ef768571 100644 --- a/src/backend/executor/nodeLockRows.c +++ b/src/backend/executor/nodeLockRows.c @@ -322,6 +322,8 @@ ExecInitLockRows(LockRows *node, EState *estate, int eflags) * then initialize outer plan */ outerPlanState(lrstate) = ExecInitNode(outerPlan, estate, eflags); + if (unlikely(!ExecPlanStillValid(estate))) + return lrstate; /* node returns unmodified slots from the outer plan */ lrstate->ps.resultopsset = true; diff --git a/src/backend/executor/nodeMaterial.c b/src/backend/executor/nodeMaterial.c index 03c514900b..c38eef099d 100644 --- a/src/backend/executor/nodeMaterial.c +++ b/src/backend/executor/nodeMaterial.c @@ -214,6 +214,8 @@ ExecInitMaterial(Material *node, EState *estate, int eflags) outerPlan = outerPlan(node); outerPlanState(matstate) = ExecInitNode(outerPlan, estate, eflags); + if (unlikely(!ExecPlanStillValid(estate))) + return matstate; /* * Initialize result type and slot. No need to initialize projection info diff --git a/src/backend/executor/nodeMemoize.c b/src/backend/executor/nodeMemoize.c index ee4749c852..a6bf66029c 100644 --- a/src/backend/executor/nodeMemoize.c +++ b/src/backend/executor/nodeMemoize.c @@ -938,6 +938,8 @@ ExecInitMemoize(Memoize *node, EState *estate, int eflags) outerNode = outerPlan(node); outerPlanState(mstate) = ExecInitNode(outerNode, estate, eflags); + if (unlikely(!ExecPlanStillValid(estate))) + return mstate; /* * Initialize return slot and type. No need to initialize projection info diff --git a/src/backend/executor/nodeMergeAppend.c b/src/backend/executor/nodeMergeAppend.c index 0a42a04b19..52c3edf278 100644 --- a/src/backend/executor/nodeMergeAppend.c +++ b/src/backend/executor/nodeMergeAppend.c @@ -120,7 +120,7 @@ ExecInitMergeAppend(MergeAppend *node, EState *estate, int eflags) mergestate->ms_prune_state = NULL; } - mergeplanstates = (PlanState **) palloc(nplans * sizeof(PlanState *)); + mergeplanstates = (PlanState **) palloc0(nplans * sizeof(PlanState *)); mergestate->mergeplans = mergeplanstates; mergestate->ms_nplans = nplans; @@ -151,6 +151,8 @@ ExecInitMergeAppend(MergeAppend *node, EState *estate, int eflags) Plan *initNode = (Plan *) list_nth(node->mergeplans, i); mergeplanstates[j++] = ExecInitNode(initNode, estate, eflags); + if (unlikely(!ExecPlanStillValid(estate))) + return mergestate; } mergestate->ps.ps_ProjInfo = NULL; diff --git a/src/backend/executor/nodeMergejoin.c b/src/backend/executor/nodeMergejoin.c index c84f53e0bd..887f519b10 100644 --- a/src/backend/executor/nodeMergejoin.c +++ b/src/backend/executor/nodeMergejoin.c @@ -1490,11 +1490,15 @@ ExecInitMergeJoin(MergeJoin *node, EState *estate, int eflags) mergestate->mj_SkipMarkRestore = node->skip_mark_restore; outerPlanState(mergestate) = ExecInitNode(outerPlan(node), estate, eflags); + if (unlikely(!ExecPlanStillValid(estate))) + return mergestate; outerDesc = ExecGetResultType(outerPlanState(mergestate)); innerPlanState(mergestate) = ExecInitNode(innerPlan(node), estate, mergestate->mj_SkipMarkRestore ? eflags : (eflags | EXEC_FLAG_MARK)); + if (unlikely(!ExecPlanStillValid(estate))) + return mergestate; innerDesc = ExecGetResultType(innerPlanState(mergestate)); /* diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c index ea043c57c1..95d909c1d0 100644 --- a/src/backend/executor/nodeModifyTable.c +++ b/src/backend/executor/nodeModifyTable.c @@ -3985,6 +3985,13 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags) linitial_int(node->resultRelations)); } + /* + * ExecInitResultRelation() may have returned without initializing + * rootResultRelInfo if the plan got invalidated, so check. + */ + if (unlikely(!ExecPlanStillValid(estate))) + return mtstate; + /* set up epqstate with dummy subplan data for the moment */ EvalPlanQualInit(&mtstate->mt_epqstate, estate, NULL, NIL, node->epqParam, node->resultRelations); @@ -4013,6 +4020,10 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags) { ExecInitResultRelation(estate, resultRelInfo, resultRelation); + /* See the comment above. */ + if (unlikely(!ExecPlanStillValid(estate))) + return mtstate; + /* * For child result relations, store the root result relation * pointer. We do so for the convenience of places that want to @@ -4039,6 +4050,8 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags) * Now we may initialize the subplan. */ outerPlanState(mtstate) = ExecInitNode(subplan, estate, eflags); + if (unlikely(!ExecPlanStillValid(estate))) + return mtstate; /* * Do additional per-result-relation initialization. diff --git a/src/backend/executor/nodeNestloop.c b/src/backend/executor/nodeNestloop.c index 1211d871ea..8d67d17e10 100644 --- a/src/backend/executor/nodeNestloop.c +++ b/src/backend/executor/nodeNestloop.c @@ -295,11 +295,15 @@ ExecInitNestLoop(NestLoop *node, EState *estate, int eflags) * values. */ outerPlanState(nlstate) = ExecInitNode(outerPlan(node), estate, eflags); + if (unlikely(!ExecPlanStillValid(estate))) + return nlstate; if (node->nestParams == NIL) eflags |= EXEC_FLAG_REWIND; else eflags &= ~EXEC_FLAG_REWIND; innerPlanState(nlstate) = ExecInitNode(innerPlan(node), estate, eflags); + if (unlikely(!ExecPlanStillValid(estate))) + return nlstate; /* * Initialize result slot, type and projection. diff --git a/src/backend/executor/nodeProjectSet.c b/src/backend/executor/nodeProjectSet.c index e9b96416d3..706cc23a21 100644 --- a/src/backend/executor/nodeProjectSet.c +++ b/src/backend/executor/nodeProjectSet.c @@ -247,6 +247,8 @@ ExecInitProjectSet(ProjectSet *node, EState *estate, int eflags) * initialize child nodes */ outerPlanState(state) = ExecInitNode(outerPlan(node), estate, eflags); + if (unlikely(!ExecPlanStillValid(estate))) + return state; /* * we don't use inner plan diff --git a/src/backend/executor/nodeRecursiveunion.c b/src/backend/executor/nodeRecursiveunion.c index f6d60bcd6c..27dc318acb 100644 --- a/src/backend/executor/nodeRecursiveunion.c +++ b/src/backend/executor/nodeRecursiveunion.c @@ -244,7 +244,11 @@ ExecInitRecursiveUnion(RecursiveUnion *node, EState *estate, int eflags) * initialize child nodes */ outerPlanState(rustate) = ExecInitNode(outerPlan(node), estate, eflags); + if (unlikely(!ExecPlanStillValid(estate))) + return rustate; innerPlanState(rustate) = ExecInitNode(innerPlan(node), estate, eflags); + if (unlikely(!ExecPlanStillValid(estate))) + return rustate; /* * If hashing, precompute fmgr lookup data for inner loop, and create the diff --git a/src/backend/executor/nodeResult.c b/src/backend/executor/nodeResult.c index f15902e840..6820d3bfd5 100644 --- a/src/backend/executor/nodeResult.c +++ b/src/backend/executor/nodeResult.c @@ -208,6 +208,8 @@ ExecInitResult(Result *node, EState *estate, int eflags) * initialize child nodes */ outerPlanState(resstate) = ExecInitNode(outerPlan(node), estate, eflags); + if (unlikely(!ExecPlanStillValid(estate))) + return resstate; /* * we don't use inner plan diff --git a/src/backend/executor/nodeSamplescan.c b/src/backend/executor/nodeSamplescan.c index a6813559e6..02051fea51 100644 --- a/src/backend/executor/nodeSamplescan.c +++ b/src/backend/executor/nodeSamplescan.c @@ -125,6 +125,8 @@ ExecInitSampleScan(SampleScan *node, EState *estate, int eflags) ExecOpenScanRelation(estate, node->scan.scanrelid, eflags); + if (unlikely(!ExecPlanStillValid(estate))) + return scanstate; /* we won't set up the HeapScanDesc till later */ scanstate->ss.ss_currentScanDesc = NULL; diff --git a/src/backend/executor/nodeSeqscan.c b/src/backend/executor/nodeSeqscan.c index 911266da07..9e3ef94388 100644 --- a/src/backend/executor/nodeSeqscan.c +++ b/src/backend/executor/nodeSeqscan.c @@ -153,6 +153,8 @@ ExecInitSeqScan(SeqScan *node, EState *estate, int eflags) ExecOpenScanRelation(estate, node->scan.scanrelid, eflags); + if (unlikely(!ExecPlanStillValid(estate))) + return scanstate; /* and create slot with the appropriate rowtype */ ExecInitScanTupleSlot(estate, &scanstate->ss, diff --git a/src/backend/executor/nodeSetOp.c b/src/backend/executor/nodeSetOp.c index 5c2861d243..475af4df24 100644 --- a/src/backend/executor/nodeSetOp.c +++ b/src/backend/executor/nodeSetOp.c @@ -528,6 +528,8 @@ ExecInitSetOp(SetOp *node, EState *estate, int eflags) if (node->strategy == SETOP_HASHED) eflags &= ~EXEC_FLAG_REWIND; outerPlanState(setopstate) = ExecInitNode(outerPlan(node), estate, eflags); + if (unlikely(!ExecPlanStillValid(estate))) + return setopstate; outerDesc = ExecGetResultType(outerPlanState(setopstate)); /* diff --git a/src/backend/executor/nodeSort.c b/src/backend/executor/nodeSort.c index c8a35b64a8..9de717aa7c 100644 --- a/src/backend/executor/nodeSort.c +++ b/src/backend/executor/nodeSort.c @@ -263,6 +263,8 @@ ExecInitSort(Sort *node, EState *estate, int eflags) eflags &= ~(EXEC_FLAG_REWIND | EXEC_FLAG_BACKWARD | EXEC_FLAG_MARK); outerPlanState(sortstate) = ExecInitNode(outerPlan(node), estate, eflags); + if (unlikely(!ExecPlanStillValid(estate))) + return sortstate; /* * Initialize scan slot and type. diff --git a/src/backend/executor/nodeSubqueryscan.c b/src/backend/executor/nodeSubqueryscan.c index 91d7ae82ce..d9c10d1f6f 100644 --- a/src/backend/executor/nodeSubqueryscan.c +++ b/src/backend/executor/nodeSubqueryscan.c @@ -124,6 +124,8 @@ ExecInitSubqueryScan(SubqueryScan *node, EState *estate, int eflags) * initialize subquery */ subquerystate->subplan = ExecInitNode(node->subplan, estate, eflags); + if (unlikely(!ExecPlanStillValid(estate))) + return subquerystate; /* * Initialize scan slot and type (needed by ExecAssignScanProjectionInfo) diff --git a/src/backend/executor/nodeTidrangescan.c b/src/backend/executor/nodeTidrangescan.c index 9147e4afa8..a7482aee50 100644 --- a/src/backend/executor/nodeTidrangescan.c +++ b/src/backend/executor/nodeTidrangescan.c @@ -378,6 +378,8 @@ ExecInitTidRangeScan(TidRangeScan *node, EState *estate, int eflags) * open the scan relation */ currentRelation = ExecOpenScanRelation(estate, node->scan.scanrelid, eflags); + if (unlikely(!ExecPlanStillValid(estate))) + return tidrangestate; tidrangestate->ss.ss_currentRelation = currentRelation; tidrangestate->ss.ss_currentScanDesc = NULL; /* no table scan here */ diff --git a/src/backend/executor/nodeTidscan.c b/src/backend/executor/nodeTidscan.c index 74ec6afdcc..657411ef19 100644 --- a/src/backend/executor/nodeTidscan.c +++ b/src/backend/executor/nodeTidscan.c @@ -523,6 +523,8 @@ ExecInitTidScan(TidScan *node, EState *estate, int eflags) * open the scan relation */ currentRelation = ExecOpenScanRelation(estate, node->scan.scanrelid, eflags); + if (unlikely(!ExecPlanStillValid(estate))) + return tidstate; tidstate->ss.ss_currentRelation = currentRelation; tidstate->ss.ss_currentScanDesc = NULL; /* no heap scan here */ diff --git a/src/backend/executor/nodeUnique.c b/src/backend/executor/nodeUnique.c index 13c556326a..ee30688417 100644 --- a/src/backend/executor/nodeUnique.c +++ b/src/backend/executor/nodeUnique.c @@ -136,6 +136,8 @@ ExecInitUnique(Unique *node, EState *estate, int eflags) * then initialize outer plan */ outerPlanState(uniquestate) = ExecInitNode(outerPlan(node), estate, eflags); + if (unlikely(!ExecPlanStillValid(estate))) + return uniquestate; /* * Initialize result slot and type. Unique nodes do no projections, so diff --git a/src/backend/executor/nodeWindowAgg.c b/src/backend/executor/nodeWindowAgg.c index c4c6f009ba..1246d7919a 100644 --- a/src/backend/executor/nodeWindowAgg.c +++ b/src/backend/executor/nodeWindowAgg.c @@ -2461,6 +2461,8 @@ ExecInitWindowAgg(WindowAgg *node, EState *estate, int eflags) */ outerPlan = outerPlan(node); outerPlanState(winstate) = ExecInitNode(outerPlan, estate, eflags); + if (unlikely(!ExecPlanStillValid(estate))) + return winstate; /* * initialize source tuple type (which is also the tuple type that we'll diff --git a/src/backend/executor/spi.c b/src/backend/executor/spi.c index 33975687b3..f2cca807ef 100644 --- a/src/backend/executor/spi.c +++ b/src/backend/executor/spi.c @@ -2668,6 +2668,7 @@ _SPI_execute_plan(SPIPlanPtr plan, const SPIExecuteOptions *options, snap = InvalidSnapshot; qdesc = CreateQueryDesc(stmt, + NULL, plansource->query_string, snap, crosscheck_snapshot, dest, diff --git a/src/backend/tcop/pquery.c b/src/backend/tcop/pquery.c index 5565f200c3..4ef349df8b 100644 --- a/src/backend/tcop/pquery.c +++ b/src/backend/tcop/pquery.c @@ -65,6 +65,7 @@ static void DoPortalRewind(Portal portal); */ QueryDesc * CreateQueryDesc(PlannedStmt *plannedstmt, + CachedPlan *cplan, const char *sourceText, Snapshot snapshot, Snapshot crosscheck_snapshot, @@ -77,6 +78,7 @@ CreateQueryDesc(PlannedStmt *plannedstmt, qd->operation = plannedstmt->commandType; /* operation */ qd->plannedstmt = plannedstmt; /* plan */ + qd->cplan = cplan; /* CachedPlan, if plan is from one */ qd->sourceText = sourceText; /* query text */ qd->snapshot = RegisterSnapshot(snapshot); /* snapshot */ /* RI check snapshot */ @@ -145,7 +147,7 @@ ProcessQuery(PlannedStmt *plan, /* * Create the QueryDesc object */ - queryDesc = CreateQueryDesc(plan, sourceText, + queryDesc = CreateQueryDesc(plan, NULL, sourceText, GetActiveSnapshot(), InvalidSnapshot, dest, params, queryEnv, 0); @@ -493,6 +495,7 @@ PortalStart(Portal portal, ParamListInfo params, * the destination to DestNone. */ queryDesc = CreateQueryDesc(linitial_node(PlannedStmt, portal->stmts), + NULL, portal->sourceText, GetActiveSnapshot(), InvalidSnapshot, diff --git a/src/include/executor/execdesc.h b/src/include/executor/execdesc.h index af2bf36dfb..4b7368a0dc 100644 --- a/src/include/executor/execdesc.h +++ b/src/include/executor/execdesc.h @@ -32,9 +32,12 @@ */ typedef struct QueryDesc { + NodeTag type; + /* These fields are provided by CreateQueryDesc */ CmdType operation; /* CMD_SELECT, CMD_UPDATE, etc. */ PlannedStmt *plannedstmt; /* planner's output (could be utility, too) */ + struct CachedPlan *cplan; /* CachedPlan, if plannedstmt is from one */ const char *sourceText; /* source text of the query */ Snapshot snapshot; /* snapshot to use for query */ Snapshot crosscheck_snapshot; /* crosscheck for RI update/delete */ @@ -57,6 +60,7 @@ typedef struct QueryDesc /* in pquery.c */ extern QueryDesc *CreateQueryDesc(PlannedStmt *plannedstmt, + struct CachedPlan *cplan, const char *sourceText, Snapshot snapshot, Snapshot crosscheck_snapshot, diff --git a/src/include/executor/executor.h b/src/include/executor/executor.h index aeebe0e0ff..72cbf120c5 100644 --- a/src/include/executor/executor.h +++ b/src/include/executor/executor.h @@ -19,6 +19,7 @@ #include "nodes/lockoptions.h" #include "nodes/parsenodes.h" #include "utils/memutils.h" +#include "utils/plancache.h" /* @@ -256,6 +257,15 @@ extern void ExecEndNode(PlanState *node); extern void ExecShutdownNode(PlanState *node); extern void ExecSetTupleBound(int64 tuples_needed, PlanState *child_node); +/* + * Is the CachedPlan in es_cachedplan still valid? + */ +static inline bool +ExecPlanStillValid(EState *estate) +{ + return estate->es_cachedplan == NULL ? true : + CachedPlanStillValid(estate->es_cachedplan); +} /* ---------------------------------------------------------------- * ExecProcNode diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h index cb714f4a19..846eb32a1d 100644 --- a/src/include/nodes/execnodes.h +++ b/src/include/nodes/execnodes.h @@ -623,6 +623,8 @@ typedef struct EState * ExecRowMarks, or NULL if none */ List *es_rteperminfos; /* List of RTEPermissionInfo */ PlannedStmt *es_plannedstmt; /* link to top of plan tree */ + struct CachedPlan *es_cachedplan; /* CachedPlan if plannedstmt is from + * one, or NULL if not */ const char *es_sourceText; /* Source text from QueryDesc */ JunkFilter *es_junkFilter; /* top-level junk filter, if any */ diff --git a/src/include/utils/plancache.h b/src/include/utils/plancache.h index 916e59d9fe..0a9e041d51 100644 --- a/src/include/utils/plancache.h +++ b/src/include/utils/plancache.h @@ -221,6 +221,20 @@ extern CachedPlan *GetCachedPlan(CachedPlanSource *plansource, ParamListInfo boundParams, ResourceOwner owner, QueryEnvironment *queryEnv); + +/* + * CachedPlanStillValid + * Returns if a cached generic plan is still valid + * + * Invoked by the executor for each relation lock acquired during the + * initialization of the plan tree within the CachedPlan. + */ +static inline bool +CachedPlanStillValid(CachedPlan *cplan) +{ + return cplan->is_valid; +} + extern void ReleaseCachedPlan(CachedPlan *plan, ResourceOwner owner); extern bool CachedPlanAllowsSimpleValidityCheck(CachedPlanSource *plansource, -- 2.35.3