public inbox for [email protected]
help / color / mirror / Atom feedFrom: Amit Langote <[email protected]>
To: 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: Jacob Champion <[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, 15 Aug 2024 21:57:40 +0900
Message-ID: <CA+HiwqHL=aGU9Y4RYXQ5VCp4L5NVdiaQLLoXN3NCQQQMKo0ByQ@mail.gmail.com> (raw)
In-Reply-To: <CA+TgmobHL_vTjOdy6KVMVeW-CQQmXXz_yU6Q9d2YjnVfFxuy6A@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>
On Thu, Aug 15, 2024 at 4:23 AM Robert Haas <[email protected]> wrote:
> On Mon, Aug 12, 2024 at 8:54 AM Amit Langote <[email protected]> wrote:
> > 1. I went through many iterations of the changes to ExecInitNode() to
> > return a partially initialized PlanState tree when it detects that the
> > CachedPlan was invalidated after locking a child table and to
> > ExecEndNode() to account for the PlanState tree sometimes being
> > partially initialized, but it still seems fragile and bug-prone to me.
> > It might be because this approach is fundamentally hard to get right
> > or I haven't invested enough effort in becoming more confident in its
> > robustness.
>
> Can you give some examples of what's going wrong, or what you think
> might go wrong?
>
> I didn't think there was a huge problem here based on previous
> discussion, but I could very well be missing some important challenge.
TBH, it's more of a hunch that people who are not involved in this
development might find the new reality, whereby the execution is not
racefree until ExecutorRun(), hard to reason about.
That's perhaps true with the other approach too whereby one would need
to consult a separate data structure that records the result of
pruning done in plancache.c to be sure if a given node of the plan
tree coming from a CachedPlan is safe to execute or do something with.
> > 2. Refactoring needed due to the ExecutorStart() API change especially
> > that pertaining to portals does not seem airtight. I'm especially
> > worried about moving the ExecutorStart() call for the
> > PORTAL_MULTI_QUERY case from where it is currently to PortalStart().
> > That requires additional bookkeeping in PortalData and I am not
> > totally sure that the snapshot handling changes after that move are
> > entirely correct.
>
> Here again, it would help to see exactly what you had to do and what
> consequences you think it might have. But it sounds like you're
> talking about moving ExecutorStart() from PortalStart() to PortalRun()
> and I agree that sounds like it might have user-visible behavioral
> consequences that we don't want.
Let's specifically looks at this block of code in PortalRunMulti():
/*
* Must always have a snapshot for plannable queries. First time
* through, take a new snapshot; for subsequent queries in the
* same portal, just update the snapshot's copy of the command
* counter.
*/
if (!active_snapshot_set)
{
Snapshot snapshot = GetTransactionSnapshot();
/* If told to, register the snapshot and save in portal */
if (setHoldSnapshot)
{
snapshot = RegisterSnapshot(snapshot);
portal->holdSnapshot = snapshot;
}
/*
* We can't have the holdSnapshot also be the active one,
* because UpdateActiveSnapshotCommandId would complain. So
* force an extra snapshot copy. Plain PushActiveSnapshot
* would have copied the transaction snapshot anyway, so this
* only adds a copy step when setHoldSnapshot is true. (It's
* okay for the command ID of the active snapshot to diverge
* from what holdSnapshot has.)
*/
PushCopiedSnapshot(snapshot);
/*
* As for PORTAL_ONE_SELECT portals, it does not seem
* necessary to maintain portal->portalSnapshot here.
*/
active_snapshot_set = true;
}
else
UpdateActiveSnapshotCommandId();
Without the patch, the code immediately following this does a
CreateQueryDesc(), which "registers" the above copied snapshot,
followed by ExecutorStart() immediately followed by ExecutorRun(), for
each query in the list for the PORTAL_RUN_MULTI case.
With the patch, CreateQueryDesc() and ExecutorStart() are moved to
PortalStart() so that QueryDescs including the PlanState trees for all
queries are built before any is run. Why? So that if ExecutorStart()
fails for any query in the list, we can simply throw out the QueryDesc
and the PlanState trees of the previous queries (NOT run them) and ask
plancache for a new CachedPlan for the list of queries. We don't have
a way to ask plancache.c to replan only a given query in the list.
Because of that reshuffling, the above block also needed to be moved
to PortalStart() along with the CommandCounterIncrement() between
queries. That requires the following non-trivial changes:
* A copy of the snapshot needs to be created for each statement after
the 1st one to be able to perform UpdateActiveSnapshotCommandId() on
it.
* In PortalRunMulti(), PushActiveSnapshot() must now be done for every
query because the executor expects the copy in the given query's
QueryDesc to match the ActiveSnapshot.
* There's no longer CCI() between queries in PortalRunMulti() because
the snapshots in each query's QueryDesc must have been adjusted to
reflect the correct command counter. I've checked but can't really be
sure if the value in the snapshot is all anyone ever uses if they want
to know the current value of the command counter.
There is likely to be performance regression for the multi-query cases
due to this handling of snapshots and maybe even correctness issues.
> > 3. The need to add *back* the fields to store the RT indexes of
> > relations that are not looked at by ExecInitNode() traversal such as
> > root partitioned tables and non-leaf partitions.
>
> I don't remember exactly why we removed those or what the benefit was,
> so I'm not sure how big of a problem it is if we have to put them
> back.
We removed those in commit 52ed730d511b after commit f2343653f5b2
removed redundant execution-time locking of non-leaf relations. So we
removed them because we realized that execution time locking is
unnecessary given that AcquireExecutorLocks() exists and now we want
to add them back because we'd like to get rid of
AcquireExecutorLocks(). :-)
I'm attaching a rebased version of the patch that implements the
current design because the cfbot has been broken for a while and also
in case you or anyone else wants to take another look. I've combined
2 patches into one -- one that dealt with the executor side changes to
account for locking and another that dealt with caller side changes to
handle executor returning when the CachedPlan becomes invalid.
--
Thanks, Amit Langote
Attachments:
[application/octet-stream] v50-0002-Add-field-to-store-parent-relids-to-Append-Merge.patch (24.5K, 2-v50-0002-Add-field-to-store-parent-relids-to-Append-Merge.patch)
download | inline diff:
From f1ad8bea2621c5a7074742516b248c9efe9dd17e Mon Sep 17 00:00:00 2001
From: Amit Langote <[email protected]>
Date: Wed, 6 Sep 2023 17:54:02 +0900
Subject: [PATCH v50 2/6] Add field to store parent relids to
Append/MergeAppend
There's no way currently in the executor to tell if the child
subplans of Append/MergeAppend are scanning partitions, and if
they indeed do, what the RT indexes of their parent/ancestor tables
are. Executor doesn't need to see those RT indexes except for
run-time pruning, in which case they can can be found in the
PartitionPruneInfo. An upcoming commit will create a need for them
to be available for the purpose of locking those parent/ancestor
tables when executing a cached plan, so add a
field called allpartrelids to Append/MergeAppend to store those
RT indexes.
In the cases where an Append/MergeAppend node containing parent
RT indexes is eligible for elision in
set_{append|mergeappend}_references(), those RT indexes are now
transferred into PlannedStmt.elidedAppendPartRels.
The code to look up partitioned parent relids for a given list of
partition scan subpaths of an Append/MergeAppend is already present
in make_partition_pruneinfo() but it's local to partprune.c. This
commit refactors that code into its own function called
add_append_subpath_partrelids() defined in appendinfo.c and
generalizes it to consider child join and aggregation paths. To
facilitate looking up of parent rels of child grouping rels in
add_append_subpath_partrelids(), parent links are now also set in
the RelOptInfos of child grouping rels too, like they are in
those of child base and join rels.
Discussion: https://postgr.es/m/CA+HiwqFGkMSge6TgC9KQzde0ohpAycLQuV7ooitEEpbKB0O_mg@mail.gmail.com
---
src/backend/executor/execParallel.c | 1 +
src/backend/optimizer/plan/createplan.c | 41 ++++++--
src/backend/optimizer/plan/planner.c | 5 +
src/backend/optimizer/plan/setrefs.c | 22 ++++
src/backend/optimizer/util/appendinfo.c | 127 ++++++++++++++++++++++++
src/backend/partitioning/partprune.c | 124 +++--------------------
src/include/nodes/pathnodes.h | 3 +
src/include/nodes/plannodes.h | 17 ++++
src/include/optimizer/appendinfo.h | 3 +
src/include/partitioning/partprune.h | 3 +-
10 files changed, 223 insertions(+), 123 deletions(-)
diff --git a/src/backend/executor/execParallel.c b/src/backend/executor/execParallel.c
index bfb3419efb..f995714d7f 100644
--- a/src/backend/executor/execParallel.c
+++ b/src/backend/executor/execParallel.c
@@ -185,6 +185,7 @@ ExecSerializePlan(Plan *plan, EState *estate)
pstmt->permInfos = estate->es_rteperminfos;
pstmt->resultRelations = NIL;
pstmt->appendRelations = NIL;
+ pstmt->elidedAppendPartRels = NIL;
/*
* Transfer only parallel-safe subplans, leaving a NULL "hole" in the list
diff --git a/src/backend/optimizer/plan/createplan.c b/src/backend/optimizer/plan/createplan.c
index 28addc1129..49c193c237 100644
--- a/src/backend/optimizer/plan/createplan.c
+++ b/src/backend/optimizer/plan/createplan.c
@@ -25,6 +25,7 @@
#include "nodes/extensible.h"
#include "nodes/makefuncs.h"
#include "nodes/nodeFuncs.h"
+#include "optimizer/appendinfo.h"
#include "optimizer/clauses.h"
#include "optimizer/cost.h"
#include "optimizer/optimizer.h"
@@ -1232,6 +1233,7 @@ create_append_plan(PlannerInfo *root, AppendPath *best_path, int flags)
Oid *nodeCollations = NULL;
bool *nodeNullsFirst = NULL;
bool consider_async = false;
+ List *allpartrelids = NIL;
/*
* The subpaths list could be empty, if every child was proven empty by
@@ -1373,15 +1375,23 @@ create_append_plan(PlannerInfo *root, AppendPath *best_path, int flags)
++nasyncplans;
}
+ /*
+ * Find partitioned parent rel(s) of the subpath's rel(s).
+ */
+ allpartrelids = add_append_subpath_partrelids(root, subpath, rel,
+ allpartrelids);
+
subplans = lappend(subplans, subplan);
}
+ plan->allpartrelids = allpartrelids;
+
/*
- * If any quals exist, they may be useful to perform further partition
- * pruning during execution. Gather information needed by the executor to
- * do partition pruning.
+ * If scanning partitions, check if there are quals that may be useful to
+ * perform further partition pruning during execution. Gather information
+ * needed by the executor to do partition pruning.
*/
- if (enable_partition_pruning)
+ if (enable_partition_pruning && allpartrelids != NIL)
{
List *prunequal;
@@ -1402,7 +1412,8 @@ create_append_plan(PlannerInfo *root, AppendPath *best_path, int flags)
partpruneinfo =
make_partition_pruneinfo(root, rel,
best_path->subpaths,
- prunequal);
+ prunequal,
+ allpartrelids);
}
plan->appendplans = subplans;
@@ -1448,6 +1459,7 @@ create_merge_append_plan(PlannerInfo *root, MergeAppendPath *best_path,
ListCell *subpaths;
RelOptInfo *rel = best_path->path.parent;
PartitionPruneInfo *partpruneinfo = NULL;
+ List *allpartrelids = NIL;
/*
* We don't have the actual creation of the MergeAppend node split out
@@ -1537,15 +1549,23 @@ create_merge_append_plan(PlannerInfo *root, MergeAppendPath *best_path,
subplan = (Plan *) sort;
}
+ /*
+ * Find partitioned parent rel(s) of the subpath's rel(s).
+ */
+ allpartrelids = add_append_subpath_partrelids(root, subpath, rel,
+ allpartrelids);
+
subplans = lappend(subplans, subplan);
}
+ node->allpartrelids = allpartrelids;
+
/*
- * If any quals exist, they may be useful to perform further partition
- * pruning during execution. Gather information needed by the executor to
- * do partition pruning.
+ * If scanning partitions, check if there are quals that may be useful to
+ * perform further partition pruning during execution. Gather information
+ * needed by the executor to do partition pruning.
*/
- if (enable_partition_pruning)
+ if (enable_partition_pruning && allpartrelids != NIL)
{
List *prunequal;
@@ -1557,7 +1577,8 @@ create_merge_append_plan(PlannerInfo *root, MergeAppendPath *best_path,
if (prunequal != NIL)
partpruneinfo = make_partition_pruneinfo(root, rel,
best_path->subpaths,
- prunequal);
+ prunequal,
+ allpartrelids);
}
node->mergeplans = subplans;
diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c
index 948afd9094..2c2e38f589 100644
--- a/src/backend/optimizer/plan/planner.c
+++ b/src/backend/optimizer/plan/planner.c
@@ -522,6 +522,7 @@ standard_planner(Query *parse, const char *query_string, int cursorOptions,
Assert(glob->finalrowmarks == NIL);
Assert(glob->resultRelations == NIL);
Assert(glob->appendRelations == NIL);
+ Assert(glob->elidedAppendPartRels == NIL);
top_plan = set_plan_references(root, top_plan);
/* ... and the subplans (both regular subplans and initplans) */
Assert(list_length(glob->subplans) == list_length(glob->subroots));
@@ -549,6 +550,7 @@ standard_planner(Query *parse, const char *query_string, int cursorOptions,
result->permInfos = glob->finalrteperminfos;
result->resultRelations = glob->resultRelations;
result->appendRelations = glob->appendRelations;
+ result->elidedAppendPartRels = glob->elidedAppendPartRels;
result->subplans = glob->subplans;
result->rewindPlanIDs = glob->rewindPlanIDs;
result->rowMarks = glob->finalrowmarks;
@@ -7941,8 +7943,11 @@ create_partitionwise_grouping_paths(PlannerInfo *root,
agg_costs, gd, &child_extra,
&child_partially_grouped_rel);
+ /* Mark as child of grouped_rel. */
+ child_grouped_rel->parent = grouped_rel;
if (child_partially_grouped_rel)
{
+ child_partially_grouped_rel->parent = grouped_rel;
partially_grouped_live_children =
lappend(partially_grouped_live_children,
child_partially_grouped_rel);
diff --git a/src/backend/optimizer/plan/setrefs.c b/src/backend/optimizer/plan/setrefs.c
index 7aed84584c..4fc5ed15aa 100644
--- a/src/backend/optimizer/plan/setrefs.c
+++ b/src/backend/optimizer/plan/setrefs.c
@@ -1757,6 +1757,10 @@ set_append_references(PlannerInfo *root,
lfirst(l) = set_plan_refs(root, (Plan *) lfirst(l), rtoffset);
}
+ /* Do this before possibly removing the MergeAppend node below. */
+ foreach(l, aplan->allpartrelids)
+ lfirst(l) = offset_relid_set((Relids) lfirst(l), rtoffset);
+
/*
* See if it's safe to get rid of the Append entirely. For this to be
* safe, there must be only one child plan and that child plan's parallel
@@ -1770,7 +1774,14 @@ set_append_references(PlannerInfo *root,
Plan *p = (Plan *) linitial(aplan->appendplans);
if (p->parallel_aware == aplan->plan.parallel_aware)
+ {
+ if (aplan->allpartrelids)
+ root->glob->elidedAppendPartRels =
+ list_concat(root->glob->elidedAppendPartRels,
+ aplan->allpartrelids);
+
return clean_up_removed_plan_level((Plan *) aplan, p);
+ }
}
/*
@@ -1832,6 +1843,10 @@ set_mergeappend_references(PlannerInfo *root,
lfirst(l) = set_plan_refs(root, (Plan *) lfirst(l), rtoffset);
}
+ /* Do this before possibly removing the MergeAppend node below. */
+ foreach(l, mplan->allpartrelids)
+ lfirst(l) = offset_relid_set((Relids) lfirst(l), rtoffset);
+
/*
* See if it's safe to get rid of the MergeAppend entirely. For this to
* be safe, there must be only one child plan and that child plan's
@@ -1846,7 +1861,14 @@ set_mergeappend_references(PlannerInfo *root,
Plan *p = (Plan *) linitial(mplan->mergeplans);
if (p->parallel_aware == mplan->plan.parallel_aware)
+ {
+ if (mplan->allpartrelids)
+ root->glob->elidedAppendPartRels =
+ list_concat(root->glob->elidedAppendPartRels,
+ mplan->allpartrelids);
+
return clean_up_removed_plan_level((Plan *) mplan, p);
+ }
}
/*
diff --git a/src/backend/optimizer/util/appendinfo.c b/src/backend/optimizer/util/appendinfo.c
index 4989722637..0569cd00a5 100644
--- a/src/backend/optimizer/util/appendinfo.c
+++ b/src/backend/optimizer/util/appendinfo.c
@@ -1038,3 +1038,130 @@ distribute_row_identity_vars(PlannerInfo *root)
}
}
}
+
+/*
+ * add_append_subpath_partrelids
+ * Look up a child subpath's rel's partitioned parent relids up to
+ * parentrel and add the bitmapset containing those into
+ * 'allpartrelids'
+ */
+List *
+add_append_subpath_partrelids(PlannerInfo *root, Path *subpath,
+ RelOptInfo *parentrel,
+ List *allpartrelids)
+{
+ RelOptInfo *pathrel = subpath->parent;
+ Relids partrelids = NULL;
+ Index top_parent;
+ ListCell *lc;
+
+ /* Nothing to do if there's no parent to begin with. */
+ if (!IS_OTHER_REL(pathrel))
+ return allpartrelids;
+
+ /*
+ * Traverse up to the pathrel's topmost partitioned parent, collecting
+ * parent relids as we go; but stop if we reach parentrel. (Normally, a
+ * pathrel's topmost partitioned parent is either parentrel or a UNION ALL
+ * appendrel child of parentrel. But when handling partitionwise joins of
+ * multi-level partitioning trees, we can see an append path whose
+ * parentrel is an intermediate partitioned table.)
+ */
+ do
+ {
+ Relids parent_relids = NULL;
+
+ /*
+ * For simple child rels, we can simply set the parent_relids to
+ * pathrel->parent->relids. But for partitionwise join and aggregate
+ * child rels, while we can use pathrel->parent to move up the tree,
+ * parent_relids must be found the hard way through AppendInfoInfos,
+ * because 1) a joinrel's relids may point to RTE_JOIN entries,
+ * 2) topmost parent grouping rel's relids field is NULL.
+ */
+ if (IS_SIMPLE_REL(pathrel))
+ {
+ pathrel = pathrel->parent;
+ /* Stop once we reach the root partitioned rel. */
+ if (!IS_PARTITIONED_REL(pathrel))
+ break;
+ parent_relids = bms_add_members(parent_relids, pathrel->relids);
+ }
+ else
+ {
+ AppendRelInfo **appinfos;
+ int nappinfos,
+ i;
+
+ appinfos = find_appinfos_by_relids(root, pathrel->relids,
+ &nappinfos);
+ for (i = 0; i < nappinfos; i++)
+ {
+ AppendRelInfo *appinfo = appinfos[i];
+
+ parent_relids = bms_add_member(parent_relids,
+ appinfo->parent_relid);
+ }
+ pfree(appinfos);
+ pathrel = pathrel->parent;
+ }
+ /* accept this level as an interesting parent */
+ partrelids = bms_add_members(partrelids, parent_relids);
+ if (pathrel == parentrel)
+ break; /* don't traverse above parentrel */
+ } while (IS_OTHER_REL(pathrel));
+
+ if (partrelids == NULL)
+ return allpartrelids;
+
+ /*
+ * Append the 'partrelids' RT index bitmapset to 'allpartrelids' or
+ * merge the RT indexes into an appropriate bitmapset already present
+ * in the list
+ *
+ * Within 'allpartrelids', there is one Bitmapset for each topmost parent
+ * partitioned rel mentioned in the query whose children's subpaths have
+ * been passed to add_append_subpath_partrelids. Each Bitmapset contains
+ * the RT indexes of the topmost parent as well as its relevant non-leaf
+ * child partitions. Since (by construction of the rangetable list) parent
+ * partitions must have lower RT indexes than their children, we can
+ * distinguish the topmost parent as being the lowest set bit in the
+ * Bitmapset.
+ *
+ * Note that the list contains only RT indexes of partitioned tables that
+ * are parents of some scan-level relation appearing in the 'subpaths' that
+ * add_append_subpath_partrelids() is dealing with. Also, "topmost"
+ * parents are not allowed to be higher than the 'parentrel' associated
+ * with the append path. In this way, we avoid expending cycles on
+ * partitioned rels that can't contribute useful pruning information for
+ * the problem at hand.
+ *
+ * (It is possible for 'parentrel' to be a child partitioned table, and it
+ * is also possible for scan-level relations to be child partitioned tables
+ * rather than leaf partitions. Hence we must construct this relation set
+ * with reference to the particular append path we're dealing with, rather
+ * than looking at the full partitioning structure represented in the
+ * RelOptInfos.)
+ */
+
+ /* We can easily get the lowest set bit this way: */
+ top_parent = bms_next_member(partrelids, -1);
+ Assert(top_parent > 0);
+
+ /* Look for a matching topmost parent */
+ foreach(lc, allpartrelids)
+ {
+ Bitmapset *currpartrelids = (Bitmapset *) lfirst(lc);
+ Index currtarget = bms_next_member(currpartrelids, -1);
+
+ if (top_parent == currtarget)
+ {
+ /* Found a match, so add any new RT indexes to this hierarchy */
+ currpartrelids = bms_add_members(currpartrelids, partrelids);
+ lfirst(lc) = currpartrelids;
+ return allpartrelids;
+ }
+ }
+ /* No match, so add the new partition hierarchy to the list */
+ return lappend(allpartrelids, partrelids);
+}
diff --git a/src/backend/partitioning/partprune.c b/src/backend/partitioning/partprune.c
index 9a1a7faac7..2afc10c40b 100644
--- a/src/backend/partitioning/partprune.c
+++ b/src/backend/partitioning/partprune.c
@@ -137,7 +137,6 @@ typedef struct PruneStepResult
} PruneStepResult;
-static List *add_part_relids(List *allpartrelids, Bitmapset *partrelids);
static List *make_partitionedrel_pruneinfo(PlannerInfo *root,
RelOptInfo *parentrel,
List *prunequal,
@@ -215,33 +214,32 @@ static void partkey_datum_from_expr(PartitionPruneContext *context,
* of scan paths for its child rels.
* 'prunequal' is a list of potential pruning quals (i.e., restriction
* clauses that are applicable to the appendrel).
+ * 'allpartrelids' contains Bitmapsets of RT indexes of partitioned parents
+ * whose partitions' Paths are in 'subpaths'; there's one Bitmapset for every
+ * partition tree involved.
*/
PartitionPruneInfo *
make_partition_pruneinfo(PlannerInfo *root, RelOptInfo *parentrel,
List *subpaths,
- List *prunequal)
+ List *prunequal,
+ List *allpartrelids)
{
PartitionPruneInfo *pruneinfo;
Bitmapset *allmatchedsubplans = NULL;
- List *allpartrelids;
List *prunerelinfos;
int *relid_subplan_map;
ListCell *lc;
int i;
+ Assert(list_length(allpartrelids) > 0);
+
/*
- * Scan the subpaths to see which ones are scans of partition child
- * relations, and identify their parent partitioned rels. (Note: we must
- * restrict the parent partitioned rels to be parentrel or children of
- * parentrel, otherwise we couldn't translate prunequal to match.)
- *
- * Also construct a temporary array to map from partition-child-relation
- * relid to the index in 'subpaths' of the scan plan for that partition.
+ * Construct a temporary array to map from partition-child-relation relid
+ * to the index in 'subpaths' of the scan plan for that partition.
* (Use of "subplan" rather than "subpath" is a bit of a misnomer, but
* we'll let it stand.) For convenience, we use 1-based indexes here, so
* that zero can represent an un-filled array entry.
*/
- allpartrelids = NIL;
relid_subplan_map = palloc0(sizeof(int) * root->simple_rel_array_size);
i = 1;
@@ -250,50 +248,9 @@ make_partition_pruneinfo(PlannerInfo *root, RelOptInfo *parentrel,
Path *path = (Path *) lfirst(lc);
RelOptInfo *pathrel = path->parent;
- /* We don't consider partitioned joins here */
- if (pathrel->reloptkind == RELOPT_OTHER_MEMBER_REL)
- {
- RelOptInfo *prel = pathrel;
- Bitmapset *partrelids = NULL;
-
- /*
- * Traverse up to the pathrel's topmost partitioned parent,
- * collecting parent relids as we go; but stop if we reach
- * parentrel. (Normally, a pathrel's topmost partitioned parent
- * is either parentrel or a UNION ALL appendrel child of
- * parentrel. But when handling partitionwise joins of
- * multi-level partitioning trees, we can see an append path whose
- * parentrel is an intermediate partitioned table.)
- */
- do
- {
- AppendRelInfo *appinfo;
-
- Assert(prel->relid < root->simple_rel_array_size);
- appinfo = root->append_rel_array[prel->relid];
- prel = find_base_rel(root, appinfo->parent_relid);
- if (!IS_PARTITIONED_REL(prel))
- break; /* reached a non-partitioned parent */
- /* accept this level as an interesting parent */
- partrelids = bms_add_member(partrelids, prel->relid);
- if (prel == parentrel)
- break; /* don't traverse above parentrel */
- } while (prel->reloptkind == RELOPT_OTHER_MEMBER_REL);
-
- if (partrelids)
- {
- /*
- * Found some relevant parent partitions, which may or may not
- * overlap with partition trees we already found. Add new
- * information to the allpartrelids list.
- */
- allpartrelids = add_part_relids(allpartrelids, partrelids);
- /* Also record the subplan in relid_subplan_map[] */
- /* No duplicates please */
- Assert(relid_subplan_map[pathrel->relid] == 0);
- relid_subplan_map[pathrel->relid] = i;
- }
- }
+ /* No duplicates please */
+ Assert(relid_subplan_map[pathrel->relid] == 0);
+ relid_subplan_map[pathrel->relid] = i;
i++;
}
@@ -359,63 +316,6 @@ make_partition_pruneinfo(PlannerInfo *root, RelOptInfo *parentrel,
return pruneinfo;
}
-/*
- * add_part_relids
- * Add new info to a list of Bitmapsets of partitioned relids.
- *
- * Within 'allpartrelids', there is one Bitmapset for each topmost parent
- * partitioned rel. Each Bitmapset contains the RT indexes of the topmost
- * parent as well as its relevant non-leaf child partitions. Since (by
- * construction of the rangetable list) parent partitions must have lower
- * RT indexes than their children, we can distinguish the topmost parent
- * as being the lowest set bit in the Bitmapset.
- *
- * 'partrelids' contains the RT indexes of a parent partitioned rel, and
- * possibly some non-leaf children, that are newly identified as parents of
- * some subpath rel passed to make_partition_pruneinfo(). These are added
- * to an appropriate member of 'allpartrelids'.
- *
- * Note that the list contains only RT indexes of partitioned tables that
- * are parents of some scan-level relation appearing in the 'subpaths' that
- * make_partition_pruneinfo() is dealing with. Also, "topmost" parents are
- * not allowed to be higher than the 'parentrel' associated with the append
- * path. In this way, we avoid expending cycles on partitioned rels that
- * can't contribute useful pruning information for the problem at hand.
- * (It is possible for 'parentrel' to be a child partitioned table, and it
- * is also possible for scan-level relations to be child partitioned tables
- * rather than leaf partitions. Hence we must construct this relation set
- * with reference to the particular append path we're dealing with, rather
- * than looking at the full partitioning structure represented in the
- * RelOptInfos.)
- */
-static List *
-add_part_relids(List *allpartrelids, Bitmapset *partrelids)
-{
- Index targetpart;
- ListCell *lc;
-
- /* We can easily get the lowest set bit this way: */
- targetpart = bms_next_member(partrelids, -1);
- Assert(targetpart > 0);
-
- /* Look for a matching topmost parent */
- foreach(lc, allpartrelids)
- {
- Bitmapset *currpartrelids = (Bitmapset *) lfirst(lc);
- Index currtarget = bms_next_member(currpartrelids, -1);
-
- if (targetpart == currtarget)
- {
- /* Found a match, so add any new RT indexes to this hierarchy */
- currpartrelids = bms_add_members(currpartrelids, partrelids);
- lfirst(lc) = currpartrelids;
- return allpartrelids;
- }
- }
- /* No match, so add the new partition hierarchy to the list */
- return lappend(allpartrelids, partrelids);
-}
-
/*
* make_partitionedrel_pruneinfo
* Build a List of PartitionedRelPruneInfos, one for each interesting
diff --git a/src/include/nodes/pathnodes.h b/src/include/nodes/pathnodes.h
index 14ccfc1ac1..73c2a70028 100644
--- a/src/include/nodes/pathnodes.h
+++ b/src/include/nodes/pathnodes.h
@@ -128,6 +128,9 @@ typedef struct PlannerGlobal
/* "flat" list of AppendRelInfos */
List *appendRelations;
+ /* "flat list of Bitmapsets of RT indexes "*/
+ List *elidedAppendPartRels;
+
/* OIDs of relations the plan depends on */
List *relationOids;
diff --git a/src/include/nodes/plannodes.h b/src/include/nodes/plannodes.h
index 1aeeaec95e..634c1908ca 100644
--- a/src/include/nodes/plannodes.h
+++ b/src/include/nodes/plannodes.h
@@ -79,6 +79,11 @@ typedef struct PlannedStmt
List *appendRelations; /* list of AppendRelInfo nodes */
+ List *elidedAppendPartRels; /* list of Bitmapsets of RT indexes of
+ * partitioned tables from Append/
+ * MergeAppend nodes that were elided
+ * in setrefs.c */
+
List *subplans; /* Plan trees for SubPlan expressions; note
* that some could be NULL */
@@ -269,6 +274,15 @@ typedef struct Append
List *appendplans;
int nasyncplans; /* # of asynchronous plans */
+ /*
+ * List of bitmapsets containing RT indexes of all partitioned tables
+ * scanned by this Append, with one bitmapset for every partitioned
+ * table appearing in the query. Each bitmapset contains the RT indexes
+ * of all non-pruned non-leaf partitions in the tree with a given
+ * partitioned table as root.
+ */
+ List *allpartrelids;
+
/*
* All 'appendplans' preceding this index are non-partial plans. All
* 'appendplans' from this index onwards are partial plans.
@@ -293,6 +307,9 @@ typedef struct MergeAppend
List *mergeplans;
+ /* See the description in Append's definition. */
+ List *allpartrelids;
+
/* these fields are just like the sort-key info in struct Sort: */
/* number of sort-key columns */
diff --git a/src/include/optimizer/appendinfo.h b/src/include/optimizer/appendinfo.h
index cc12c9c743..8e3d61c708 100644
--- a/src/include/optimizer/appendinfo.h
+++ b/src/include/optimizer/appendinfo.h
@@ -46,5 +46,8 @@ extern void add_row_identity_columns(PlannerInfo *root, Index rtindex,
RangeTblEntry *target_rte,
Relation target_relation);
extern void distribute_row_identity_vars(PlannerInfo *root);
+extern List *add_append_subpath_partrelids(PlannerInfo *root, Path *subpath,
+ RelOptInfo *parentrel,
+ List *allpartrelids);
#endif /* APPENDINFO_H */
diff --git a/src/include/partitioning/partprune.h b/src/include/partitioning/partprune.h
index bd490d154f..1587298812 100644
--- a/src/include/partitioning/partprune.h
+++ b/src/include/partitioning/partprune.h
@@ -73,7 +73,8 @@ typedef struct PartitionPruneContext
extern PartitionPruneInfo *make_partition_pruneinfo(struct PlannerInfo *root,
struct RelOptInfo *parentrel,
List *subpaths,
- List *prunequal);
+ List *prunequal,
+ List *allpartrelids);
extern Bitmapset *prune_append_rel_partitions(struct RelOptInfo *rel);
extern Bitmapset *get_matching_partitions(PartitionPruneContext *context,
List *pruning_steps);
--
2.43.0
[application/octet-stream] v50-0001-Assorted-tightening-in-various-ExecEnd-routines.patch (31.3K, 3-v50-0001-Assorted-tightening-in-various-ExecEnd-routines.patch)
download | inline diff:
From 28cadedabb59e5e3386f4d9a309b77bdc7047143 Mon Sep 17 00:00:00 2001
From: Amit Langote <[email protected]>
Date: Thu, 28 Sep 2023 16:56:29 +0900
Subject: [PATCH v50 1/6] 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 4d7c92d63c..2d5234dee3 100644
--- a/src/backend/executor/execMain.c
+++ b/src/backend/executor/execMain.c
@@ -2986,6 +2986,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 61480733a1..dbf4920363 100644
--- a/src/backend/executor/nodeHash.c
+++ b/src/backend/executor/nodeHash.c
@@ -412,13 +412,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 5429e68734..592c098b9f 100644
--- a/src/backend/executor/nodeHashjoin.c
+++ b/src/backend/executor/nodeHashjoin.c
@@ -870,7 +870,7 @@ ExecEndHashJoin(HashJoinState *node)
/*
* Free hash table
*/
- if (node->hj_HashTable)
+ if (node->hj_HashTable != NULL)
{
ExecHashTableDestroy(node->hj_HashTable);
node->hj_HashTable = NULL;
@@ -880,7 +880,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 4913e49319..062a780f29 100644
--- a/src/backend/executor/nodeModifyTable.c
+++ b/src/backend/executor/nodeModifyTable.c
@@ -4721,7 +4721,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;
}
}
@@ -4729,12 +4731,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;
+ }
}
/*
@@ -4746,6 +4752,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] v50-0003-Assert-that-relations-needing-their-permissions-.patch (2.2K, 4-v50-0003-Assert-that-relations-needing-their-permissions-.patch)
download | inline diff:
From 1657389c3c44b1765919773ad9cbaaab8a72fa64 Mon Sep 17 00:00:00 2001
From: Amit Langote <[email protected]>
Date: Mon, 25 Sep 2023 11:52:02 +0900
Subject: [PATCH v50 3/6] Assert that relations needing their permissions
checked are locked
---
src/backend/executor/execMain.c | 13 +++++++++++++
src/backend/storage/lmgr/lmgr.c | 1 +
src/backend/utils/cache/lsyscache.c | 1 -
3 files changed, 14 insertions(+), 1 deletion(-)
diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c
index 2d5234dee3..5e1b8a42e8 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"
@@ -602,6 +603,18 @@ ExecCheckPermissions(List *rangeTable, List *rteperminfos,
(rte->rtekind == RTE_SUBQUERY &&
rte->relkind == RELKIND_VIEW));
+ /*
+ * Relations whose permissions need to be checked must already
+ * have been locked by the parser or by GetCachedPlan() if a
+ * cached plan is being executed.
+ *
+ * XXX Maybe we should we skip calling ExecCheckPermissions from
+ * InitPlan in a parallel worker.
+ */
+ Assert(IsParallelWorker() ||
+ CheckRelationOidLockedByMe(rte->relid, AccessShareLock,
+ true));
+
(void) getRTEPermissionInfo(rteperminfos, rte);
/* Many-to-one mapping not allowed */
Assert(!bms_is_member(rte->perminfoindex, indexset));
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/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
--
2.43.0
[application/octet-stream] v50-0005-Don-t-lock-child-tables-in-GetCachedPlan.patch (24.0K, 5-v50-0005-Don-t-lock-child-tables-in-GetCachedPlan.patch)
download | inline diff:
From a1aef6d727c0a6c6f3305a623f6337e35d007be5 Mon Sep 17 00:00:00 2001
From: Amit Langote <[email protected]>
Date: Wed, 6 Sep 2023 17:54:15 +0900
Subject: [PATCH v50 5/6] Don't lock child tables in GetCachedPlan()
Currently, GetCachedPlan() takes a lock on all relations contained in
a cached plan before returning it as a valid plan to its callers for
execution. One disadvantage is that if the plan contains partitions
that are prunable with conditions involving EXTERN parameters and
other stable expressions (known as "initial pruning"), many of them
would be locked unnecessarily, because only those that survive
initial pruning need to have been locked. Locking all partitions this
way causes significant delay when there are many partitions. Note
that initial pruning occurs during executor's initialization of the
plan, that is, ExecInitNode().
Previous commits have made all the necessary adjustment to make the
executor lock child tables, to detect invalidation of the CachedPlan
resulting from that, and to retry the execution with a new CachePlan.
So, this commit simply removes the code in plancache.c that does the
"for execution" locking, aka AcquireExecutorLocks().
Discussion: https://postgr.es/m/CA+HiwqFGkMSge6TgC9KQzde0ohpAycLQuV7ooitEEpbKB0O_mg@mail.gmail.com
---
src/backend/executor/execMain.c | 2 +-
src/backend/utils/cache/plancache.c | 154 +++++++----------
src/test/modules/delay_execution/Makefile | 3 +-
.../modules/delay_execution/delay_execution.c | 68 +++++++-
.../expected/cached-plan-replan.out | 158 ++++++++++++++++++
.../specs/cached-plan-replan.spec | 61 +++++++
6 files changed, 340 insertions(+), 106 deletions(-)
create mode 100644 src/test/modules/delay_execution/expected/cached-plan-replan.out
create mode 100644 src/test/modules/delay_execution/specs/cached-plan-replan.spec
diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c
index cf059cb850..168ab553ac 100644
--- a/src/backend/executor/execMain.c
+++ b/src/backend/executor/execMain.c
@@ -1508,7 +1508,7 @@ ExecGetAncestorResultRels(EState *estate, ResultRelInfo *resultRelInfo)
/*
* All ancestors up to the root target relation must have been
- * locked by the planner or AcquireExecutorLocks().
+ * locked by the planner or ExecLockAppendNonLeafPartitions().
*/
ancRel = table_open(ancOid, NoLock);
rInfo = makeNode(ResultRelInfo);
diff --git a/src/backend/utils/cache/plancache.c b/src/backend/utils/cache/plancache.c
index 5af1a168ec..ad33d611f9 100644
--- a/src/backend/utils/cache/plancache.c
+++ b/src/backend/utils/cache/plancache.c
@@ -103,13 +103,13 @@ static void ReleaseGenericPlan(CachedPlanSource *plansource);
static List *RevalidateCachedQuery(CachedPlanSource *plansource,
QueryEnvironment *queryEnv);
static bool CheckCachedPlan(CachedPlanSource *plansource);
+static bool GenericPlanIsValid(CachedPlan *cplan);
static CachedPlan *BuildCachedPlan(CachedPlanSource *plansource, List *qlist,
ParamListInfo boundParams, QueryEnvironment *queryEnv);
static bool choose_custom_plan(CachedPlanSource *plansource,
ParamListInfo boundParams);
static double cached_plan_cost(CachedPlan *plan, bool include_planner);
static Query *QueryListGetPrimaryStmt(List *stmts);
-static void AcquireExecutorLocks(List *stmt_list, bool acquire);
static void AcquirePlannerLocks(List *stmt_list, bool acquire);
static void ScanQueryForLocks(Query *parsetree, bool acquire);
static bool ScanQueryWalker(Node *node, bool *acquire);
@@ -815,8 +815,13 @@ 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.)
+ * If the plan includes child relations introduced by the planner, they
+ * wouldn't be locked yet. This is because AcquirePlannerLocks() only locks
+ * relations present in the original query's range table (before planner
+ * entry). Hence, the plan might become stale if child relations are modified
+ * concurrently. During the plan initialization, the executor must ensure the
+ * plan (CachedPlan) remains valid after locking each child table. If found
+ * invalid, the caller should be prompted to recreate the plan.
*/
static bool
CheckCachedPlan(CachedPlanSource *plansource)
@@ -830,60 +835,56 @@ CheckCachedPlan(CachedPlanSource *plansource)
if (!plan)
return false;
- Assert(plan->magic == CACHEDPLAN_MAGIC);
- /* Generic plans are never one-shot */
- Assert(!plan->is_oneshot);
+ if (GenericPlanIsValid(plan))
+ return true;
/*
- * If plan isn't valid for current role, we can't use it.
+ * Plan has been invalidated, so unlink it from the parent and release it.
*/
- if (plan->is_valid && plan->dependsOnRole &&
- plan->planRoleId != GetUserId())
- plan->is_valid = false;
+ ReleaseGenericPlan(plansource);
- /*
- * If it appears valid, acquire locks and recheck; this is much the same
- * logic as in RevalidateCachedQuery, but for a plan.
- */
- if (plan->is_valid)
+ return false;
+}
+
+/*
+ * GenericPlanIsValid
+ * Is a generic plan still valid?
+ *
+ * It may have gone stale due to concurrent schema modifications of relations
+ * mentioned in the plan or a couple of other things mentioned below.
+ */
+static bool
+GenericPlanIsValid(CachedPlan *cplan)
+{
+ Assert(cplan != NULL);
+ Assert(cplan->magic == CACHEDPLAN_MAGIC);
+ /* Generic plans are never one-shot */
+ Assert(!cplan->is_oneshot);
+
+ if (cplan->is_valid)
{
/*
* Plan must have positive refcount because it is referenced by
* plansource; so no need to fear it disappears under us here.
*/
- Assert(plan->refcount > 0);
-
- AcquireExecutorLocks(plan->stmt_list, true);
+ Assert(cplan->refcount > 0);
/*
- * If plan was transient, check to see if TransactionXmin has
- * advanced, and if so invalidate it.
+ * If plan isn't valid for current role, we can't use it.
*/
- if (plan->is_valid &&
- TransactionIdIsValid(plan->saved_xmin) &&
- !TransactionIdEquals(plan->saved_xmin, TransactionXmin))
- plan->is_valid = false;
+ if (cplan->dependsOnRole && cplan->planRoleId != GetUserId())
+ cplan->is_valid = false;
/*
- * By now, if any invalidation has happened, the inval callback
- * functions will have marked the plan invalid.
+ * If plan was transient, check to see if TransactionXmin has
+ * advanced, and if so invalidate it.
*/
- if (plan->is_valid)
- {
- /* Successfully revalidated and locked the query. */
- return true;
- }
-
- /* Oops, the race case happened. Release useless locks. */
- AcquireExecutorLocks(plan->stmt_list, false);
+ if (TransactionIdIsValid(cplan->saved_xmin) &&
+ !TransactionIdEquals(cplan->saved_xmin, TransactionXmin))
+ cplan->is_valid = false;
}
- /*
- * Plan has been invalidated, so unlink it from the parent and release it.
- */
- ReleaseGenericPlan(plansource);
-
- return false;
+ return cplan->is_valid;
}
/*
@@ -1153,8 +1154,16 @@ cached_plan_cost(CachedPlan *plan, bool include_planner)
* plan or a custom plan for the given parameters: the caller does not know
* which it will get.
*
- * On return, the plan is valid and we have sufficient locks to begin
- * execution.
+ * Typically, the plan returned by this function is valid. However, a caveat
+ * arises with inheritance/partition child tables. These aren't locked by
+ * this function, as we only lock tables directly mentioned in the original
+ * query here. The task of locking these child tables falls to the executor
+ * during plan tree setup. If acquiring these locks invalidates the plan, the
+ * executor should inform the caller to regenerate the plan by invoking this
+ * function again. The reason for this deferred child table locking mechanism
+ * is efficiency: not all might need to be locked. Some could be pruned during
+ * executor initialization, especially if their corresponding plan nodes
+ * facilitate partition pruning.
*
* On return, the refcount of the plan has been incremented; a later
* ReleaseCachedPlan() call is expected. If "owner" is not NULL then
@@ -1189,7 +1198,10 @@ GetCachedPlan(CachedPlanSource *plansource, ParamListInfo boundParams,
{
if (CheckCachedPlan(plansource))
{
- /* We want a generic plan, and we already have a valid one */
+ /*
+ * We want a generic plan, and we already have a valid one, though
+ * see the header comment.
+ */
plan = plansource->gplan;
Assert(plan->magic == CACHEDPLAN_MAGIC);
}
@@ -1387,8 +1399,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 the executor would need to take additional locks, that is, in
+ * addition to those taken by AcquirePlannerLocks() on a given query.
*/
foreach(lc, plan->stmt_list)
{
@@ -1764,58 +1776,6 @@ QueryListGetPrimaryStmt(List *stmts)
return NULL;
}
-/*
- * AcquireExecutorLocks: acquire locks needed for execution of a cached plan;
- * or release them if acquire is false.
- */
-static void
-AcquireExecutorLocks(List *stmt_list, bool acquire)
-{
- ListCell *lc1;
-
- foreach(lc1, stmt_list)
- {
- PlannedStmt *plannedstmt = lfirst_node(PlannedStmt, lc1);
- ListCell *lc2;
-
- if (plannedstmt->commandType == CMD_UTILITY)
- {
- /*
- * Ignore utility statements, except those (such as EXPLAIN) that
- * contain a parsed-but-not-planned query. Note: it's okay to use
- * ScanQueryForLocks, even though the query hasn't been through
- * rule rewriting, because rewriting doesn't change the query
- * representation.
- */
- Query *query = UtilityContainsQuery(plannedstmt->utilityStmt);
-
- if (query)
- ScanQueryForLocks(query, acquire);
- continue;
- }
-
- foreach(lc2, plannedstmt->rtable)
- {
- RangeTblEntry *rte = (RangeTblEntry *) lfirst(lc2);
-
- if (!(rte->rtekind == RTE_RELATION ||
- (rte->rtekind == RTE_SUBQUERY && OidIsValid(rte->relid))))
- continue;
-
- /*
- * Acquire the appropriate type of lock on each relation OID. Note
- * that we don't actually try to open the rel, and hence will not
- * fail if it's been dropped entirely --- we'll just transiently
- * acquire a non-conflicting lock.
- */
- if (acquire)
- LockRelationOid(rte->relid, rte->rellockmode);
- else
- UnlockRelationOid(rte->relid, rte->rellockmode);
- }
- }
-}
-
/*
* AcquirePlannerLocks: acquire locks needed for planning of a querytree list;
* or release them if acquire is false.
diff --git a/src/test/modules/delay_execution/Makefile b/src/test/modules/delay_execution/Makefile
index 70f24e846d..2fca84d027 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-replan
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..8bb6f0319a 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,46 @@ delay_execution_planner(Query *parse, const char *query_string,
return result;
}
+/* ExecutorStart_hook function to provide the desired delay */
+static bool
+delay_execution_ExecutorStart(QueryDesc *queryDesc, CachedPlan *cplan,
+ int eflags)
+{
+ bool plan_valid;
+
+ /* 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)
+ plan_valid = prev_ExecutorStart_hook(queryDesc, cplan, eflags);
+ else
+ plan_valid = standard_ExecutorStart(queryDesc, cplan, eflags);
+
+ if (executor_start_lock_id != 0)
+ elog(NOTICE, "Finished ExecutorStart(): CachedPlan is %s",
+ plan_valid ? "valid" : "not valid");
+
+ return plan_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 +128,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-replan.out b/src/test/modules/delay_execution/expected/cached-plan-replan.out
new file mode 100644
index 0000000000..122d81f2ee
--- /dev/null
+++ b/src/test/modules/delay_execution/expected/cached-plan-replan.out
@@ -0,0 +1,158 @@
+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: 1
+ -> Bitmap Heap Scan on foo11 foo_1
+ Recheck Cond: (a = $1)
+ -> Bitmap Index Scan on foo11_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 foo11_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: 1
+ -> Seq Scan on foo11 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 = 1;
+ EXPLAIN (COSTS OFF) EXECUTE q2;
+s1: NOTICE: Finished ExecutorStart(): CachedPlan is valid
+QUERY PLAN
+----------------------------------
+Bitmap Heap Scan on foo11 foo
+ Recheck Cond: (a = 1)
+ -> Bitmap Index Scan on foo11_a
+ Index Cond: (a = 1)
+(4 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 foo11_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
+---------------------
+Seq Scan on foo11 foo
+ Filter: (a = 1)
+(2 rows)
+
+
+starting permutation: s1prep3 s2lock s1exec3 s2dropi s2unlock
+step s1prep3: SET plan_cache_mode = force_generic_plan;
+ SET enable_partitionwise_aggregate = on;
+ SET enable_partitionwise_join = on;
+ PREPARE q3 AS SELECT t1.a, count(t2.b) FROM foo t1, foo t2 WHERE t1.a = t2.a GROUP BY 1;
+ EXPLAIN (COSTS OFF) EXECUTE q3;
+s1: NOTICE: Finished ExecutorStart(): CachedPlan is valid
+QUERY PLAN
+------------------------------------------------------------
+Append
+ -> GroupAggregate
+ Group Key: t1.a
+ -> Merge Join
+ Merge Cond: (t1.a = t2.a)
+ -> Index Only Scan using foo11_a on foo11 t1
+ -> Materialize
+ -> Index Scan using foo11_a on foo11 t2
+ -> GroupAggregate
+ Group Key: t1_1.a
+ -> Merge Join
+ Merge Cond: (t1_1.a = t2_1.a)
+ -> Sort
+ Sort Key: t1_1.a
+ -> Seq Scan on foo2 t1_1
+ -> Sort
+ Sort Key: t2_1.a
+ -> Seq Scan on foo2 t2_1
+(18 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 foo11_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
+QUERY PLAN
+---------------------------------------------
+Append
+ -> GroupAggregate
+ Group Key: t1.a
+ -> Merge Join
+ Merge Cond: (t1.a = t2.a)
+ -> Sort
+ Sort Key: t1.a
+ -> Seq Scan on foo11 t1
+ -> Sort
+ Sort Key: t2.a
+ -> Seq Scan on foo11 t2
+ -> GroupAggregate
+ Group Key: t1_1.a
+ -> Merge Join
+ Merge Cond: (t1_1.a = t2_1.a)
+ -> Sort
+ Sort Key: t1_1.a
+ -> Seq Scan on foo2 t1_1
+ -> Sort
+ Sort Key: t2_1.a
+ -> Seq Scan on foo2 t2_1
+(21 rows)
+
diff --git a/src/test/modules/delay_execution/specs/cached-plan-replan.spec b/src/test/modules/delay_execution/specs/cached-plan-replan.spec
new file mode 100644
index 0000000000..2d0607b176
--- /dev/null
+++ b/src/test/modules/delay_execution/specs/cached-plan-replan.spec
@@ -0,0 +1,61 @@
+# 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 foo1 PARTITION OF foo FOR VALUES IN (1) PARTITION BY LIST (a);
+ CREATE TABLE foo11 PARTITION OF foo1 FOR VALUES IN (1);
+ CREATE INDEX foo11_a ON foo11 (a);
+ CREATE TABLE foo2 PARTITION OF foo FOR VALUES IN (2);
+ CREATE VIEW foov AS SELECT * FROM foo;
+}
+
+teardown
+{
+ DROP VIEW foov;
+ DROP TABLE foo;
+}
+
+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); }
+
+# no Append case (only one partition selected by the planner)
+step "s1prep2" { SET plan_cache_mode = force_generic_plan;
+ PREPARE q2 AS SELECT * FROM foov WHERE a = 1;
+ EXPLAIN (COSTS OFF) EXECUTE q2; }
+
+# Append with partition-wise join aggregate and join plans as child subplans
+step "s1prep3" { SET plan_cache_mode = force_generic_plan;
+ SET enable_partitionwise_aggregate = on;
+ SET enable_partitionwise_join = on;
+ PREPARE q3 AS SELECT t1.a, count(t2.b) FROM foo t1, foo t2 WHERE t1.a = t2.a GROUP BY 1;
+ 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 foo11_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
[application/octet-stream] v50-0004-Preparations-to-allow-executor-to-take-locks-in-.patch (98.2K, 6-v50-0004-Preparations-to-allow-executor-to-take-locks-in-.patch)
download | inline diff:
From cef620d80a764fa0787d074c3cc4dacba73ed190 Mon Sep 17 00:00:00 2001
From: Amit Langote <[email protected]>
Date: Wed, 6 Sep 2023 17:53:46 +0900
Subject: [PATCH v50 4/6] Preparations to allow executor to take locks in some
cases
This does two things on the executor side:
* Make the executor take locks on the child tables if the plan
comes from a CachedPlan. To determine that a given range table
relation is a child table, ExecGetRangeTableRelation() now
examines RangeTblEntry.inFromCl. To that end, the planner now
sets inFromCl to false in the child tables' RTEs. Also, the
to deal with the cases where an unlocked child table might have
been concurrently dropped, ExecGetRangeTableRelation() is now
made to use try_table_open() replacing the existing table_open().
* Add checks at various points during the executor's initialization
of the plan tree to determine whether the originating CachedPlan
has become invalid as a result of taking locks on the relations
referenced in the plan. This includes addding the check after
every call to ExecOpenScanRelation() / ExecGetRangeTableRelation()
and to ExecInitNode(), including the recursive ones to initialize
child nodes.
If a given ExecInit*() function or any function called by it detects
that the plan has become invalid, it should return immediately even
if the PlanState node it's building may only be partially valid.
That is crucial for two reasons depending on where the check is:
* The checks following ExecOpenScanRelation() /
ExecGetRangeTableRelation() may find that the relation being
opened has been dropped concurrently or that the plan has become
invalid. In this case, some operations in the code that follows
may no longer be safe to do. For example, it might try to
dereference a NULL pointer in the case where the relation was
dropped.
* For the checks following ExecInitNode(), the returned child
PlanState node might be only partially invalid. The code that
follows may misbehave if it depends on inspecting the child
PlanState. Note that this commit adds the check following all
calls of ExecInitNode() that exist in the code base, even at
sites where there is no code that might misbehave today, because
it might misbehave in the future. It seems like a good idea to
put the guards in place today rather than in the future when the
need arises.
To pass the CachedPlan that the executor will use for these checks,
ExecutorStart() (and ExecutorStart_hook) now gets a new parameter
CachedPlan *cplan.
Changes on the side of the callers of ExecutorStart():
ExecutorStart() (and ExecutorStart_hook()) now return a Boolean
telling the caller if the plan initialization failed. When it
returns false due to the CachedPlan becoming invalid, the execution
should be reattempted using a fresh CachedPlan. Actually, a new
function TryExecutorStart() is added to use by the call sites that
supply the PlannedStmt to execute from a CachedPlan, which takes
care of cleaning up the partially initialized execution state
including planstate tree.
For the replan loop in that context, it makes more sense to have
ExecutorStart() either in the same scope or closer to where
GetCachedPlan() is invoked. So this commit modifies the following
sites:
* ExplainOnePlan() now returns a Boolean to tell the caller that
TryExecutorStart() failed, so the caller should retry with a new
plan.
* The ExecutorStart() call in _SPI_pquery() is moved to its caller
_SPI_execute_plan() and replaced by TryExecutorStart().
* The ExecutorStart() call in PortalRunMulti() is moved to
PortalStart() and replaced by TryExecutorStart(). This requires a
new List field in PortalData to store the QueryDescs created in
PortalStart() and a new memory context for those. One unintended
consequence is that CommandCounterIncrement() between queries in
the PORTAL_MULTI_QUERY case is now done in the loop in PortalStart()
and not in PortalRunMulti(). That still works because the Snapshot
registered in QueryDesc/EState is updated to account for the CCI().
This commit also adds a new flag to EState called es_canceled that
complements es_finished to denote the new scenario where
ExecutorStart() returns with a partially setup planstate tree. Also,
to reset the AFTER trigger state that would have been set up in the
ExecutorStart(), this adds a new function AfterTriggerCancelQuery()
which is called from ExecutorEnd() (not ExecutorFinish()) when
es_canceled is true.
Note that this commit by itself doesn't make any functional change,
because ExecutorStart() currently always returns true. The changes
to make it check if the CachedPlan has become invalid will be
added in the upcoming patches.
Reviewed-by: Robert Haas
Discussion: https://postgr.es/m/CA+HiwqFGkMSge6TgC9KQzde0ohpAycLQuV7ooitEEpbKB0O_mg@mail.gmail.comk
---
contrib/auto_explain/auto_explain.c | 15 +-
.../pg_stat_statements/pg_stat_statements.c | 15 +-
contrib/postgres_fdw/postgres_fdw.c | 10 +-
src/backend/commands/copyto.c | 5 +-
src/backend/commands/createas.c | 9 +-
src/backend/commands/explain.c | 42 ++-
src/backend/commands/extension.c | 6 +-
src/backend/commands/matview.c | 10 +-
src/backend/commands/portalcmds.c | 6 +-
src/backend/commands/prepare.c | 30 +-
src/backend/commands/trigger.c | 13 +
src/backend/executor/README | 36 ++-
src/backend/executor/execMain.c | 144 ++++++++--
src/backend/executor/execParallel.c | 7 +-
src/backend/executor/execPartition.c | 2 +
src/backend/executor/execProcnode.c | 7 +
src/backend/executor/execUtils.c | 98 ++++++-
src/backend/executor/functions.c | 8 +-
src/backend/executor/nodeAgg.c | 2 +
src/backend/executor/nodeAppend.c | 24 +-
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 | 6 +-
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 | 18 +-
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 | 46 ++-
src/backend/optimizer/util/inherit.c | 7 +
src/backend/parser/analyze.c | 7 +-
src/backend/tcop/postgres.c | 17 +-
src/backend/tcop/pquery.c | 263 ++++++++++--------
src/backend/utils/mmgr/portalmem.c | 9 +
src/include/commands/explain.h | 4 +-
src/include/commands/trigger.h | 1 +
src/include/executor/executor.h | 21 +-
src/include/nodes/execnodes.h | 5 +
src/include/nodes/parsenodes.h | 8 +-
src/include/tcop/pquery.h | 5 +-
src/include/utils/plancache.h | 14 +
src/include/utils/portal.h | 2 +
68 files changed, 804 insertions(+), 217 deletions(-)
diff --git a/contrib/auto_explain/auto_explain.c b/contrib/auto_explain/auto_explain.c
index 677c135f59..1b4b8ad8b6 100644
--- a/contrib/auto_explain/auto_explain.c
+++ b/contrib/auto_explain/auto_explain.c
@@ -78,7 +78,8 @@ static ExecutorRun_hook_type prev_ExecutorRun = NULL;
static ExecutorFinish_hook_type prev_ExecutorFinish = NULL;
static ExecutorEnd_hook_type prev_ExecutorEnd = NULL;
-static void explain_ExecutorStart(QueryDesc *queryDesc, int eflags);
+static bool explain_ExecutorStart(QueryDesc *queryDesc, CachedPlan *cplan,
+ int eflags);
static void explain_ExecutorRun(QueryDesc *queryDesc,
ScanDirection direction,
uint64 count, bool execute_once);
@@ -258,9 +259,11 @@ _PG_init(void)
/*
* ExecutorStart hook: start up logging if needed
*/
-static void
-explain_ExecutorStart(QueryDesc *queryDesc, int eflags)
+static bool
+explain_ExecutorStart(QueryDesc *queryDesc, CachedPlan *cplan, int eflags)
{
+ bool plan_valid;
+
/*
* At the beginning of each top-level statement, decide whether we'll
* sample this statement. If nested-statement explaining is enabled,
@@ -296,9 +299,9 @@ explain_ExecutorStart(QueryDesc *queryDesc, int eflags)
}
if (prev_ExecutorStart)
- prev_ExecutorStart(queryDesc, eflags);
+ plan_valid = prev_ExecutorStart(queryDesc, cplan, eflags);
else
- standard_ExecutorStart(queryDesc, eflags);
+ plan_valid = standard_ExecutorStart(queryDesc, cplan, eflags);
if (auto_explain_enabled())
{
@@ -316,6 +319,8 @@ explain_ExecutorStart(QueryDesc *queryDesc, int eflags)
MemoryContextSwitchTo(oldcxt);
}
}
+
+ return plan_valid;
}
/*
diff --git a/contrib/pg_stat_statements/pg_stat_statements.c b/contrib/pg_stat_statements/pg_stat_statements.c
index 362d222f63..74d359f61b 100644
--- a/contrib/pg_stat_statements/pg_stat_statements.c
+++ b/contrib/pg_stat_statements/pg_stat_statements.c
@@ -329,7 +329,8 @@ static PlannedStmt *pgss_planner(Query *parse,
const char *query_string,
int cursorOptions,
ParamListInfo boundParams);
-static void pgss_ExecutorStart(QueryDesc *queryDesc, int eflags);
+static bool pgss_ExecutorStart(QueryDesc *queryDesc, CachedPlan *cplan,
+ int eflags);
static void pgss_ExecutorRun(QueryDesc *queryDesc,
ScanDirection direction,
uint64 count, bool execute_once);
@@ -984,13 +985,15 @@ pgss_planner(Query *parse,
/*
* ExecutorStart hook: start up tracking if needed
*/
-static void
-pgss_ExecutorStart(QueryDesc *queryDesc, int eflags)
+static bool
+pgss_ExecutorStart(QueryDesc *queryDesc, CachedPlan *cplan, int eflags)
{
+ bool plan_valid;
+
if (prev_ExecutorStart)
- prev_ExecutorStart(queryDesc, eflags);
+ plan_valid = prev_ExecutorStart(queryDesc, cplan, eflags);
else
- standard_ExecutorStart(queryDesc, eflags);
+ plan_valid = standard_ExecutorStart(queryDesc, cplan, eflags);
/*
* If query has queryId zero, don't track it. This prevents double
@@ -1013,6 +1016,8 @@ pgss_ExecutorStart(QueryDesc *queryDesc, int eflags)
MemoryContextSwitchTo(oldcxt);
}
}
+
+ return plan_valid;
}
/*
diff --git a/contrib/postgres_fdw/postgres_fdw.c b/contrib/postgres_fdw/postgres_fdw.c
index fc65d81e21..e7080f8953 100644
--- a/contrib/postgres_fdw/postgres_fdw.c
+++ b/contrib/postgres_fdw/postgres_fdw.c
@@ -2137,7 +2137,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;
@@ -2671,7 +2675,11 @@ postgresBeginDirectModify(ForeignScanState *node, int eflags)
/* Get info about foreign table. */
rtindex = node->resultRelInfo->ri_RangeTableIndex;
if (fsplan->scan.scanrelid == 0)
+ {
dmstate->rel = ExecOpenScanRelation(estate, rtindex, eflags);
+ if (unlikely(dmstate->rel == NULL || !ExecPlanStillValid(estate)))
+ return;
+ }
else
dmstate->rel = node->ss.ss_currentRelation;
table = GetForeignTable(RelationGetRelid(dmstate->rel));
diff --git a/src/backend/commands/copyto.c b/src/backend/commands/copyto.c
index eb1d3d8fbb..e88c3a6760 100644
--- a/src/backend/commands/copyto.c
+++ b/src/backend/commands/copyto.c
@@ -561,8 +561,11 @@ BeginCopyTo(ParseState *pstate,
* Call ExecutorStart to prepare the plan for execution.
*
* ExecutorStart computes a result tupdesc for us
+ *
+ * Plan can't become invalid, because there's no CachedPlan.
*/
- ExecutorStart(cstate->queryDesc, 0);
+ if (!ExecutorStart(cstate->queryDesc, NULL, 0))
+ elog(ERROR, "unexpected failure running ExecutorStart()");
tupDesc = cstate->queryDesc->tupDesc;
}
diff --git a/src/backend/commands/createas.c b/src/backend/commands/createas.c
index 0b629b1f79..720bc1c72f 100644
--- a/src/backend/commands/createas.c
+++ b/src/backend/commands/createas.c
@@ -328,8 +328,13 @@ ExecCreateTableAs(ParseState *pstate, CreateTableAsStmt *stmt,
GetActiveSnapshot(), InvalidSnapshot,
dest, params, queryEnv, 0);
- /* call ExecutorStart to prepare the plan for execution */
- ExecutorStart(queryDesc, GetIntoRelEFlags(into));
+ /*
+ * call ExecutorStart to prepare the plan for execution
+ *
+ * Plan can't become invalid, because there's no CachedPlan.
+ */
+ if (!ExecutorStart(queryDesc, NULL, GetIntoRelEFlags(into)))
+ elog(ERROR, "unexpected failure running ExecutorStart()");
/* run the plan to completion */
ExecutorRun(queryDesc, ForwardScanDirection, 0, true);
diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.c
index 5771aabf40..d2de259062 100644
--- a/src/backend/commands/explain.c
+++ b/src/backend/commands/explain.c
@@ -506,10 +506,11 @@ standard_ExplainOneQuery(Query *query, int cursorOptions,
BufferUsageAccumDiff(&bufusage, &pgBufferUsage, &bufusage_start);
}
- /* run it (if needed) and produce output */
- ExplainOnePlan(plan, into, es, queryString, params, queryEnv,
- &planduration, (es->buffers ? &bufusage : NULL),
- es->memory ? &mem_counters : NULL);
+ /* run it (if needed) and produce output; no CachedPlan, no replanning! */
+ if (!ExplainOnePlan(plan, NULL, into, es, queryString, params, queryEnv,
+ &planduration, (es->buffers ? &bufusage : NULL),
+ es->memory ? &mem_counters : NULL))
+ elog(ERROR, "unexpected failure to finish ExplainOnePlan()");
}
/*
@@ -613,9 +614,13 @@ ExplainOneUtility(Node *utilityStmt, IntoClause *into, ExplainState *es,
* This is exported because it's called back from prepare.c in the
* EXPLAIN EXECUTE case, and because an index advisor plugin would need
* to call it.
+ *
+ * Returns true if execution succeeds, false otherwise. Latter only possible
+ * if cplan != NULL and gets invalidated during ExecutorStart().
*/
-void
-ExplainOnePlan(PlannedStmt *plannedstmt, IntoClause *into, ExplainState *es,
+bool
+ExplainOnePlan(PlannedStmt *plannedstmt, CachedPlan *cplan,
+ IntoClause *into, ExplainState *es,
const char *queryString, ParamListInfo params,
QueryEnvironment *queryEnv, const instr_time *planduration,
const BufferUsage *bufusage,
@@ -685,8 +690,16 @@ ExplainOnePlan(PlannedStmt *plannedstmt, IntoClause *into, ExplainState *es,
if (into)
eflags |= GetIntoRelEFlags(into);
- /* call ExecutorStart to prepare the plan for execution */
- ExecutorStart(queryDesc, eflags);
+ /*
+ * Call TryExecutorStart to prepare the plan for execution. A cached plan
+ * may get invalidated during plan intialization.
+ */
+ if (!TryExecutorStart(queryDesc, cplan, eflags))
+ {
+ /* Clean up. */
+ PopActiveSnapshot();
+ return false;
+ }
/* Execute the plan for statistics if asked for */
if (es->analyze)
@@ -798,6 +811,8 @@ ExplainOnePlan(PlannedStmt *plannedstmt, IntoClause *into, ExplainState *es,
es);
ExplainCloseGroup("Query", NULL, true, es);
+
+ return true;
}
/*
@@ -5194,6 +5209,17 @@ ExplainDummyGroup(const char *objtype, const char *labelname, ExplainState *es)
}
}
+/*
+ * Discard output buffer for a fresh restart.
+ */
+void
+ExplainResetOutput(ExplainState *es)
+{
+ Assert(es->str);
+ resetStringInfo(es->str);
+ ExplainBeginOutput(es);
+}
+
/*
* Emit the start-of-output boilerplate.
*
diff --git a/src/backend/commands/extension.c b/src/backend/commands/extension.c
index 1643c8c69a..c3003ae1c6 100644
--- a/src/backend/commands/extension.c
+++ b/src/backend/commands/extension.c
@@ -802,7 +802,11 @@ execute_sql_string(const char *sql)
GetActiveSnapshot(), NULL,
dest, NULL, NULL, 0);
- ExecutorStart(qdesc, 0);
+ /*
+ * Plan can't become invalid, because there's no CachedPlan.
+ */
+ if (!ExecutorStart(qdesc, NULL, 0))
+ elog(ERROR, "unexpected failure running ExecutorStart()");
ExecutorRun(qdesc, ForwardScanDirection, 0, true);
ExecutorFinish(qdesc);
ExecutorEnd(qdesc);
diff --git a/src/backend/commands/matview.c b/src/backend/commands/matview.c
index 91f0fd6ea3..ac4dfa9a71 100644
--- a/src/backend/commands/matview.c
+++ b/src/backend/commands/matview.c
@@ -442,8 +442,14 @@ refresh_matview_datafill(DestReceiver *dest, Query *query,
GetActiveSnapshot(), InvalidSnapshot,
dest, NULL, NULL, 0);
- /* call ExecutorStart to prepare the plan for execution */
- ExecutorStart(queryDesc, 0);
+ /*
+ * call ExecutorStart to prepare the plan for execution
+ *
+ * OK to ignore the return value; plan can't become invalid,
+ * because there's no CachedPlan.
+ */
+ if (!ExecutorStart(queryDesc, NULL, 0))
+ elog(ERROR, "unexpected failure running ExecutorStart()");
/* run the plan */
ExecutorRun(queryDesc, ForwardScanDirection, 0, true);
diff --git a/src/backend/commands/portalcmds.c b/src/backend/commands/portalcmds.c
index 4f6acf6719..cea3e8eca4 100644
--- a/src/backend/commands/portalcmds.c
+++ b/src/backend/commands/portalcmds.c
@@ -142,9 +142,11 @@ PerformCursorOpen(ParseState *pstate, DeclareCursorStmt *cstmt, ParamListInfo pa
/*
* Start execution, inserting parameters if any.
+ *
+ * Plan can't become invalid here, because there's no CachedPlan.
*/
- PortalStart(portal, params, 0, GetActiveSnapshot());
-
+ if (!PortalStart(portal, params, 0, GetActiveSnapshot(), NULL))
+ elog(ERROR, "unexpected failure running PortalStart()");
Assert(portal->strategy == PORTAL_ONE_SELECT);
/*
diff --git a/src/backend/commands/prepare.c b/src/backend/commands/prepare.c
index 07257d4db9..bf4917f22f 100644
--- a/src/backend/commands/prepare.c
+++ b/src/backend/commands/prepare.c
@@ -180,6 +180,7 @@ ExecuteQuery(ParseState *pstate,
paramLI = EvaluateParams(pstate, entry, stmt->params, estate);
}
+replan:
/* Create a new portal to run the query in */
portal = CreateNewPortal();
/* Don't display the portal in pg_cursors, it is for internal use only */
@@ -248,9 +249,16 @@ ExecuteQuery(ParseState *pstate,
}
/*
- * Run the portal as appropriate.
+ * Run the portal as appropriate. If the portal has a cached plan and
+ * it's found to be invalidated during the initialization of its plan
+ * trees, the plan must be regenerated.
*/
- PortalStart(portal, paramLI, eflags, GetActiveSnapshot());
+ if (!PortalStart(portal, paramLI, eflags, GetActiveSnapshot(), cplan))
+ {
+ Assert(cplan != NULL);
+ PortalDrop(portal, false);
+ goto replan;
+ }
(void) PortalRun(portal, count, false, true, dest, dest, qc);
@@ -571,7 +579,7 @@ ExplainExecuteQuery(ExecuteStmt *execstmt, IntoClause *into, ExplainState *es,
{
PreparedStatement *entry;
const char *query_string;
- CachedPlan *cplan;
+ CachedPlan *cplan = NULL;
List *plan_list;
ListCell *p;
ParamListInfo paramLI = NULL;
@@ -628,6 +636,7 @@ ExplainExecuteQuery(ExecuteStmt *execstmt, IntoClause *into, ExplainState *es,
}
/* Replan if needed, and acquire a transient refcount */
+replan:
cplan = GetCachedPlan(entry->plansource, paramLI,
CurrentResourceOwner, queryEnv);
@@ -655,9 +664,18 @@ 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,
- &planduration, (es->buffers ? &bufusage : NULL),
- es->memory ? &mem_counters : NULL);
+ {
+ if (!ExplainOnePlan(pstmt, cplan, into, es, query_string, paramLI,
+ queryEnv, &planduration,
+ (es->buffers ? &bufusage : NULL),
+ es->memory ? &mem_counters : NULL))
+ {
+ Assert(cplan != NULL);
+ ExplainResetOutput(es);
+ ReleaseCachedPlan(cplan, CurrentResourceOwner);
+ goto replan;
+ }
+ }
else
ExplainOneUtility(pstmt->utilityStmt, into, es, query_string,
paramLI, queryEnv);
diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c
index 170360edda..bb410d7313 100644
--- a/src/backend/commands/trigger.c
+++ b/src/backend/commands/trigger.c
@@ -5023,6 +5023,19 @@ AfterTriggerBeginQuery(void)
afterTriggers.query_depth++;
}
+/* ----------
+ * AfterTriggerCancelQuery()
+ *
+ * Called from ExecutorEnd() if the query execution was canceled.
+ * ----------
+ */
+void
+AfterTriggerCancelQuery(void)
+{
+ /* Set to a value denoting that no query is active. */
+ afterTriggers.query_depth = -1;
+}
+
/* ----------
* AfterTriggerEndQuery()
diff --git a/src/backend/executor/README b/src/backend/executor/README
index 642d63be61..6cd840d3a7 100644
--- a/src/backend/executor/README
+++ b/src/backend/executor/README
@@ -280,6 +280,34 @@ 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, there can be relations that remain unlocked. The function
+GetCachedPlan() locks relations existing in the query's range table pre-planning
+but doesn't account for those added during the planning phase. Consequently,
+inheritance child tables, introduced to the query's range table during planning,
+won't be locked when the cached plan reaches the executor.
+
+The decision to defer locking child tables with GetCachedPlan() arises from the
+fact that not all might be accessed during plan execution. For instance, if
+child tables are partitions, some might be omitted due to pruning at
+execution-initialization-time. Thus, the responsibility of locking these child
+tables is pushed to execution-initialization-time, taking place in ExecInitNode()
+for plan nodes encompassing these tables.
+
+This approach opens a window where a cached plan tree with child tables could
+become outdated if another backend modifies these tables before ExecInitNode()
+locks them. Given this, the executor has the added duty to confirm the plan
+tree's validity whenever it locks a child table post execution-initialization-
+pruning. This validation is done by checking the CachedPlan.is_valid attribute
+of the CachedPlan provided. If the plan tree is outdated (is_valid=false), the
+executor halts any further initialization and alerts the caller that they should
+retry execution with another freshly created plan tree.
Query Processing Control Flow
-----------------------------
@@ -316,7 +344,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 the caller of
+ExecutorStart(), which must redo the steps from CreateQueryDesc with a new
+plan tree.
+
+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 5e1b8a42e8..cf059cb850 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"
@@ -72,7 +73,7 @@ ExecutorEnd_hook_type ExecutorEnd_hook = NULL;
ExecutorCheckPerms_hook_type ExecutorCheckPerms_hook = NULL;
/* decls for local routines only used within this module */
-static void InitPlan(QueryDesc *queryDesc, int eflags);
+static bool InitPlan(QueryDesc *queryDesc, CachedPlan *cplan, int eflags);
static void CheckValidRowMarkRel(Relation rel, RowMarkType markType);
static void ExecPostprocessPlan(EState *estate);
static void ExecEndPlan(PlanState *planstate, EState *estate);
@@ -112,6 +113,13 @@ static void EvalPlanQualStart(EPQState *epqstate, Plan *planTree);
*
* eflags contains flag bits as described in executor.h.
*
+ * Plan initialization may fail if the input plan tree is found to have been
+ * invalidated, which can happen if it comes from a CachedPlan ('cplan').
+ *
+ * Returns true if plan was successfully initialized and false otherwise. If
+ * the latter, the caller must call ExecutorEnd() on 'queryDesc' to clean up
+ * after failed plan initialization.
+ *
* NB: the CurrentMemoryContext when this is called will become the parent
* of the per-query context used for this Executor invocation.
*
@@ -121,8 +129,8 @@ static void EvalPlanQualStart(EPQState *epqstate, Plan *planTree);
*
* ----------------------------------------------------------------
*/
-void
-ExecutorStart(QueryDesc *queryDesc, int eflags)
+bool
+ExecutorStart(QueryDesc *queryDesc, CachedPlan *cplan, int eflags)
{
/*
* In some cases (e.g. an EXECUTE statement) a query execution will skip
@@ -133,14 +141,34 @@ ExecutorStart(QueryDesc *queryDesc, int eflags)
pgstat_report_query_id(queryDesc->plannedstmt->queryId, false);
if (ExecutorStart_hook)
- (*ExecutorStart_hook) (queryDesc, eflags);
- else
- standard_ExecutorStart(queryDesc, eflags);
+ return (*ExecutorStart_hook) (queryDesc, cplan, eflags);
+
+ return standard_ExecutorStart(queryDesc, cplan, eflags);
}
-void
-standard_ExecutorStart(QueryDesc *queryDesc, int eflags)
+/*
+ * Variant of ExecutorStart() that handles cleaning up if the input CachedPlan
+ * becomes invalid due to locks being taken during ExecutorStart().
+ */
+bool
+TryExecutorStart(QueryDesc *queryDesc, CachedPlan *cplan, int eflags)
{
+ bool plan_valid = ExecutorStart(queryDesc, cplan, eflags);
+
+ if (!plan_valid)
+ {
+ Assert(cplan != NULL);
+ ExecutorEnd(queryDesc);
+ FreeQueryDesc(queryDesc);
+ }
+
+ return plan_valid;
+}
+
+bool
+standard_ExecutorStart(QueryDesc *queryDesc, CachedPlan *cplan, int eflags)
+{
+ bool plan_valid;
EState *estate;
MemoryContext oldcontext;
@@ -259,9 +287,14 @@ standard_ExecutorStart(QueryDesc *queryDesc, int eflags)
/*
* Initialize the plan state tree
*/
- InitPlan(queryDesc, eflags);
+ plan_valid = InitPlan(queryDesc, cplan, eflags);
+
+ /* Mark execution as canceled if plan won't be executed. */
+ estate->es_canceled = !plan_valid;
MemoryContextSwitchTo(oldcontext);
+
+ return plan_valid;
}
/* ----------------------------------------------------------------
@@ -321,6 +354,7 @@ standard_ExecutorRun(QueryDesc *queryDesc,
estate = queryDesc->estate;
Assert(estate != NULL);
+ Assert(!estate->es_canceled);
Assert(!(estate->es_top_eflags & EXEC_FLAG_EXPLAIN_ONLY));
/* caller must ensure the query's snapshot is active */
@@ -428,7 +462,7 @@ standard_ExecutorFinish(QueryDesc *queryDesc)
Assert(!(estate->es_top_eflags & EXEC_FLAG_EXPLAIN_ONLY));
/* This should be run once and only once per Executor instance */
- Assert(!estate->es_finished);
+ Assert(!estate->es_finished && !estate->es_canceled);
/* Switch into per-query memory context */
oldcontext = MemoryContextSwitchTo(estate->es_query_cxt);
@@ -487,11 +521,11 @@ 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 canceled. This Assert is needed because ExecutorFinish is
+ * new as of 9.1, and callers might forget to call it.
*/
- Assert(estate->es_finished ||
+ Assert(estate->es_finished || estate->es_canceled ||
(estate->es_top_eflags & EXEC_FLAG_EXPLAIN_ONLY));
/*
@@ -505,6 +539,14 @@ standard_ExecutorEnd(QueryDesc *queryDesc)
UnregisterSnapshot(estate->es_snapshot);
UnregisterSnapshot(estate->es_crosscheck_snapshot);
+ /*
+ * Cancel trigger execution too if the query execution was canceled.
+ */
+ if (estate->es_canceled &&
+ !(estate->es_top_eflags &
+ (EXEC_FLAG_SKIP_TRIGGERS | EXEC_FLAG_EXPLAIN_ONLY)))
+ AfterTriggerCancelQuery();
+
/*
* Must switch out of context before destroying it
*/
@@ -834,24 +876,55 @@ ExecCheckXactReadOnly(PlannedStmt *plannedstmt)
PreventCommandIfParallelMode(CreateCommandName((Node *) plannedstmt));
}
+/* Lock the relations found in PlannedStmt.elidedAppendPartRels. */
+static void
+ExecLockElidedAppendPartRels(EState *estate, List *elidedAppendPartRels)
+{
+ ListCell *lc;
+
+ foreach(lc, elidedAppendPartRels)
+ {
+ Bitmapset *partrelids = castNode(Bitmapset, lfirst(lc));
+ int rti = -1;
+
+ while ((rti = bms_next_member(partrelids, rti)) > 0)
+ {
+ RangeTblEntry *rte = exec_rt_fetch(rti, estate);
+
+ /*
+ * Don't lock any partitioned tables mentioned in the query,
+ * because they would already have been locked before entering the
+ * executor.
+ */
+ if (!rte->inFromCl)
+ LockRelationOid(rte->relid, rte->rellockmode);
+ else
+ Assert(CheckRelationOidLockedByMe(rte->relid, rte->rellockmode, true));
+ }
+ }
+}
/* ----------------------------------------------------------------
* InitPlan
*
* Initializes the query plan: open files, allocate storage
* and start up the rule manager
+ *
+ * Returns true if the plan tree is successfully initialized for execution,
+ * false otherwise. The latter case may occur if the CachedPlan that provides
+ * the plan tree ('cplan') got invalidated during the initialization.
* ----------------------------------------------------------------
*/
-static void
-InitPlan(QueryDesc *queryDesc, int eflags)
+static bool
+InitPlan(QueryDesc *queryDesc, CachedPlan *cplan, int eflags)
{
CmdType operation = queryDesc->operation;
PlannedStmt *plannedstmt = queryDesc->plannedstmt;
Plan *plan = plannedstmt->planTree;
List *rangeTable = plannedstmt->rtable;
EState *estate = queryDesc->estate;
- PlanState *planstate;
- TupleDesc tupType;
+ PlanState *planstate = NULL;
+ TupleDesc tupType = NULL;
ListCell *l;
int i;
@@ -865,7 +938,16 @@ InitPlan(QueryDesc *queryDesc, int eflags)
*/
ExecInitRangeTable(estate, rangeTable, plannedstmt->permInfos);
+ /*
+ * With range table in place, lock partitioned tables whose
+ * Append/MergeAppend nodes have been removed by the planner.
+ */
+ if (cplan != NULL)
+ ExecLockElidedAppendPartRels(estate,
+ plannedstmt->elidedAppendPartRels);
+
estate->es_plannedstmt = plannedstmt;
+ estate->es_cachedplan = cplan;
/*
* Next, build the ExecRowMark array from the PlanRowMark(s), if any.
@@ -897,6 +979,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 false;
break;
case ROW_MARK_COPY:
/* no physical table access is required */
@@ -967,6 +1052,8 @@ InitPlan(QueryDesc *queryDesc, int eflags)
estate->es_subplanstates = lappend(estate->es_subplanstates,
subplanstate);
+ if (unlikely(!ExecPlanStillValid(estate)))
+ return false;
i++;
}
@@ -977,6 +1064,8 @@ InitPlan(QueryDesc *queryDesc, int eflags)
* processing tuples.
*/
planstate = ExecInitNode(plan, estate, eflags);
+ if (unlikely(!ExecPlanStillValid(estate)))
+ return false;
/*
* Get the tuple descriptor describing the type of tuples to return.
@@ -1018,8 +1107,12 @@ InitPlan(QueryDesc *queryDesc, int eflags)
}
}
+ Assert(queryDesc->tupDesc == NULL);
queryDesc->tupDesc = tupType;
+ Assert(queryDesc->planstate == NULL);
queryDesc->planstate = planstate;
+
+ return true;
}
/*
@@ -2847,7 +2940,8 @@ EvalPlanQualStart(EPQState *epqstate, Plan *planTree)
* Child EPQ EStates share the parent's copy of unchanging state such as
* the snapshot, rangetable, and external Param info. They need their own
* copies of local state, including a tuple table, es_param_exec_vals,
- * result-rel info, etc.
+ * result-rel info, etc. Also, we don't pass the parent's copy of the
+ * CachedPlan, because no new locks will be taken for EvalPlanQual().
*/
rcestate->es_direction = ForwardScanDirection;
rcestate->es_snapshot = parentestate->es_snapshot;
@@ -2936,6 +3030,14 @@ EvalPlanQualStart(EPQState *epqstate, Plan *planTree)
subplanstate = ExecInitNode(subplan, rcestate, 0);
rcestate->es_subplanstates = lappend(rcestate->es_subplanstates,
subplanstate);
+
+ /*
+ * All the necessary locks must already have been taken when
+ * initializing the parent's copy of subplanstate, so the CachedPlan,
+ * if any, should not have become invalid during ExecInitNode().
+ */
+ if (!ExecPlanStillValid(rcestate))
+ elog(ERROR, "unexpected failure to initialize subplan in EvalPlanQualStart()");
}
/*
@@ -2977,6 +3079,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 f995714d7f..e8ca60d143 100644
--- a/src/backend/executor/execParallel.c
+++ b/src/backend/executor/execParallel.c
@@ -1439,7 +1439,12 @@ ParallelQueryMain(dsm_segment *seg, shm_toc *toc)
/* Start up the executor */
queryDesc->plannedstmt->jitFlags = fpes->jit_flags;
- ExecutorStart(queryDesc, fpes->eflags);
+ /*
+ * OK to ignore the return value; plan can't become invalid,
+ * because there's no CachedPlan.
+ */
+ if (!ExecutorStart(queryDesc, NULL, fpes->eflags))
+ elog(ERROR, "unexpected failure running ExecutorStart()");
/* Special executor initialization steps for parallel workers */
queryDesc->planstate->state->es_query_dsa = area;
diff --git a/src/backend/executor/execPartition.c b/src/backend/executor/execPartition.c
index 7651886229..97a43513ce 100644
--- a/src/backend/executor/execPartition.c
+++ b/src/backend/executor/execPartition.c
@@ -1935,6 +1935,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 5737f9f4eb..edf1c24e0e 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_canceled = 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 NULL;
/*
* 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
@@ -757,6 +782,9 @@ ExecInitRangeTable(EState *estate, List *rangeTable, List *permInfos)
* Open the Relation for a range table entry, if not already done
*
* The Relations will be closed again in ExecEndPlan().
+ *
+ * Returned value may be NULL if the relation is a child relation that is not
+ * already locked.
*/
Relation
ExecGetRangeTableRelation(EState *estate, Index rti)
@@ -773,7 +801,31 @@ ExecGetRangeTableRelation(EState *estate, Index rti)
Assert(rte->rtekind == RTE_RELATION);
- if (!IsParallelWorker())
+ if (IsParallelWorker() ||
+ (estate->es_cachedplan != NULL && !rte->inFromCl))
+ {
+ /*
+ * Take a lock if we are a parallel worker or if this is a child
+ * table referenced in a cached plan.
+ *
+ * Parallel workers need to have their own local lock on the
+ * relation. This ensures sane behavior in case the parent process
+ * exits before we do.
+ *
+ * When executing a cached plan, child tables must be locked
+ * here, because plancache.c (GetCachedPlan()) would only have
+ * locked tables mentioned in the query, that is, tables whose
+ * RTEs' inFromCl is true.
+ *
+ * Note that we use try_table_open() here, because without a lock
+ * held on the relation, it may have disappeared from under us.
+ */
+ rel = try_table_open(rte->relid, rte->rellockmode);
+ if (unlikely(!ExecPlanStillValid(estate)))
+ elog(DEBUG2, "CachedPlan invalidated on locking relation %u",
+ rte->relid);
+ }
+ else
{
/*
* In a normal query, we should already have the appropriate lock,
@@ -786,15 +838,6 @@ ExecGetRangeTableRelation(EState *estate, Index rti)
Assert(rte->rellockmode == AccessShareLock ||
CheckRelationLockedByMe(rel, rte->rellockmode, false));
}
- else
- {
- /*
- * 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.
- */
- rel = table_open(rte->relid, rte->rellockmode);
- }
estate->es_relations[rti - 1] = rel;
}
@@ -802,6 +845,38 @@ ExecGetRangeTableRelation(EState *estate, Index rti)
return rel;
}
+/*
+ * ExecLockAppendPartRels
+ * Lock non-leaf partitions whose child partitions are scanned by a given
+ * Append/MergeAppend node
+ */
+void
+ExecLockAppendPartRels(EState *estate, List *allpartrelids)
+{
+ ListCell *l;
+
+ foreach(l, allpartrelids)
+ {
+ Bitmapset *partrelids = lfirst_node(Bitmapset, l);
+ int rti = -1;
+
+ while ((rti = bms_next_member(partrelids, rti)) > 0)
+ {
+ RangeTblEntry *rte = exec_rt_fetch(rti, estate);
+
+ /*
+ * Don't lock any partitioned tables mentioned in the query,
+ * because they would already have been locked before entering the
+ * executor.
+ */
+ if (!rte->inFromCl)
+ LockRelationOid(rte->relid, rte->rellockmode);
+ else
+ Assert(CheckRelationOidLockedByMe(rte->relid, rte->rellockmode, true));
+ }
+ }
+}
+
/*
* ExecInitResultRelation
* Open relation given by the passed-in RT index and fill its
@@ -817,6 +892,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/functions.c b/src/backend/executor/functions.c
index 692854e2b3..8f65242f33 100644
--- a/src/backend/executor/functions.c
+++ b/src/backend/executor/functions.c
@@ -864,7 +864,13 @@ postquel_start(execution_state *es, SQLFunctionCachePtr fcache)
eflags = EXEC_FLAG_SKIP_TRIGGERS;
else
eflags = 0; /* default run-to-completion flags */
- ExecutorStart(es->qd, eflags);
+
+ /*
+ * OK to ignore the return value; plan can't become invalid,
+ * because there's no CachedPlan.
+ */
+ if (!ExecutorStart(es->qd, NULL, eflags))
+ elog(ERROR, "unexpected failure running ExecutorStart()");
}
es->status = F_EXEC_RUN;
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..72437f729f 100644
--- a/src/backend/executor/nodeAppend.c
+++ b/src/backend/executor/nodeAppend.c
@@ -133,6 +133,20 @@ ExecInitAppend(Append *node, EState *estate, int eflags)
appendstate->as_syncdone = false;
appendstate->as_begun = false;
+ /*
+ * Lock non-leaf partitions whose leaf children are present in
+ * node->appendplans. Only need to do so if executing a cached
+ * plan, because child tables present in cached plans are not
+ * locked before execution.
+ *
+ * XXX - some of the non-leaf partitions may also be mentioned in
+ * part_prune_info, which would get locked again in
+ * ExecInitPartitionPruning() because it calls
+ * ExecGetRangeTableRelation() which locks child tables.
+ */
+ if (estate->es_cachedplan)
+ ExecLockAppendPartRels(estate, node->allpartrelids);
+
/* If run-time partition pruning is enabled, then set that up now */
if (node->part_prune_info != NULL)
{
@@ -185,8 +199,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 +237,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 dbf4920363..df55e697c0 100644
--- a/src/backend/executor/nodeHash.c
+++ b/src/backend/executor/nodeHash.c
@@ -385,6 +385,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 592c098b9f..5e11975702 100644
--- a/src/backend/executor/nodeHashjoin.c
+++ b/src/backend/executor/nodeHashjoin.c
@@ -760,8 +760,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..109a90fe74 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,7 +585,9 @@ 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);
+ if (unlikely(!ExecPlanStillValid(estate)))
+ return indexstate;
indexstate->ioss_RelationDesc = indexRelation;
/*
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..6e00c90916 100644
--- a/src/backend/executor/nodeMergeAppend.c
+++ b/src/backend/executor/nodeMergeAppend.c
@@ -81,6 +81,20 @@ ExecInitMergeAppend(MergeAppend *node, EState *estate, int eflags)
mergestate->ps.state = estate;
mergestate->ps.ExecProcNode = ExecMergeAppend;
+ /*
+ * Lock non-leaf partitions whose leaf children are present in
+ * node->mergeplans. Only need to do so if executing a cached
+ * plan, because child tables present in cached plans are not
+ * locked before execution.
+ *
+ * XXX - some of the non-leaf partitions may also be mentioned in
+ * part_prune_info, which would get locked again in
+ * ExecInitPartitionPruning() because it calls
+ * ExecGetRangeTableRelation() which locks child tables.
+ */
+ if (estate->es_cachedplan)
+ ExecLockAppendPartRels(estate, node->allpartrelids);
+
/* If run-time partition pruning is enabled, then set that up now */
if (node->part_prune_info != NULL)
{
@@ -120,7 +134,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 +165,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 062a780f29..aff089799d 100644
--- a/src/backend/executor/nodeModifyTable.c
+++ b/src/backend/executor/nodeModifyTable.c
@@ -4274,6 +4274,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);
@@ -4306,6 +4313,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
@@ -4332,6 +4343,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 d6516b1bca..5885a1d056 100644
--- a/src/backend/executor/spi.c
+++ b/src/backend/executor/spi.c
@@ -70,7 +70,7 @@ 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, uint64 tcount);
static void _SPI_error_callback(void *arg);
@@ -1581,6 +1581,7 @@ SPI_cursor_open_internal(const char *name, SPIPlanPtr plan,
Snapshot snapshot;
MemoryContext oldcontext;
Portal portal;
+ bool plan_valid;
SPICallbackArg spicallbackarg;
ErrorContextCallback spierrcontext;
@@ -1622,6 +1623,7 @@ SPI_cursor_open_internal(const char *name, SPIPlanPtr plan,
_SPI_current->processed = 0;
_SPI_current->tuptable = NULL;
+replan:
/* Create the portal */
if (name == NULL || name[0] == '\0')
{
@@ -1765,15 +1767,24 @@ SPI_cursor_open_internal(const char *name, SPIPlanPtr plan,
}
/*
- * Start portal execution.
+ * Start portal execution. If the portal contains a cached plan, it must
+ * be recreated if the cached plan was found to have been invalidated when
+ * initializing one of the plan trees contained in it.
*/
- PortalStart(portal, paramLI, 0, snapshot);
+ plan_valid = PortalStart(portal, paramLI, 0, snapshot, cplan);
Assert(portal->strategy != PORTAL_MULTI_QUERY);
/* Pop the error context stack */
error_context_stack = spierrcontext.previous;
+ if (!plan_valid)
+ {
+ Assert(cplan != NULL);
+ PortalDrop(portal, false);
+ goto replan;
+ }
+
/* Pop the SPI stack */
_SPI_end_call(true);
@@ -2568,6 +2579,7 @@ _SPI_execute_plan(SPIPlanPtr plan, const SPIExecuteOptions *options,
* Replan if needed, and increment plan refcount. If it's a saved
* plan, the refcount must be backed by the plan_owner.
*/
+replan:
cplan = GetCachedPlan(plansource, options->params,
plan_owner, _SPI_current->queryEnv);
@@ -2677,6 +2689,7 @@ _SPI_execute_plan(SPIPlanPtr plan, const SPIExecuteOptions *options,
{
QueryDesc *qdesc;
Snapshot snap;
+ int eflags;
if (ActiveSnapshotSet())
snap = GetActiveSnapshot();
@@ -2690,8 +2703,20 @@ _SPI_execute_plan(SPIPlanPtr plan, const SPIExecuteOptions *options,
options->params,
_SPI_current->queryEnv,
0);
- res = _SPI_pquery(qdesc, fire_triggers,
- canSetTag ? options->tcount : 0);
+
+ /* Select execution options */
+ if (fire_triggers)
+ eflags = 0; /* default run-to-completion flags */
+ else
+ eflags = EXEC_FLAG_SKIP_TRIGGERS;
+
+ if (!TryExecutorStart(qdesc, cplan, eflags))
+ {
+ ReleaseCachedPlan(cplan, plan_owner);
+ goto replan;
+ }
+
+ res = _SPI_pquery(qdesc, canSetTag ? options->tcount : 0);
FreeQueryDesc(qdesc);
}
else
@@ -2865,10 +2890,9 @@ _SPI_convert_params(int nargs, Oid *argtypes,
}
static int
-_SPI_pquery(QueryDesc *queryDesc, bool fire_triggers, uint64 tcount)
+_SPI_pquery(QueryDesc *queryDesc, uint64 tcount)
{
int operation = queryDesc->operation;
- int eflags;
int res;
switch (operation)
@@ -2915,14 +2939,6 @@ _SPI_pquery(QueryDesc *queryDesc, bool fire_triggers, uint64 tcount)
ResetUsage();
#endif
- /* Select execution options */
- if (fire_triggers)
- eflags = 0; /* default run-to-completion flags */
- else
- eflags = EXEC_FLAG_SKIP_TRIGGERS;
-
- ExecutorStart(queryDesc, eflags);
-
ExecutorRun(queryDesc, ForwardScanDirection, tcount, true);
_SPI_current->processed = queryDesc->estate->es_processed;
diff --git a/src/backend/optimizer/util/inherit.c b/src/backend/optimizer/util/inherit.c
index 4797312ae5..be6e4ddfdf 100644
--- a/src/backend/optimizer/util/inherit.c
+++ b/src/backend/optimizer/util/inherit.c
@@ -493,6 +493,13 @@ expand_single_inheritance_child(PlannerInfo *root, RangeTblEntry *parentrte,
}
else
childrte->inh = false;
+
+ /*
+ * Flag child tables as indirectly referenced in the query. This is to
+ * allow ExecGetRangeTableRelation() recognize them as inheritance
+ * child tables.
+ */
+ childrte->inFromCl = false;
childrte->securityQuals = NIL;
/* No permission checking for child RTEs. */
diff --git a/src/backend/parser/analyze.c b/src/backend/parser/analyze.c
index e901203424..fe7ad7b82f 100644
--- a/src/backend/parser/analyze.c
+++ b/src/backend/parser/analyze.c
@@ -3327,10 +3327,9 @@ transformLockingClause(ParseState *pstate, Query *qry, LockingClause *lc,
/*
* Lock all regular tables used in query and its subqueries. We
* examine inFromCl to exclude auto-added RTEs, particularly NEW/OLD
- * in rules. This is a bit of an abuse of a mostly-obsolete flag, but
- * it's convenient. We can't rely on the namespace mechanism that has
- * largely replaced inFromCl, since for example we need to lock
- * base-relation RTEs even if they are masked by upper joins.
+ * in rules. We can't rely on the namespace mechanism since for
+ * example we need to lock base-relation RTEs even if they are masked
+ * by upper joins.
*/
i = 0;
foreach(rt, qry->rtable)
diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
index 8bc6bea113..4f2196aaaf 100644
--- a/src/backend/tcop/postgres.c
+++ b/src/backend/tcop/postgres.c
@@ -1241,8 +1241,11 @@ exec_simple_query(const char *query_string)
/*
* Start the portal. No parameters here.
+ *
+ * Plan can't become invalid here, because there's no CachedPlan.
*/
- PortalStart(portal, NULL, 0, InvalidSnapshot);
+ if (!PortalStart(portal, NULL, 0, InvalidSnapshot, NULL))
+ elog(ERROR, "unexpected failure running PortalStart()");
/*
* Select the appropriate output format: text unless we are doing a
@@ -1747,6 +1750,7 @@ exec_bind_message(StringInfo input_message)
"commands ignored until end of transaction block"),
errdetail_abort()));
+replan:
/*
* Create the portal. Allow silent replacement of an existing portal only
* if the unnamed portal is specified.
@@ -2034,9 +2038,16 @@ exec_bind_message(StringInfo input_message)
PopActiveSnapshot();
/*
- * And we're ready to start portal execution.
+ * Start portal execution. If the portal contains a cached plan, it must
+ * be recreated if the cached plan was found to have been invalidated when
+ * initializing one of the plan trees contained in it.
*/
- PortalStart(portal, params, 0, InvalidSnapshot);
+ if (!PortalStart(portal, params, 0, InvalidSnapshot, cplan))
+ {
+ Assert(cplan != NULL);
+ PortalDrop(portal, false);
+ goto replan;
+ }
/*
* Apply the result format requests to the portal.
diff --git a/src/backend/tcop/pquery.c b/src/backend/tcop/pquery.c
index a1f8d03db1..ea33da4c2a 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"
@@ -34,8 +35,7 @@
*/
Portal ActivePortal = NULL;
-
-static void ProcessQuery(PlannedStmt *plan,
+static void ProcessQuery(QueryDesc *queryDesc,
const char *sourceText,
ParamListInfo params,
QueryEnvironment *queryEnv,
@@ -115,13 +115,12 @@ FreeQueryDesc(QueryDesc *qdesc)
pfree(qdesc);
}
-
/*
* ProcessQuery
* Execute a single plannable query within a PORTAL_MULTI_QUERY,
* PORTAL_ONE_RETURNING, or PORTAL_ONE_MOD_WITH portal
*
- * plan: the plan tree for the query
+ * queryDesc: QueryDesc created in PortalStart()
* sourceText: the source text of the query
* params: any parameters needed
* dest: where to send results
@@ -133,26 +132,14 @@ FreeQueryDesc(QueryDesc *qdesc)
* error; otherwise the executor's memory usage will be leaked.
*/
static void
-ProcessQuery(PlannedStmt *plan,
+ProcessQuery(QueryDesc *queryDesc,
const char *sourceText,
ParamListInfo params,
QueryEnvironment *queryEnv,
DestReceiver *dest,
QueryCompletion *qc)
{
- QueryDesc *queryDesc;
-
- /*
- * Create the QueryDesc object
- */
- queryDesc = CreateQueryDesc(plan, sourceText,
- GetActiveSnapshot(), InvalidSnapshot,
- dest, params, queryEnv, 0);
-
- /*
- * Call ExecutorStart to prepare the plan for execution
- */
- ExecutorStart(queryDesc, 0);
+ queryDesc->dest = dest;
/*
* Run the plan to completion.
@@ -426,19 +413,22 @@ FetchStatementTargetList(Node *stmt)
* presently ignored for non-PORTAL_ONE_SELECT portals (it's only intended
* to be used for cursors).
*
- * On return, portal is ready to accept PortalRun() calls, and the result
- * tupdesc (if any) is known.
+ * True is returned if portal is ready to accept PortalRun() calls, and the
+ * result tupdesc (if any) is known. False if the plan tree is no longer
+ * valid, in which case, the caller must retry after generating a new
+ * CachedPlan.
*/
-void
+bool
PortalStart(Portal portal, ParamListInfo params,
- int eflags, Snapshot snapshot)
+ int eflags, Snapshot snapshot,
+ CachedPlan *cplan)
{
Portal saveActivePortal;
ResourceOwner saveResourceOwner;
- MemoryContext savePortalContext;
MemoryContext oldContext;
QueryDesc *queryDesc;
- int myeflags;
+ int myeflags = 0;
+ bool plan_valid = true;
Assert(PortalIsValid(portal));
Assert(portal->status == PORTAL_DEFINED);
@@ -448,15 +438,13 @@ PortalStart(Portal portal, ParamListInfo params,
*/
saveActivePortal = ActivePortal;
saveResourceOwner = CurrentResourceOwner;
- savePortalContext = PortalContext;
PG_TRY();
{
ActivePortal = portal;
if (portal->resowner)
CurrentResourceOwner = portal->resowner;
- PortalContext = portal->portalContext;
- oldContext = MemoryContextSwitchTo(PortalContext);
+ oldContext = MemoryContextSwitchTo(portal->queryContext);
/* Must remember portal param list, if any */
portal->portalParams = params;
@@ -472,6 +460,8 @@ PortalStart(Portal portal, ParamListInfo params,
switch (portal->strategy)
{
case PORTAL_ONE_SELECT:
+ case PORTAL_ONE_RETURNING:
+ case PORTAL_ONE_MOD_WITH:
/* Must set snapshot before starting executor. */
if (snapshot)
@@ -489,8 +479,8 @@ PortalStart(Portal portal, ParamListInfo params,
*/
/*
- * Create QueryDesc in portal's context; for the moment, set
- * the destination to DestNone.
+ * Create QueryDesc in portal->queryContext; for the moment,
+ * set the destination to DestNone.
*/
queryDesc = CreateQueryDesc(linitial_node(PlannedStmt, portal->stmts),
portal->sourceText,
@@ -501,30 +491,49 @@ PortalStart(Portal portal, ParamListInfo params,
portal->queryEnv,
0);
+ /* Remember for PortalRunMulti(). */
+ if (portal->strategy == PORTAL_ONE_RETURNING ||
+ portal->strategy == PORTAL_ONE_MOD_WITH)
+ portal->qdescs = list_make1(queryDesc);
+
/*
* If it's a scrollable cursor, executor needs to support
* REWIND and backwards scan, as well as whatever the caller
* might've asked for.
*/
- if (portal->cursorOptions & CURSOR_OPT_SCROLL)
+ if (portal->strategy == PORTAL_ONE_SELECT &&
+ (portal->cursorOptions & CURSOR_OPT_SCROLL))
myeflags = eflags | EXEC_FLAG_REWIND | EXEC_FLAG_BACKWARD;
else
myeflags = eflags;
/*
- * Call ExecutorStart to prepare the plan for execution
+ * Call TryExecutorStart to prepare the plan for execution. A
+ * cached plan may get invalidated during plan intialization.
*/
- ExecutorStart(queryDesc, myeflags);
+ if (!TryExecutorStart(queryDesc, cplan, myeflags))
+ {
+ PopActiveSnapshot();
+ plan_valid = false;
+ goto plan_init_failed;
+ }
/*
- * This tells PortalCleanup to shut down the executor
+ * This tells PortalCleanup to shut down the executor, though
+ * not needed for queries handled by PortalRunMulti().
*/
- portal->queryDesc = queryDesc;
+ if (portal->strategy == PORTAL_ONE_SELECT)
+ portal->queryDesc = queryDesc;
/*
- * Remember tuple descriptor (computed by ExecutorStart)
+ * Remember tuple descriptor (computed by ExecutorStart),
+ * though make it independent of QueryDesc for queries handled
+ * by PortalRunMulti().
*/
- portal->tupDesc = queryDesc->tupDesc;
+ if (portal->strategy != PORTAL_ONE_SELECT)
+ portal->tupDesc = CreateTupleDescCopy(queryDesc->tupDesc);
+ else
+ portal->tupDesc = queryDesc->tupDesc;
/*
* Reset cursor position data to "start of query"
@@ -536,29 +545,6 @@ PortalStart(Portal portal, ParamListInfo params,
PopActiveSnapshot();
break;
- case PORTAL_ONE_RETURNING:
- case PORTAL_ONE_MOD_WITH:
-
- /*
- * We don't start the executor until we are told to run the
- * portal. We do need to set up the result tupdesc.
- */
- {
- PlannedStmt *pstmt;
-
- pstmt = PortalGetPrimaryStmt(portal);
- portal->tupDesc =
- ExecCleanTypeFromTL(pstmt->planTree->targetlist);
- }
-
- /*
- * Reset cursor position data to "start of query"
- */
- portal->atStart = true;
- portal->atEnd = false; /* allow fetches */
- portal->portalPos = 0;
- break;
-
case PORTAL_UTIL_SELECT:
/*
@@ -581,7 +567,79 @@ PortalStart(Portal portal, ParamListInfo params,
break;
case PORTAL_MULTI_QUERY:
- /* Need do nothing now */
+ {
+ ListCell *lc;
+ bool first = true;
+
+ myeflags = eflags;
+ foreach(lc, portal->stmts)
+ {
+ PlannedStmt *plan = lfirst_node(PlannedStmt, lc);
+ bool is_utility = (plan->utilityStmt != NULL);
+
+ /*
+ * Push the snapshot to be used by the executor.
+ */
+ if (!is_utility)
+ {
+ /*
+ * Must copy the snapshot for all statements
+ * except thec first as we'll need to update its
+ * command ID.
+ */
+ if (!first)
+ PushCopiedSnapshot(GetTransactionSnapshot());
+ else
+ PushActiveSnapshot(GetTransactionSnapshot());
+ }
+
+ /*
+ * From the 2nd statement onwards, update the command
+ * ID and the snapshot to match.
+ */
+ if (!first)
+ {
+ CommandCounterIncrement();
+ UpdateActiveSnapshotCommandId();
+ }
+
+ first = false;
+
+ /*
+ * Create the QueryDesc. DestReceiver will be set in
+ * PortalRunMulti() before calling ExecutorRun().
+ */
+ queryDesc = CreateQueryDesc(plan,
+ portal->sourceText,
+ !is_utility ?
+ GetActiveSnapshot() :
+ InvalidSnapshot,
+ InvalidSnapshot,
+ NULL,
+ params,
+ portal->queryEnv, 0);
+
+ /* Remember for PortalRunMulti() */
+ portal->qdescs = lappend(portal->qdescs, queryDesc);
+
+ if (is_utility)
+ continue;
+
+ /*
+ * Call ExecutorStart to prepare the plan for
+ * execution. A cached plan may get invalidated
+ * during plan intialization.
+ */
+ if (!ExecutorStart(queryDesc, cplan, myeflags))
+ {
+ PopActiveSnapshot();
+ plan_valid = false;
+ goto plan_init_failed;
+ }
+ PopActiveSnapshot();
+ }
+ }
+
portal->tupDesc = NULL;
break;
}
@@ -594,19 +652,20 @@ PortalStart(Portal portal, ParamListInfo params,
/* Restore global vars and propagate error */
ActivePortal = saveActivePortal;
CurrentResourceOwner = saveResourceOwner;
- PortalContext = savePortalContext;
PG_RE_THROW();
}
PG_END_TRY();
+ portal->status = PORTAL_READY;
+
+plan_init_failed:
MemoryContextSwitchTo(oldContext);
ActivePortal = saveActivePortal;
CurrentResourceOwner = saveResourceOwner;
- PortalContext = savePortalContext;
- portal->status = PORTAL_READY;
+ return plan_valid;
}
/*
@@ -1193,8 +1252,8 @@ PortalRunMulti(Portal portal,
DestReceiver *dest, DestReceiver *altdest,
QueryCompletion *qc)
{
- bool active_snapshot_set = false;
- ListCell *stmtlist_item;
+ bool holdSnapshotSet = false;
+ ListCell *qdesc_item;
/*
* If the destination is DestRemoteExecute, change to DestNone. The
@@ -1215,9 +1274,10 @@ PortalRunMulti(Portal portal,
* Loop to handle the individual queries generated from a single parsetree
* by analysis and rewrite.
*/
- foreach(stmtlist_item, portal->stmts)
+ foreach(qdesc_item, portal->qdescs)
{
- PlannedStmt *pstmt = lfirst_node(PlannedStmt, stmtlist_item);
+ QueryDesc *qdesc = (QueryDesc *) lfirst(qdesc_item);
+ PlannedStmt *pstmt = qdesc->plannedstmt;
/*
* If we got a cancel signal in prior command, quit
@@ -1234,48 +1294,33 @@ PortalRunMulti(Portal portal,
if (log_executor_stats)
ResetUsage();
+ /* Push the snapshot for plannable queries. */
+ PushActiveSnapshot(qdesc->snapshot);
+
/*
- * Must always have a snapshot for plannable queries. First time
- * through, take a new snapshot; for subsequent queries in the
- * same portal, just update the snapshot's copy of the command
- * counter.
+ * If told to, register the snapshot and save in portal
+ *
+ * Note that the command ID of qdesc->snapshot for 2nd query
+ * onwards would have been updated in PortalStart() to account
+ * for CCI() done between queries, but it's okay for the command
+ * ID of the active snapshot to diverge from what holdSnapshot
+ * has.
*/
- if (!active_snapshot_set)
+ if (setHoldSnapshot && !holdSnapshotSet)
{
- Snapshot snapshot = GetTransactionSnapshot();
-
- /* If told to, register the snapshot and save in portal */
- if (setHoldSnapshot)
- {
- snapshot = RegisterSnapshot(snapshot);
- portal->holdSnapshot = snapshot;
- }
-
- /*
- * We can't have the holdSnapshot also be the active one,
- * because UpdateActiveSnapshotCommandId would complain. So
- * force an extra snapshot copy. Plain PushActiveSnapshot
- * would have copied the transaction snapshot anyway, so this
- * only adds a copy step when setHoldSnapshot is true. (It's
- * okay for the command ID of the active snapshot to diverge
- * from what holdSnapshot has.)
- */
- PushCopiedSnapshot(snapshot);
-
- /*
- * As for PORTAL_ONE_SELECT portals, it does not seem
- * necessary to maintain portal->portalSnapshot here.
- */
-
- active_snapshot_set = true;
+ portal->holdSnapshot = RegisterSnapshot(qdesc->snapshot);
+ holdSnapshotSet = true;
}
- else
- UpdateActiveSnapshotCommandId();
+
+ /*
+ * As for PORTAL_ONE_SELECT portals, it does not seem
+ * necessary to maintain portal->portalSnapshot here.
+ */
if (pstmt->canSetTag)
{
/* statement can set tag string */
- ProcessQuery(pstmt,
+ ProcessQuery(qdesc,
portal->sourceText,
portal->portalParams,
portal->queryEnv,
@@ -1284,13 +1329,15 @@ PortalRunMulti(Portal portal,
else
{
/* stmt added by rewrite cannot set tag */
- ProcessQuery(pstmt,
+ ProcessQuery(qdesc,
portal->sourceText,
portal->portalParams,
portal->queryEnv,
altdest, NULL);
}
+ PopActiveSnapshot();
+
if (log_executor_stats)
ShowUsage("EXECUTOR STATISTICS");
@@ -1311,7 +1358,6 @@ PortalRunMulti(Portal portal,
*/
if (pstmt->canSetTag)
{
- Assert(!active_snapshot_set);
/* statement can set tag string */
PortalRunUtility(portal, pstmt, isTopLevel, false,
dest, qc);
@@ -1342,19 +1388,8 @@ PortalRunMulti(Portal portal,
*/
if (portal->stmts == NIL)
break;
-
- /*
- * Increment command counter between queries, but not after the last
- * one.
- */
- if (lnext(portal->stmts, stmtlist_item) != NULL)
- CommandCounterIncrement();
}
- /* Pop the snapshot if we pushed one. */
- if (active_snapshot_set)
- PopActiveSnapshot();
-
/*
* If a query completion data was supplied, use it. Otherwise use the
* portal's query completion data.
diff --git a/src/backend/utils/mmgr/portalmem.c b/src/backend/utils/mmgr/portalmem.c
index 4a24613537..2d5ba638f7 100644
--- a/src/backend/utils/mmgr/portalmem.c
+++ b/src/backend/utils/mmgr/portalmem.c
@@ -200,6 +200,13 @@ CreatePortal(const char *name, bool allowDup, bool dupSilent)
portal->portalContext = AllocSetContextCreate(TopPortalContext,
"PortalContext",
ALLOCSET_SMALL_SIZES);
+ /*
+ * initialize portal's query context to store QueryDescs created during
+ * PortalStart() and then used in PortalRun().
+ */
+ portal->queryContext = AllocSetContextCreate(TopPortalContext,
+ "PortalQueryContext",
+ ALLOCSET_SMALL_SIZES);
/* create a resource owner for the portal */
portal->resowner = ResourceOwnerCreate(CurTransactionResourceOwner,
@@ -223,6 +230,7 @@ CreatePortal(const char *name, bool allowDup, bool dupSilent)
/* for named portals reuse portal->name copy */
MemoryContextSetIdentifier(portal->portalContext, portal->name[0] ? portal->name : "<unnamed>");
+ MemoryContextSetIdentifier(portal->queryContext, portal->name[0] ? portal->name : "<unnamed>");
return portal;
}
@@ -593,6 +601,7 @@ PortalDrop(Portal portal, bool isTopCommit)
/* release subsidiary storage */
MemoryContextDelete(portal->portalContext);
+ MemoryContextDelete(portal->queryContext);
/* release portal struct (it's in TopPortalContext) */
pfree(portal);
diff --git a/src/include/commands/explain.h b/src/include/commands/explain.h
index 9b8b351d9a..78292c69f9 100644
--- a/src/include/commands/explain.h
+++ b/src/include/commands/explain.h
@@ -101,7 +101,8 @@ extern void ExplainOneUtility(Node *utilityStmt, IntoClause *into,
ExplainState *es, const char *queryString,
ParamListInfo params, QueryEnvironment *queryEnv);
-extern void ExplainOnePlan(PlannedStmt *plannedstmt, IntoClause *into,
+extern bool ExplainOnePlan(PlannedStmt *stmt, CachedPlan *cplan,
+ IntoClause *into,
ExplainState *es, const char *queryString,
ParamListInfo params, QueryEnvironment *queryEnv,
const instr_time *planduration,
@@ -118,6 +119,7 @@ extern void ExplainQueryParameters(ExplainState *es, ParamListInfo params, int m
extern void ExplainBeginOutput(ExplainState *es);
extern void ExplainEndOutput(ExplainState *es);
+extern void ExplainResetOutput(ExplainState *es);
extern void ExplainSeparatePlans(ExplainState *es);
extern void ExplainPropertyList(const char *qlabel, List *data,
diff --git a/src/include/commands/trigger.h b/src/include/commands/trigger.h
index 8a5a9fe642..d3e6cf24cb 100644
--- a/src/include/commands/trigger.h
+++ b/src/include/commands/trigger.h
@@ -257,6 +257,7 @@ extern void ExecASTruncateTriggers(EState *estate,
extern void AfterTriggerBeginXact(void);
extern void AfterTriggerBeginQuery(void);
+extern void AfterTriggerCancelQuery(void);
extern void AfterTriggerEndQuery(EState *estate);
extern void AfterTriggerFireDeferred(void);
extern void AfterTriggerEndXact(bool isCommit);
diff --git a/src/include/executor/executor.h b/src/include/executor/executor.h
index 9770752ea3..1dbc0997ae 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"
/*
@@ -72,7 +73,8 @@
/* Hook for plugins to get control in ExecutorStart() */
-typedef void (*ExecutorStart_hook_type) (QueryDesc *queryDesc, int eflags);
+typedef bool (*ExecutorStart_hook_type) (QueryDesc *queryDesc, CachedPlan *cplan,
+ int eflags);
extern PGDLLIMPORT ExecutorStart_hook_type ExecutorStart_hook;
/* Hook for plugins to get control in ExecutorRun() */
@@ -197,8 +199,10 @@ ExecGetJunkAttribute(TupleTableSlot *slot, AttrNumber attno, bool *isNull)
/*
* prototypes from functions in execMain.c
*/
-extern void ExecutorStart(QueryDesc *queryDesc, int eflags);
-extern void standard_ExecutorStart(QueryDesc *queryDesc, int eflags);
+extern bool ExecutorStart(QueryDesc *queryDesc, CachedPlan *cplan, int eflags);
+extern bool TryExecutorStart(QueryDesc *queryDesc, CachedPlan *cplan, int eflags);
+extern bool standard_ExecutorStart(QueryDesc *queryDesc, CachedPlan *cplan,
+ int eflags);
extern void ExecutorRun(QueryDesc *queryDesc,
ScanDirection direction, uint64 count, bool execute_once);
extern void standard_ExecutorRun(QueryDesc *queryDesc,
@@ -257,6 +261,15 @@ extern void ExecEndNode(PlanState *node);
extern void ExecShutdownNode(PlanState *node);
extern void ExecSetTupleBound(int64 tuples_needed, PlanState *child_node);
+/*
+ * Is the CachedPlan in es_cachedplan still valid?
+ */
+static inline bool
+ExecPlanStillValid(EState *estate)
+{
+ return estate->es_cachedplan == NULL ? true :
+ CachedPlanStillValid(estate->es_cachedplan);
+}
/* ----------------------------------------------------------------
* ExecProcNode
@@ -578,6 +591,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);
@@ -590,6 +604,7 @@ exec_rt_fetch(Index rti, EState *estate)
}
extern Relation ExecGetRangeTableRelation(EState *estate, Index rti);
+extern void ExecLockAppendPartRels(EState *estate, List *allpartrelids);
extern void ExecInitResultRelation(EState *estate, ResultRelInfo *resultRelInfo,
Index rti);
diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h
index c3670f7158..4bc6d9d461 100644
--- a/src/include/nodes/execnodes.h
+++ b/src/include/nodes/execnodes.h
@@ -631,6 +631,8 @@ typedef struct EState
* ExecRowMarks, or NULL if none */
List *es_rteperminfos; /* List of RTEPermissionInfo */
PlannedStmt *es_plannedstmt; /* link to top of plan tree */
+ struct CachedPlan *es_cachedplan; /* CachedPlan if plannedstmt is from
+ * one, or NULL if not */
const char *es_sourceText; /* Source text from QueryDesc */
JunkFilter *es_junkFilter; /* top-level junk filter, if any */
@@ -676,6 +678,9 @@ 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_canceled; /* true when execution was canceled
+ * upon encountering that plan was invalided
+ * during ExecInitNode() */
List *es_exprcontexts; /* List of ExprContexts within EState */
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index 85a62b538e..e8f14982c0 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -1009,11 +1009,15 @@ typedef struct PartitionCmd
*
* inFromCl marks those range variables that are listed in the FROM clause.
* It's false for RTEs that are added to a query behind the scenes, such
- * as the NEW and OLD variables for a rule, or the subqueries of a UNION.
+ * as the NEW and OLD variables for a rule, or the subqueries of a UNION,
+ * or the RTEs of inheritance child tables that are added by the planner.
* This flag is not used during parsing (except in transformLockingClause,
* q.v.); the parser now uses a separate "namespace" data structure to
* control visibility. But it is needed by ruleutils.c to determine
- * whether RTEs should be shown in decompiled queries.
+ * whether RTEs should be shown in decompiled queries. The executor uses
+ * this to ascertain if an RTE_RELATION entry is for a table explicitly
+ * named in the query or a child table added by the planner. This
+ * distinction is vital when child tables in a plan must be locked.
*
* securityQuals is a list of security barrier quals (boolean expressions),
* to be tested in the listed order before returning a row from the
diff --git a/src/include/tcop/pquery.h b/src/include/tcop/pquery.h
index 073fb323bc..274510598f 100644
--- a/src/include/tcop/pquery.h
+++ b/src/include/tcop/pquery.h
@@ -29,8 +29,9 @@ extern List *FetchPortalTargetList(Portal portal);
extern List *FetchStatementTargetList(Node *stmt);
-extern void PortalStart(Portal portal, ParamListInfo params,
- int eflags, Snapshot snapshot);
+extern bool PortalStart(Portal portal, ParamListInfo params,
+ int eflags, Snapshot snapshot,
+ CachedPlan *cplan);
extern void PortalSetResultFormat(Portal portal, int nFormats,
int16 *formats);
diff --git a/src/include/utils/plancache.h b/src/include/utils/plancache.h
index a90dfdf906..f88e2abad2 100644
--- a/src/include/utils/plancache.h
+++ b/src/include/utils/plancache.h
@@ -223,6 +223,20 @@ extern CachedPlan *GetCachedPlan(CachedPlanSource *plansource,
ParamListInfo boundParams,
ResourceOwner owner,
QueryEnvironment *queryEnv);
+
+/*
+ * CachedPlanStillValid
+ * Returns if a cached generic plan is still valid
+ *
+ * Invoked by the executor for each relation lock acquired during the
+ * initialization of the plan tree within the CachedPlan.
+ */
+static inline bool
+CachedPlanStillValid(CachedPlan *cplan)
+{
+ return cplan->is_valid;
+}
+
extern void ReleaseCachedPlan(CachedPlan *plan, ResourceOwner owner);
extern bool CachedPlanAllowsSimpleValidityCheck(CachedPlanSource *plansource,
diff --git a/src/include/utils/portal.h b/src/include/utils/portal.h
index 29f49829f2..c0707c4876 100644
--- a/src/include/utils/portal.h
+++ b/src/include/utils/portal.h
@@ -138,6 +138,8 @@ typedef struct PortalData
QueryCompletion qc; /* command completion data for executed query */
List *stmts; /* list of PlannedStmts */
CachedPlan *cplan; /* CachedPlan, if stmts are from one */
+ List *qdescs; /* list of QueryDescs */
+ MemoryContext queryContext; /* memory for QueryDescs and children */
ParamListInfo portalParams; /* params to pass to query */
QueryEnvironment *queryEnv; /* environment for query */
--
2.43.0
[application/octet-stream] v50-0006-Track-opened-range-table-relations-in-a-List-in-.patch (2.5K, 7-v50-0006-Track-opened-range-table-relations-in-a-List-in-.patch)
download | inline diff:
From c72ca47d9d9e9af4e7b422d217d126bc7cd14ba4 Mon Sep 17 00:00:00 2001
From: Amit Langote <[email protected]>
Date: Wed, 6 Sep 2023 17:54:19 +0900
Subject: [PATCH v50 6/6] Track opened range table relations in a List in
EState
This makes ExecCloseRangeTableRelations faster when there are many
relations in the range table but only a few are opened during
execution, such as when run-time pruning kicks in on an Append
containing thousands of partition subplans.
Discussion: https://postgr.es/m/CA+HiwqFGkMSge6TgC9KQzde0ohpAycLQuV7ooitEEpbKB0O_mg@mail.gmail.com
---
src/backend/executor/execMain.c | 9 +++++----
src/backend/executor/execUtils.c | 3 +++
src/include/nodes/execnodes.h | 2 ++
3 files changed, 10 insertions(+), 4 deletions(-)
diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c
index 168ab553ac..8e067ab7d7 100644
--- a/src/backend/executor/execMain.c
+++ b/src/backend/executor/execMain.c
@@ -1682,12 +1682,13 @@ ExecCloseResultRelations(EState *estate)
void
ExecCloseRangeTableRelations(EState *estate)
{
- int i;
+ ListCell *lc;
- for (i = 0; i < estate->es_range_table_size; i++)
+ foreach(lc, estate->es_opened_relations)
{
- if (estate->es_relations[i])
- table_close(estate->es_relations[i], NoLock);
+ Relation rel = lfirst(lc);
+
+ table_close(rel, NoLock);
}
}
diff --git a/src/backend/executor/execUtils.c b/src/backend/executor/execUtils.c
index edf1c24e0e..2e4e748559 100644
--- a/src/backend/executor/execUtils.c
+++ b/src/backend/executor/execUtils.c
@@ -840,6 +840,9 @@ ExecGetRangeTableRelation(EState *estate, Index rti)
}
estate->es_relations[rti - 1] = rel;
+ if (rel != NULL)
+ estate->es_opened_relations = lappend(estate->es_opened_relations,
+ rel);
}
return rel;
diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h
index 4bc6d9d461..655b6f1b8d 100644
--- a/src/include/nodes/execnodes.h
+++ b/src/include/nodes/execnodes.h
@@ -627,6 +627,8 @@ typedef struct EState
Index es_range_table_size; /* size of the range table arrays */
Relation *es_relations; /* Array of per-range-table-entry Relation
* pointers, or NULL if not yet opened */
+ List *es_opened_relations; /* List of non-NULL entries in
+ * es_relations in no specific order */
struct ExecRowMark **es_rowmarks; /* Array of per-range-table-entry
* ExecRowMarks, or NULL if none */
List *es_rteperminfos; /* List of RTEPermissionInfo */
--
2.43.0
view thread (3+ messages)
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+HiwqHL=aGU9Y4RYXQ5VCp4L5NVdiaQLLoXN3NCQQQMKo0ByQ@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