public inbox for [email protected]
help / color / mirror / Atom feedFrom: Amit Langote <[email protected]>
To: Junwang Zhao <[email protected]>
Cc: Robert Haas <[email protected]>
Cc: Alvaro Herrera <[email protected]>
Cc: Andres Freund <[email protected]>
Cc: Daniel Gustafsson <[email protected]>
Cc: David Rowley <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Cc: Thom Brown <[email protected]>
Cc: Tom Lane <[email protected]>
Subject: Re: generic plans and "initial" pruning
Date: Thu, 5 Sep 2024 18:55:47 +0900
Message-ID: <CA+HiwqGSOge3eT3kcm_nxCSA3Ut+d0jtchi8g8J9uXi-kyC7Jw@mail.gmail.com> (raw)
In-Reply-To: <CA+HiwqGJ6O=-EcqUHv43ussnm8yJj47tS833M1oksgt5tdHq3Q@mail.gmail.com>
References: <CA+HiwqFpZ80UJKr4tZus4Omgg7YESzFXKSwSHRW2Ap2=XSVyUA@mail.gmail.com>
<[email protected]>
<CA+HiwqF+3tv=tuB9EVfOj9YcXhSq477X+1RKOpJ5JqCCj3qgww@mail.gmail.com>
<CA+TgmobHL_vTjOdy6KVMVeW-CQQmXXz_yU6Q9d2YjnVfFxuy6A@mail.gmail.com>
<CA+HiwqHL=aGU9Y4RYXQ5VCp4L5NVdiaQLLoXN3NCQQQMKo0ByQ@mail.gmail.com>
<CA+TgmoabYD=pnccFLzbbREFsqkFgE4EZ+FdHoTOhgCqn4jP2Cw@mail.gmail.com>
<CA+HiwqE_rQ9pZnkXeoHdds2kgAiT7XNNHZW8gTGicBdXv0rwnw@mail.gmail.com>
<CA+TgmoY2drv9PmrRAC7AR77mkx09sOh-+5qJkHB_hLKeHRNqzQ@mail.gmail.com>
<CA+HiwqHkjicWzfAjB6_SVsVmKF6omQ4EBHr+GTUgJNN7WiUDag@mail.gmail.com>
<CA+TgmoaZxb4JTimK8MmbXEeCwtzyfx7uGYjq565s2pY9i1GN+Q@mail.gmail.com>
<CA+HiwqHzKO9FT-CjFWo6OmkiCSYmbPspKXVex96tOBKf6S_x_w@mail.gmail.com>
<CA+TgmoZGWyMXutfen-NNv9=QM7eCHn9R1bpLZ9N4sRURMOCK2A@mail.gmail.com>
<CA+HiwqHNb9jrwOFHfmASfiGc=SnqXs7THwQ_Rta=z+ognYV8qw@mail.gmail.com>
<CA+HiwqH9u1RWn9OEa=VQQpJagB0hDLCY+=fSyBC4ZkeU6Gg2HA@mail.gmail.com>
<CAEG8a3LTsA4KcbvHYGbFX6w27Rv-7m5RZMHrH+JupGw+cm2Ung@mail.gmail.com>
<CA+HiwqGJ6O=-EcqUHv43ussnm8yJj47tS833M1oksgt5tdHq3Q@mail.gmail.com>
On Mon, Sep 2, 2024 at 5:19 PM Amit Langote <[email protected]> wrote:
> On Sat, Aug 31, 2024 at 9:30 PM Junwang Zhao <[email protected]> wrote:
> > @@ -1241,7 +1244,7 @@ GetCachedPlan(CachedPlanSource *plansource,
> > ParamListInfo boundParams,
> > if (customplan)
> > {
> > /* Build a custom plan */
> > - plan = BuildCachedPlan(plansource, qlist, boundParams, queryEnv);
> > + plan = BuildCachedPlan(plansource, qlist, boundParams, queryEnv, true);
> >
> > Is the *true* here a typo? Seems it should be *false* for custom plan?
>
> That's correct, thanks for catching that. Will fix.
Done.
I've also rewritten the new GetSingleCachedPlan() function in 0003.
The most glaring bug in the previous version was that the transient
CachedPlan it creates cannot be seen by PlanCacheRelCallback() et al
functions because it was intentionally not linked to the
CachedPlanSource, so if the CachedPlan would not be invalidated even
if some prunable relation got changed before it is locked during
ExecutorStart(). I've added a new list standalone_plan_list to add
these to and changed the inval callback functions to invalidate any
plans contained in them.
Another thing I found out through testing is that CachedPlanSource can
have become invalid since leaving GetCachedPlan() (actually even
before returning from that function) because of
PlanCacheSysCallback(), which drops/invalidates *all* plans when a
syscache is invalidated. There are comments in plancache.c (see
BuildCachedPlan()) saying that such invalidations are, in theory,
false positives, but that gave me a pause nonetheless.
Finally, instead of calling GetCachedPlan() from GetSingleCachedPlan()
to create a plan for only the query whose plan got invalidated, which
required a bunch of care to ensure that the CachedPlanSource is not
overwritten with the information about this single-query planning,
I've made GetSingleCachedPlan() create the PlannedStmt and the
detached CachedPlan object on its own, borrowing the minimal necessary
code from BuildCachedPlan() to do so.
--
Thanks, Amit Langote
Attachments:
[application/octet-stream] v52-0001-Defer-locking-of-runtime-prunable-relations-to-e.patch (32.8K, 2-v52-0001-Defer-locking-of-runtime-prunable-relations-to-e.patch)
download | inline diff:
From 74906c4bbc42b362f7a5608774af68615a299912 Mon Sep 17 00:00:00 2001
From: Amit Langote <[email protected]>
Date: Wed, 7 Aug 2024 18:25:51 +0900
Subject: [PATCH v52 1/3] Defer locking of runtime-prunable relations to
executor
When preparing a cached plan for execution, plancache.c locks the
relations contained in the plan's range table to ensure it is safe for
execution. However, this simplistic approach, implemented in
AcquireExecutorLocks(), results in unnecessarily locking relations that
might be pruned during "initial" runtime pruning.
To optimize this, the locking is now deferred for relations that are
subject to "initial" runtime pruning. The planner now provides a set
of "unprunable" relations, available through the new
PlannedStmt.unprunableRelids field. AcquireExecutorLocks() will now
only lock those relations.
PlannedStmt.unprunableRelids is populated by subtracting the set of
initially prunable relids from the set of all RT indexes. The prunable
relids set is constructed by examining all PartitionPruneInfos during
set_plan_refs() and storing the RT indexes of partitions subject to
"initial" pruning steps. While at it, some duplicated code in
set_append_references() and set_mergeappend_references() that
constructs the prunable relids set has been refactored into a common
function.
To enable the executor to determine whether the plan tree it's
executing is a cached one, the CachedPlan is now made available via
the QueryDesc. The executor can call CachedPlanRequiresLocking(),
which returns true if the CachedPlan is a reusable generic plan that
might contain relations needing to be locked. If so, the executor
will lock any relation that is not in PlannedStmt.unprunableRelids.
Finally, an Assert has been added in ExecCheckPermissions() to ensure
that all relations whose permissions are checked have been properly
locked. This helps catch any accidental omission of relations from the
unprunableRelids set that should have their permissions checked.
This deferment introduces a window in which prunable relations may be
altered by concurrent DDL, potentially causing the plan to become
invalid. As a result, the executor might attempt to run an invalid plan,
leading to errors such as being unable to locate a partition-only index
during ExecInitIndexScan(). Future commits will introduce changes to
ready the executor to check plan validity during ExecutorStart() and
retry with a newly created plan if the original one becomes invalid
after taking deferred locks.
---
src/backend/commands/copyto.c | 2 +-
src/backend/commands/createas.c | 2 +-
src/backend/commands/explain.c | 7 ++--
src/backend/commands/extension.c | 1 +
src/backend/commands/matview.c | 2 +-
src/backend/commands/prepare.c | 3 +-
src/backend/executor/execMain.c | 18 ++++++++
src/backend/executor/execParallel.c | 9 +++-
src/backend/executor/execUtils.c | 30 +++++++++++++-
src/backend/executor/functions.c | 1 +
src/backend/executor/spi.c | 1 +
src/backend/optimizer/plan/planner.c | 2 +
src/backend/optimizer/plan/setrefs.c | 62 +++++++++++++++-------------
src/backend/partitioning/partprune.c | 24 ++++++++++-
src/backend/storage/lmgr/lmgr.c | 1 +
src/backend/tcop/pquery.c | 10 ++++-
src/backend/utils/cache/lsyscache.c | 1 -
src/backend/utils/cache/plancache.c | 40 +++++++++++-------
src/include/commands/explain.h | 5 ++-
src/include/executor/execdesc.h | 2 +
src/include/nodes/execnodes.h | 2 +
src/include/nodes/pathnodes.h | 6 +++
src/include/nodes/plannodes.h | 11 +++++
src/include/utils/plancache.h | 10 +++++
24 files changed, 195 insertions(+), 57 deletions(-)
diff --git a/src/backend/commands/copyto.c b/src/backend/commands/copyto.c
index 91de442f43..db976f928a 100644
--- a/src/backend/commands/copyto.c
+++ b/src/backend/commands/copyto.c
@@ -552,7 +552,7 @@ 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 0b629b1f79..57a3375cad 100644
--- a/src/backend/commands/createas.c
+++ b/src/backend/commands/createas.c
@@ -324,7 +324,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 11df4a04d4..a83ea07db1 100644
--- a/src/backend/commands/explain.c
+++ b/src/backend/commands/explain.c
@@ -507,7 +507,7 @@ standard_ExplainOneQuery(Query *query, int cursorOptions,
}
/* run it (if needed) and produce output */
- ExplainOnePlan(plan, into, es, queryString, params, queryEnv,
+ ExplainOnePlan(plan, NULL, into, es, queryString, params, queryEnv,
&planduration, (es->buffers ? &bufusage : NULL),
es->memory ? &mem_counters : NULL);
}
@@ -615,7 +615,8 @@ ExplainOneUtility(Node *utilityStmt, IntoClause *into, ExplainState *es,
* to call it.
*/
void
-ExplainOnePlan(PlannedStmt *plannedstmt, IntoClause *into, ExplainState *es,
+ExplainOnePlan(PlannedStmt *plannedstmt, CachedPlan *cplan,
+ IntoClause *into, ExplainState *es,
const char *queryString, ParamListInfo params,
QueryEnvironment *queryEnv, const instr_time *planduration,
const BufferUsage *bufusage,
@@ -671,7 +672,7 @@ ExplainOnePlan(PlannedStmt *plannedstmt, IntoClause *into, ExplainState *es,
dest = None_Receiver;
/* Create a QueryDesc for the query */
- queryDesc = CreateQueryDesc(plannedstmt, queryString,
+ queryDesc = CreateQueryDesc(plannedstmt, cplan, queryString,
GetActiveSnapshot(), InvalidSnapshot,
dest, params, queryEnv, instrument_option);
diff --git a/src/backend/commands/extension.c b/src/backend/commands/extension.c
index 1643c8c69a..3f7f4306fe 100644
--- a/src/backend/commands/extension.c
+++ b/src/backend/commands/extension.c
@@ -798,6 +798,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 91f0fd6ea3..a7a79583ec 100644
--- a/src/backend/commands/matview.c
+++ b/src/backend/commands/matview.c
@@ -438,7 +438,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/commands/prepare.c b/src/backend/commands/prepare.c
index 07257d4db9..311b9ebd5b 100644
--- a/src/backend/commands/prepare.c
+++ b/src/backend/commands/prepare.c
@@ -655,7 +655,8 @@ ExplainExecuteQuery(ExecuteStmt *execstmt, IntoClause *into, ExplainState *es,
PlannedStmt *pstmt = lfirst_node(PlannedStmt, p);
if (pstmt->commandType != CMD_UTILITY)
- ExplainOnePlan(pstmt, into, es, query_string, paramLI, queryEnv,
+ ExplainOnePlan(pstmt, cplan, into, es, query_string, paramLI,
+ queryEnv,
&planduration, (es->buffers ? &bufusage : NULL),
es->memory ? &mem_counters : NULL);
else
diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c
index 29e186fa73..271f9d93fc 100644
--- a/src/backend/executor/execMain.c
+++ b/src/backend/executor/execMain.c
@@ -52,6 +52,7 @@
#include "miscadmin.h"
#include "parser/parse_relation.h"
#include "rewrite/rewriteHandler.h"
+#include "storage/lmgr.h"
#include "tcop/utility.h"
#include "utils/acl.h"
#include "utils/backend_status.h"
@@ -597,6 +598,21 @@ ExecCheckPermissions(List *rangeTable, List *rteperminfos,
(rte->rtekind == RTE_SUBQUERY &&
rte->relkind == RELKIND_VIEW));
+ /*
+ * Ensure that we have at least an AccessShareLock on relations
+ * whose permissions need to be checked.
+ *
+ * Skip this check in a parallel worker because locks won't be
+ * taken until ExecInitNode() performs plan initialization.
+ *
+ * XXX: ExecCheckPermissions() in a parallel worker may be
+ * redundant with the checks done in the leader process, so this
+ * should be reviewed to ensure it’s necessary.
+ */
+ Assert(IsParallelWorker() ||
+ CheckRelationOidLockedByMe(rte->relid, AccessShareLock,
+ true));
+
(void) getRTEPermissionInfo(rteperminfos, rte);
/* Many-to-one mapping not allowed */
Assert(!bms_is_member(rte->perminfoindex, indexset));
@@ -829,6 +845,7 @@ InitPlan(QueryDesc *queryDesc, int eflags)
{
CmdType operation = queryDesc->operation;
PlannedStmt *plannedstmt = queryDesc->plannedstmt;
+ CachedPlan *cachedplan = queryDesc->cplan;
Plan *plan = plannedstmt->planTree;
List *rangeTable = plannedstmt->rtable;
EState *estate = queryDesc->estate;
@@ -848,6 +865,7 @@ InitPlan(QueryDesc *queryDesc, int eflags)
ExecInitRangeTable(estate, rangeTable, plannedstmt->permInfos);
estate->es_plannedstmt = plannedstmt;
+ estate->es_cachedplan = cachedplan;
/*
* Next, build the ExecRowMark array from the PlanRowMark(s), if any.
diff --git a/src/backend/executor/execParallel.c b/src/backend/executor/execParallel.c
index bfb3419efb..03b48e12b4 100644
--- a/src/backend/executor/execParallel.c
+++ b/src/backend/executor/execParallel.c
@@ -1256,8 +1256,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. */
+ /*
+ * Create a QueryDesc for the query. We pass NULL for cachedplan, because
+ * we don't have a pointer to the CachedPlan in the leader's process. It's
+ * fine because the only reason the executor needs to see it is to decide
+ * if it should take locks on certain relations, but paraller workers
+ * always take locks anyway.
+ */
return CreateQueryDesc(pstmt,
+ NULL,
queryString,
GetActiveSnapshot(), InvalidSnapshot,
receiver, paramLI, NULL, instrument_options);
diff --git a/src/backend/executor/execUtils.c b/src/backend/executor/execUtils.c
index 5737f9f4eb..6dfd5a26b7 100644
--- a/src/backend/executor/execUtils.c
+++ b/src/backend/executor/execUtils.c
@@ -752,6 +752,26 @@ ExecInitRangeTable(EState *estate, List *rangeTable, List *permInfos)
estate->es_rowmarks = NULL;
}
+/*
+ * ExecShouldLockRelation
+ * Determine if the relation should be locked.
+ *
+ * The relation does not need to be locked if we are not running a cached
+ * plan or if it has already been locked as an unprunable relation.
+ *
+ * Lock the relation if it might be one of the prunable relations mentioned
+ * in the cached plan.
+ */
+static bool
+ExecShouldLockRelation(EState *estate, Index rtindex)
+{
+ if (estate->es_cachedplan == NULL ||
+ bms_is_member(rtindex, estate->es_plannedstmt->unprunableRelids))
+ return false;
+
+ return CachedPlanRequiresLocking(estate->es_cachedplan);
+}
+
/*
* ExecGetRangeTableRelation
* Open the Relation for a range table entry, if not already done
@@ -773,7 +793,7 @@ ExecGetRangeTableRelation(EState *estate, Index rti)
Assert(rte->rtekind == RTE_RELATION);
- if (!IsParallelWorker())
+ if (!IsParallelWorker() && !ExecShouldLockRelation(estate, rti))
{
/*
* In a normal query, we should already have the appropriate lock,
@@ -789,9 +809,17 @@ ExecGetRangeTableRelation(EState *estate, Index rti)
else
{
/*
+ * Lock relation either if we are a parallel worker or if
+ * ExecShouldLockRelation() says we should.
+ *
* If we are a parallel worker, we need to obtain our own local
* lock on the relation. This ensures sane behavior in case the
* parent process exits before we do.
+ *
+ * ExecShouldLockRelation() would return true if the RT index is
+ * that of a prunable relation and we're running a cached generic
+ * plan. AcquireExecutorLocks() of plancache.c would have locked
+ * only the unprunable relations in the plan tree.
*/
rel = table_open(rte->relid, rte->rellockmode);
}
diff --git a/src/backend/executor/functions.c b/src/backend/executor/functions.c
index 692854e2b3..6f6f45e0ad 100644
--- a/src/backend/executor/functions.c
+++ b/src/backend/executor/functions.c
@@ -840,6 +840,7 @@ postquel_start(execution_state *es, SQLFunctionCachePtr fcache)
dest = None_Receiver;
es->qd = CreateQueryDesc(es->stmt,
+ NULL,
fcache->src,
GetActiveSnapshot(),
InvalidSnapshot,
diff --git a/src/backend/executor/spi.c b/src/backend/executor/spi.c
index d6516b1bca..902793b02b 100644
--- a/src/backend/executor/spi.c
+++ b/src/backend/executor/spi.c
@@ -2684,6 +2684,7 @@ _SPI_execute_plan(SPIPlanPtr plan, const SPIExecuteOptions *options,
snap = InvalidSnapshot;
qdesc = CreateQueryDesc(stmt,
+ cplan,
plansource->query_string,
snap, crosscheck_snapshot,
dest,
diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c
index b5827d3980..cb9b6f0147 100644
--- a/src/backend/optimizer/plan/planner.c
+++ b/src/backend/optimizer/plan/planner.c
@@ -546,6 +546,8 @@ standard_planner(Query *parse, const char *query_string, int cursorOptions,
result->parallelModeNeeded = glob->parallelModeNeeded;
result->planTree = top_plan;
result->rtable = glob->finalrtable;
+ result->unprunableRelids = bms_difference(bms_add_range(NULL, 1, list_length(result->rtable)),
+ glob->prunableRelids);
result->permInfos = glob->finalrteperminfos;
result->resultRelations = glob->resultRelations;
result->appendRelations = glob->appendRelations;
diff --git a/src/backend/optimizer/plan/setrefs.c b/src/backend/optimizer/plan/setrefs.c
index 7aed84584c..b6be0e5730 100644
--- a/src/backend/optimizer/plan/setrefs.c
+++ b/src/backend/optimizer/plan/setrefs.c
@@ -154,6 +154,9 @@ static Plan *set_append_references(PlannerInfo *root,
static Plan *set_mergeappend_references(PlannerInfo *root,
MergeAppend *mplan,
int rtoffset);
+static void set_part_prune_references(PartitionPruneInfo *pinfo,
+ PlannerGlobal *glob,
+ int rtoffset);
static void set_hash_references(PlannerInfo *root, Plan *plan, int rtoffset);
static Relids offset_relid_set(Relids relids, int rtoffset);
static Node *fix_scan_expr(PlannerInfo *root, Node *node,
@@ -1783,20 +1786,8 @@ set_append_references(PlannerInfo *root,
aplan->apprelids = offset_relid_set(aplan->apprelids, rtoffset);
if (aplan->part_prune_info)
- {
- foreach(l, aplan->part_prune_info->prune_infos)
- {
- List *prune_infos = lfirst(l);
- ListCell *l2;
-
- foreach(l2, prune_infos)
- {
- PartitionedRelPruneInfo *pinfo = lfirst(l2);
-
- pinfo->rtindex += rtoffset;
- }
- }
- }
+ set_part_prune_references(aplan->part_prune_info, root->glob,
+ rtoffset);
/* We don't need to recurse to lefttree or righttree ... */
Assert(aplan->plan.lefttree == NULL);
@@ -1859,20 +1850,8 @@ set_mergeappend_references(PlannerInfo *root,
mplan->apprelids = offset_relid_set(mplan->apprelids, rtoffset);
if (mplan->part_prune_info)
- {
- foreach(l, mplan->part_prune_info->prune_infos)
- {
- List *prune_infos = lfirst(l);
- ListCell *l2;
-
- foreach(l2, prune_infos)
- {
- PartitionedRelPruneInfo *pinfo = lfirst(l2);
-
- pinfo->rtindex += rtoffset;
- }
- }
- }
+ set_part_prune_references(mplan->part_prune_info, root->glob,
+ rtoffset);
/* We don't need to recurse to lefttree or righttree ... */
Assert(mplan->plan.lefttree == NULL);
@@ -1881,6 +1860,33 @@ set_mergeappend_references(PlannerInfo *root,
return (Plan *) mplan;
}
+/*
+ * Updates RT indexes in PartitionedRelPruneInfos contained in pinfo and adds
+ * the RT indexes of "prunable" relations into glob->prunableRelids.
+ */
+static void
+set_part_prune_references(PartitionPruneInfo *pinfo, PlannerGlobal *glob,
+ int rtoffset)
+{
+ ListCell *l;
+
+ foreach(l, pinfo->prune_infos)
+ {
+ List *prune_infos = lfirst(l);
+ ListCell *l2;
+
+ foreach(l2, prune_infos)
+ {
+ PartitionedRelPruneInfo *prelinfo = lfirst(l2);
+
+ prelinfo->rtindex += rtoffset;
+ if (prelinfo->initial_pruning_steps != NIL)
+ glob->prunableRelids = bms_add_members(glob->prunableRelids,
+ prelinfo->present_part_rtis);
+ }
+ }
+}
+
/*
* set_hash_references
* Do set_plan_references processing on a Hash node
diff --git a/src/backend/partitioning/partprune.c b/src/backend/partitioning/partprune.c
index 9a1a7faac7..8e27e35df2 100644
--- a/src/backend/partitioning/partprune.c
+++ b/src/backend/partitioning/partprune.c
@@ -634,6 +634,7 @@ make_partitionedrel_pruneinfo(PlannerInfo *root, RelOptInfo *parentrel,
PartitionedRelPruneInfo *pinfo = lfirst(lc);
RelOptInfo *subpart = find_base_rel(root, pinfo->rtindex);
Bitmapset *present_parts;
+ Bitmapset *present_part_rtis;
int nparts = subpart->nparts;
int *subplan_map;
int *subpart_map;
@@ -650,7 +651,7 @@ make_partitionedrel_pruneinfo(PlannerInfo *root, RelOptInfo *parentrel,
subpart_map = (int *) palloc(nparts * sizeof(int));
memset(subpart_map, -1, nparts * sizeof(int));
relid_map = (Oid *) palloc0(nparts * sizeof(Oid));
- present_parts = NULL;
+ present_parts = present_part_rtis = NULL;
i = -1;
while ((i = bms_next_member(subpart->live_parts, i)) >= 0)
@@ -664,15 +665,35 @@ make_partitionedrel_pruneinfo(PlannerInfo *root, RelOptInfo *parentrel,
subplan_map[i] = subplanidx = relid_subplan_map[partrel->relid] - 1;
subpart_map[i] = subpartidx = relid_subpart_map[partrel->relid] - 1;
relid_map[i] = planner_rt_fetch(partrel->relid, root)->relid;
+
+ /*
+ * Track the RT indexes of partitions to ensure they are included
+ * in the prunableRelids set of relations that are locked during
+ * execution. This ensures that if the plan is cached, these
+ * partitions are locked when the plan is reused.
+ *
+ * Partitions without a subplan and sub-partitioned partitions
+ * where none of the sub-partitions have a subplan due to
+ * constraint exclusion are not included in this set. Instead,
+ * they are added to the unprunableRelids set, and the relations
+ * in this set are locked by AcquireExecutorLocks() before
+ * executing a cached plan.
+ */
if (subplanidx >= 0)
{
present_parts = bms_add_member(present_parts, i);
+ present_part_rtis = bms_add_member(present_part_rtis,
+ partrel->relid);
/* Record finding this subplan */
subplansfound = bms_add_member(subplansfound, subplanidx);
}
else if (subpartidx >= 0)
+ {
present_parts = bms_add_member(present_parts, i);
+ present_part_rtis = bms_add_member(present_part_rtis,
+ partrel->relid);
+ }
}
/*
@@ -684,6 +705,7 @@ make_partitionedrel_pruneinfo(PlannerInfo *root, RelOptInfo *parentrel,
/* Record the maps and other information. */
pinfo->present_parts = present_parts;
+ pinfo->present_part_rtis = present_part_rtis;
pinfo->nparts = nparts;
pinfo->subplan_map = subplan_map;
pinfo->subpart_map = subpart_map;
diff --git a/src/backend/storage/lmgr/lmgr.c b/src/backend/storage/lmgr/lmgr.c
index 094522acb4..a1c89f5d72 100644
--- a/src/backend/storage/lmgr/lmgr.c
+++ b/src/backend/storage/lmgr/lmgr.c
@@ -26,6 +26,7 @@
#include "storage/procarray.h"
#include "storage/sinvaladt.h"
#include "utils/inval.h"
+#include "utils/lsyscache.h"
/*
diff --git a/src/backend/tcop/pquery.c b/src/backend/tcop/pquery.c
index a1f8d03db1..6e8f6b1b8f 100644
--- a/src/backend/tcop/pquery.c
+++ b/src/backend/tcop/pquery.c
@@ -36,6 +36,7 @@ Portal ActivePortal = NULL;
static void ProcessQuery(PlannedStmt *plan,
+ CachedPlan *cplan,
const char *sourceText,
ParamListInfo params,
QueryEnvironment *queryEnv,
@@ -65,6 +66,7 @@ static void DoPortalRewind(Portal portal);
*/
QueryDesc *
CreateQueryDesc(PlannedStmt *plannedstmt,
+ CachedPlan *cplan,
const char *sourceText,
Snapshot snapshot,
Snapshot crosscheck_snapshot,
@@ -77,6 +79,7 @@ CreateQueryDesc(PlannedStmt *plannedstmt,
qd->operation = plannedstmt->commandType; /* operation */
qd->plannedstmt = plannedstmt; /* plan */
+ qd->cplan = cplan; /* CachedPlan supplying the plannedstmt */
qd->sourceText = sourceText; /* query text */
qd->snapshot = RegisterSnapshot(snapshot); /* snapshot */
/* RI check snapshot */
@@ -122,6 +125,7 @@ FreeQueryDesc(QueryDesc *qdesc)
* PORTAL_ONE_RETURNING, or PORTAL_ONE_MOD_WITH portal
*
* plan: the plan tree for the query
+ * cplan: CachedPlan supplying the plan
* sourceText: the source text of the query
* params: any parameters needed
* dest: where to send results
@@ -134,6 +138,7 @@ FreeQueryDesc(QueryDesc *qdesc)
*/
static void
ProcessQuery(PlannedStmt *plan,
+ CachedPlan *cplan,
const char *sourceText,
ParamListInfo params,
QueryEnvironment *queryEnv,
@@ -145,7 +150,7 @@ ProcessQuery(PlannedStmt *plan,
/*
* Create the QueryDesc object
*/
- queryDesc = CreateQueryDesc(plan, sourceText,
+ queryDesc = CreateQueryDesc(plan, cplan, sourceText,
GetActiveSnapshot(), InvalidSnapshot,
dest, params, queryEnv, 0);
@@ -493,6 +498,7 @@ PortalStart(Portal portal, ParamListInfo params,
* the destination to DestNone.
*/
queryDesc = CreateQueryDesc(linitial_node(PlannedStmt, portal->stmts),
+ portal->cplan,
portal->sourceText,
GetActiveSnapshot(),
InvalidSnapshot,
@@ -1276,6 +1282,7 @@ PortalRunMulti(Portal portal,
{
/* statement can set tag string */
ProcessQuery(pstmt,
+ portal->cplan,
portal->sourceText,
portal->portalParams,
portal->queryEnv,
@@ -1285,6 +1292,7 @@ PortalRunMulti(Portal portal,
{
/* stmt added by rewrite cannot set tag */
ProcessQuery(pstmt,
+ portal->cplan,
portal->sourceText,
portal->portalParams,
portal->queryEnv,
diff --git a/src/backend/utils/cache/lsyscache.c b/src/backend/utils/cache/lsyscache.c
index 48a280d089..f647821382 100644
--- a/src/backend/utils/cache/lsyscache.c
+++ b/src/backend/utils/cache/lsyscache.c
@@ -2113,7 +2113,6 @@ get_rel_relam(Oid relid)
return result;
}
-
/* ---------- TRANSFORM CACHE ---------- */
Oid
diff --git a/src/backend/utils/cache/plancache.c b/src/backend/utils/cache/plancache.c
index 5af1a168ec..5b75dadf13 100644
--- a/src/backend/utils/cache/plancache.c
+++ b/src/backend/utils/cache/plancache.c
@@ -104,7 +104,8 @@ static List *RevalidateCachedQuery(CachedPlanSource *plansource,
QueryEnvironment *queryEnv);
static bool CheckCachedPlan(CachedPlanSource *plansource);
static CachedPlan *BuildCachedPlan(CachedPlanSource *plansource, List *qlist,
- ParamListInfo boundParams, QueryEnvironment *queryEnv);
+ ParamListInfo boundParams, QueryEnvironment *queryEnv,
+ bool generic);
static bool choose_custom_plan(CachedPlanSource *plansource,
ParamListInfo boundParams);
static double cached_plan_cost(CachedPlan *plan, bool include_planner);
@@ -815,8 +816,11 @@ RevalidateCachedQuery(CachedPlanSource *plansource,
* Caller must have already called RevalidateCachedQuery to verify that the
* querytree is up to date.
*
- * On a "true" return, we have acquired the locks needed to run the plan.
- * (We must do this for the "true" result to be race-condition-free.)
+ * On a "true" return, we have acquired locks on the "unprunableRelids" set
+ * for all plans in plansource->stmt_list. The plans are not completely
+ * race-condition-free until the executor takes locks on the set of prunable
+ * relations that survive initial runtime pruning during executor
+ * initialization;
*/
static bool
CheckCachedPlan(CachedPlanSource *plansource)
@@ -893,10 +897,10 @@ CheckCachedPlan(CachedPlanSource *plansource)
* or it can be set to NIL if we need to re-copy the plansource's query_list.
*
* To build a generic, parameter-value-independent plan, pass NULL for
- * boundParams. To build a custom plan, pass the actual parameter values via
- * boundParams. For best effect, the PARAM_FLAG_CONST flag should be set on
- * each parameter value; otherwise the planner will treat the value as a
- * hint rather than a hard constant.
+ * boundParams, and true for generic. To build a custom plan, pass the actual
+ * parameter values via boundParams, and false for generic. For best effect,
+ * the PARAM_FLAG_CONST flag should be set on each parameter value; otherwise
+ * the planner will treat the value as a hint rather than a hard constant.
*
* Planning work is done in the caller's memory context. The finished plan
* is in a child memory context, which typically should get reparented
@@ -904,7 +908,8 @@ CheckCachedPlan(CachedPlanSource *plansource)
*/
static CachedPlan *
BuildCachedPlan(CachedPlanSource *plansource, List *qlist,
- ParamListInfo boundParams, QueryEnvironment *queryEnv)
+ ParamListInfo boundParams, QueryEnvironment *queryEnv,
+ bool generic)
{
CachedPlan *plan;
List *plist;
@@ -1026,6 +1031,7 @@ BuildCachedPlan(CachedPlanSource *plansource, List *qlist,
plan->refcount = 0;
plan->context = plan_context;
plan->is_oneshot = plansource->is_oneshot;
+ plan->is_generic = generic;
plan->is_saved = false;
plan->is_valid = true;
@@ -1196,7 +1202,7 @@ GetCachedPlan(CachedPlanSource *plansource, ParamListInfo boundParams,
else
{
/* Build a new generic plan */
- plan = BuildCachedPlan(plansource, qlist, NULL, queryEnv);
+ plan = BuildCachedPlan(plansource, qlist, NULL, queryEnv, true);
/* Just make real sure plansource->gplan is clear */
ReleaseGenericPlan(plansource);
/* Link the new generic plan into the plansource */
@@ -1241,7 +1247,7 @@ GetCachedPlan(CachedPlanSource *plansource, ParamListInfo boundParams,
if (customplan)
{
/* Build a custom plan */
- plan = BuildCachedPlan(plansource, qlist, boundParams, queryEnv);
+ plan = BuildCachedPlan(plansource, qlist, boundParams, queryEnv, false);
/* Accumulate total costs of custom plans */
plansource->total_custom_cost += cached_plan_cost(plan, true);
@@ -1387,8 +1393,8 @@ CachedPlanAllowsSimpleValidityCheck(CachedPlanSource *plansource,
}
/*
- * Reject if AcquireExecutorLocks would have anything to do. This is
- * probably unnecessary given the previous check, but let's be safe.
+ * Reject if there are any lockable relations. This is probably
+ * unnecessary given the previous check, but let's be safe.
*/
foreach(lc, plan->stmt_list)
{
@@ -1776,7 +1782,7 @@ AcquireExecutorLocks(List *stmt_list, bool acquire)
foreach(lc1, stmt_list)
{
PlannedStmt *plannedstmt = lfirst_node(PlannedStmt, lc1);
- ListCell *lc2;
+ int rtindex;
if (plannedstmt->commandType == CMD_UTILITY)
{
@@ -1794,9 +1800,13 @@ AcquireExecutorLocks(List *stmt_list, bool acquire)
continue;
}
- foreach(lc2, plannedstmt->rtable)
+ rtindex = -1;
+ while ((rtindex = bms_next_member(plannedstmt->unprunableRelids,
+ rtindex)) >= 0)
{
- RangeTblEntry *rte = (RangeTblEntry *) lfirst(lc2);
+ RangeTblEntry *rte = list_nth_node(RangeTblEntry,
+ plannedstmt->rtable,
+ rtindex - 1);
if (!(rte->rtekind == RTE_RELATION ||
(rte->rtekind == RTE_SUBQUERY && OidIsValid(rte->relid))))
diff --git a/src/include/commands/explain.h b/src/include/commands/explain.h
index 9b8b351d9a..bf326eeb70 100644
--- a/src/include/commands/explain.h
+++ b/src/include/commands/explain.h
@@ -101,8 +101,9 @@ extern void ExplainOneUtility(Node *utilityStmt, IntoClause *into,
ExplainState *es, const char *queryString,
ParamListInfo params, QueryEnvironment *queryEnv);
-extern void ExplainOnePlan(PlannedStmt *plannedstmt, IntoClause *into,
- ExplainState *es, const char *queryString,
+extern void ExplainOnePlan(PlannedStmt *plannedstmt, CachedPlan *cplan,
+ IntoClause *into, ExplainState *es,
+ const char *queryString,
ParamListInfo params, QueryEnvironment *queryEnv,
const instr_time *planduration,
const BufferUsage *bufusage,
diff --git a/src/include/executor/execdesc.h b/src/include/executor/execdesc.h
index 0a7274e26c..0e7245435d 100644
--- a/src/include/executor/execdesc.h
+++ b/src/include/executor/execdesc.h
@@ -35,6 +35,7 @@ typedef struct QueryDesc
/* These fields are provided by CreateQueryDesc */
CmdType operation; /* CMD_SELECT, CMD_UPDATE, etc. */
PlannedStmt *plannedstmt; /* planner's output (could be utility, too) */
+ CachedPlan *cplan; /* CachedPlan that supplies the plannedstmt */
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 +58,7 @@ typedef struct QueryDesc
/* in pquery.c */
extern QueryDesc *CreateQueryDesc(PlannedStmt *plannedstmt,
+ CachedPlan *cplan,
const char *sourceText,
Snapshot snapshot,
Snapshot crosscheck_snapshot,
diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h
index af7d8fd1e7..ee089505a0 100644
--- a/src/include/nodes/execnodes.h
+++ b/src/include/nodes/execnodes.h
@@ -42,6 +42,7 @@
#include "storage/condition_variable.h"
#include "utils/hsearch.h"
#include "utils/queryenvironment.h"
+#include "utils/plancache.h"
#include "utils/reltrigger.h"
#include "utils/sharedtuplestore.h"
#include "utils/snapshot.h"
@@ -633,6 +634,7 @@ typedef struct EState
* ExecRowMarks, or NULL if none */
List *es_rteperminfos; /* List of RTEPermissionInfo */
PlannedStmt *es_plannedstmt; /* link to top of plan tree */
+ CachedPlan *es_cachedplan; /* CachedPlan supplying the plannedstmt */
const char *es_sourceText; /* Source text from QueryDesc */
JunkFilter *es_junkFilter; /* top-level junk filter, if any */
diff --git a/src/include/nodes/pathnodes.h b/src/include/nodes/pathnodes.h
index 540d021592..2466157b25 100644
--- a/src/include/nodes/pathnodes.h
+++ b/src/include/nodes/pathnodes.h
@@ -116,6 +116,12 @@ typedef struct PlannerGlobal
/* "flat" rangetable for executor */
List *finalrtable;
+ /*
+ * RT indexes of relations subject to removal from the plan due to runtime
+ * pruning at plan initialization time
+ */
+ Bitmapset *prunableRelids;
+
/* "flat" list of RTEPermissionInfos */
List *finalrteperminfos;
diff --git a/src/include/nodes/plannodes.h b/src/include/nodes/plannodes.h
index 62cd6a6666..ae608812f1 100644
--- a/src/include/nodes/plannodes.h
+++ b/src/include/nodes/plannodes.h
@@ -71,6 +71,10 @@ typedef struct PlannedStmt
List *rtable; /* list of RangeTblEntry nodes */
+ Bitmapset *unprunableRelids; /* RT indexes of relations that are not
+ * subject to runtime pruning; for
+ * AcquireExecutorLocks() */
+
List *permInfos; /* list of RTEPermissionInfo nodes for rtable
* entries needing one */
@@ -1459,6 +1463,13 @@ typedef struct PartitionedRelPruneInfo
/* Indexes of all partitions which subplans or subparts are present for */
Bitmapset *present_parts;
+ /*
+ * RT indexes of all partitions which subplans or subparts are present
+ * for; only used during planning to help in the construction of
+ * PlannerGlobal.prunableRelids.
+ */
+ Bitmapset *present_part_rtis;
+
/* Length of the following arrays: */
int nparts;
diff --git a/src/include/utils/plancache.h b/src/include/utils/plancache.h
index a90dfdf906..0b5ee007ca 100644
--- a/src/include/utils/plancache.h
+++ b/src/include/utils/plancache.h
@@ -149,6 +149,7 @@ typedef struct CachedPlan
int magic; /* should equal CACHEDPLAN_MAGIC */
List *stmt_list; /* list of PlannedStmts */
bool is_oneshot; /* is it a "oneshot" plan? */
+ bool is_generic; /* is it a reusable generic plan? */
bool is_saved; /* is CachedPlan in a long-lived context? */
bool is_valid; /* is the stmt_list currently valid? */
Oid planRoleId; /* Role ID the plan was created for */
@@ -235,4 +236,13 @@ extern bool CachedPlanIsSimplyValid(CachedPlanSource *plansource,
extern CachedExpression *GetCachedExpression(Node *expr);
extern void FreeCachedExpression(CachedExpression *cexpr);
+/*
+ * CachedPlanRequiresLocking: should the executor acquire locks?
+ */
+static inline bool
+CachedPlanRequiresLocking(CachedPlan *cplan)
+{
+ return cplan->is_generic;
+}
+
#endif /* PLANCACHE_H */
--
2.43.0
[application/octet-stream] v52-0002-Assorted-tightening-in-various-ExecEnd-routines.patch (31.3K, 3-v52-0002-Assorted-tightening-in-various-ExecEnd-routines.patch)
download | inline diff:
From a4077c425a1874036d2937bb93a6116cfd3640cb Mon Sep 17 00:00:00 2001
From: Amit Langote <[email protected]>
Date: Thu, 28 Sep 2023 16:56:29 +0900
Subject: [PATCH v52 2/3] Assorted tightening in various ExecEnd()* routines
This includes adding NULLness checks on pointers before cleaning them
up. Many ExecEnd*() routines already perform this check, but a few
are missing them. These NULLness checks might seem redundant as
things stand since the ExecEnd*() routines operate under the
assumption that their matching ExecInit* routine would have fully
executed, ensuring pointers are set. However, that assumption seems a
bit shaky in the face of future changes.
This also adds a guard at the begigging of EvalPlanQualEnd() to return
early if the EPQState does not appear to have been initialized. That
case can happen if the corresponding ExecInit*() routine returned
early without calling EvalPlanQualInit().
While at it, this commit ensures that pointers are consistently set
to NULL after cleanup in all ExecEnd*() routines.
Finally, for enhanced consistency, the format of NULLness checks has
been standardized to "if (pointer != NULL)", replacing the previous
"if (pointer)" style.
Reviewed-by: Robert Haas
Discussion: https://postgr.es/m/CA+HiwqFGkMSge6TgC9KQzde0ohpAycLQuV7ooitEEpbKB0O_mg@mail.gmail.com
---
src/backend/executor/execMain.c | 4 ++
src/backend/executor/nodeAgg.c | 27 +++++++++----
src/backend/executor/nodeAppend.c | 3 ++
src/backend/executor/nodeBitmapAnd.c | 4 +-
src/backend/executor/nodeBitmapHeapscan.c | 46 ++++++++++++++--------
src/backend/executor/nodeBitmapIndexscan.c | 23 ++++++-----
src/backend/executor/nodeBitmapOr.c | 4 +-
src/backend/executor/nodeCtescan.c | 3 +-
src/backend/executor/nodeForeignscan.c | 17 ++++----
src/backend/executor/nodeGather.c | 1 +
src/backend/executor/nodeGatherMerge.c | 1 +
src/backend/executor/nodeGroup.c | 6 +--
src/backend/executor/nodeHash.c | 6 +--
src/backend/executor/nodeHashjoin.c | 4 +-
src/backend/executor/nodeIncrementalSort.c | 13 +++++-
src/backend/executor/nodeIndexonlyscan.c | 25 ++++++------
src/backend/executor/nodeIndexscan.c | 23 ++++++-----
src/backend/executor/nodeLimit.c | 1 +
src/backend/executor/nodeLockRows.c | 1 +
src/backend/executor/nodeMaterial.c | 5 ++-
src/backend/executor/nodeMemoize.c | 7 +++-
src/backend/executor/nodeMergeAppend.c | 3 ++
src/backend/executor/nodeMergejoin.c | 2 +
src/backend/executor/nodeModifyTable.c | 11 +++++-
src/backend/executor/nodeNestloop.c | 2 +
src/backend/executor/nodeProjectSet.c | 1 +
src/backend/executor/nodeRecursiveunion.c | 24 +++++++++--
src/backend/executor/nodeResult.c | 1 +
src/backend/executor/nodeSamplescan.c | 7 +++-
src/backend/executor/nodeSeqscan.c | 16 +++-----
src/backend/executor/nodeSetOp.c | 6 ++-
src/backend/executor/nodeSort.c | 5 ++-
src/backend/executor/nodeSubqueryscan.c | 1 +
src/backend/executor/nodeTableFuncscan.c | 4 +-
src/backend/executor/nodeTidrangescan.c | 12 ++++--
src/backend/executor/nodeTidscan.c | 8 +++-
src/backend/executor/nodeUnique.c | 1 +
src/backend/executor/nodeWindowAgg.c | 41 +++++++++++++------
38 files changed, 246 insertions(+), 123 deletions(-)
diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c
index 271f9d93fc..0f6dbd1e2b 100644
--- a/src/backend/executor/execMain.c
+++ b/src/backend/executor/execMain.c
@@ -2999,6 +2999,10 @@ EvalPlanQualEnd(EPQState *epqstate)
MemoryContext oldcontext;
ListCell *l;
+ /* Nothing to do if no EvalPlanQualInit() was done to begin with. */
+ if (epqstate->parentestate == NULL)
+ return;
+
rtsize = epqstate->parentestate->es_range_table_size;
/*
diff --git a/src/backend/executor/nodeAgg.c b/src/backend/executor/nodeAgg.c
index 53ead77ece..0dfba5ca16 100644
--- a/src/backend/executor/nodeAgg.c
+++ b/src/backend/executor/nodeAgg.c
@@ -4303,7 +4303,6 @@ GetAggInitVal(Datum textInitVal, Oid transtype)
void
ExecEndAgg(AggState *node)
{
- PlanState *outerPlan;
int transno;
int numGroupingSets = Max(node->maxsets, 1);
int setno;
@@ -4313,7 +4312,7 @@ ExecEndAgg(AggState *node)
* worker back into shared memory so that it can be picked up by the main
* process to report in EXPLAIN ANALYZE.
*/
- if (node->shared_info && IsParallelWorker())
+ if (node->shared_info != NULL && IsParallelWorker())
{
AggregateInstrumentation *si;
@@ -4326,10 +4325,16 @@ ExecEndAgg(AggState *node)
/* Make sure we have closed any open tuplesorts */
- if (node->sort_in)
+ if (node->sort_in != NULL)
+ {
tuplesort_end(node->sort_in);
- if (node->sort_out)
+ node->sort_in = NULL;
+ }
+ if (node->sort_out != NULL)
+ {
tuplesort_end(node->sort_out);
+ node->sort_out = NULL;
+ }
hashagg_reset_spill_state(node);
@@ -4345,19 +4350,25 @@ ExecEndAgg(AggState *node)
for (setno = 0; setno < numGroupingSets; setno++)
{
- if (pertrans->sortstates[setno])
+ if (pertrans->sortstates[setno] != NULL)
tuplesort_end(pertrans->sortstates[setno]);
}
}
/* And ensure any agg shutdown callbacks have been called */
for (setno = 0; setno < numGroupingSets; setno++)
+ {
ReScanExprContext(node->aggcontexts[setno]);
- if (node->hashcontext)
+ node->aggcontexts[setno] = NULL;
+ }
+ if (node->hashcontext != NULL)
+ {
ReScanExprContext(node->hashcontext);
+ node->hashcontext = NULL;
+ }
- outerPlan = outerPlanState(node);
- ExecEndNode(outerPlan);
+ ExecEndNode(outerPlanState(node));
+ outerPlanState(node) = NULL;
}
void
diff --git a/src/backend/executor/nodeAppend.c b/src/backend/executor/nodeAppend.c
index ca0f54d676..86d75b1a7e 100644
--- a/src/backend/executor/nodeAppend.c
+++ b/src/backend/executor/nodeAppend.c
@@ -399,7 +399,10 @@ ExecEndAppend(AppendState *node)
* shut down each of the subscans
*/
for (i = 0; i < nplans; i++)
+ {
ExecEndNode(appendplans[i]);
+ appendplans[i] = NULL;
+ }
}
void
diff --git a/src/backend/executor/nodeBitmapAnd.c b/src/backend/executor/nodeBitmapAnd.c
index 9c9c666872..ae391222bf 100644
--- a/src/backend/executor/nodeBitmapAnd.c
+++ b/src/backend/executor/nodeBitmapAnd.c
@@ -192,8 +192,8 @@ ExecEndBitmapAnd(BitmapAndState *node)
*/
for (i = 0; i < nplans; i++)
{
- if (bitmapplans[i])
- ExecEndNode(bitmapplans[i]);
+ ExecEndNode(bitmapplans[i]);
+ bitmapplans[i] = NULL;
}
}
diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c
index 3c63bdd93d..19f18ab817 100644
--- a/src/backend/executor/nodeBitmapHeapscan.c
+++ b/src/backend/executor/nodeBitmapHeapscan.c
@@ -625,8 +625,6 @@ ExecReScanBitmapHeapScan(BitmapHeapScanState *node)
void
ExecEndBitmapHeapScan(BitmapHeapScanState *node)
{
- TableScanDesc scanDesc;
-
/*
* When ending a parallel worker, copy the statistics gathered by the
* worker back into shared memory so that it can be picked up by the main
@@ -650,38 +648,54 @@ ExecEndBitmapHeapScan(BitmapHeapScanState *node)
si->lossy_pages += node->stats.lossy_pages;
}
- /*
- * extract information from the node
- */
- scanDesc = node->ss.ss_currentScanDesc;
-
/*
* close down subplans
*/
ExecEndNode(outerPlanState(node));
+ outerPlanState(node) = NULL;
/*
* release bitmaps and buffers if any
*/
- if (node->tbmiterator)
+ if (node->tbmiterator != NULL)
+ {
tbm_end_iterate(node->tbmiterator);
- if (node->prefetch_iterator)
+ node->tbmiterator = NULL;
+ }
+ if (node->prefetch_iterator != NULL)
+ {
tbm_end_iterate(node->prefetch_iterator);
- if (node->tbm)
+ node->prefetch_iterator = NULL;
+ }
+ if (node->tbm != NULL)
+ {
tbm_free(node->tbm);
- if (node->shared_tbmiterator)
+ node->tbm = NULL;
+ }
+ if (node->shared_tbmiterator != NULL)
+ {
tbm_end_shared_iterate(node->shared_tbmiterator);
- if (node->shared_prefetch_iterator)
+ node->shared_tbmiterator = NULL;
+ }
+ if (node->shared_prefetch_iterator != NULL)
+ {
tbm_end_shared_iterate(node->shared_prefetch_iterator);
+ node->shared_prefetch_iterator = NULL;
+ }
if (node->pvmbuffer != InvalidBuffer)
+ {
ReleaseBuffer(node->pvmbuffer);
+ node->pvmbuffer = InvalidBuffer;
+ }
/*
- * close heap scan
+ * close heap scan (no-op if we didn't start it)
*/
- if (scanDesc)
- table_endscan(scanDesc);
-
+ if (node->ss.ss_currentScanDesc != NULL)
+ {
+ table_endscan(node->ss.ss_currentScanDesc);
+ node->ss.ss_currentScanDesc = NULL;
+ }
}
/* ----------------------------------------------------------------
diff --git a/src/backend/executor/nodeBitmapIndexscan.c b/src/backend/executor/nodeBitmapIndexscan.c
index 6df8e17ec8..4669e8d0ce 100644
--- a/src/backend/executor/nodeBitmapIndexscan.c
+++ b/src/backend/executor/nodeBitmapIndexscan.c
@@ -174,22 +174,21 @@ ExecReScanBitmapIndexScan(BitmapIndexScanState *node)
void
ExecEndBitmapIndexScan(BitmapIndexScanState *node)
{
- Relation indexRelationDesc;
- IndexScanDesc indexScanDesc;
-
- /*
- * extract information from the node
- */
- indexRelationDesc = node->biss_RelationDesc;
- indexScanDesc = node->biss_ScanDesc;
+ /* close the scan (no-op if we didn't start it) */
+ if (node->biss_ScanDesc != NULL)
+ {
+ index_endscan(node->biss_ScanDesc);
+ node->biss_ScanDesc = NULL;
+ }
/*
* close the index relation (no-op if we didn't open it)
*/
- if (indexScanDesc)
- index_endscan(indexScanDesc);
- if (indexRelationDesc)
- index_close(indexRelationDesc, NoLock);
+ if (node->biss_RelationDesc != NULL)
+ {
+ index_close(node->biss_RelationDesc, NoLock);
+ node->biss_RelationDesc = NULL;
+ }
}
/* ----------------------------------------------------------------
diff --git a/src/backend/executor/nodeBitmapOr.c b/src/backend/executor/nodeBitmapOr.c
index 7029536c64..de439235d2 100644
--- a/src/backend/executor/nodeBitmapOr.c
+++ b/src/backend/executor/nodeBitmapOr.c
@@ -210,8 +210,8 @@ ExecEndBitmapOr(BitmapOrState *node)
*/
for (i = 0; i < nplans; i++)
{
- if (bitmapplans[i])
- ExecEndNode(bitmapplans[i]);
+ ExecEndNode(bitmapplans[i]);
+ bitmapplans[i] = NULL;
}
}
diff --git a/src/backend/executor/nodeCtescan.c b/src/backend/executor/nodeCtescan.c
index 8081eed887..7cea943988 100644
--- a/src/backend/executor/nodeCtescan.c
+++ b/src/backend/executor/nodeCtescan.c
@@ -290,10 +290,11 @@ ExecEndCteScan(CteScanState *node)
/*
* If I am the leader, free the tuplestore.
*/
- if (node->leader == node)
+ if (node->leader != NULL && node->leader == node)
{
tuplestore_end(node->cte_table);
node->cte_table = NULL;
+ node->leader = NULL;
}
}
diff --git a/src/backend/executor/nodeForeignscan.c b/src/backend/executor/nodeForeignscan.c
index fe4ae55c0f..1357ccf3c9 100644
--- a/src/backend/executor/nodeForeignscan.c
+++ b/src/backend/executor/nodeForeignscan.c
@@ -300,17 +300,20 @@ ExecEndForeignScan(ForeignScanState *node)
EState *estate = node->ss.ps.state;
/* Let the FDW shut down */
- if (plan->operation != CMD_SELECT)
+ if (node->fdwroutine != NULL)
{
- if (estate->es_epq_active == NULL)
- node->fdwroutine->EndDirectModify(node);
+ if (plan->operation != CMD_SELECT)
+ {
+ if (estate->es_epq_active == NULL)
+ node->fdwroutine->EndDirectModify(node);
+ }
+ else
+ node->fdwroutine->EndForeignScan(node);
}
- else
- node->fdwroutine->EndForeignScan(node);
/* Shut down any outer plan. */
- if (outerPlanState(node))
- ExecEndNode(outerPlanState(node));
+ ExecEndNode(outerPlanState(node));
+ outerPlanState(node) = NULL;
}
/* ----------------------------------------------------------------
diff --git a/src/backend/executor/nodeGather.c b/src/backend/executor/nodeGather.c
index 5d4ffe989c..cae5ea1f92 100644
--- a/src/backend/executor/nodeGather.c
+++ b/src/backend/executor/nodeGather.c
@@ -244,6 +244,7 @@ void
ExecEndGather(GatherState *node)
{
ExecEndNode(outerPlanState(node)); /* let children clean up first */
+ outerPlanState(node) = NULL;
ExecShutdownGather(node);
}
diff --git a/src/backend/executor/nodeGatherMerge.c b/src/backend/executor/nodeGatherMerge.c
index 45f6017c29..b36cd89e7d 100644
--- a/src/backend/executor/nodeGatherMerge.c
+++ b/src/backend/executor/nodeGatherMerge.c
@@ -284,6 +284,7 @@ void
ExecEndGatherMerge(GatherMergeState *node)
{
ExecEndNode(outerPlanState(node)); /* let children clean up first */
+ outerPlanState(node) = NULL;
ExecShutdownGatherMerge(node);
}
diff --git a/src/backend/executor/nodeGroup.c b/src/backend/executor/nodeGroup.c
index da32bec181..807429e504 100644
--- a/src/backend/executor/nodeGroup.c
+++ b/src/backend/executor/nodeGroup.c
@@ -225,10 +225,8 @@ ExecInitGroup(Group *node, EState *estate, int eflags)
void
ExecEndGroup(GroupState *node)
{
- PlanState *outerPlan;
-
- outerPlan = outerPlanState(node);
- ExecEndNode(outerPlan);
+ ExecEndNode(outerPlanState(node));
+ outerPlanState(node) = NULL;
}
void
diff --git a/src/backend/executor/nodeHash.c b/src/backend/executor/nodeHash.c
index 570a90ebe1..a913d5b50c 100644
--- a/src/backend/executor/nodeHash.c
+++ b/src/backend/executor/nodeHash.c
@@ -427,13 +427,11 @@ ExecInitHash(Hash *node, EState *estate, int eflags)
void
ExecEndHash(HashState *node)
{
- PlanState *outerPlan;
-
/*
* shut down the subplan
*/
- outerPlan = outerPlanState(node);
- ExecEndNode(outerPlan);
+ ExecEndNode(outerPlanState(node));
+ outerPlanState(node) = NULL;
}
diff --git a/src/backend/executor/nodeHashjoin.c b/src/backend/executor/nodeHashjoin.c
index 2f7170604d..901c9e9be7 100644
--- a/src/backend/executor/nodeHashjoin.c
+++ b/src/backend/executor/nodeHashjoin.c
@@ -950,7 +950,7 @@ ExecEndHashJoin(HashJoinState *node)
/*
* Free hash table
*/
- if (node->hj_HashTable)
+ if (node->hj_HashTable != NULL)
{
ExecHashTableDestroy(node->hj_HashTable);
node->hj_HashTable = NULL;
@@ -960,7 +960,9 @@ ExecEndHashJoin(HashJoinState *node)
* clean up subtrees
*/
ExecEndNode(outerPlanState(node));
+ outerPlanState(node) = NULL;
ExecEndNode(innerPlanState(node));
+ innerPlanState(node) = NULL;
}
/*
diff --git a/src/backend/executor/nodeIncrementalSort.c b/src/backend/executor/nodeIncrementalSort.c
index 2ce5ed5ec8..010bcfafa8 100644
--- a/src/backend/executor/nodeIncrementalSort.c
+++ b/src/backend/executor/nodeIncrementalSort.c
@@ -1078,8 +1078,16 @@ ExecEndIncrementalSort(IncrementalSortState *node)
{
SO_printf("ExecEndIncrementalSort: shutting down sort node\n");
- ExecDropSingleTupleTableSlot(node->group_pivot);
- ExecDropSingleTupleTableSlot(node->transfer_tuple);
+ if (node->group_pivot != NULL)
+ {
+ ExecDropSingleTupleTableSlot(node->group_pivot);
+ node->group_pivot = NULL;
+ }
+ if (node->transfer_tuple != NULL)
+ {
+ ExecDropSingleTupleTableSlot(node->transfer_tuple);
+ node->transfer_tuple = NULL;
+ }
/*
* Release tuplesort resources.
@@ -1099,6 +1107,7 @@ ExecEndIncrementalSort(IncrementalSortState *node)
* Shut down the subplan.
*/
ExecEndNode(outerPlanState(node));
+ outerPlanState(node) = NULL;
SO_printf("ExecEndIncrementalSort: sort node shutdown\n");
}
diff --git a/src/backend/executor/nodeIndexonlyscan.c b/src/backend/executor/nodeIndexonlyscan.c
index 612c673895..481d479760 100644
--- a/src/backend/executor/nodeIndexonlyscan.c
+++ b/src/backend/executor/nodeIndexonlyscan.c
@@ -397,15 +397,6 @@ ExecReScanIndexOnlyScan(IndexOnlyScanState *node)
void
ExecEndIndexOnlyScan(IndexOnlyScanState *node)
{
- Relation indexRelationDesc;
- IndexScanDesc indexScanDesc;
-
- /*
- * extract information from the node
- */
- indexRelationDesc = node->ioss_RelationDesc;
- indexScanDesc = node->ioss_ScanDesc;
-
/* Release VM buffer pin, if any. */
if (node->ioss_VMBuffer != InvalidBuffer)
{
@@ -413,13 +404,21 @@ ExecEndIndexOnlyScan(IndexOnlyScanState *node)
node->ioss_VMBuffer = InvalidBuffer;
}
+ /* close the scan (no-op if we didn't start it) */
+ if (node->ioss_ScanDesc != NULL)
+ {
+ index_endscan(node->ioss_ScanDesc);
+ node->ioss_ScanDesc = NULL;
+ }
+
/*
* close the index relation (no-op if we didn't open it)
*/
- if (indexScanDesc)
- index_endscan(indexScanDesc);
- if (indexRelationDesc)
- index_close(indexRelationDesc, NoLock);
+ if (node->ioss_RelationDesc != NULL)
+ {
+ index_close(node->ioss_RelationDesc, NoLock);
+ node->ioss_RelationDesc = NULL;
+ }
}
/* ----------------------------------------------------------------
diff --git a/src/backend/executor/nodeIndexscan.c b/src/backend/executor/nodeIndexscan.c
index 8000feff4c..a8172d8b82 100644
--- a/src/backend/executor/nodeIndexscan.c
+++ b/src/backend/executor/nodeIndexscan.c
@@ -784,22 +784,21 @@ ExecIndexAdvanceArrayKeys(IndexArrayKeyInfo *arrayKeys, int numArrayKeys)
void
ExecEndIndexScan(IndexScanState *node)
{
- Relation indexRelationDesc;
- IndexScanDesc indexScanDesc;
-
- /*
- * extract information from the node
- */
- indexRelationDesc = node->iss_RelationDesc;
- indexScanDesc = node->iss_ScanDesc;
+ /* close the scan (no-op if we didn't start it) */
+ if (node->iss_ScanDesc != NULL)
+ {
+ index_endscan(node->iss_ScanDesc);
+ node->iss_ScanDesc = NULL;
+ }
/*
* close the index relation (no-op if we didn't open it)
*/
- if (indexScanDesc)
- index_endscan(indexScanDesc);
- if (indexRelationDesc)
- index_close(indexRelationDesc, NoLock);
+ if (node->iss_RelationDesc != NULL)
+ {
+ index_close(node->iss_RelationDesc, NoLock);
+ node->iss_RelationDesc = NULL;
+ }
}
/* ----------------------------------------------------------------
diff --git a/src/backend/executor/nodeLimit.c b/src/backend/executor/nodeLimit.c
index e6f1fb1562..eb7b6e52be 100644
--- a/src/backend/executor/nodeLimit.c
+++ b/src/backend/executor/nodeLimit.c
@@ -534,6 +534,7 @@ void
ExecEndLimit(LimitState *node)
{
ExecEndNode(outerPlanState(node));
+ outerPlanState(node) = NULL;
}
diff --git a/src/backend/executor/nodeLockRows.c b/src/backend/executor/nodeLockRows.c
index 41754ddfea..0d3489195b 100644
--- a/src/backend/executor/nodeLockRows.c
+++ b/src/backend/executor/nodeLockRows.c
@@ -387,6 +387,7 @@ ExecEndLockRows(LockRowsState *node)
/* We may have shut down EPQ already, but no harm in another call */
EvalPlanQualEnd(&node->lr_epqstate);
ExecEndNode(outerPlanState(node));
+ outerPlanState(node) = NULL;
}
diff --git a/src/backend/executor/nodeMaterial.c b/src/backend/executor/nodeMaterial.c
index 22e1787fbd..883e3f3933 100644
--- a/src/backend/executor/nodeMaterial.c
+++ b/src/backend/executor/nodeMaterial.c
@@ -243,13 +243,16 @@ ExecEndMaterial(MaterialState *node)
* Release tuplestore resources
*/
if (node->tuplestorestate != NULL)
+ {
tuplestore_end(node->tuplestorestate);
- node->tuplestorestate = NULL;
+ node->tuplestorestate = NULL;
+ }
/*
* shut down the subplan
*/
ExecEndNode(outerPlanState(node));
+ outerPlanState(node) = NULL;
}
/* ----------------------------------------------------------------
diff --git a/src/backend/executor/nodeMemoize.c b/src/backend/executor/nodeMemoize.c
index df8e3fff08..690dee1daa 100644
--- a/src/backend/executor/nodeMemoize.c
+++ b/src/backend/executor/nodeMemoize.c
@@ -1128,12 +1128,17 @@ ExecEndMemoize(MemoizeState *node)
}
/* Remove the cache context */
- MemoryContextDelete(node->tableContext);
+ if (node->tableContext != NULL)
+ {
+ MemoryContextDelete(node->tableContext);
+ node->tableContext = NULL;
+ }
/*
* shut down the subplan
*/
ExecEndNode(outerPlanState(node));
+ outerPlanState(node) = NULL;
}
void
diff --git a/src/backend/executor/nodeMergeAppend.c b/src/backend/executor/nodeMergeAppend.c
index e1b9b984a7..3236444cf1 100644
--- a/src/backend/executor/nodeMergeAppend.c
+++ b/src/backend/executor/nodeMergeAppend.c
@@ -333,7 +333,10 @@ ExecEndMergeAppend(MergeAppendState *node)
* shut down each of the subscans
*/
for (i = 0; i < nplans; i++)
+ {
ExecEndNode(mergeplans[i]);
+ mergeplans[i] = NULL;
+ }
}
void
diff --git a/src/backend/executor/nodeMergejoin.c b/src/backend/executor/nodeMergejoin.c
index 29c54fcd75..926e631d88 100644
--- a/src/backend/executor/nodeMergejoin.c
+++ b/src/backend/executor/nodeMergejoin.c
@@ -1647,7 +1647,9 @@ ExecEndMergeJoin(MergeJoinState *node)
* shut down the subplans
*/
ExecEndNode(innerPlanState(node));
+ innerPlanState(node) = NULL;
ExecEndNode(outerPlanState(node));
+ outerPlanState(node) = NULL;
MJ1_printf("ExecEndMergeJoin: %s\n",
"node processing ended");
diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c
index 8bf4c80d4a..9e56f9c36c 100644
--- a/src/backend/executor/nodeModifyTable.c
+++ b/src/backend/executor/nodeModifyTable.c
@@ -4724,7 +4724,9 @@ ExecEndModifyTable(ModifyTableState *node)
for (j = 0; j < resultRelInfo->ri_NumSlotsInitialized; j++)
{
ExecDropSingleTupleTableSlot(resultRelInfo->ri_Slots[j]);
+ resultRelInfo->ri_Slots[j] = NULL;
ExecDropSingleTupleTableSlot(resultRelInfo->ri_PlanSlots[j]);
+ resultRelInfo->ri_PlanSlots[j] = NULL;
}
}
@@ -4732,12 +4734,16 @@ ExecEndModifyTable(ModifyTableState *node)
* Close all the partitioned tables, leaf partitions, and their indices
* and release the slot used for tuple routing, if set.
*/
- if (node->mt_partition_tuple_routing)
+ if (node->mt_partition_tuple_routing != NULL)
{
ExecCleanupTupleRouting(node, node->mt_partition_tuple_routing);
+ node->mt_partition_tuple_routing = NULL;
- if (node->mt_root_tuple_slot)
+ if (node->mt_root_tuple_slot != NULL)
+ {
ExecDropSingleTupleTableSlot(node->mt_root_tuple_slot);
+ node->mt_root_tuple_slot = NULL;
+ }
}
/*
@@ -4749,6 +4755,7 @@ ExecEndModifyTable(ModifyTableState *node)
* shut down subplan
*/
ExecEndNode(outerPlanState(node));
+ outerPlanState(node) = NULL;
}
void
diff --git a/src/backend/executor/nodeNestloop.c b/src/backend/executor/nodeNestloop.c
index 7f4bf6c4db..01f3d56a3b 100644
--- a/src/backend/executor/nodeNestloop.c
+++ b/src/backend/executor/nodeNestloop.c
@@ -367,7 +367,9 @@ ExecEndNestLoop(NestLoopState *node)
* close down subplans
*/
ExecEndNode(outerPlanState(node));
+ outerPlanState(node) = NULL;
ExecEndNode(innerPlanState(node));
+ innerPlanState(node) = NULL;
NL1_printf("ExecEndNestLoop: %s\n",
"node processing ended");
diff --git a/src/backend/executor/nodeProjectSet.c b/src/backend/executor/nodeProjectSet.c
index e483730015..ca9a5e2ed2 100644
--- a/src/backend/executor/nodeProjectSet.c
+++ b/src/backend/executor/nodeProjectSet.c
@@ -331,6 +331,7 @@ ExecEndProjectSet(ProjectSetState *node)
* shut down subplans
*/
ExecEndNode(outerPlanState(node));
+ outerPlanState(node) = NULL;
}
void
diff --git a/src/backend/executor/nodeRecursiveunion.c b/src/backend/executor/nodeRecursiveunion.c
index c7f8a19fa4..7680142c7b 100644
--- a/src/backend/executor/nodeRecursiveunion.c
+++ b/src/backend/executor/nodeRecursiveunion.c
@@ -272,20 +272,36 @@ void
ExecEndRecursiveUnion(RecursiveUnionState *node)
{
/* Release tuplestores */
- tuplestore_end(node->working_table);
- tuplestore_end(node->intermediate_table);
+ if (node->working_table != NULL)
+ {
+ tuplestore_end(node->working_table);
+ node->working_table = NULL;
+ }
+ if (node->intermediate_table != NULL)
+ {
+ tuplestore_end(node->intermediate_table);
+ node->intermediate_table = NULL;
+ }
/* free subsidiary stuff including hashtable */
- if (node->tempContext)
+ if (node->tempContext != NULL)
+ {
MemoryContextDelete(node->tempContext);
- if (node->tableContext)
+ node->tempContext = NULL;
+ }
+ if (node->tableContext != NULL)
+ {
MemoryContextDelete(node->tableContext);
+ node->tableContext = NULL;
+ }
/*
* close down subplans
*/
ExecEndNode(outerPlanState(node));
+ outerPlanState(node) = NULL;
ExecEndNode(innerPlanState(node));
+ innerPlanState(node) = NULL;
}
/* ----------------------------------------------------------------
diff --git a/src/backend/executor/nodeResult.c b/src/backend/executor/nodeResult.c
index 348361e7f4..e3cfc9b772 100644
--- a/src/backend/executor/nodeResult.c
+++ b/src/backend/executor/nodeResult.c
@@ -243,6 +243,7 @@ ExecEndResult(ResultState *node)
* shut down subplans
*/
ExecEndNode(outerPlanState(node));
+ outerPlanState(node) = NULL;
}
void
diff --git a/src/backend/executor/nodeSamplescan.c b/src/backend/executor/nodeSamplescan.c
index 714b076e64..6ab91001bc 100644
--- a/src/backend/executor/nodeSamplescan.c
+++ b/src/backend/executor/nodeSamplescan.c
@@ -181,14 +181,17 @@ ExecEndSampleScan(SampleScanState *node)
/*
* Tell sampling function that we finished the scan.
*/
- if (node->tsmroutine->EndSampleScan)
+ if (node->tsmroutine != NULL && node->tsmroutine->EndSampleScan)
node->tsmroutine->EndSampleScan(node);
/*
- * close heap scan
+ * close heap scan (no-op if we didn't start it)
*/
if (node->ss.ss_currentScanDesc)
+ {
table_endscan(node->ss.ss_currentScanDesc);
+ node->ss.ss_currentScanDesc = NULL;
+ }
}
/* ----------------------------------------------------------------
diff --git a/src/backend/executor/nodeSeqscan.c b/src/backend/executor/nodeSeqscan.c
index 7cb12a11c2..b052775e5b 100644
--- a/src/backend/executor/nodeSeqscan.c
+++ b/src/backend/executor/nodeSeqscan.c
@@ -183,18 +183,14 @@ ExecInitSeqScan(SeqScan *node, EState *estate, int eflags)
void
ExecEndSeqScan(SeqScanState *node)
{
- TableScanDesc scanDesc;
-
- /*
- * get information from node
- */
- scanDesc = node->ss.ss_currentScanDesc;
-
/*
- * close heap scan
+ * close heap scan (no-op if we didn't start it)
*/
- if (scanDesc != NULL)
- table_endscan(scanDesc);
+ if (node->ss.ss_currentScanDesc != NULL)
+ {
+ table_endscan(node->ss.ss_currentScanDesc);
+ node->ss.ss_currentScanDesc = NULL;
+ }
}
/* ----------------------------------------------------------------
diff --git a/src/backend/executor/nodeSetOp.c b/src/backend/executor/nodeSetOp.c
index a8ac68b482..fe34b2134f 100644
--- a/src/backend/executor/nodeSetOp.c
+++ b/src/backend/executor/nodeSetOp.c
@@ -583,10 +583,14 @@ void
ExecEndSetOp(SetOpState *node)
{
/* free subsidiary stuff including hashtable */
- if (node->tableContext)
+ if (node->tableContext != NULL)
+ {
MemoryContextDelete(node->tableContext);
+ node->tableContext = NULL;
+ }
ExecEndNode(outerPlanState(node));
+ outerPlanState(node) = NULL;
}
diff --git a/src/backend/executor/nodeSort.c b/src/backend/executor/nodeSort.c
index 3fc925d7b4..af852464d0 100644
--- a/src/backend/executor/nodeSort.c
+++ b/src/backend/executor/nodeSort.c
@@ -307,13 +307,16 @@ ExecEndSort(SortState *node)
* Release tuplesort resources
*/
if (node->tuplesortstate != NULL)
+ {
tuplesort_end((Tuplesortstate *) node->tuplesortstate);
- node->tuplesortstate = NULL;
+ node->tuplesortstate = NULL;
+ }
/*
* shut down the subplan
*/
ExecEndNode(outerPlanState(node));
+ outerPlanState(node) = NULL;
SO1_printf("ExecEndSort: %s\n",
"sort node shutdown");
diff --git a/src/backend/executor/nodeSubqueryscan.c b/src/backend/executor/nodeSubqueryscan.c
index 782097eaf2..0b2612183a 100644
--- a/src/backend/executor/nodeSubqueryscan.c
+++ b/src/backend/executor/nodeSubqueryscan.c
@@ -171,6 +171,7 @@ ExecEndSubqueryScan(SubqueryScanState *node)
* close down subquery
*/
ExecEndNode(node->subplan);
+ node->subplan = NULL;
}
/* ----------------------------------------------------------------
diff --git a/src/backend/executor/nodeTableFuncscan.c b/src/backend/executor/nodeTableFuncscan.c
index f483221bb8..778d25d511 100644
--- a/src/backend/executor/nodeTableFuncscan.c
+++ b/src/backend/executor/nodeTableFuncscan.c
@@ -223,8 +223,10 @@ ExecEndTableFuncScan(TableFuncScanState *node)
* Release tuplestore resources
*/
if (node->tupstore != NULL)
+ {
tuplestore_end(node->tupstore);
- node->tupstore = NULL;
+ node->tupstore = NULL;
+ }
}
/* ----------------------------------------------------------------
diff --git a/src/backend/executor/nodeTidrangescan.c b/src/backend/executor/nodeTidrangescan.c
index 9aa7683d7e..702ee884d2 100644
--- a/src/backend/executor/nodeTidrangescan.c
+++ b/src/backend/executor/nodeTidrangescan.c
@@ -326,10 +326,14 @@ ExecReScanTidRangeScan(TidRangeScanState *node)
void
ExecEndTidRangeScan(TidRangeScanState *node)
{
- TableScanDesc scan = node->ss.ss_currentScanDesc;
-
- if (scan != NULL)
- table_endscan(scan);
+ /*
+ * close heap scan (no-op if we didn't start it)
+ */
+ if (node->ss.ss_currentScanDesc != NULL)
+ {
+ table_endscan(node->ss.ss_currentScanDesc);
+ node->ss.ss_currentScanDesc = NULL;
+ }
}
/* ----------------------------------------------------------------
diff --git a/src/backend/executor/nodeTidscan.c b/src/backend/executor/nodeTidscan.c
index 864a9013b6..f375951699 100644
--- a/src/backend/executor/nodeTidscan.c
+++ b/src/backend/executor/nodeTidscan.c
@@ -469,8 +469,14 @@ ExecReScanTidScan(TidScanState *node)
void
ExecEndTidScan(TidScanState *node)
{
- if (node->ss.ss_currentScanDesc)
+ /*
+ * close heap scan (no-op if we didn't start it)
+ */
+ if (node->ss.ss_currentScanDesc != NULL)
+ {
table_endscan(node->ss.ss_currentScanDesc);
+ node->ss.ss_currentScanDesc = NULL;
+ }
}
/* ----------------------------------------------------------------
diff --git a/src/backend/executor/nodeUnique.c b/src/backend/executor/nodeUnique.c
index a125923e93..b82d0e9ad5 100644
--- a/src/backend/executor/nodeUnique.c
+++ b/src/backend/executor/nodeUnique.c
@@ -168,6 +168,7 @@ void
ExecEndUnique(UniqueState *node)
{
ExecEndNode(outerPlanState(node));
+ outerPlanState(node) = NULL;
}
diff --git a/src/backend/executor/nodeWindowAgg.c b/src/backend/executor/nodeWindowAgg.c
index 3221fa1522..561d7e731d 100644
--- a/src/backend/executor/nodeWindowAgg.c
+++ b/src/backend/executor/nodeWindowAgg.c
@@ -1351,11 +1351,14 @@ release_partition(WindowAggState *winstate)
* any aggregate temp data). We don't rely on retail pfree because some
* aggregates might have allocated data we don't have direct pointers to.
*/
- MemoryContextReset(winstate->partcontext);
- MemoryContextReset(winstate->aggcontext);
+ if (winstate->partcontext != NULL)
+ MemoryContextReset(winstate->partcontext);
+ if (winstate->aggcontext != NULL)
+ MemoryContextReset(winstate->aggcontext);
for (i = 0; i < winstate->numaggs; i++)
{
- if (winstate->peragg[i].aggcontext != winstate->aggcontext)
+ if (winstate->peragg[i].aggcontext != NULL &&
+ winstate->peragg[i].aggcontext != winstate->aggcontext)
MemoryContextReset(winstate->peragg[i].aggcontext);
}
@@ -2681,24 +2684,40 @@ ExecInitWindowAgg(WindowAgg *node, EState *estate, int eflags)
void
ExecEndWindowAgg(WindowAggState *node)
{
- PlanState *outerPlan;
int i;
release_partition(node);
for (i = 0; i < node->numaggs; i++)
{
- if (node->peragg[i].aggcontext != node->aggcontext)
+ if (node->peragg[i].aggcontext != NULL &&
+ node->peragg[i].aggcontext != node->aggcontext)
MemoryContextDelete(node->peragg[i].aggcontext);
}
- MemoryContextDelete(node->partcontext);
- MemoryContextDelete(node->aggcontext);
+ if (node->partcontext != NULL)
+ {
+ MemoryContextDelete(node->partcontext);
+ node->partcontext = NULL;
+ }
+ if (node->aggcontext != NULL)
+ {
+ MemoryContextDelete(node->aggcontext);
+ node->aggcontext = NULL;
+ }
- pfree(node->perfunc);
- pfree(node->peragg);
+ if (node->perfunc != NULL)
+ {
+ pfree(node->perfunc);
+ node->perfunc = NULL;
+ }
+ if (node->peragg != NULL)
+ {
+ pfree(node->peragg);
+ node->peragg = NULL;
+ }
- outerPlan = outerPlanState(node);
- ExecEndNode(outerPlan);
+ ExecEndNode(outerPlanState(node));
+ outerPlanState(node) = NULL;
}
/* -----------------
--
2.43.0
[application/octet-stream] v52-0003-Handle-CachedPlan-invalidation-in-the-executor.patch (92.1K, 4-v52-0003-Handle-CachedPlan-invalidation-in-the-executor.patch)
download | inline diff:
From 533abeac5857c4ac2950c8eb1699485b46cce9c7 Mon Sep 17 00:00:00 2001
From: Amit Langote <[email protected]>
Date: Thu, 22 Aug 2024 19:38:13 +0900
Subject: [PATCH v52 3/3] Handle CachedPlan invalidation in the executor
This commit makes changes to handle cases where a cached plan
becomes invalid before deferred locks on prunable relations are taken.
* Add checks at various points in ExecutorStart() and its called
functions to determine if the plan becomes invalid. If detected,
the function and its callers return immediately. A previous commit
ensures any partially initialized PlanState tree objects are cleaned
up appropriately.
* Introduce ExecutorStartExt(), a wrapper over ExecutorStart(), to
handle cases where plan initialization is aborted due to invalidation.
ExecutorStartExt() creates a new transient CachedPlan if needed and
retries execution. This new entry point is only required for sites
using plancache.c. It requires passing the QueryDesc, eflags,
CachedPlanSource, and query_index (index in CachedPlanSource.query_list).
* Add GetSingleCachedPlan() in plancache.c to create a transient
CachedPlan for a specified query in the given CachedPlanSource.
Such CachedPlans are tracked in a separate global list for the
plancache invalidation callbacks to check.
This also adds isolation tests using the delay_execution test module
to verify scenarios where a CachedPlan becomes invalid before the
deferred locks are taken.
All ExecutorStart_hook implementations now must add the following
block after the ExecutorStart() call to ensure it doesn't work with an
invalid plan:
/* The plan may have become invalid during ExecutorStart() */
if (!ExecPlanStillValid(queryDesc->estate))
return;
Reviewed-by: Robert Haas
Discussion: https://postgr.es/m/CA+HiwqFGkMSge6TgC9KQzde0ohpAycLQuV7ooitEEpbKB0O_mg@mail.gmail.comk
---
contrib/auto_explain/auto_explain.c | 4 +
.../pg_stat_statements/pg_stat_statements.c | 4 +
contrib/postgres_fdw/postgres_fdw.c | 36 ++-
src/backend/commands/explain.c | 8 +-
src/backend/commands/portalcmds.c | 1 +
src/backend/commands/prepare.c | 10 +-
src/backend/commands/trigger.c | 14 ++
src/backend/executor/README | 32 ++-
src/backend/executor/execMain.c | 99 ++++++++-
src/backend/executor/execParallel.c | 4 +-
src/backend/executor/execPartition.c | 10 +
src/backend/executor/execProcnode.c | 7 +
src/backend/executor/execUtils.c | 42 +++-
src/backend/executor/nodeAgg.c | 2 +
src/backend/executor/nodeAppend.c | 12 +-
src/backend/executor/nodeBitmapAnd.c | 2 +
src/backend/executor/nodeBitmapHeapscan.c | 4 +
src/backend/executor/nodeBitmapIndexscan.c | 6 +-
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 | 7 +-
src/backend/executor/nodeIndexscan.c | 8 +-
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 | 6 +-
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 | 3 +
src/backend/executor/nodeSeqscan.c | 3 +
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 | 19 +-
src/backend/tcop/postgres.c | 4 +-
src/backend/tcop/pquery.c | 31 ++-
src/backend/utils/cache/plancache.c | 206 ++++++++++++++++++
src/backend/utils/mmgr/portalmem.c | 4 +-
src/include/commands/explain.h | 1 +
src/include/commands/trigger.h | 1 +
src/include/executor/execdesc.h | 1 +
src/include/executor/executor.h | 18 ++
src/include/nodes/execnodes.h | 1 +
src/include/utils/plancache.h | 26 +++
src/include/utils/portal.h | 4 +-
src/test/modules/delay_execution/Makefile | 3 +-
.../modules/delay_execution/delay_execution.c | 63 +++++-
.../expected/cached-plan-inval.out | 175 +++++++++++++++
src/test/modules/delay_execution/meson.build | 1 +
.../specs/cached-plan-inval.spec | 65 ++++++
66 files changed, 962 insertions(+), 58 deletions(-)
create mode 100644 src/test/modules/delay_execution/expected/cached-plan-inval.out
create mode 100644 src/test/modules/delay_execution/specs/cached-plan-inval.spec
diff --git a/contrib/auto_explain/auto_explain.c b/contrib/auto_explain/auto_explain.c
index 677c135f59..3675ce9a88 100644
--- a/contrib/auto_explain/auto_explain.c
+++ b/contrib/auto_explain/auto_explain.c
@@ -300,6 +300,10 @@ explain_ExecutorStart(QueryDesc *queryDesc, int eflags)
else
standard_ExecutorStart(queryDesc, eflags);
+ /* The plan may have become invalid during ExecutorStart() */
+ if (!ExecPlanStillValid(queryDesc->estate))
+ return;
+
if (auto_explain_enabled())
{
/*
diff --git a/contrib/pg_stat_statements/pg_stat_statements.c b/contrib/pg_stat_statements/pg_stat_statements.c
index 362d222f63..98a328b79f 100644
--- a/contrib/pg_stat_statements/pg_stat_statements.c
+++ b/contrib/pg_stat_statements/pg_stat_statements.c
@@ -992,6 +992,10 @@ pgss_ExecutorStart(QueryDesc *queryDesc, int eflags)
else
standard_ExecutorStart(queryDesc, eflags);
+ /* The plan may have become invalid during ExecutorStart() */
+ if (!ExecPlanStillValid(queryDesc->estate))
+ return;
+
/*
* If query has queryId zero, don't track it. This prevents double
* counting of optimizable statements that are directly contained in
diff --git a/contrib/postgres_fdw/postgres_fdw.c b/contrib/postgres_fdw/postgres_fdw.c
index adc62576d1..65f4ffe5ee 100644
--- a/contrib/postgres_fdw/postgres_fdw.c
+++ b/contrib/postgres_fdw/postgres_fdw.c
@@ -2144,7 +2144,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;
@@ -2650,8 +2654,9 @@ postgresBeginDirectModify(ForeignScanState *node, int eflags)
{
ForeignScan *fsplan = (ForeignScan *) node->ss.ps.plan;
EState *estate = node->ss.ps.state;
+ Relation rel = node->ss.ss_currentRelation;
PgFdwDirectModifyState *dmstate;
- Index rtindex;
+ Index rtindex = node->resultRelInfo->ri_RangeTableIndex;
Oid userid;
ForeignTable *table;
UserMapping *user;
@@ -2663,24 +2668,32 @@ postgresBeginDirectModify(ForeignScanState *node, int eflags)
if (eflags & EXEC_FLAG_EXPLAIN_ONLY)
return;
+ /*
+ * Open the foreign table using the RT index given in the ResultRelInfo if
+ * the ScanState doesn't provide it. If the plan becomes invalid as a
+ * result of taking a lock in ExecOpenScanRelation(), do nothing, in which
+ * case node->fdw_state remains NULL.
+ */
+ if (rel == NULL)
+ {
+ Assert(fsplan->scan.scanrelid == 0);
+ rel = ExecOpenScanRelation(estate, rtindex, eflags);
+ if (unlikely(rel == NULL || !ExecPlanStillValid(estate)))
+ return;
+ }
+
/*
* We'll save private state in node->fdw_state.
*/
dmstate = (PgFdwDirectModifyState *) palloc0(sizeof(PgFdwDirectModifyState));
node->fdw_state = (void *) dmstate;
+ dmstate->rel = rel;
/*
* Identify which user to do the remote access as. This should match what
* ExecCheckPermissions() does.
*/
userid = OidIsValid(fsplan->checkAsUser) ? fsplan->checkAsUser : GetUserId();
-
- /* Get info about foreign table. */
- rtindex = node->resultRelInfo->ri_RangeTableIndex;
- if (fsplan->scan.scanrelid == 0)
- dmstate->rel = ExecOpenScanRelation(estate, rtindex, eflags);
- else
- dmstate->rel = node->ss.ss_currentRelation;
table = GetForeignTable(RelationGetRelid(dmstate->rel));
user = GetUserMapping(userid, table->serverid);
@@ -2811,7 +2824,10 @@ postgresEndDirectModify(ForeignScanState *node)
{
PgFdwDirectModifyState *dmstate = (PgFdwDirectModifyState *) node->fdw_state;
- /* if dmstate is NULL, we are in EXPLAIN; nothing to do */
+ /*
+ * Nothing to do if dmstate is NULL, either because we are in EXPLAIN or
+ * dmstate wasn't initialized due to aborted plan initialization.
+ */
if (dmstate == NULL)
return;
diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.c
index a83ea07db1..a7643360a7 100644
--- a/src/backend/commands/explain.c
+++ b/src/backend/commands/explain.c
@@ -507,7 +507,8 @@ standard_ExplainOneQuery(Query *query, int cursorOptions,
}
/* run it (if needed) and produce output */
- ExplainOnePlan(plan, NULL, into, es, queryString, params, queryEnv,
+ ExplainOnePlan(plan, NULL, NULL, -1, into, es, queryString, params,
+ queryEnv,
&planduration, (es->buffers ? &bufusage : NULL),
es->memory ? &mem_counters : NULL);
}
@@ -616,6 +617,7 @@ ExplainOneUtility(Node *utilityStmt, IntoClause *into, ExplainState *es,
*/
void
ExplainOnePlan(PlannedStmt *plannedstmt, CachedPlan *cplan,
+ CachedPlanSource *plansource, int query_index,
IntoClause *into, ExplainState *es,
const char *queryString, ParamListInfo params,
QueryEnvironment *queryEnv, const instr_time *planduration,
@@ -686,8 +688,8 @@ ExplainOnePlan(PlannedStmt *plannedstmt, CachedPlan *cplan,
if (into)
eflags |= GetIntoRelEFlags(into);
- /* call ExecutorStart to prepare the plan for execution */
- ExecutorStart(queryDesc, eflags);
+ /* Call ExecutorStartExt to prepare the plan for execution. */
+ ExecutorStartExt(queryDesc, eflags, plansource, query_index);
/* Execute the plan for statistics if asked for */
if (es->analyze)
diff --git a/src/backend/commands/portalcmds.c b/src/backend/commands/portalcmds.c
index 4f6acf6719..4b1503c05e 100644
--- a/src/backend/commands/portalcmds.c
+++ b/src/backend/commands/portalcmds.c
@@ -107,6 +107,7 @@ PerformCursorOpen(ParseState *pstate, DeclareCursorStmt *cstmt, ParamListInfo pa
queryString,
CMDTAG_SELECT, /* cursor's query is always a SELECT */
list_make1(plan),
+ NULL,
NULL);
/*----------
diff --git a/src/backend/commands/prepare.c b/src/backend/commands/prepare.c
index 311b9ebd5b..4cd79a6e3a 100644
--- a/src/backend/commands/prepare.c
+++ b/src/backend/commands/prepare.c
@@ -202,7 +202,8 @@ ExecuteQuery(ParseState *pstate,
query_string,
entry->plansource->commandTag,
plan_list,
- cplan);
+ cplan,
+ entry->plansource);
/*
* For CREATE TABLE ... AS EXECUTE, we must verify that the prepared
@@ -583,6 +584,7 @@ ExplainExecuteQuery(ExecuteStmt *execstmt, IntoClause *into, ExplainState *es,
MemoryContextCounters mem_counters;
MemoryContext planner_ctx = NULL;
MemoryContext saved_ctx = NULL;
+ int i = 0;
if (es->memory)
{
@@ -655,8 +657,8 @@ ExplainExecuteQuery(ExecuteStmt *execstmt, IntoClause *into, ExplainState *es,
PlannedStmt *pstmt = lfirst_node(PlannedStmt, p);
if (pstmt->commandType != CMD_UTILITY)
- ExplainOnePlan(pstmt, cplan, into, es, query_string, paramLI,
- queryEnv,
+ ExplainOnePlan(pstmt, cplan, entry->plansource, i,
+ into, es, query_string, paramLI, queryEnv,
&planduration, (es->buffers ? &bufusage : NULL),
es->memory ? &mem_counters : NULL);
else
@@ -668,6 +670,8 @@ ExplainExecuteQuery(ExecuteStmt *execstmt, IntoClause *into, ExplainState *es,
/* Separate plans with an appropriate separator */
if (lnext(plan_list, p) != NULL)
ExplainSeparatePlans(es);
+
+ i++;
}
if (estate)
diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c
index 170360edda..91e4b821a0 100644
--- a/src/backend/commands/trigger.c
+++ b/src/backend/commands/trigger.c
@@ -5119,6 +5119,20 @@ AfterTriggerEndQuery(EState *estate)
afterTriggers.query_depth--;
}
+/* ----------
+ * AfterTriggerAbortQuery()
+ *
+ * Called by ExecutorEnd() if the query execution was aborted due to the
+ * plan becoming invalid during initialization.
+ * ----------
+ */
+void
+AfterTriggerAbortQuery(void)
+{
+ /* Revert the actions of AfterTriggerBeginQuery(). */
+ afterTriggers.query_depth--;
+}
+
/*
* AfterTriggerFreeQuery
diff --git a/src/backend/executor/README b/src/backend/executor/README
index 642d63be61..e583df5be0 100644
--- a/src/backend/executor/README
+++ b/src/backend/executor/README
@@ -280,6 +280,28 @@ are typically reset to empty once per tuple. Per-tuple contexts are usually
associated with ExprContexts, and commonly each PlanState node has its own
ExprContext to evaluate its qual and targetlist expressions in.
+Relation Locking
+----------------
+
+Typically, when the executor initializes a plan tree for execution, it doesn't
+lock non-index relations if the plan tree is freshly generated and not derived
+from a CachedPlan. This is because such locks have already been established
+during the query's parsing, rewriting, and planning phases. However, with a
+cached plan tree, some relations may remain unlocked. The function
+AcquireExecutorLocks() only locks unprunable relations in the plan, deferring
+the locking of prunable ones to executor initialization. This avoids
+unnecessary locking of relations that will be pruned during "initial" runtime
+pruning in the ExecInitNode() routine of nodes containing the pruning info.
+
+This approach creates a window where a cached plan tree with child tables
+could become outdated if another backend modifies these tables before
+ExecInitNode() locks them. As a result, the executor has the added duty to
+verify the plan tree's validity whenever it locks a child table after
+execution-initialization-pruning. This validation is done by checking the
+CachedPlan.is_valid attribute. If the plan tree is outdated (is_valid=false),
+the executor halts further initialization, cleans up the partially initialized
+PlanState tree, and retries execution after creating a new transient
+CachedPlan.
Query Processing Control Flow
-----------------------------
@@ -288,7 +310,7 @@ This is a sketch of control flow for full query processing:
CreateQueryDesc
- ExecutorStart
+ ExecutorStart or ExecutorStartExt
CreateExecutorState
creates per-query context
switch to per-query context to run ExecInitNode
@@ -316,7 +338,13 @@ This is a sketch of control flow for full query processing:
FreeQueryDesc
-Per above comments, it's not really critical for ExecEndNode to free any
+As mentioned in the "Relation Locking" section, if the plan tree is found to
+be stale during one of the recursive calls of ExecInitNode() after taking a
+lock on a child table, the control is immmediately returned to
+ExecutorStartExt(), which will create a new plan tree and perform the
+steps starting from CreateExecutorState() again.
+
+Per above comments, it's not really critical for ExecEndPlan to free any
memory; it'll all go away in FreeExecutorState anyway. However, we do need to
be careful to close relations, drop buffer pins, etc, so we do need to scan
the plan state tree to find these sorts of resources.
diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c
index 0f6dbd1e2b..000d02a337 100644
--- a/src/backend/executor/execMain.c
+++ b/src/backend/executor/execMain.c
@@ -58,6 +58,7 @@
#include "utils/backend_status.h"
#include "utils/lsyscache.h"
#include "utils/partcache.h"
+#include "utils/plancache.h"
#include "utils/rls.h"
#include "utils/snapmgr.h"
@@ -133,6 +134,60 @@ ExecutorStart(QueryDesc *queryDesc, int eflags)
standard_ExecutorStart(queryDesc, eflags);
}
+/*
+ * A variant of ExecutorStart() that handles cleanup and replanning if the
+ * input CachedPlan becomes invalid due to locks being taken during
+ * ExecutorStartInternal(). If that happens, a new CachedPlan is created
+ * only for the at the index 'query_index' in plansource->query_list, which
+ * is released separately from the original CachedPlan.
+ */
+void
+ExecutorStartExt(QueryDesc *queryDesc, int eflags,
+ CachedPlanSource *plansource,
+ int query_index)
+{
+ if (queryDesc->cplan == NULL)
+ {
+ ExecutorStart(queryDesc, eflags);
+ return;
+ }
+
+ while (1)
+ {
+ ExecutorStart(queryDesc, eflags);
+ if (!CachedPlanValid(queryDesc->cplan))
+ {
+ CachedPlan *cplan;
+
+ /*
+ * The plan got invalidated, so try with a new updated plan.
+ *
+ * But first undo what ExecutorStart() would've done. Mark
+ * execution as aborted to ensure that AFTER trigger state is
+ * properly reset.
+ */
+ queryDesc->estate->es_aborted = true;
+ ExecutorEnd(queryDesc);
+
+ cplan = GetSingleCachedPlan(plansource, query_index,
+ queryDesc->queryEnv);
+
+ /*
+ * Install the new transient cplan into the QueryDesc replacing
+ * the old one so that executor initialization code can see it.
+ * Mark it as in use by us and ask FreeQueryDesc() to release it.
+ */
+ cplan->refcount = 1;
+ queryDesc->cplan = cplan;
+ queryDesc->cplan_release = true;
+ queryDesc->plannedstmt = linitial_node(PlannedStmt,
+ queryDesc->cplan->stmt_list);
+ }
+ else
+ break; /* ExecutorStart() succeeded! */
+ }
+}
+
void
standard_ExecutorStart(QueryDesc *queryDesc, int eflags)
{
@@ -316,6 +371,7 @@ standard_ExecutorRun(QueryDesc *queryDesc,
estate = queryDesc->estate;
Assert(estate != NULL);
+ Assert(!estate->es_aborted);
Assert(!(estate->es_top_eflags & EXEC_FLAG_EXPLAIN_ONLY));
/* caller must ensure the query's snapshot is active */
@@ -422,8 +478,11 @@ standard_ExecutorFinish(QueryDesc *queryDesc)
Assert(estate != NULL);
Assert(!(estate->es_top_eflags & EXEC_FLAG_EXPLAIN_ONLY));
- /* This should be run once and only once per Executor instance */
- Assert(!estate->es_finished);
+ /*
+ * This should be run once and only once per Executor instance and never
+ * if the execution was aborted.
+ */
+ Assert(!estate->es_finished && !estate->es_aborted);
/* Switch into per-query memory context */
oldcontext = MemoryContextSwitchTo(estate->es_query_cxt);
@@ -482,11 +541,10 @@ standard_ExecutorEnd(QueryDesc *queryDesc)
Assert(estate != NULL);
/*
- * Check that ExecutorFinish was called, unless in EXPLAIN-only mode. This
- * Assert is needed because ExecutorFinish is new as of 9.1, and callers
- * might forget to call it.
+ * Check that ExecutorFinish was called, unless in EXPLAIN-only mode or if
+ * execution was aborted.
*/
- Assert(estate->es_finished ||
+ Assert(estate->es_finished || estate->es_aborted ||
(estate->es_top_eflags & EXEC_FLAG_EXPLAIN_ONLY));
/*
@@ -500,6 +558,14 @@ standard_ExecutorEnd(QueryDesc *queryDesc)
UnregisterSnapshot(estate->es_snapshot);
UnregisterSnapshot(estate->es_crosscheck_snapshot);
+ /*
+ * Reset AFTER trigger module if the query execution was aborted.
+ */
+ if (estate->es_aborted &&
+ !(estate->es_top_eflags &
+ (EXEC_FLAG_SKIP_TRIGGERS | EXEC_FLAG_EXPLAIN_ONLY)))
+ AfterTriggerAbortQuery();
+
/*
* Must switch out of context before destroying it
*/
@@ -832,7 +898,6 @@ ExecCheckXactReadOnly(PlannedStmt *plannedstmt)
PreventCommandIfParallelMode(CreateCommandName((Node *) plannedstmt));
}
-
/* ----------------------------------------------------------------
* InitPlan
*
@@ -897,6 +962,9 @@ InitPlan(QueryDesc *queryDesc, int eflags)
case ROW_MARK_KEYSHARE:
case ROW_MARK_REFERENCE:
relation = ExecGetRangeTableRelation(estate, rc->rti);
+ if (unlikely(relation == NULL ||
+ !ExecPlanStillValid(estate)))
+ return;
break;
case ROW_MARK_COPY:
/* no physical table access is required */
@@ -967,6 +1035,8 @@ InitPlan(QueryDesc *queryDesc, int eflags)
estate->es_subplanstates = lappend(estate->es_subplanstates,
subplanstate);
+ if (unlikely(!ExecPlanStillValid(estate)))
+ return;
i++;
}
@@ -977,6 +1047,8 @@ InitPlan(QueryDesc *queryDesc, int eflags)
* processing tuples.
*/
planstate = ExecInitNode(plan, estate, eflags);
+ if (unlikely(!ExecPlanStillValid(estate)))
+ return;
/*
* Get the tuple descriptor describing the type of tuples to return.
@@ -2858,6 +2930,7 @@ EvalPlanQualStart(EPQState *epqstate, Plan *planTree)
rcestate->es_rowmarks = parentestate->es_rowmarks;
rcestate->es_rteperminfos = parentestate->es_rteperminfos;
rcestate->es_plannedstmt = parentestate->es_plannedstmt;
+ rcestate->es_cachedplan = parentestate->es_cachedplan;
rcestate->es_junkFilter = parentestate->es_junkFilter;
rcestate->es_output_cid = parentestate->es_output_cid;
rcestate->es_queryEnv = parentestate->es_queryEnv;
@@ -2936,6 +3009,14 @@ EvalPlanQualStart(EPQState *epqstate, Plan *planTree)
subplanstate = ExecInitNode(subplan, rcestate, 0);
rcestate->es_subplanstates = lappend(rcestate->es_subplanstates,
subplanstate);
+
+ /*
+ * All necessary locks should have been taken when initializing the
+ * parent's copy of subplanstate, so the CachedPlan, if any, should
+ * not have become invalid during the above ExecInitNode().
+ */
+ if (!ExecPlanStillValid(rcestate))
+ elog(ERROR, "unexpected failure to initialize subplan in EvalPlanQualStart()");
}
/*
@@ -2977,6 +3058,10 @@ EvalPlanQualStart(EPQState *epqstate, Plan *planTree)
*/
epqstate->recheckplanstate = ExecInitNode(planTree, rcestate, 0);
+ /* See the comment above. */
+ if (!ExecPlanStillValid(rcestate))
+ elog(ERROR, "unexpected failure to initialize main plantree in EvalPlanQualStart()");
+
MemoryContextSwitchTo(oldcontext);
}
diff --git a/src/backend/executor/execParallel.c b/src/backend/executor/execParallel.c
index 03b48e12b4..2017433c64 100644
--- a/src/backend/executor/execParallel.c
+++ b/src/backend/executor/execParallel.c
@@ -1263,9 +1263,7 @@ ExecParallelGetQueryDesc(shm_toc *toc, DestReceiver *receiver,
* if it should take locks on certain relations, but paraller workers
* always take locks anyway.
*/
- return CreateQueryDesc(pstmt,
- NULL,
- queryString,
+ return CreateQueryDesc(pstmt, NULL, queryString,
GetActiveSnapshot(), InvalidSnapshot,
receiver, paramLI, NULL, instrument_options);
}
diff --git a/src/backend/executor/execPartition.c b/src/backend/executor/execPartition.c
index 7651886229..38cd97b59c 100644
--- a/src/backend/executor/execPartition.c
+++ b/src/backend/executor/execPartition.c
@@ -1794,6 +1794,9 @@ adjust_partition_colnos_using_map(List *colnos, AttrMap *attrMap)
* If subplans are indeed pruned, subplan_map arrays contained in the returned
* PartitionPruneState are re-sequenced to not count those, though only if the
* maps will be needed for subsequent execution pruning passes.
+ *
+ * Returns NULL if the plan has become invalid after taking the locks to
+ * create the PartitionPruneState in CreatePartitionPruneState().
*/
PartitionPruneState *
ExecInitPartitionPruning(PlanState *planstate,
@@ -1809,6 +1812,8 @@ ExecInitPartitionPruning(PlanState *planstate,
/* Create the working data structure for pruning */
prunestate = CreatePartitionPruneState(planstate, pruneinfo);
+ if (!ExecPlanStillValid(estate))
+ return NULL;
/*
* Perform an initial partition prune pass, if required.
@@ -1860,6 +1865,9 @@ ExecInitPartitionPruning(PlanState *planstate,
* stored in each PartitionedRelPruningData can be re-used each time we
* re-evaluate which partitions match the pruning steps provided in each
* PartitionedRelPruneInfo.
+ *
+ * Returns NULL if the plan has become invalid after taking a lock to create
+ * a PartitionedRelPruningData.
*/
static PartitionPruneState *
CreatePartitionPruneState(PlanState *planstate, PartitionPruneInfo *pruneinfo)
@@ -1935,6 +1943,8 @@ CreatePartitionPruneState(PlanState *planstate, PartitionPruneInfo *pruneinfo)
* duration of this executor run.
*/
partrel = ExecGetRangeTableRelation(estate, pinfo->rtindex);
+ if (unlikely(partrel == NULL || !ExecPlanStillValid(estate)))
+ return NULL;
partkey = RelationGetPartitionKey(partrel);
partdesc = PartitionDirectoryLookup(estate->es_partition_directory,
partrel);
diff --git a/src/backend/executor/execProcnode.c b/src/backend/executor/execProcnode.c
index 34f28dfece..7689d34dd0 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 *
@@ -388,6 +392,9 @@ ExecInitNode(Plan *node, EState *estate, int eflags)
break;
}
+ if (unlikely(!ExecPlanStillValid(estate)))
+ return result;
+
ExecSetExecProcNode(result, result->ExecProcNode);
/*
diff --git a/src/backend/executor/execUtils.c b/src/backend/executor/execUtils.c
index 6dfd5a26b7..39b388e6b4 100644
--- a/src/backend/executor/execUtils.c
+++ b/src/backend/executor/execUtils.c
@@ -146,6 +146,7 @@ CreateExecutorState(void)
estate->es_top_eflags = 0;
estate->es_instrument = 0;
estate->es_finished = false;
+ estate->es_aborted = false;
estate->es_exprcontexts = NIL;
@@ -691,6 +692,8 @@ ExecRelationIsTargetRelation(EState *estate, Index scanrelid)
*
* Open the heap relation to be scanned by a base-level scan plan node.
* This should be called during the node's ExecInit routine.
+ *
+ * NULL is returned if the relation is found to have been dropped.
* ----------------------------------------------------------------
*/
Relation
@@ -700,6 +703,8 @@ ExecOpenScanRelation(EState *estate, Index scanrelid, int eflags)
/* Open the relation. */
rel = ExecGetRangeTableRelation(estate, scanrelid);
+ if (unlikely(rel == NULL || !ExecPlanStillValid(estate)))
+ return rel;
/*
* Complain if we're attempting a scan of an unscannable relation, except
@@ -717,6 +722,26 @@ ExecOpenScanRelation(EState *estate, Index scanrelid, int eflags)
return rel;
}
+/* ----------------------------------------------------------------
+ * ExecOpenScanIndexRelation
+ *
+ * Open the index relation to be scanned by an index scan plan node.
+ * This should be called during the node's ExecInit routine.
+ * ----------------------------------------------------------------
+ */
+Relation
+ExecOpenScanIndexRelation(EState *estate, Oid indexid, int lockmode)
+{
+ Relation rel;
+
+ /* Open the index. */
+ rel = index_open(indexid, lockmode);
+ if (unlikely(!ExecPlanStillValid(estate)))
+ elog(DEBUG2, "CachedPlan invalidated on locking index %u", indexid);
+
+ return rel;
+}
+
/*
* ExecInitRangeTable
* Set up executor's range-table-related data
@@ -776,8 +801,12 @@ ExecShouldLockRelation(EState *estate, Index rtindex)
* ExecGetRangeTableRelation
* Open the Relation for a range table entry, if not already done
*
- * The Relations will be closed again in ExecEndPlan().
+ * The Relations will be closed in ExecEndPlan().
+ *
+ * The returned value may be NULL if the relation is a prunable relation
+ * that has not been locked and may have been concurrently dropped.
*/
+
Relation
ExecGetRangeTableRelation(EState *estate, Index rti)
{
@@ -820,8 +849,14 @@ ExecGetRangeTableRelation(EState *estate, Index rti)
* that of a prunable relation and we're running a cached generic
* plan. AcquireExecutorLocks() of plancache.c would have locked
* only the unprunable relations in the plan tree.
+ *
+ * Note that we use try_table_open() here, because without a lock
+ * held on the relation, it may have disappeared from under us.
*/
- rel = table_open(rte->relid, rte->rellockmode);
+ rel = try_table_open(rte->relid, rte->rellockmode);
+ if (unlikely(!ExecPlanStillValid(estate)))
+ elog(DEBUG2, "CachedPlan invalidated on locking relation %u",
+ rte->relid);
}
estate->es_relations[rti - 1] = rel;
@@ -845,6 +880,9 @@ ExecInitResultRelation(EState *estate, ResultRelInfo *resultRelInfo,
Relation resultRelationDesc;
resultRelationDesc = ExecGetRangeTableRelation(estate, rti);
+ if (unlikely(resultRelationDesc == NULL ||
+ !ExecPlanStillValid(estate)))
+ return;
InitResultRelInfo(resultRelInfo,
resultRelationDesc,
rti,
diff --git a/src/backend/executor/nodeAgg.c b/src/backend/executor/nodeAgg.c
index 0dfba5ca16..8c40d8c520 100644
--- a/src/backend/executor/nodeAgg.c
+++ b/src/backend/executor/nodeAgg.c
@@ -3303,6 +3303,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 86d75b1a7e..3c82a1ceab 100644
--- a/src/backend/executor/nodeAppend.c
+++ b/src/backend/executor/nodeAppend.c
@@ -147,6 +147,8 @@ ExecInitAppend(Append *node, EState *estate, int eflags)
list_length(node->appendplans),
node->part_prune_info,
&validsubplans);
+ if (!ExecPlanStillValid(estate))
+ return appendstate;
appendstate->as_prune_state = prunestate;
nplans = bms_num_members(validsubplans);
@@ -185,8 +187,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 +225,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 ae391222bf..168c440692 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 19f18ab817..b13cae1cbb 100644
--- a/src/backend/executor/nodeBitmapHeapscan.c
+++ b/src/backend/executor/nodeBitmapHeapscan.c
@@ -754,11 +754,15 @@ ExecInitBitmapHeapScan(BitmapHeapScan *node, EState *estate, int eflags)
* open the scan relation
*/
currentRelation = ExecOpenScanRelation(estate, node->scan.scanrelid, eflags);
+ if (unlikely(currentRelation == NULL || !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/nodeBitmapIndexscan.c b/src/backend/executor/nodeBitmapIndexscan.c
index 4669e8d0ce..f04a53e9be 100644
--- a/src/backend/executor/nodeBitmapIndexscan.c
+++ b/src/backend/executor/nodeBitmapIndexscan.c
@@ -252,7 +252,11 @@ ExecInitBitmapIndexScan(BitmapIndexScan *node, EState *estate, int eflags)
/* Open the index relation. */
lockmode = exec_rt_fetch(node->scan.scanrelid, estate)->rellockmode;
- indexstate->biss_RelationDesc = index_open(node->indexid, lockmode);
+ indexstate->biss_RelationDesc = ExecOpenScanIndexRelation(estate,
+ node->indexid,
+ lockmode);
+ if (unlikely(!ExecPlanStillValid(estate)))
+ return indexstate;
/*
* Initialize index-specific scan state
diff --git a/src/backend/executor/nodeBitmapOr.c b/src/backend/executor/nodeBitmapOr.c
index de439235d2..980b68dd82 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 e559cd2346..2a7c5dccd8 100644
--- a/src/backend/executor/nodeCustom.c
+++ b/src/backend/executor/nodeCustom.c
@@ -58,6 +58,8 @@ ExecInitCustomScan(CustomScan *cscan, EState *estate, int eflags)
if (scanrelid > 0)
{
scan_rel = ExecOpenScanRelation(estate, scanrelid, eflags);
+ if (unlikely(scan_rel == NULL || !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 1357ccf3c9..90d5878ae3 100644
--- a/src/backend/executor/nodeForeignscan.c
+++ b/src/backend/executor/nodeForeignscan.c
@@ -172,6 +172,8 @@ ExecInitForeignScan(ForeignScan *node, EState *estate, int eflags)
if (scanrelid > 0)
{
currentRelation = ExecOpenScanRelation(estate, scanrelid, eflags);
+ if (unlikely(currentRelation == NULL || !ExecPlanStillValid(estate)))
+ return scanstate;
scanstate->ss.ss_currentRelation = currentRelation;
fdwroutine = GetFdwRoutineForRelation(currentRelation, true);
}
@@ -263,6 +265,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 cae5ea1f92..67548aa7ba 100644
--- a/src/backend/executor/nodeGather.c
+++ b/src/backend/executor/nodeGather.c
@@ -84,6 +84,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 b36cd89e7d..cf0e074359 100644
--- a/src/backend/executor/nodeGatherMerge.c
+++ b/src/backend/executor/nodeGatherMerge.c
@@ -103,6 +103,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 807429e504..6d0fd9e7b4 100644
--- a/src/backend/executor/nodeGroup.c
+++ b/src/backend/executor/nodeGroup.c
@@ -184,6 +184,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 a913d5b50c..e71d131d18 100644
--- a/src/backend/executor/nodeHash.c
+++ b/src/backend/executor/nodeHash.c
@@ -396,6 +396,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 901c9e9be7..3c870de1c5 100644
--- a/src/backend/executor/nodeHashjoin.c
+++ b/src/backend/executor/nodeHashjoin.c
@@ -758,8 +758,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 010bcfafa8..af723ea755 100644
--- a/src/backend/executor/nodeIncrementalSort.c
+++ b/src/backend/executor/nodeIncrementalSort.c
@@ -1040,6 +1040,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 481d479760..0fba8f7d5a 100644
--- a/src/backend/executor/nodeIndexonlyscan.c
+++ b/src/backend/executor/nodeIndexonlyscan.c
@@ -531,6 +531,8 @@ ExecInitIndexOnlyScan(IndexOnlyScan *node, EState *estate, int eflags)
* open the scan relation
*/
currentRelation = ExecOpenScanRelation(estate, node->scan.scanrelid, eflags);
+ if (unlikely(currentRelation == NULL || !ExecPlanStillValid(estate)))
+ return indexstate;
indexstate->ss.ss_currentRelation = currentRelation;
indexstate->ss.ss_currentScanDesc = NULL; /* no heap scan here */
@@ -583,9 +585,12 @@ ExecInitIndexOnlyScan(IndexOnlyScan *node, EState *estate, int eflags)
/* Open the index relation. */
lockmode = exec_rt_fetch(node->scan.scanrelid, estate)->rellockmode;
- indexRelation = index_open(node->indexid, lockmode);
+ indexRelation = ExecOpenScanIndexRelation(estate, node->indexid, lockmode);
indexstate->ioss_RelationDesc = indexRelation;
+ if (unlikely(!ExecPlanStillValid(estate)))
+ return indexstate;
+
/*
* Initialize index-specific scan state
*/
diff --git a/src/backend/executor/nodeIndexscan.c b/src/backend/executor/nodeIndexscan.c
index a8172d8b82..db28aeb3d6 100644
--- a/src/backend/executor/nodeIndexscan.c
+++ b/src/backend/executor/nodeIndexscan.c
@@ -907,6 +907,8 @@ ExecInitIndexScan(IndexScan *node, EState *estate, int eflags)
* open the scan relation
*/
currentRelation = ExecOpenScanRelation(estate, node->scan.scanrelid, eflags);
+ if (unlikely(currentRelation == NULL || !ExecPlanStillValid(estate)))
+ return indexstate;
indexstate->ss.ss_currentRelation = currentRelation;
indexstate->ss.ss_currentScanDesc = NULL; /* no heap scan here */
@@ -951,7 +953,11 @@ ExecInitIndexScan(IndexScan *node, EState *estate, int eflags)
/* Open the index relation. */
lockmode = exec_rt_fetch(node->scan.scanrelid, estate)->rellockmode;
- indexstate->iss_RelationDesc = index_open(node->indexid, lockmode);
+ indexstate->iss_RelationDesc = ExecOpenScanIndexRelation(estate,
+ node->indexid,
+ lockmode);
+ if (unlikely(!ExecPlanStillValid(estate)))
+ return indexstate;
/*
* Initialize index-specific scan state
diff --git a/src/backend/executor/nodeLimit.c b/src/backend/executor/nodeLimit.c
index eb7b6e52be..369c904577 100644
--- a/src/backend/executor/nodeLimit.c
+++ b/src/backend/executor/nodeLimit.c
@@ -475,6 +475,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 0d3489195b..9077858413 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 883e3f3933..972962d44d 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 690dee1daa..6aaab743b5 100644
--- a/src/backend/executor/nodeMemoize.c
+++ b/src/backend/executor/nodeMemoize.c
@@ -973,6 +973,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 3236444cf1..a82f0a71a0 100644
--- a/src/backend/executor/nodeMergeAppend.c
+++ b/src/backend/executor/nodeMergeAppend.c
@@ -95,6 +95,8 @@ ExecInitMergeAppend(MergeAppend *node, EState *estate, int eflags)
list_length(node->mergeplans),
node->part_prune_info,
&validsubplans);
+ if (!ExecPlanStillValid(estate))
+ return mergestate;
mergestate->ms_prune_state = prunestate;
nplans = bms_num_members(validsubplans);
@@ -120,7 +122,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 +153,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 926e631d88..53cb1ff207 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 9e56f9c36c..8debfbd3ec 100644
--- a/src/backend/executor/nodeModifyTable.c
+++ b/src/backend/executor/nodeModifyTable.c
@@ -4277,6 +4277,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);
@@ -4309,6 +4316,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
@@ -4335,6 +4346,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 01f3d56a3b..34eafbb6e0 100644
--- a/src/backend/executor/nodeNestloop.c
+++ b/src/backend/executor/nodeNestloop.c
@@ -294,11 +294,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 ca9a5e2ed2..f834499479 100644
--- a/src/backend/executor/nodeProjectSet.c
+++ b/src/backend/executor/nodeProjectSet.c
@@ -254,6 +254,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 7680142c7b..5dd3285c41 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 e3cfc9b772..7d7c2aa786 100644
--- a/src/backend/executor/nodeResult.c
+++ b/src/backend/executor/nodeResult.c
@@ -207,6 +207,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 6ab91001bc..3afdaeecd7 100644
--- a/src/backend/executor/nodeSamplescan.c
+++ b/src/backend/executor/nodeSamplescan.c
@@ -121,6 +121,9 @@ ExecInitSampleScan(SampleScan *node, EState *estate, int eflags)
ExecOpenScanRelation(estate,
node->scan.scanrelid,
eflags);
+ if (unlikely(scanstate->ss.ss_currentRelation == NULL ||
+ !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 b052775e5b..f7fb64a4a2 100644
--- a/src/backend/executor/nodeSeqscan.c
+++ b/src/backend/executor/nodeSeqscan.c
@@ -153,6 +153,9 @@ ExecInitSeqScan(SeqScan *node, EState *estate, int eflags)
ExecOpenScanRelation(estate,
node->scan.scanrelid,
eflags);
+ if (unlikely(scanstate->ss.ss_currentRelation == NULL ||
+ !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 fe34b2134f..2231d8b82f 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 af852464d0..fb76e4c01b 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 0b2612183a..b5b538fa91 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 702ee884d2..a76836d021 100644
--- a/src/backend/executor/nodeTidrangescan.c
+++ b/src/backend/executor/nodeTidrangescan.c
@@ -377,6 +377,8 @@ ExecInitTidRangeScan(TidRangeScan *node, EState *estate, int eflags)
* open the scan relation
*/
currentRelation = ExecOpenScanRelation(estate, node->scan.scanrelid, eflags);
+ if (unlikely(currentRelation == NULL || !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 f375951699..088babf572 100644
--- a/src/backend/executor/nodeTidscan.c
+++ b/src/backend/executor/nodeTidscan.c
@@ -522,6 +522,8 @@ ExecInitTidScan(TidScan *node, EState *estate, int eflags)
* open the scan relation
*/
currentRelation = ExecOpenScanRelation(estate, node->scan.scanrelid, eflags);
+ if (unlikely(currentRelation == NULL || !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 b82d0e9ad5..cb46b2d5d0 100644
--- a/src/backend/executor/nodeUnique.c
+++ b/src/backend/executor/nodeUnique.c
@@ -135,6 +135,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 561d7e731d..1b96f51fe8 100644
--- a/src/backend/executor/nodeWindowAgg.c
+++ b/src/backend/executor/nodeWindowAgg.c
@@ -2464,6 +2464,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 902793b02b..b754827013 100644
--- a/src/backend/executor/spi.c
+++ b/src/backend/executor/spi.c
@@ -70,7 +70,8 @@ static int _SPI_execute_plan(SPIPlanPtr plan, const SPIExecuteOptions *options,
static ParamListInfo _SPI_convert_params(int nargs, Oid *argtypes,
Datum *Values, const char *Nulls);
-static int _SPI_pquery(QueryDesc *queryDesc, bool fire_triggers, uint64 tcount);
+static int _SPI_pquery(QueryDesc *queryDesc, bool fire_triggers, uint64 tcount,
+ CachedPlanSource *plansource, int query_index);
static void _SPI_error_callback(void *arg);
@@ -1682,7 +1683,8 @@ SPI_cursor_open_internal(const char *name, SPIPlanPtr plan,
query_string,
plansource->commandTag,
stmt_list,
- cplan);
+ cplan,
+ plansource);
/*
* Set up options for portal. Default SCROLL type is chosen the same way
@@ -2494,6 +2496,7 @@ _SPI_execute_plan(SPIPlanPtr plan, const SPIExecuteOptions *options,
CachedPlanSource *plansource = (CachedPlanSource *) lfirst(lc1);
List *stmt_list;
ListCell *lc2;
+ int i = 0;
spicallbackarg.query = plansource->query_string;
@@ -2691,8 +2694,9 @@ _SPI_execute_plan(SPIPlanPtr plan, const SPIExecuteOptions *options,
options->params,
_SPI_current->queryEnv,
0);
- res = _SPI_pquery(qdesc, fire_triggers,
- canSetTag ? options->tcount : 0);
+
+ res = _SPI_pquery(qdesc, fire_triggers, canSetTag ? options->tcount : 0,
+ plansource, i);
FreeQueryDesc(qdesc);
}
else
@@ -2789,6 +2793,8 @@ _SPI_execute_plan(SPIPlanPtr plan, const SPIExecuteOptions *options,
my_res = res;
goto fail;
}
+
+ i++;
}
/* Done with this plan, so release refcount */
@@ -2866,7 +2872,8 @@ _SPI_convert_params(int nargs, Oid *argtypes,
}
static int
-_SPI_pquery(QueryDesc *queryDesc, bool fire_triggers, uint64 tcount)
+_SPI_pquery(QueryDesc *queryDesc, bool fire_triggers, uint64 tcount,
+ CachedPlanSource *plansource, int query_index)
{
int operation = queryDesc->operation;
int eflags;
@@ -2922,7 +2929,7 @@ _SPI_pquery(QueryDesc *queryDesc, bool fire_triggers, uint64 tcount)
else
eflags = EXEC_FLAG_SKIP_TRIGGERS;
- ExecutorStart(queryDesc, eflags);
+ ExecutorStartExt(queryDesc, eflags, plansource, query_index);
ExecutorRun(queryDesc, ForwardScanDirection, tcount, true);
diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
index 8bc6bea113..ccbc27b575 100644
--- a/src/backend/tcop/postgres.c
+++ b/src/backend/tcop/postgres.c
@@ -1237,6 +1237,7 @@ exec_simple_query(const char *query_string)
query_string,
commandTag,
plantree_list,
+ NULL,
NULL);
/*
@@ -2027,7 +2028,8 @@ exec_bind_message(StringInfo input_message)
query_string,
psrc->commandTag,
cplan->stmt_list,
- cplan);
+ cplan,
+ psrc);
/* Done with the snapshot used for parameter I/O and parsing/planning */
if (snapshot_set)
diff --git a/src/backend/tcop/pquery.c b/src/backend/tcop/pquery.c
index 6e8f6b1b8f..c2ebddaa84 100644
--- a/src/backend/tcop/pquery.c
+++ b/src/backend/tcop/pquery.c
@@ -19,6 +19,7 @@
#include "access/xact.h"
#include "commands/prepare.h"
+#include "executor/execdesc.h"
#include "executor/tstoreReceiver.h"
#include "miscadmin.h"
#include "pg_trace.h"
@@ -37,6 +38,8 @@ Portal ActivePortal = NULL;
static void ProcessQuery(PlannedStmt *plan,
CachedPlan *cplan,
+ CachedPlanSource *plansource,
+ int query_index,
const char *sourceText,
ParamListInfo params,
QueryEnvironment *queryEnv,
@@ -80,6 +83,7 @@ CreateQueryDesc(PlannedStmt *plannedstmt,
qd->operation = plannedstmt->commandType; /* operation */
qd->plannedstmt = plannedstmt; /* plan */
qd->cplan = cplan; /* CachedPlan supplying the plannedstmt */
+ qd->cplan_release = false;
qd->sourceText = sourceText; /* query text */
qd->snapshot = RegisterSnapshot(snapshot); /* snapshot */
/* RI check snapshot */
@@ -114,6 +118,13 @@ FreeQueryDesc(QueryDesc *qdesc)
UnregisterSnapshot(qdesc->snapshot);
UnregisterSnapshot(qdesc->crosscheck_snapshot);
+ /*
+ * Release CachedPlan if requested. The CachedPlan is not associated with
+ * a ResourceOwner when release_cplan is true; see ExecutorStartExt().
+ */
+ if (qdesc->cplan_release)
+ ReleaseCachedPlan(qdesc->cplan, NULL);
+
/* Only the QueryDesc itself need be freed */
pfree(qdesc);
}
@@ -126,6 +137,8 @@ FreeQueryDesc(QueryDesc *qdesc)
*
* plan: the plan tree for the query
* cplan: CachedPlan supplying the plan
+ * plansource: CachedPlanSource supplying the cplan
+ * query_index: index of the query in plansource->query_list
* sourceText: the source text of the query
* params: any parameters needed
* dest: where to send results
@@ -139,6 +152,8 @@ FreeQueryDesc(QueryDesc *qdesc)
static void
ProcessQuery(PlannedStmt *plan,
CachedPlan *cplan,
+ CachedPlanSource *plansource,
+ int query_index,
const char *sourceText,
ParamListInfo params,
QueryEnvironment *queryEnv,
@@ -157,7 +172,7 @@ ProcessQuery(PlannedStmt *plan,
/*
* Call ExecutorStart to prepare the plan for execution
*/
- ExecutorStart(queryDesc, 0);
+ ExecutorStartExt(queryDesc, 0, plansource, query_index);
/*
* Run the plan to completion.
@@ -518,9 +533,12 @@ PortalStart(Portal portal, ParamListInfo params,
myeflags = eflags;
/*
- * Call ExecutorStart to prepare the plan for execution
+ * ExecutorStartExt() to prepare the plan for execution. If
+ * the portal is using a cached plan, it may get invalidated
+ * during plan intialization, in which case a new one is
+ * created and saved in the QueryDesc.
*/
- ExecutorStart(queryDesc, myeflags);
+ ExecutorStartExt(queryDesc, myeflags, portal->plansource, 0);
/*
* This tells PortalCleanup to shut down the executor
@@ -1201,6 +1219,7 @@ PortalRunMulti(Portal portal,
{
bool active_snapshot_set = false;
ListCell *stmtlist_item;
+ int i = 0;
/*
* If the destination is DestRemoteExecute, change to DestNone. The
@@ -1283,6 +1302,8 @@ PortalRunMulti(Portal portal,
/* statement can set tag string */
ProcessQuery(pstmt,
portal->cplan,
+ portal->plansource,
+ i,
portal->sourceText,
portal->portalParams,
portal->queryEnv,
@@ -1293,6 +1314,8 @@ PortalRunMulti(Portal portal,
/* stmt added by rewrite cannot set tag */
ProcessQuery(pstmt,
portal->cplan,
+ portal->plansource,
+ i,
portal->sourceText,
portal->portalParams,
portal->queryEnv,
@@ -1357,6 +1380,8 @@ PortalRunMulti(Portal portal,
*/
if (lnext(portal->stmts, stmtlist_item) != NULL)
CommandCounterIncrement();
+
+ i++;
}
/* Pop the snapshot if we pushed one. */
diff --git a/src/backend/utils/cache/plancache.c b/src/backend/utils/cache/plancache.c
index 5b75dadf13..d33f871ea2 100644
--- a/src/backend/utils/cache/plancache.c
+++ b/src/backend/utils/cache/plancache.c
@@ -94,6 +94,14 @@
*/
static dlist_head saved_plan_list = DLIST_STATIC_INIT(saved_plan_list);
+/*
+ * Head of the backend's list of "standalone" CachedPlans that are not
+ * associated with a CachedPlanSource, created by GetSingleCachedPlan() for
+ * transient use by the executor in certain scenarios where they're needed
+ * only for one execution of the plan.
+ */
+static dlist_head standalone_plan_list = DLIST_STATIC_INIT(standalone_plan_list);
+
/*
* This is the head of the backend's list of CachedExpressions.
*/
@@ -905,6 +913,8 @@ CheckCachedPlan(CachedPlanSource *plansource)
* Planning work is done in the caller's memory context. The finished plan
* is in a child memory context, which typically should get reparented
* (unless this is a one-shot plan, in which case we don't copy the plan).
+ *
+ * Note: When changing this, you should also look at GetSingleCachedPlan().
*/
static CachedPlan *
BuildCachedPlan(CachedPlanSource *plansource, List *qlist,
@@ -1034,6 +1044,7 @@ BuildCachedPlan(CachedPlanSource *plansource, List *qlist,
plan->is_generic = generic;
plan->is_saved = false;
plan->is_valid = true;
+ plan->is_standalone = false;
/* assign generation number to new plan */
plan->generation = ++(plansource->generation);
@@ -1282,6 +1293,121 @@ GetCachedPlan(CachedPlanSource *plansource, ParamListInfo boundParams,
return plan;
}
+/*
+ * Create a fresh CachedPlan for the query_index'th query in the provided
+ * CachedPlanSource.
+ *
+ * The created CachedPlan is standalone, meaning it is not tracked in the
+ * CachedPlanSource. The CachedPlan and its plan trees are allocated in a
+ * child context of the caller's memory context. The caller must ensure they
+ * remain valid until execution is complete, after which the plan should be
+ * released by calling ReleaseCachedPlan().
+ *
+ * This function primarily supports ExecutorStartExt(), which handles cases
+ * where the original generic CachedPlan becomes invalid after prunable
+ * relations are locked.
+ */
+CachedPlan *
+GetSingleCachedPlan(CachedPlanSource *plansource, int query_index,
+ QueryEnvironment *queryEnv)
+{
+ List *query_list = plansource->query_list,
+ *plan_list;
+ CachedPlan *plan = plansource->gplan;
+ MemoryContext oldcxt = CurrentMemoryContext,
+ plan_context;
+ PlannedStmt *plannedstmt;
+
+ Assert(ActiveSnapshotSet());
+
+ /* Sanity checks */
+ if (plan == NULL)
+ elog(ERROR, "GetSingleCachedPlan() called in the wrong context: plansource->gplan is NULL");
+ else if (plan->is_valid)
+ elog(ERROR, "GetSingleCachedPlan() called in the wrong context: plansource->gplan->is_valid");
+
+ /*
+ * The plansource might have become invalid since GetCachedPlan(). See the
+ * comment in BuildCachedPlan() for details on why this might happen.
+ *
+ * The risk is greater here because this function is called from the
+ * executor, meaning much more processing may have occurred compared to
+ * when BuildCachedPlan() is called from GetCachedPlan().
+ */
+ if (!plansource->is_valid)
+ query_list = RevalidateCachedQuery(plansource, queryEnv);
+ Assert(query_list != NIL);
+
+ /*
+ * Build a new generic plan for the query_index'th query, but make a copy
+ * to be scribbled on by the planner
+ */
+ query_list = list_make1(copyObject(list_nth_node(Query, query_list,
+ query_index)));
+ plan_list = pg_plan_queries(query_list, plansource->query_string,
+ plansource->cursor_options, NULL);
+
+ list_free_deep(query_list);
+
+ /*
+ * Make a dedicated memory context for the CachedPlan and its subsidiary
+ * data so that we can release it in ReleaseCachedPlan() that will be
+ * called in FreeQueryDesc().
+ */
+ plan_context = AllocSetContextCreate(CurrentMemoryContext,
+ "Standalone CachedPlan",
+ ALLOCSET_START_SMALL_SIZES);
+ MemoryContextCopyAndSetIdentifier(plan_context, plansource->query_string);
+
+ /*
+ * Copy plan into the new context.
+ */
+ MemoryContextSwitchTo(plan_context);
+ plan_list = copyObject(plan_list);
+
+ /*
+ * Create and fill the CachedPlan struct within the new context.
+ */
+ plan = (CachedPlan *) palloc(sizeof(CachedPlan));
+ plan->magic = CACHEDPLAN_MAGIC;
+ plan->stmt_list = plan_list;
+
+ plan->planRoleId = GetUserId();
+ Assert(list_length(plan_list) == 1);
+ plannedstmt = linitial_node(PlannedStmt, plan_list);
+
+ /*
+ * CachedPlan is dependent on role either if RLS affected the rewrite
+ * phase or if a role dependency was injected during planning. And it's
+ * transient if any plan is marked so.
+ */
+ plan->dependsOnRole = plansource->dependsOnRLS || plannedstmt->dependsOnRole;
+ if (plannedstmt->transientPlan)
+ {
+ Assert(TransactionIdIsNormal(TransactionXmin));
+ plan->saved_xmin = TransactionXmin;
+ }
+ else
+ plan->saved_xmin = InvalidTransactionId;
+ plan->refcount = 0;
+ plan->context = plan_context;
+ plan->is_oneshot = false;
+ plan->is_generic = true;
+ plan->is_saved = false;
+ plan->is_valid = true;
+ plan->is_standalone = true;
+ plan->generation = 1;
+ MemoryContextSwitchTo(oldcxt);
+
+ /*
+ * Add the entry to the global list of "standalone" cached plans. It is
+ * removed from the list by ReleaseCachedPlan().
+ */
+ dlist_push_tail(&standalone_plan_list, &plan->node);
+
+ return plan;
+}
+
/*
* ReleaseCachedPlan: release active use of a cached plan.
*
@@ -1309,6 +1435,10 @@ ReleaseCachedPlan(CachedPlan *plan, ResourceOwner owner)
/* Mark it no longer valid */
plan->magic = 0;
+ /* Remove from the global list if we are a standalone plan. */
+ if (plan->is_standalone)
+ dlist_delete(&plan->node);
+
/* One-shot plans do not own their context, so we can't free them */
if (!plan->is_oneshot)
MemoryContextDelete(plan->context);
@@ -2066,6 +2196,33 @@ PlanCacheRelCallback(Datum arg, Oid relid)
cexpr->is_valid = false;
}
}
+
+ /* Finally, invalidate any standalone cached plans */
+ dlist_foreach(iter, &standalone_plan_list)
+ {
+ CachedPlan *cplan = dlist_container(CachedPlan,
+ node, iter.cur);
+
+ Assert(cplan->magic == CACHEDPLAN_MAGIC);
+
+ if (cplan->is_valid)
+ {
+ ListCell *lc;
+
+ foreach(lc, cplan->stmt_list)
+ {
+ PlannedStmt *plannedstmt = lfirst_node(PlannedStmt, lc);
+
+ if (plannedstmt->commandType == CMD_UTILITY)
+ continue; /* Ignore utility statements */
+ if ((relid == InvalidOid) ? plannedstmt->relationOids != NIL :
+ list_member_oid(plannedstmt->relationOids, relid))
+ cplan->is_valid = false;
+ if (!cplan->is_valid)
+ break; /* out of stmt_list scan */
+ }
+ }
+ }
}
/*
@@ -2176,6 +2333,44 @@ PlanCacheObjectCallback(Datum arg, int cacheid, uint32 hashvalue)
}
}
}
+
+ /* Finally, invalidate any standalone cached plans */
+ dlist_foreach(iter, &standalone_plan_list)
+ {
+ CachedPlan *cplan = dlist_container(CachedPlan,
+ node, iter.cur);
+
+ Assert(cplan->magic == CACHEDPLAN_MAGIC);
+
+ if (cplan->is_valid)
+ {
+ ListCell *lc;
+
+ foreach(lc, cplan->stmt_list)
+ {
+ PlannedStmt *plannedstmt = lfirst_node(PlannedStmt, lc);
+ ListCell *lc3;
+
+ if (plannedstmt->commandType == CMD_UTILITY)
+ continue; /* Ignore utility statements */
+ foreach(lc3, plannedstmt->invalItems)
+ {
+ PlanInvalItem *item = (PlanInvalItem *) lfirst(lc3);
+
+ if (item->cacheId != cacheid)
+ continue;
+ if (hashvalue == 0 ||
+ item->hashValue == hashvalue)
+ {
+ cplan->is_valid = false;
+ break; /* out of invalItems scan */
+ }
+ }
+ if (!cplan->is_valid)
+ break; /* out of stmt_list scan */
+ }
+ }
+ }
}
/*
@@ -2235,6 +2430,17 @@ ResetPlanCache(void)
cexpr->is_valid = false;
}
+
+ /* Finally, invalidate any standalone cached plans */
+ dlist_foreach(iter, &standalone_plan_list)
+ {
+ CachedPlan *cplan = dlist_container(CachedPlan,
+ node, iter.cur);
+
+ Assert(cplan->magic == CACHEDPLAN_MAGIC);
+
+ cplan->is_valid = false;
+ }
}
/*
diff --git a/src/backend/utils/mmgr/portalmem.c b/src/backend/utils/mmgr/portalmem.c
index 4a24613537..bf70fd4ce7 100644
--- a/src/backend/utils/mmgr/portalmem.c
+++ b/src/backend/utils/mmgr/portalmem.c
@@ -284,7 +284,8 @@ PortalDefineQuery(Portal portal,
const char *sourceText,
CommandTag commandTag,
List *stmts,
- CachedPlan *cplan)
+ CachedPlan *cplan,
+ CachedPlanSource *plansource)
{
Assert(PortalIsValid(portal));
Assert(portal->status == PORTAL_NEW);
@@ -299,6 +300,7 @@ PortalDefineQuery(Portal portal,
portal->commandTag = commandTag;
portal->stmts = stmts;
portal->cplan = cplan;
+ portal->plansource = plansource;
portal->status = PORTAL_DEFINED;
}
diff --git a/src/include/commands/explain.h b/src/include/commands/explain.h
index bf326eeb70..652e1afbf7 100644
--- a/src/include/commands/explain.h
+++ b/src/include/commands/explain.h
@@ -102,6 +102,7 @@ extern void ExplainOneUtility(Node *utilityStmt, IntoClause *into,
ParamListInfo params, QueryEnvironment *queryEnv);
extern void ExplainOnePlan(PlannedStmt *plannedstmt, CachedPlan *cplan,
+ CachedPlanSource *plansource, int plan_index,
IntoClause *into, ExplainState *es,
const char *queryString,
ParamListInfo params, QueryEnvironment *queryEnv,
diff --git a/src/include/commands/trigger.h b/src/include/commands/trigger.h
index 8a5a9fe642..db21561c8c 100644
--- a/src/include/commands/trigger.h
+++ b/src/include/commands/trigger.h
@@ -258,6 +258,7 @@ extern void ExecASTruncateTriggers(EState *estate,
extern void AfterTriggerBeginXact(void);
extern void AfterTriggerBeginQuery(void);
extern void AfterTriggerEndQuery(EState *estate);
+extern void AfterTriggerAbortQuery(void);
extern void AfterTriggerFireDeferred(void);
extern void AfterTriggerEndXact(bool isCommit);
extern void AfterTriggerBeginSubXact(void);
diff --git a/src/include/executor/execdesc.h b/src/include/executor/execdesc.h
index 0e7245435d..f6cb6479c0 100644
--- a/src/include/executor/execdesc.h
+++ b/src/include/executor/execdesc.h
@@ -36,6 +36,7 @@ typedef struct QueryDesc
CmdType operation; /* CMD_SELECT, CMD_UPDATE, etc. */
PlannedStmt *plannedstmt; /* planner's output (could be utility, too) */
CachedPlan *cplan; /* CachedPlan that supplies the plannedstmt */
+ bool cplan_release; /* Should FreeQueryDesc() release cplan? */
const char *sourceText; /* source text of the query */
Snapshot snapshot; /* snapshot to use for query */
Snapshot crosscheck_snapshot; /* crosscheck for RI update/delete */
diff --git a/src/include/executor/executor.h b/src/include/executor/executor.h
index 69c3ebff00..084d8d5d91 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"
/*
@@ -198,6 +199,8 @@ ExecGetJunkAttribute(TupleTableSlot *slot, AttrNumber attno, bool *isNull)
* prototypes from functions in execMain.c
*/
extern void ExecutorStart(QueryDesc *queryDesc, int eflags);
+extern void ExecutorStartExt(QueryDesc *queryDesc, int eflags,
+ CachedPlanSource *plansource, int query_index);
extern void standard_ExecutorStart(QueryDesc *queryDesc, int eflags);
extern void ExecutorRun(QueryDesc *queryDesc,
ScanDirection direction, uint64 count, bool execute_once);
@@ -261,6 +264,20 @@ 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?
+ *
+ * Called at various points during ExecutorStart() because invalidation
+ * messages that affect the plan might be received after locks have been
+ * taken on runtime-prunable relations. The caller should take appropriate
+ * action if the plan has become invalid.
+ */
+static inline bool
+ExecPlanStillValid(EState *estate)
+{
+ return estate->es_cachedplan == NULL ? true :
+ CachedPlanValid(estate->es_cachedplan);
+}
/* ----------------------------------------------------------------
* ExecProcNode
@@ -589,6 +606,7 @@ extern void ExecCreateScanSlotFromOuterPlan(EState *estate,
extern bool ExecRelationIsTargetRelation(EState *estate, Index scanrelid);
extern Relation ExecOpenScanRelation(EState *estate, Index scanrelid, int eflags);
+extern Relation ExecOpenScanIndexRelation(EState *estate, Oid indexid, int lockmode);
extern void ExecInitRangeTable(EState *estate, List *rangeTable, List *permInfos);
extern void ExecCloseRangeTableRelations(EState *estate);
diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h
index ee089505a0..2a8e5bd784 100644
--- a/src/include/nodes/execnodes.h
+++ b/src/include/nodes/execnodes.h
@@ -680,6 +680,7 @@ typedef struct EState
int es_top_eflags; /* eflags passed to ExecutorStart */
int es_instrument; /* OR of InstrumentOption flags */
bool es_finished; /* true when ExecutorFinish is done */
+ bool es_aborted; /* true when execution was aborted */
List *es_exprcontexts; /* List of ExprContexts within EState */
diff --git a/src/include/utils/plancache.h b/src/include/utils/plancache.h
index 0b5ee007ca..154f68f671 100644
--- a/src/include/utils/plancache.h
+++ b/src/include/utils/plancache.h
@@ -18,6 +18,7 @@
#include "access/tupdesc.h"
#include "lib/ilist.h"
#include "nodes/params.h"
+#include "nodes/parsenodes.h"
#include "tcop/cmdtag.h"
#include "utils/queryenvironment.h"
#include "utils/resowner.h"
@@ -152,6 +153,8 @@ typedef struct CachedPlan
bool is_generic; /* is it a reusable generic plan? */
bool is_saved; /* is CachedPlan in a long-lived context? */
bool is_valid; /* is the stmt_list currently valid? */
+ bool is_standalone; /* is it not associated with a
+ * CachedPlanSource? */
Oid planRoleId; /* Role ID the plan was created for */
bool dependsOnRole; /* is plan specific to that role? */
TransactionId saved_xmin; /* if valid, replan when TransactionXmin
@@ -159,6 +162,12 @@ typedef struct CachedPlan
int generation; /* parent's generation number for this plan */
int refcount; /* count of live references to this struct */
MemoryContext context; /* context containing this CachedPlan */
+
+ /*
+ * If the plan is not associated with a CachedPlanSource, it is saved in
+ * a separate global list.
+ */
+ dlist_node node; /* list link, if is_standalone */
} CachedPlan;
/*
@@ -224,6 +233,10 @@ extern CachedPlan *GetCachedPlan(CachedPlanSource *plansource,
ParamListInfo boundParams,
ResourceOwner owner,
QueryEnvironment *queryEnv);
+extern CachedPlan *GetSingleCachedPlan(CachedPlanSource *plansource,
+ int query_index,
+ QueryEnvironment *queryEnv);
+
extern void ReleaseCachedPlan(CachedPlan *plan, ResourceOwner owner);
extern bool CachedPlanAllowsSimpleValidityCheck(CachedPlanSource *plansource,
@@ -245,4 +258,17 @@ CachedPlanRequiresLocking(CachedPlan *cplan)
return cplan->is_generic;
}
+/*
+ * CachedPlanValid
+ * Returns whether a cached generic plan is still valid.
+ *
+ * Invoked by the executor to check if the plan has not been invalidated after
+ * taking locks during the initialization of the plan.
+ */
+static inline bool
+CachedPlanValid(CachedPlan *cplan)
+{
+ return cplan->is_valid;
+}
+
#endif /* PLANCACHE_H */
diff --git a/src/include/utils/portal.h b/src/include/utils/portal.h
index 29f49829f2..58c3828d2c 100644
--- a/src/include/utils/portal.h
+++ b/src/include/utils/portal.h
@@ -138,6 +138,7 @@ typedef struct PortalData
QueryCompletion qc; /* command completion data for executed query */
List *stmts; /* list of PlannedStmts */
CachedPlan *cplan; /* CachedPlan, if stmts are from one */
+ CachedPlanSource *plansource; /* CachedPlanSource, for cplan */
ParamListInfo portalParams; /* params to pass to query */
QueryEnvironment *queryEnv; /* environment for query */
@@ -241,7 +242,8 @@ extern void PortalDefineQuery(Portal portal,
const char *sourceText,
CommandTag commandTag,
List *stmts,
- CachedPlan *cplan);
+ CachedPlan *cplan,
+ CachedPlanSource *plansource);
extern PlannedStmt *PortalGetPrimaryStmt(Portal portal);
extern void PortalCreateHoldStore(Portal portal);
extern void PortalHashTableDeleteAll(void);
diff --git a/src/test/modules/delay_execution/Makefile b/src/test/modules/delay_execution/Makefile
index 70f24e846d..3eeb097fde 100644
--- a/src/test/modules/delay_execution/Makefile
+++ b/src/test/modules/delay_execution/Makefile
@@ -8,7 +8,8 @@ OBJS = \
delay_execution.o
ISOLATION = partition-addition \
- partition-removal-1
+ partition-removal-1 \
+ cached-plan-inval
ifdef USE_PGXS
PG_CONFIG = pg_config
diff --git a/src/test/modules/delay_execution/delay_execution.c b/src/test/modules/delay_execution/delay_execution.c
index 155c8a8d55..304ca77f7b 100644
--- a/src/test/modules/delay_execution/delay_execution.c
+++ b/src/test/modules/delay_execution/delay_execution.c
@@ -1,14 +1,18 @@
/*-------------------------------------------------------------------------
*
* delay_execution.c
- * Test module to allow delay between parsing and execution of a query.
+ * Test module to introduce delay at various points during execution of a
+ * query to test that execution proceeds safely in light of concurrent
+ * changes.
*
* The delay is implemented by taking and immediately releasing a specified
* advisory lock. If another process has previously taken that lock, the
* current process will be blocked until the lock is released; otherwise,
* there's no effect. This allows an isolationtester script to reliably
- * test behaviors where some specified action happens in another backend
- * between parsing and execution of any desired query.
+ * test behaviors where some specified action happens in another backend in
+ * a couple of cases: 1) between parsing and execution of any desired query
+ * when using the planner_hook, 2) between RevalidateCachedQuery() and
+ * ExecutorStart() when using the ExecutorStart_hook.
*
* Copyright (c) 2020-2024, PostgreSQL Global Development Group
*
@@ -22,6 +26,7 @@
#include <limits.h>
+#include "executor/executor.h"
#include "optimizer/planner.h"
#include "utils/builtins.h"
#include "utils/guc.h"
@@ -32,9 +37,11 @@ PG_MODULE_MAGIC;
/* GUC: advisory lock ID to use. Zero disables the feature. */
static int post_planning_lock_id = 0;
+static int executor_start_lock_id = 0;
-/* Save previous planner hook user to be a good citizen */
+/* Save previous hook users to be a good citizen */
static planner_hook_type prev_planner_hook = NULL;
+static ExecutorStart_hook_type prev_ExecutorStart_hook = NULL;
/* planner_hook function to provide the desired delay */
@@ -70,11 +77,41 @@ delay_execution_planner(Query *parse, const char *query_string,
return result;
}
+/* ExecutorStart_hook function to provide the desired delay */
+static void
+delay_execution_ExecutorStart(QueryDesc *queryDesc, int eflags)
+{
+ /* If enabled, delay by taking and releasing the specified lock */
+ if (executor_start_lock_id != 0)
+ {
+ DirectFunctionCall1(pg_advisory_lock_int8,
+ Int64GetDatum((int64) executor_start_lock_id));
+ DirectFunctionCall1(pg_advisory_unlock_int8,
+ Int64GetDatum((int64) executor_start_lock_id));
+
+ /*
+ * Ensure that we notice any pending invalidations, since the advisory
+ * lock functions don't do this.
+ */
+ AcceptInvalidationMessages();
+ }
+
+ /* Now start the executor, possibly via a previous hook user */
+ if (prev_ExecutorStart_hook)
+ prev_ExecutorStart_hook(queryDesc, eflags);
+ else
+ standard_ExecutorStart(queryDesc, eflags);
+
+ if (executor_start_lock_id != 0)
+ elog(NOTICE, "Finished ExecutorStart(): CachedPlan is %s",
+ CachedPlanValid(queryDesc->cplan) ? "valid" : "not valid");
+}
+
/* Module load function */
void
_PG_init(void)
{
- /* Set up the GUC to control which lock is used */
+ /* Set up GUCs to control which lock is used */
DefineCustomIntVariable("delay_execution.post_planning_lock_id",
"Sets the advisory lock ID to be locked/unlocked after planning.",
"Zero disables the delay.",
@@ -86,10 +123,22 @@ _PG_init(void)
NULL,
NULL,
NULL);
-
+ DefineCustomIntVariable("delay_execution.executor_start_lock_id",
+ "Sets the advisory lock ID to be locked/unlocked before starting execution.",
+ "Zero disables the delay.",
+ &executor_start_lock_id,
+ 0,
+ 0, INT_MAX,
+ PGC_USERSET,
+ 0,
+ NULL,
+ NULL,
+ NULL);
MarkGUCPrefixReserved("delay_execution");
- /* Install our hook */
+ /* Install our hooks. */
prev_planner_hook = planner_hook;
planner_hook = delay_execution_planner;
+ prev_ExecutorStart_hook = ExecutorStart_hook;
+ ExecutorStart_hook = delay_execution_ExecutorStart;
}
diff --git a/src/test/modules/delay_execution/expected/cached-plan-inval.out b/src/test/modules/delay_execution/expected/cached-plan-inval.out
new file mode 100644
index 0000000000..e8efb6d9d9
--- /dev/null
+++ b/src/test/modules/delay_execution/expected/cached-plan-inval.out
@@ -0,0 +1,175 @@
+Parsed test spec with 2 sessions
+
+starting permutation: s1prep s2lock s1exec s2dropi s2unlock
+step s1prep: SET plan_cache_mode = force_generic_plan;
+ PREPARE q AS SELECT * FROM foov WHERE a = $1 FOR UPDATE;
+ EXPLAIN (COSTS OFF) EXECUTE q (1);
+QUERY PLAN
+------------------------------------------------
+LockRows
+ -> Append
+ Subplans Removed: 2
+ -> Bitmap Heap Scan on foo12_1 foo_1
+ Recheck Cond: (a = $1)
+ -> Bitmap Index Scan on foo12_1_a
+ Index Cond: (a = $1)
+(7 rows)
+
+step s2lock: SELECT pg_advisory_lock(12345);
+pg_advisory_lock
+----------------
+
+(1 row)
+
+step s1exec: LOAD 'delay_execution';
+ SET delay_execution.executor_start_lock_id = 12345;
+ EXPLAIN (COSTS OFF) EXECUTE q (1); <waiting ...>
+step s2dropi: DROP INDEX foo12_1_a;
+step s2unlock: SELECT pg_advisory_unlock(12345);
+pg_advisory_unlock
+------------------
+t
+(1 row)
+
+step s1exec: <... completed>
+s1: NOTICE: Finished ExecutorStart(): CachedPlan is not valid
+s1: NOTICE: Finished ExecutorStart(): CachedPlan is valid
+QUERY PLAN
+-------------------------------------
+LockRows
+ -> Append
+ Subplans Removed: 2
+ -> Seq Scan on foo12_1 foo_1
+ Filter: (a = $1)
+(5 rows)
+
+
+starting permutation: s1prep2 s2lock s1exec2 s2dropi s2unlock
+step s1prep2: SET plan_cache_mode = force_generic_plan;
+ PREPARE q2 AS SELECT * FROM foov WHERE a = one() or a = two();
+ EXPLAIN (COSTS OFF) EXECUTE q2;
+s1: NOTICE: Finished ExecutorStart(): CachedPlan is valid
+QUERY PLAN
+--------------------------------------------------
+Append
+ Subplans Removed: 1
+ -> Bitmap Heap Scan on foo12_1 foo_1
+ Recheck Cond: ((a = one()) OR (a = two()))
+ -> BitmapOr
+ -> Bitmap Index Scan on foo12_1_a
+ Index Cond: (a = one())
+ -> Bitmap Index Scan on foo12_1_a
+ Index Cond: (a = two())
+ -> Seq Scan on foo12_2 foo_2
+ Filter: ((a = one()) OR (a = two()))
+(11 rows)
+
+step s2lock: SELECT pg_advisory_lock(12345);
+pg_advisory_lock
+----------------
+
+(1 row)
+
+step s1exec2: LOAD 'delay_execution';
+ SET delay_execution.executor_start_lock_id = 12345;
+ EXPLAIN (COSTS OFF) EXECUTE q2; <waiting ...>
+step s2dropi: DROP INDEX foo12_1_a;
+step s2unlock: SELECT pg_advisory_unlock(12345);
+pg_advisory_unlock
+------------------
+t
+(1 row)
+
+step s1exec2: <... completed>
+s1: NOTICE: Finished ExecutorStart(): CachedPlan is not valid
+s1: NOTICE: Finished ExecutorStart(): CachedPlan is valid
+QUERY PLAN
+--------------------------------------------
+Append
+ Subplans Removed: 1
+ -> Seq Scan on foo12_1 foo_1
+ Filter: ((a = one()) OR (a = two()))
+ -> Seq Scan on foo12_2 foo_2
+ Filter: ((a = one()) OR (a = two()))
+(6 rows)
+
+
+starting permutation: s1prep3 s2lock s1exec3 s2dropi s2unlock
+step s1prep3: SET plan_cache_mode = force_generic_plan;
+ PREPARE q3 AS UPDATE foov SET a = a WHERE a = one() or a = two();
+ EXPLAIN (COSTS OFF) EXECUTE q3;
+s1: NOTICE: Finished ExecutorStart(): CachedPlan is valid
+s1: NOTICE: Finished ExecutorStart(): CachedPlan is valid
+QUERY PLAN
+--------------------------------------------------------
+Append
+ Subplans Removed: 1
+ -> Bitmap Heap Scan on foo12_1 foo_1
+ Recheck Cond: ((a = one()) OR (a = two()))
+ -> BitmapOr
+ -> Bitmap Index Scan on foo12_1_a
+ Index Cond: (a = one())
+ -> Bitmap Index Scan on foo12_1_a
+ Index Cond: (a = two())
+ -> Seq Scan on foo12_2 foo_2
+ Filter: ((a = one()) OR (a = two()))
+
+Update on foo
+ Update on foo12_1 foo_1
+ Update on foo12_2 foo_2
+ Update on foo3 foo
+ -> Append
+ Subplans Removed: 1
+ -> Bitmap Heap Scan on foo12_1 foo_1
+ Recheck Cond: ((a = one()) OR (a = two()))
+ -> BitmapOr
+ -> Bitmap Index Scan on foo12_1_a
+ Index Cond: (a = one())
+ -> Bitmap Index Scan on foo12_1_a
+ Index Cond: (a = two())
+ -> Seq Scan on foo12_2 foo_2
+ Filter: ((a = one()) OR (a = two()))
+(27 rows)
+
+step s2lock: SELECT pg_advisory_lock(12345);
+pg_advisory_lock
+----------------
+
+(1 row)
+
+step s1exec3: LOAD 'delay_execution';
+ SET delay_execution.executor_start_lock_id = 12345;
+ EXPLAIN (COSTS OFF) EXECUTE q3; <waiting ...>
+step s2dropi: DROP INDEX foo12_1_a;
+step s2unlock: SELECT pg_advisory_unlock(12345);
+pg_advisory_unlock
+------------------
+t
+(1 row)
+
+step s1exec3: <... completed>
+s1: NOTICE: Finished ExecutorStart(): CachedPlan is not valid
+s1: NOTICE: Finished ExecutorStart(): CachedPlan is valid
+s1: NOTICE: Finished ExecutorStart(): CachedPlan is not valid
+s1: NOTICE: Finished ExecutorStart(): CachedPlan is valid
+QUERY PLAN
+--------------------------------------------------
+Append
+ Subplans Removed: 1
+ -> Seq Scan on foo12_1 foo_1
+ Filter: ((a = one()) OR (a = two()))
+ -> Seq Scan on foo12_2 foo_2
+ Filter: ((a = one()) OR (a = two()))
+
+Update on foo
+ Update on foo12_1 foo_1
+ Update on foo12_2 foo_2
+ Update on foo3 foo
+ -> Append
+ Subplans Removed: 1
+ -> Seq Scan on foo12_1 foo_1
+ Filter: ((a = one()) OR (a = two()))
+ -> Seq Scan on foo12_2 foo_2
+ Filter: ((a = one()) OR (a = two()))
+(17 rows)
+
diff --git a/src/test/modules/delay_execution/meson.build b/src/test/modules/delay_execution/meson.build
index 41f3ac0b89..5a70b183d0 100644
--- a/src/test/modules/delay_execution/meson.build
+++ b/src/test/modules/delay_execution/meson.build
@@ -24,6 +24,7 @@ tests += {
'specs': [
'partition-addition',
'partition-removal-1',
+ 'cached-plan-inval',
],
},
}
diff --git a/src/test/modules/delay_execution/specs/cached-plan-inval.spec b/src/test/modules/delay_execution/specs/cached-plan-inval.spec
new file mode 100644
index 0000000000..5b1f72b4a8
--- /dev/null
+++ b/src/test/modules/delay_execution/specs/cached-plan-inval.spec
@@ -0,0 +1,65 @@
+# Test to check that invalidation of cached generic plans during ExecutorStart
+# correctly triggers replanning and re-execution.
+
+setup
+{
+ CREATE TABLE foo (a int, b text) PARTITION BY LIST(a);
+ CREATE TABLE foo12 PARTITION OF foo FOR VALUES IN (1, 2) PARTITION BY LIST (a);
+ CREATE TABLE foo12_1 PARTITION OF foo12 FOR VALUES IN (1);
+ CREATE TABLE foo12_2 PARTITION OF foo12 FOR VALUES IN (2);
+ CREATE INDEX foo12_1_a ON foo12_1 (a);
+ CREATE TABLE foo3 PARTITION OF foo FOR VALUES IN (3);
+ CREATE VIEW foov AS SELECT * FROM foo;
+ CREATE FUNCTION one () RETURNS int AS $$ BEGIN RETURN 1; END; $$ LANGUAGE PLPGSQL STABLE;
+ CREATE FUNCTION two () RETURNS int AS $$ BEGIN RETURN 2; END; $$ LANGUAGE PLPGSQL STABLE;
+ CREATE RULE update_foo AS ON UPDATE TO foo DO ALSO SELECT 1;
+}
+
+teardown
+{
+ DROP VIEW foov;
+ DROP RULE update_foo ON foo;
+ DROP TABLE foo;
+ DROP FUNCTION one(), two();
+}
+
+session "s1"
+# Append with run-time pruning
+step "s1prep" { SET plan_cache_mode = force_generic_plan;
+ PREPARE q AS SELECT * FROM foov WHERE a = $1 FOR UPDATE;
+ EXPLAIN (COSTS OFF) EXECUTE q (1); }
+
+# Another case with Append with run-time pruning
+step "s1prep2" { SET plan_cache_mode = force_generic_plan;
+ PREPARE q2 AS SELECT * FROM foov WHERE a = one() or a = two();
+ EXPLAIN (COSTS OFF) EXECUTE q2; }
+
+# Case with a rule adding another query
+step "s1prep3" { SET plan_cache_mode = force_generic_plan;
+ PREPARE q3 AS UPDATE foov SET a = a WHERE a = one() or a = two();
+ EXPLAIN (COSTS OFF) EXECUTE q3; }
+
+# Executes a generic plan
+step "s1exec" { LOAD 'delay_execution';
+ SET delay_execution.executor_start_lock_id = 12345;
+ EXPLAIN (COSTS OFF) EXECUTE q (1); }
+step "s1exec2" { LOAD 'delay_execution';
+ SET delay_execution.executor_start_lock_id = 12345;
+ EXPLAIN (COSTS OFF) EXECUTE q2; }
+step "s1exec3" { LOAD 'delay_execution';
+ SET delay_execution.executor_start_lock_id = 12345;
+ EXPLAIN (COSTS OFF) EXECUTE q3; }
+
+session "s2"
+step "s2lock" { SELECT pg_advisory_lock(12345); }
+step "s2unlock" { SELECT pg_advisory_unlock(12345); }
+step "s2dropi" { DROP INDEX foo12_1_a; }
+
+# While "s1exec", etc. wait to acquire the advisory lock, "s2drop" is able to
+# drop the index being used in the cached plan. When "s1exec" is then
+# unblocked and initializes the cached plan for execution, it detects the
+# concurrent index drop and causes the cached plan to be discarded and
+# recreated without the index.
+permutation "s1prep" "s2lock" "s1exec" "s2dropi" "s2unlock"
+permutation "s1prep2" "s2lock" "s1exec2" "s2dropi" "s2unlock"
+permutation "s1prep3" "s2lock" "s1exec3" "s2dropi" "s2unlock"
--
2.43.0
view thread (29+ 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: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: generic plans and "initial" pruning
In-Reply-To: <CA+HiwqGSOge3eT3kcm_nxCSA3Ut+d0jtchi8g8J9uXi-kyC7Jw@mail.gmail.com>
* 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