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, 28 Sep 2023 17:26:27 +0900
Message-ID: <CA+HiwqG9YF-xxnXRW604LkJbTvFABwfSBgrNcQFMCL5-QG=P0g@mail.gmail.com> (raw)
In-Reply-To: <CA+HiwqFM6qE-qc5SQyQYXf-OTsPOaCVq=qDqBnyra_mthySLRw@mail.gmail.com>
References: <[email protected]>
<[email protected]>
<[email protected]>
<CA+HiwqFhQ8tLMLQAWYRWiBQUrWSLM8qhVzJ4B5jWr=orUXfSyA@mail.gmail.com>
<[email protected]>
<CA+HiwqGFm_5aUqPnt=WCarkJ2ZU6F8kD8pFeGurHP+NZWS8KQw@mail.gmail.com>
<CA+HiwqEDr9m3NGrmiOgatCnRPwD95=MHgWQdwvnoMyQd3k9-Yw@mail.gmail.com>
<CA+HiwqGTrQ=ywAmB2zP81jcENZh1vLuyJaC2-xhWvBsnXWgZYQ@mail.gmail.com>
<CA+HiwqGe0W2C+SrKcxrk-r4JjO0sCfL581p1M2bzr_LSrzGn+g@mail.gmail.com>
<[email protected]>
<CA+HiwqGBumxzWctnUy33dHy2uGCtfmcqKyw4FONJNyitJvujWw@mail.gmail.com>
<CA+HiwqHmdH2bcUtBGncwB7iJ9N0VTkUo4YPYFNtJL_f3kkau=g@mail.gmail.com>
<CA+HiwqHKTxfaYc=e4mVOv0iDm3vVK56WOBCddzYdXKaWQqniww@mail.gmail.com>
<CA+HiwqHQ1PM+HXoEdvutj0huhu2cfmuPa8wtctor0NNADzZVvA@mail.gmail.com>
<CA+HiwqH=cbBocfSmyjd_N7ZceZ3RtXuQ=rNkAfdn+RwqMGY9fQ@mail.gmail.com>
<CA+HiwqHoZSM4A0HKoTERmp=_stQjpjmomgg=rCf_4x4qCpxbZA@mail.gmail.com>
<[email protected]>
<CA+HiwqFfC7ANtb+HAHYuR4wnwYbQdbK5B0ee0fjtNwTt+TOdwg@mail.gmail.com>
<CA+HiwqH3jY-W=bekWxFF=B+9tpS42_1sJGsre1Ks0ueQjhta2Q@mail.gmail.com>
<CA+HiwqEAHH=_PVG87rSHhQxmbHQ1dxSd58BVg=dHHfsgCeQFHw@mail.gmail.com>
<[email protected]>
<CA+HiwqHrkhNe=EUixymT0Nynp78Dnaqnf5qQnCowJd3ZSzXvFg@mail.gmail.com>
<CA+HiwqEDbf=+s73hAF0PigWORRx+YWwbCQtuuWtHzc3ko_DGpw@mail.gmail.com>
<CAA-aLv5EDpYBaZrPjE_kkaoERQmAPHO=fm-FwDsw3xJG5gb8Lg@mail.gmail.com>
<CA+HiwqHBbptyxjQx7964DjitA8FVNs1MN=uwrzRy=oOD0Hy3ag@mail.gmail.com>
<CA+HiwqEP3j25702EeergM7o8GqC79Dx-3gHKnvfa8oRJiXBDgA@mail.gmail.com>
<CA+HiwqE=qxN5C-oN5vguBZOZGyDRAMV2EW1pO_hObcpf6X5QwQ@mail.gmail.com>
<CA+Tgmoan++3tDKjBysnp3QdJc_f+zKYFezXQw9jUPvRY=kZ9Cg@mail.gmail.com>
<CA+HiwqGFYpPRxQVUfeds1QX1U6o1QkKMYjHrjn0+0XEcgUV7+A@mail.gmail.com>
<CA+TgmoacPSTXkPFivji-kA=DSp3jMi0TLonmtckDeq3p3=UP9w@mail.gmail.com>
<CA+HiwqFRJ9NsF5s_Yno3kQ4rLtkWxb86fikeUdjseub8j8rHeA@mail.gmail.com>
<CA+HiwqG=5uLuwR+8xNR-HN_5mj6c9t4kqVuwrcfhOuKONgDiGw@mail.gmail.com>
<CA+TgmoYhqXKbpcJH+Oa=J=c-=zxwoTHQenbOtsLsft0a0_J_og@mail.gmail.com>
<CA+HiwqGjaDzk8Q1Gapx8bnrFHTry92u52C8dEHKvZsVkq2VpJg@mail.gmail.com>
<CA+TgmobC6_4xasJoccq7t7A6ZRVo7PQN+NJJHDmipnNY-eONog@mail.gmail.com>
<CA+HiwqGSmTE37MmERJREqnO=w5NQHLPutDoq4sAvvXQXWwUPCw@mail.gmail.com>
<CA+TgmoYP-Aimev3Wa6O10TC3u_eRsODAZo0Lff3Ey-2mHdZAzw@mail.gmail.com>
<CA+HiwqFOLQt054o08yQZdXXEVM=4Sr8smLAspJsbYjLNxRG5Pw@mail.gmail.com>
<CA+HiwqFM6qE-qc5SQyQYXf-OTsPOaCVq=qDqBnyra_mthySLRw@mail.gmail.com>
On Tue, Sep 26, 2023 at 10:06 PM Amit Langote <[email protected]> wrote:
> On Mon, Sep 25, 2023 at 9:57 PM Amit Langote <[email protected]> wrote:
> > On Wed, Sep 6, 2023 at 11:20 PM Robert Haas <[email protected]> wrote:
> > > - Is there any point to all of these early exit cases? For example, in
> > > ExecInitBitmapAnd, why exit early if initialization fails? Why not
> > > just plunge ahead and if initialization failed the caller will notice
> > > that and when we ExecEndNode some of the child node pointers will be
> > > NULL but who cares? The obvious disadvantage of this approach is that
> > > we're doing a bunch of unnecessary initialization, but we're also
> > > speeding up the common case where we don't need to abort by avoiding a
> > > branch that will rarely be taken. I'm not quite sure what the right
> > > thing to do is here.
> > I thought about this some and figured that adding the
> > is-CachedPlan-still-valid tests in the following places should suffice
> > after all:
> >
> > 1. In InitPlan() right after the top-level ExecInitNode() calls
> > 2. In ExecInit*() functions of Scan nodes, right after
> > ExecOpenScanRelation() calls
>
> After sleeping on this, I think we do need the checks after all the
> ExecInitNode() calls too, because we have many instances of the code
> like the following one:
>
> outerPlanState(gatherstate) = ExecInitNode(outerNode, estate, eflags);
> tupDesc = ExecGetResultType(outerPlanState(gatherstate));
> <some code that dereferences outDesc>
>
> If outerNode is a SeqScan and ExecInitSeqScan() returned early because
> ExecOpenScanRelation() detected that plan was invalidated, then
> tupDesc would be NULL in this case, causing the code to crash.
>
> Now one might say that perhaps we should only add the
> is-CachedPlan-valid test in the instances where there is an actual
> risk of such misbehavior, but that could lead to confusion, now or
> later. It seems better to add them after every ExecInitNode() call
> while we're inventing the notion, because doing so relieves the
> authors of future enhancements of the ExecInit*() routines from
> worrying about any of this.
>
> Attached 0003 should show how that turned out.
>
> Updated 0002 as mentioned in the previous reply -- setting pointers to
> NULL after freeing them more consistently across various ExecEnd*()
> routines and using the `if (pointer != NULL)` style over the `if
> (pointer)` more consistently.
>
> Updated 0001's commit message to remove the mention of its relation to
> any future commits. I intend to push it tomorrow.
Pushed that one. Here are the rebased patches.
0001 seems ready to me, but I'll wait a couple more days for others to
weigh in. Just to highlight a kind of change that others may have
differing opinions on, consider this hunk from the patch:
- MemoryContextDelete(node->aggcontext);
+ if (node->aggcontext != NULL)
+ {
+ MemoryContextDelete(node->aggcontext);
+ node->aggcontext = NULL;
+ }
...
+ ExecEndNode(outerPlanState(node));
+ outerPlanState(node) = NULL;
So the patch wants to enhance the consistency of setting the pointer
to NULL after freeing part. Robert mentioned his preference for doing
it in the patch, which I agree with.
--
Thanks, Amit Langote
EDB: http://www.enterprisedb.com
Attachments:
[application/octet-stream] v48-0007-Delay-locking-of-child-tables-in-cached-plans-un.patch (25.2K, 2-v48-0007-Delay-locking-of-child-tables-in-cached-plans-un.patch)
download | inline diff:
From 8c0ff924890d173ddd9c6c087a725aeaccd210a0 Mon Sep 17 00:00:00 2001
From: Amit Langote <[email protected]>
Date: Wed, 6 Sep 2023 17:54:15 +0900
Subject: [PATCH v48 7/8] Delay locking of child tables in cached plans until
ExecutorStart()
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/spi.c | 2 +-
src/backend/tcop/pquery.c | 6 +-
src/backend/utils/cache/plancache.c | 154 +++++++----------
src/test/modules/delay_execution/Makefile | 3 +-
.../modules/delay_execution/delay_execution.c | 67 +++++++-
.../expected/cached-plan-replan.out | 158 ++++++++++++++++++
.../specs/cached-plan-replan.spec | 61 +++++++
7 files changed, 343 insertions(+), 108 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/spi.c b/src/backend/executor/spi.c
index 814ff1390f..9c4ed74240 100644
--- a/src/backend/executor/spi.c
+++ b/src/backend/executor/spi.c
@@ -2680,7 +2680,7 @@ replan:
snap = InvalidSnapshot;
qdesc = CreateQueryDesc(stmt,
- NULL,
+ cplan,
plansource->query_string,
snap, crosscheck_snapshot,
dest,
diff --git a/src/backend/tcop/pquery.c b/src/backend/tcop/pquery.c
index fcf9925ed4..8d0772ae29 100644
--- a/src/backend/tcop/pquery.c
+++ b/src/backend/tcop/pquery.c
@@ -412,7 +412,7 @@ PortalStart(Portal portal, ParamListInfo params,
* set the destination to DestNone.
*/
queryDesc = CreateQueryDesc(linitial_node(PlannedStmt, portal->stmts),
- NULL,
+ portal->cplan,
portal->sourceText,
GetActiveSnapshot(),
InvalidSnapshot,
@@ -443,6 +443,7 @@ PortalStart(Portal portal, ParamListInfo params,
*/
if (!ExecutorStart(queryDesc, myeflags))
{
+ Assert(queryDesc->cplan);
ExecutorEnd(queryDesc);
FreeQueryDesc(queryDesc);
PopActiveSnapshot();
@@ -542,7 +543,7 @@ PortalStart(Portal portal, ParamListInfo params,
* PortalRunMulti() before calling ExecutorRun().
*/
queryDesc = CreateQueryDesc(plan,
- NULL,
+ portal->cplan,
portal->sourceText,
!is_utility ?
GetActiveSnapshot() :
@@ -566,6 +567,7 @@ PortalStart(Portal portal, ParamListInfo params,
if (!ExecutorStart(queryDesc, myeflags))
{
PopActiveSnapshot();
+ Assert(queryDesc->cplan);
ExecutorEnd(queryDesc);
FreeQueryDesc(queryDesc);
plan_valid = false;
diff --git a/src/backend/utils/cache/plancache.c b/src/backend/utils/cache/plancache.c
index 7d4168f82f..35d903cb98 100644
--- a/src/backend/utils/cache/plancache.c
+++ b/src/backend/utils/cache/plancache.c
@@ -104,13 +104,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);
@@ -792,8 +792,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)
@@ -807,60 +812,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;
}
/*
@@ -1130,8 +1131,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
@@ -1166,7 +1175,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);
}
@@ -1364,8 +1376,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)
{
@@ -1741,58 +1753,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 7cd76eb34b..ce189156ad 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-2023, 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,45 @@ 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, 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, eflags);
+ else
+ plan_valid = standard_ExecutorStart(queryDesc, 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 +127,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.35.3
[application/octet-stream] v48-0006-Add-field-to-store-parent-relids-to-Append-Merge.patch (26.1K, 3-v48-0006-Add-field-to-store-parent-relids-to-Append-Merge.patch)
download | inline diff:
From bf5f136b6c6eaf278f2aefde525074afbef3958f Mon Sep 17 00:00:00 2001
From: Amit Langote <[email protected]>
Date: Wed, 6 Sep 2023 17:54:02 +0900
Subject: [PATCH v48 6/8] 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 their RT indexes except for
run-time pruning, in which case they can can be found in the
PartitionPruneInfo. A future commit will create a need for them to
be available at all times 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. This also adds a function called
ExecLockAppendNonLeafTables() to lock those tables.
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 aggregate 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/execMain.c | 2 +-
src/backend/executor/execUtils.c | 33 ++++++
src/backend/executor/nodeAppend.c | 14 +++
src/backend/executor/nodeMergeAppend.c | 14 +++
src/backend/optimizer/plan/createplan.c | 41 ++++++--
src/backend/optimizer/plan/planner.c | 3 +
src/backend/optimizer/plan/setrefs.c | 4 +
src/backend/optimizer/util/appendinfo.c | 134 ++++++++++++++++++++++++
src/backend/partitioning/partprune.c | 124 +++-------------------
src/include/executor/executor.h | 1 +
src/include/nodes/plannodes.h | 14 +++
src/include/optimizer/appendinfo.h | 3 +
src/include/partitioning/partprune.h | 3 +-
13 files changed, 266 insertions(+), 124 deletions(-)
diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c
index ffc62e379a..2804ec70f1 100644
--- a/src/backend/executor/execMain.c
+++ b/src/backend/executor/execMain.c
@@ -1475,7 +1475,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/executor/execUtils.c b/src/backend/executor/execUtils.c
index 117773706a..2b7a08c9ba 100644
--- a/src/backend/executor/execUtils.c
+++ b/src/backend/executor/execUtils.c
@@ -827,6 +827,39 @@ ExecGetRangeTableRelation(EState *estate, Index rti)
return rel;
}
+/*
+ * ExecLockAppendNonLeafPartitions
+ * Lock non-leaf partitions whose child partitions are scanned by a given
+ * Append/MergeAppend node
+ */
+void
+ExecLockAppendNonLeafPartitions(EState *estate, List *allpartrelids)
+{
+ ListCell *l;
+
+ /* This should get called only when executing cached plans. */
+ Assert(estate->es_cachedplan != NULL);
+ foreach(l, allpartrelids)
+ {
+ Bitmapset *partrelids = lfirst_node(Bitmapset, l);
+ int i = -1;
+
+ while ((i = bms_next_member(partrelids, i)) > 0)
+ {
+ RangeTblEntry *rte = exec_rt_fetch(i, estate);
+
+ /*
+ * Don't lock the root parent mentioned in the query, because it
+ * should already have been locked before entering the executor.
+ */
+ if (!rte->inFromCl)
+ LockRelationOid(rte->relid, rte->rellockmode);
+ else
+ Assert(CheckRelLockedByMe(rte->relid, rte->rellockmode, true));
+ }
+ }
+}
+
/*
* ExecInitResultRelation
* Open relation given by the passed-in RT index and fill its
diff --git a/src/backend/executor/nodeAppend.c b/src/backend/executor/nodeAppend.c
index 53ca9dc85d..4759511f87 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 if they are would get locked again in
+ * ExecInitPartitionPruning() because it calls
+ * ExecGetRangeTableRelation() which locks child tables.
+ */
+ if (estate->es_cachedplan)
+ ExecLockAppendNonLeafPartitions(estate, node->allpartrelids);
+
/* If run-time partition pruning is enabled, then set that up now */
if (node->part_prune_info != NULL)
{
diff --git a/src/backend/executor/nodeMergeAppend.c b/src/backend/executor/nodeMergeAppend.c
index 52c3edf278..158210aac1 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 if they are would get locked again in
+ * ExecInitPartitionPruning() because it calls
+ * ExecGetRangeTableRelation() which locks child tables.
+ */
+ if (estate->es_cachedplan)
+ ExecLockAppendNonLeafPartitions(estate, node->allpartrelids);
+
/* If run-time partition pruning is enabled, then set that up now */
if (node->part_prune_info != NULL)
{
diff --git a/src/backend/optimizer/plan/createplan.c b/src/backend/optimizer/plan/createplan.c
index 34ca6d4ac2..d1f4f606bf 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"
@@ -1229,6 +1230,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
@@ -1370,15 +1372,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;
@@ -1399,7 +1409,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;
@@ -1445,6 +1456,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
@@ -1534,15 +1546,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;
@@ -1554,7 +1574,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 44efb1f4eb..f97bc09113 100644
--- a/src/backend/optimizer/plan/planner.c
+++ b/src/backend/optimizer/plan/planner.c
@@ -7855,8 +7855,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 7962200885..8e256dd779 100644
--- a/src/backend/optimizer/plan/setrefs.c
+++ b/src/backend/optimizer/plan/setrefs.c
@@ -1766,6 +1766,8 @@ set_append_references(PlannerInfo *root,
set_dummy_tlist_references((Plan *) aplan, rtoffset);
aplan->apprelids = offset_relid_set(aplan->apprelids, rtoffset);
+ foreach(l, aplan->allpartrelids)
+ lfirst(l) = offset_relid_set((Relids) lfirst(l), rtoffset);
if (aplan->part_prune_info)
{
@@ -1842,6 +1844,8 @@ set_mergeappend_references(PlannerInfo *root,
set_dummy_tlist_references((Plan *) mplan, rtoffset);
mplan->apprelids = offset_relid_set(mplan->apprelids, rtoffset);
+ foreach(l, mplan->allpartrelids)
+ lfirst(l) = offset_relid_set((Relids) lfirst(l), rtoffset);
if (mplan->part_prune_info)
{
diff --git a/src/backend/optimizer/util/appendinfo.c b/src/backend/optimizer/util/appendinfo.c
index f456b3b0a4..5bd8e82b9b 100644
--- a/src/backend/optimizer/util/appendinfo.c
+++ b/src/backend/optimizer/util/appendinfo.c
@@ -41,6 +41,7 @@ static void make_inh_translation_list(Relation oldrelation,
AppendRelInfo *appinfo);
static Node *adjust_appendrel_attrs_mutator(Node *node,
adjust_appendrel_attrs_context *context);
+static List *add_part_relids(List *allpartrelids, Bitmapset *partrelids);
/*
@@ -1035,3 +1036,136 @@ 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 *prel = subpath->parent;
+ Relids partrelids = NULL;
+
+ /* Nothing to do if there's no parent to begin with. */
+ if (!IS_OTHER_REL(prel))
+ 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
+ * prel->parent->relids. But for partitionwise join and aggregate
+ * child rels, while we can use prel->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(prel))
+ {
+ prel = prel->parent;
+ /* Stop once we reach the root partitioned rel. */
+ if (!IS_PARTITIONED_REL(prel))
+ break;
+ parent_relids = bms_add_members(parent_relids, prel->relids);
+ }
+ else
+ {
+ AppendRelInfo **appinfos;
+ int nappinfos,
+ i;
+
+ appinfos = find_appinfos_by_relids(root, prel->relids,
+ &nappinfos);
+ for (i = 0; i < nappinfos; i++)
+ {
+ AppendRelInfo *appinfo = appinfos[i];
+
+ parent_relids = bms_add_member(parent_relids,
+ appinfo->parent_relid);
+ }
+ pfree(appinfos);
+ prel = prel->parent;
+ }
+ /* accept this level as an interesting parent */
+ partrelids = bms_add_members(partrelids, parent_relids);
+ if (prel == parentrel)
+ break; /* don't traverse above parentrel */
+ } while (IS_OTHER_REL(prel));
+
+ if (partrelids == NULL)
+ return allpartrelids;
+
+ return add_part_relids(allpartrelids, partrelids);
+}
+
+/*
+ * 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);
+}
diff --git a/src/backend/partitioning/partprune.c b/src/backend/partitioning/partprune.c
index 7179b22a05..213512a5f4 100644
--- a/src/backend/partitioning/partprune.c
+++ b/src/backend/partitioning/partprune.c
@@ -138,7 +138,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,
@@ -218,33 +217,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;
@@ -253,50 +251,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++;
}
@@ -362,63 +319,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/executor/executor.h b/src/include/executor/executor.h
index 10c5cda169..74a471e3e3 100644
--- a/src/include/executor/executor.h
+++ b/src/include/executor/executor.h
@@ -601,6 +601,7 @@ exec_rt_fetch(Index rti, EState *estate)
extern Relation ExecGetRangeTableRelation(EState *estate, Index rti);
extern void ExecInitResultRelation(EState *estate, ResultRelInfo *resultRelInfo,
Index rti);
+extern void ExecLockAppendNonLeafPartitions(EState *estate, List *allpartrelids);
extern int executor_errposition(EState *estate, int location);
diff --git a/src/include/nodes/plannodes.h b/src/include/nodes/plannodes.h
index 1b787fe031..7a5f3ba625 100644
--- a/src/include/nodes/plannodes.h
+++ b/src/include/nodes/plannodes.h
@@ -267,6 +267,13 @@ typedef struct Append
List *appendplans;
int nasyncplans; /* # of asynchronous plans */
+ /*
+ * RTIs of all partitioned tables whose children are scanned by
+ * appendplans. The list contains a bitmapset for every partition tree
+ * covered by this Append.
+ */
+ List *allpartrelids;
+
/*
* All 'appendplans' preceding this index are non-partial plans. All
* 'appendplans' from this index onwards are partial plans.
@@ -291,6 +298,13 @@ typedef struct MergeAppend
List *mergeplans;
+ /*
+ * RTIs of all partitioned tables whose children are scanned by
+ * mergeplans. The list contains a bitmapset for every partition tree
+ * covered by this MergeAppend.
+ */
+ 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 a05f91f77d..1621a7319a 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 8636e04e37..caa774a111 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.35.3
[application/octet-stream] v48-0008-Track-opened-range-table-relations-in-a-List-in-.patch (2.5K, 4-v48-0008-Track-opened-range-table-relations-in-a-List-in-.patch)
download | inline diff:
From fef4457e294bcc6b48a910f148816b2d163905ec Mon Sep 17 00:00:00 2001
From: Amit Langote <[email protected]>
Date: Wed, 6 Sep 2023 17:54:19 +0900
Subject: [PATCH v48 8/8] 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 2804ec70f1..d559c1de61 100644
--- a/src/backend/executor/execMain.c
+++ b/src/backend/executor/execMain.c
@@ -1649,12 +1649,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 2b7a08c9ba..1dfef44495 100644
--- a/src/backend/executor/execUtils.c
+++ b/src/backend/executor/execUtils.c
@@ -822,6 +822,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 bb5734edb5..8bbe1f6b14 100644
--- a/src/include/nodes/execnodes.h
+++ b/src/include/nodes/execnodes.h
@@ -619,6 +619,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.35.3
[application/octet-stream] v48-0005-Assert-that-relations-needing-their-permissions-.patch (5.1K, 5-v48-0005-Assert-that-relations-needing-their-permissions-.patch)
download | inline diff:
From ff1dfbb0df6d86acb5d4d6dabee623d74df17ab7 Mon Sep 17 00:00:00 2001
From: Amit Langote <[email protected]>
Date: Mon, 25 Sep 2023 11:52:02 +0900
Subject: [PATCH v48 5/8] Assert that relations needing their permissions
checked are locked
---
src/backend/executor/execMain.c | 11 +++++++
src/backend/storage/lmgr/lmgr.c | 45 +++++++++++++++++++++++++++++
src/backend/utils/cache/lsyscache.c | 21 ++++++++++++++
src/include/storage/lmgr.h | 1 +
src/include/utils/lsyscache.h | 1 +
5 files changed, 79 insertions(+)
diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c
index 5755336abd..ffc62e379a 100644
--- a/src/backend/executor/execMain.c
+++ b/src/backend/executor/execMain.c
@@ -626,6 +626,17 @@ 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() ||
+ CheckRelLockedByMe(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 ee9b89a672..c807e9cdcc 100644
--- a/src/backend/storage/lmgr/lmgr.c
+++ b/src/backend/storage/lmgr/lmgr.c
@@ -27,6 +27,7 @@
#include "storage/procarray.h"
#include "storage/sinvaladt.h"
#include "utils/inval.h"
+#include "utils/lsyscache.h"
/*
@@ -364,6 +365,50 @@ CheckRelationLockedByMe(Relation relation, LOCKMODE lockmode, bool orstronger)
return false;
}
+/*
+ * CheckRelLockedByMe
+ *
+ * Returns true if current transaction holds a lock on the given relation of
+ * mode 'lockmode'. If 'orstronger' is true, a stronger lockmode is also OK.
+ * ("Stronger" is defined as "numerically higher", which is a bit
+ * semantically dubious but is OK for the purposes we use this for.)
+ */
+bool
+CheckRelLockedByMe(Oid relid, LOCKMODE lockmode, bool orstronger)
+{
+ Oid dbId = get_rel_relisshared(relid) ? InvalidOid : MyDatabaseId;
+ LOCKTAG tag;
+
+ SET_LOCKTAG_RELATION(tag, dbId, relid);
+
+ if (LockHeldByMe(&tag, lockmode))
+ return true;
+
+ if (orstronger)
+ {
+ LOCKMODE slockmode;
+
+ for (slockmode = lockmode + 1;
+ slockmode <= MaxLockMode;
+ slockmode++)
+ {
+ if (LockHeldByMe(&tag, slockmode))
+ {
+#ifdef NOT_USED
+ /* Sometimes this might be useful for debugging purposes */
+ elog(WARNING, "lock mode %s substituted for %s on relation %s",
+ GetLockmodeName(tag.locktag_lockmethodid, slockmode),
+ GetLockmodeName(tag.locktag_lockmethodid, lockmode),
+ RelationGetRelationName(relation));
+#endif
+ return true;
+ }
+ }
+ }
+
+ return false;
+}
+
/*
* LockHasWaitersRelation
*
diff --git a/src/backend/utils/cache/lsyscache.c b/src/backend/utils/cache/lsyscache.c
index fc6d267e44..2725d02312 100644
--- a/src/backend/utils/cache/lsyscache.c
+++ b/src/backend/utils/cache/lsyscache.c
@@ -2095,6 +2095,27 @@ get_rel_persistence(Oid relid)
return result;
}
+/*
+ * get_rel_relisshared
+ *
+ * Returns if the given relation is shared or not
+ */
+bool
+get_rel_relisshared(Oid relid)
+{
+ HeapTuple tp;
+ Form_pg_class reltup;
+ bool result;
+
+ tp = SearchSysCache1(RELOID, ObjectIdGetDatum(relid));
+ if (!HeapTupleIsValid(tp))
+ elog(ERROR, "cache lookup failed for relation %u", relid);
+ reltup = (Form_pg_class) GETSTRUCT(tp);
+ result = reltup->relisshared;
+ ReleaseSysCache(tp);
+
+ return result;
+}
/* ---------- TRANSFORM CACHE ---------- */
diff --git a/src/include/storage/lmgr.h b/src/include/storage/lmgr.h
index 4ee91e3cf9..598bf2688a 100644
--- a/src/include/storage/lmgr.h
+++ b/src/include/storage/lmgr.h
@@ -48,6 +48,7 @@ extern bool ConditionalLockRelation(Relation relation, LOCKMODE lockmode);
extern void UnlockRelation(Relation relation, LOCKMODE lockmode);
extern bool CheckRelationLockedByMe(Relation relation, LOCKMODE lockmode,
bool orstronger);
+extern bool CheckRelLockedByMe(Oid relid, LOCKMODE lockmode, bool orstronger);
extern bool LockHasWaitersRelation(Relation relation, LOCKMODE lockmode);
extern void LockRelationIdForSession(LockRelId *relid, LOCKMODE lockmode);
diff --git a/src/include/utils/lsyscache.h b/src/include/utils/lsyscache.h
index f5fdbfe116..a024e5dcd0 100644
--- a/src/include/utils/lsyscache.h
+++ b/src/include/utils/lsyscache.h
@@ -140,6 +140,7 @@ extern char get_rel_relkind(Oid relid);
extern bool get_rel_relispartition(Oid relid);
extern Oid get_rel_tablespace(Oid relid);
extern char get_rel_persistence(Oid relid);
+extern bool get_rel_relisshared(Oid relid);
extern Oid get_transform_fromsql(Oid typid, Oid langid, List *trftypes);
extern Oid get_transform_tosql(Oid typid, Oid langid, List *trftypes);
extern bool get_typisdefined(Oid typid);
--
2.35.3
[application/octet-stream] v48-0004-Teach-the-executor-to-lock-child-tables-in-some-.patch (11.1K, 6-v48-0004-Teach-the-executor-to-lock-child-tables-in-some-.patch)
download | inline diff:
From 49787b3c0ed2a14ca5a68f33b47e892cf90b913f Mon Sep 17 00:00:00 2001
From: Amit Langote <[email protected]>
Date: Fri, 22 Sep 2023 18:17:15 +0900
Subject: [PATCH v48 4/8] Teach the executor to lock child tables in some cases
An upcoming commit will move the locking of child tables referenced
in a cached plan tree from GetCachedPlan() to the executor
initialization of the plan tree in ExecutorStart(). This commit
teaches ExecGetRangeTableRelation() to lock child tables if
EState.es_cachedplan points to a CachedPlan.
The executor must now deal with the cases where an unlocked child
table might have been concurrently dropped, so this modifies
ExecGetRangeTableRelation() to use try_table_open(). All of its
callers (and those of ExecOpenScanRelation() that calls it) must
now account for the child table disappearing, which means to abort
initializing the table's Scan node in the middle.
ExecGetRangeTableRelation() now examines inFromCl field of an RTE
to determine that a given range table relation is a child table, so
this commit also makes the planner set inFromCl to false in the
child tables' RTEs that it manufactures.
Discussion: https://postgr.es/m/CA+HiwqFGkMSge6TgC9KQzde0ohpAycLQuV7ooitEEpbKB0O_mg@mail.gmail.comk
---
src/backend/executor/README | 36 +++++++++++++++++++++++-
src/backend/executor/execPartition.c | 2 ++
src/backend/executor/execUtils.c | 41 +++++++++++++++++++++-------
src/backend/optimizer/util/inherit.c | 7 +++++
src/backend/parser/analyze.c | 7 ++---
src/include/nodes/parsenodes.h | 8 ++++--
6 files changed, 84 insertions(+), 17 deletions(-)
diff --git a/src/backend/executor/README b/src/backend/executor/README
index 17775a49e2..6d2240610d 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/execPartition.c b/src/backend/executor/execPartition.c
index eb8a87fd63..84978c5525 100644
--- a/src/backend/executor/execPartition.c
+++ b/src/backend/executor/execPartition.c
@@ -1927,6 +1927,8 @@ CreatePartitionPruneState(PlanState *planstate, PartitionPruneInfo *pruneinfo)
* duration of this executor run.
*/
partrel = ExecGetRangeTableRelation(estate, pinfo->rtindex);
+ if (unlikely(partrel == NULL))
+ return NULL;
partkey = RelationGetPartitionKey(partrel);
partdesc = PartitionDirectoryLookup(estate->es_partition_directory,
partrel);
diff --git a/src/backend/executor/execUtils.c b/src/backend/executor/execUtils.c
index f0f5740c26..117773706a 100644
--- a/src/backend/executor/execUtils.c
+++ b/src/backend/executor/execUtils.c
@@ -697,6 +697,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
@@ -706,6 +708,8 @@ ExecOpenScanRelation(EState *estate, Index scanrelid, int eflags)
/* Open the relation. */
rel = ExecGetRangeTableRelation(estate, scanrelid);
+ if (unlikely(rel == NULL))
+ return NULL;
/*
* Complain if we're attempting a scan of an unscannable relation, except
@@ -763,6 +767,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)
@@ -779,7 +786,28 @@ 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);
+ }
+ else
{
/*
* In a normal query, we should already have the appropriate lock,
@@ -792,15 +820,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;
}
@@ -823,6 +842,8 @@ ExecInitResultRelation(EState *estate, ResultRelInfo *resultRelInfo,
Relation resultRelationDesc;
resultRelationDesc = ExecGetRangeTableRelation(estate, rti);
+ if (unlikely(resultRelationDesc == NULL))
+ return;
InitResultRelInfo(resultRelInfo,
resultRelationDesc,
rti,
diff --git a/src/backend/optimizer/util/inherit.c b/src/backend/optimizer/util/inherit.c
index 94de855a22..1b30c0ff87 100644
--- a/src/backend/optimizer/util/inherit.c
+++ b/src/backend/optimizer/util/inherit.c
@@ -492,6 +492,13 @@ expand_single_inheritance_child(PlannerInfo *root, RangeTblEntry *parentrte,
}
else
childrte->inh = false;
+
+ /*
+ * Flag child tables as indirectly referenced in the query. This helps
+ * the executor's ExecGetRangeTableRelation() recognize them as
+ * inheritance children.
+ */
+ childrte->inFromCl = false;
childrte->securityQuals = NIL;
/*
diff --git a/src/backend/parser/analyze.c b/src/backend/parser/analyze.c
index 7a1dfb6364..cf269f8c53 100644
--- a/src/backend/parser/analyze.c
+++ b/src/backend/parser/analyze.c
@@ -3305,10 +3305,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/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index f637937cd2..acf87580a1 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -994,11 +994,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
--
2.35.3
[application/octet-stream] v48-0003-Adjustments-to-allow-ExecutorStart-to-sometimes-.patch (49.3K, 7-v48-0003-Adjustments-to-allow-ExecutorStart-to-sometimes-.patch)
download | inline diff:
From 36eb6e04907d4ab44ad424e11d54f9589309d0d2 Mon Sep 17 00:00:00 2001
From: Amit Langote <[email protected]>
Date: Wed, 6 Sep 2023 17:53:46 +0900
Subject: [PATCH v48 3/8] Adjustments to allow ExecutorStart() to sometimes
fail
Upon passing a plan tree from a CachedPlan to the executor, there's a
possibility that ExecutorStart() might return an incompletely set up
planstate tree. This can happen if the CachedPlan undergoes invalidation
during the ExecInitNode() initialization process. In such cases, the
execution should be reattempted using a fresh CachedPlan. Also, any
partially initialized EState must be cleaned up by invoking both
ExecutorEnd() and FreeExecutorState().
ExecutorStart() (and ExecutorStart_hook()) now return a Boolean telling
the caller if the plan initialization failed.
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:
* The ExecutorStart() call in ExplainOnePlan() is moved into a new
function ExplainQueryDesc() along with CreateQueryDesc(). Callers
of ExplainOnePlan() should now call the new function first.
* The ExecutorStart() call in _SPI_pquery() is moved to its caller
_SPI_execute_plan().
* The ExecutorStart() call in PortalRunMulti() is moved to
PortalStart(). 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 the CachedPlan is not passed into the executor yet.
---
contrib/auto_explain/auto_explain.c | 12 +-
.../pg_stat_statements/pg_stat_statements.c | 12 +-
src/backend/commands/copyto.c | 5 +-
src/backend/commands/createas.c | 9 +-
src/backend/commands/explain.c | 145 +++++---
src/backend/commands/extension.c | 6 +-
src/backend/commands/matview.c | 9 +-
src/backend/commands/portalcmds.c | 6 +-
src/backend/commands/prepare.c | 31 +-
src/backend/commands/trigger.c | 13 +
src/backend/executor/execMain.c | 44 ++-
src/backend/executor/execParallel.c | 6 +-
src/backend/executor/execUtils.c | 1 +
src/backend/executor/functions.c | 7 +-
src/backend/executor/spi.c | 48 ++-
src/backend/tcop/postgres.c | 18 +-
src/backend/tcop/pquery.c | 346 +++++++++---------
src/backend/utils/mmgr/portalmem.c | 9 +
src/include/commands/explain.h | 7 +-
src/include/commands/trigger.h | 1 +
src/include/executor/executor.h | 6 +-
src/include/nodes/execnodes.h | 3 +
src/include/tcop/pquery.h | 2 +-
src/include/utils/portal.h | 2 +
24 files changed, 466 insertions(+), 282 deletions(-)
diff --git a/contrib/auto_explain/auto_explain.c b/contrib/auto_explain/auto_explain.c
index c3ac27ae99..a0630d7944 100644
--- a/contrib/auto_explain/auto_explain.c
+++ b/contrib/auto_explain/auto_explain.c
@@ -78,7 +78,7 @@ 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, int eflags);
static void explain_ExecutorRun(QueryDesc *queryDesc,
ScanDirection direction,
uint64 count, bool execute_once);
@@ -258,9 +258,11 @@ _PG_init(void)
/*
* ExecutorStart hook: start up logging if needed
*/
-static void
+static bool
explain_ExecutorStart(QueryDesc *queryDesc, 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 +298,9 @@ explain_ExecutorStart(QueryDesc *queryDesc, int eflags)
}
if (prev_ExecutorStart)
- prev_ExecutorStart(queryDesc, eflags);
+ plan_valid = prev_ExecutorStart(queryDesc, eflags);
else
- standard_ExecutorStart(queryDesc, eflags);
+ plan_valid = standard_ExecutorStart(queryDesc, eflags);
if (auto_explain_enabled())
{
@@ -316,6 +318,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 a46f2db352..58cb62e872 100644
--- a/contrib/pg_stat_statements/pg_stat_statements.c
+++ b/contrib/pg_stat_statements/pg_stat_statements.c
@@ -330,7 +330,7 @@ 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, int eflags);
static void pgss_ExecutorRun(QueryDesc *queryDesc,
ScanDirection direction,
uint64 count, bool execute_once);
@@ -967,13 +967,15 @@ pgss_planner(Query *parse,
/*
* ExecutorStart hook: start up tracking if needed
*/
-static void
+static bool
pgss_ExecutorStart(QueryDesc *queryDesc, int eflags)
{
+ bool plan_valid;
+
if (prev_ExecutorStart)
- prev_ExecutorStart(queryDesc, eflags);
+ plan_valid = prev_ExecutorStart(queryDesc, eflags);
else
- standard_ExecutorStart(queryDesc, eflags);
+ plan_valid = standard_ExecutorStart(queryDesc, eflags);
/*
* If query has queryId zero, don't track it. This prevents double
@@ -996,6 +998,8 @@ pgss_ExecutorStart(QueryDesc *queryDesc, int eflags)
MemoryContextSwitchTo(oldcxt);
}
}
+
+ return plan_valid;
}
/*
diff --git a/src/backend/commands/copyto.c b/src/backend/commands/copyto.c
index 0e3547c35b..f7730c8702 100644
--- a/src/backend/commands/copyto.c
+++ b/src/backend/commands/copyto.c
@@ -568,8 +568,11 @@ BeginCopyTo(ParseState *pstate,
* Call ExecutorStart to prepare the plan for execution.
*
* ExecutorStart computes a result tupdesc for us
+ *
+ * OK to ignore the return value; plan can't become invalid,
+ * because there's no CachedPlan.
*/
- ExecutorStart(cstate->queryDesc, 0);
+ (void) ExecutorStart(cstate->queryDesc, 0);
tupDesc = cstate->queryDesc->tupDesc;
}
diff --git a/src/backend/commands/createas.c b/src/backend/commands/createas.c
index 18b07c0200..4a950c03ff 100644
--- a/src/backend/commands/createas.c
+++ b/src/backend/commands/createas.c
@@ -329,8 +329,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
+ *
+ * OK to ignore the return value; plan can't become invalid,
+ * because there's no CachedPlan.
+ */
+ (void) ExecutorStart(queryDesc, GetIntoRelEFlags(into));
/* 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 281c47b2ee..8d1fe5738b 100644
--- a/src/backend/commands/explain.c
+++ b/src/backend/commands/explain.c
@@ -393,6 +393,7 @@ ExplainOneQuery(Query *query, int cursorOptions,
else
{
PlannedStmt *plan;
+ QueryDesc *queryDesc;
instr_time planstart,
planduration;
BufferUsage bufusage_start,
@@ -415,12 +416,90 @@ ExplainOneQuery(Query *query, int cursorOptions,
BufferUsageAccumDiff(&bufusage, &pgBufferUsage, &bufusage_start);
}
+ queryDesc = ExplainQueryDesc(plan, NULL, queryString, into, es,
+ params, queryEnv);
+ Assert(queryDesc);
+
/* run it (if needed) and produce output */
- ExplainOnePlan(plan, into, es, queryString, params, queryEnv,
+ ExplainOnePlan(queryDesc, into, es, queryString, params, queryEnv,
&planduration, (es->buffers ? &bufusage : NULL));
}
}
+/*
+ * ExplainQueryDesc
+ * Set up QueryDesc for EXPLAINing a given plan
+ *
+ * This returns NULL if cplan is found to have been invalidated after
+ * calling ExecutorStart().
+ */
+QueryDesc *
+ExplainQueryDesc(PlannedStmt *stmt, CachedPlan *cplan,
+ const char *queryString, IntoClause *into, ExplainState *es,
+ ParamListInfo params, QueryEnvironment *queryEnv)
+{
+ QueryDesc *queryDesc;
+ DestReceiver *dest;
+ int eflags;
+ int instrument_option = 0;
+
+ /*
+ * Normally we discard the query's output, but if explaining CREATE TABLE
+ * AS, we'd better use the appropriate tuple receiver.
+ */
+ if (into)
+ dest = CreateIntoRelDestReceiver(into);
+ else
+ dest = None_Receiver;
+
+ if (es->analyze && es->timing)
+ instrument_option |= INSTRUMENT_TIMER;
+ else if (es->analyze)
+ instrument_option |= INSTRUMENT_ROWS;
+
+ if (es->buffers)
+ instrument_option |= INSTRUMENT_BUFFERS;
+ if (es->wal)
+ instrument_option |= INSTRUMENT_WAL;
+
+ /*
+ * Use a snapshot with an updated command ID to ensure this query sees
+ * results of any previously executed queries.
+ */
+ PushCopiedSnapshot(GetActiveSnapshot());
+ UpdateActiveSnapshotCommandId();
+
+ /* Create a QueryDesc for the query */
+ queryDesc = CreateQueryDesc(stmt, cplan, queryString,
+ GetActiveSnapshot(), InvalidSnapshot,
+ dest, params, queryEnv, instrument_option);
+
+ /* Select execution options */
+ if (es->analyze)
+ eflags = 0; /* default run-to-completion flags */
+ else
+ eflags = EXEC_FLAG_EXPLAIN_ONLY;
+ if (es->generic)
+ eflags |= EXEC_FLAG_EXPLAIN_GENERIC;
+ if (into)
+ eflags |= GetIntoRelEFlags(into);
+
+ /*
+ * Call ExecutorStart to prepare the plan for execution. A cached plan
+ * may get invalidated during plan intialization.
+ */
+ if (!ExecutorStart(queryDesc, eflags))
+ {
+ /* Clean up. */
+ ExecutorEnd(queryDesc);
+ FreeQueryDesc(queryDesc);
+ PopActiveSnapshot();
+ return NULL;
+ }
+
+ return queryDesc;
+}
+
/*
* ExplainOneUtility -
* print out the execution plan for one utility statement
@@ -524,29 +603,16 @@ ExplainOneUtility(Node *utilityStmt, IntoClause *into, ExplainState *es,
* to call it.
*/
void
-ExplainOnePlan(PlannedStmt *plannedstmt, IntoClause *into, ExplainState *es,
+ExplainOnePlan(QueryDesc *queryDesc,
+ IntoClause *into, ExplainState *es,
const char *queryString, ParamListInfo params,
QueryEnvironment *queryEnv, const instr_time *planduration,
const BufferUsage *bufusage)
{
- DestReceiver *dest;
- QueryDesc *queryDesc;
instr_time starttime;
double totaltime = 0;
- int eflags;
- int instrument_option = 0;
-
- Assert(plannedstmt->commandType != CMD_UTILITY);
- if (es->analyze && es->timing)
- instrument_option |= INSTRUMENT_TIMER;
- else if (es->analyze)
- instrument_option |= INSTRUMENT_ROWS;
-
- if (es->buffers)
- instrument_option |= INSTRUMENT_BUFFERS;
- if (es->wal)
- instrument_option |= INSTRUMENT_WAL;
+ Assert(queryDesc->plannedstmt->commandType != CMD_UTILITY);
/*
* We always collect timing for the entire statement, even when node-level
@@ -555,40 +621,6 @@ ExplainOnePlan(PlannedStmt *plannedstmt, IntoClause *into, ExplainState *es,
*/
INSTR_TIME_SET_CURRENT(starttime);
- /*
- * Use a snapshot with an updated command ID to ensure this query sees
- * results of any previously executed queries.
- */
- PushCopiedSnapshot(GetActiveSnapshot());
- UpdateActiveSnapshotCommandId();
-
- /*
- * Normally we discard the query's output, but if explaining CREATE TABLE
- * AS, we'd better use the appropriate tuple receiver.
- */
- if (into)
- dest = CreateIntoRelDestReceiver(into);
- else
- dest = None_Receiver;
-
- /* Create a QueryDesc for the query */
- queryDesc = CreateQueryDesc(plannedstmt, NULL, queryString,
- GetActiveSnapshot(), InvalidSnapshot,
- dest, params, queryEnv, instrument_option);
-
- /* Select execution options */
- if (es->analyze)
- eflags = 0; /* default run-to-completion flags */
- else
- eflags = EXEC_FLAG_EXPLAIN_ONLY;
- if (es->generic)
- eflags |= EXEC_FLAG_EXPLAIN_GENERIC;
- if (into)
- eflags |= GetIntoRelEFlags(into);
-
- /* call ExecutorStart to prepare the plan for execution */
- ExecutorStart(queryDesc, eflags);
-
/* Execute the plan for statistics if asked for */
if (es->analyze)
{
@@ -4873,6 +4905,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 b287a2e84c..127d2a3b0a 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);
+ /*
+ * OK to ignore the return value; plan can't become invalid,
+ * because there's no CachedPlan.
+ */
+ (void) ExecutorStart(qdesc, 0);
ExecutorRun(qdesc, ForwardScanDirection, 0, true);
ExecutorFinish(qdesc);
ExecutorEnd(qdesc);
diff --git a/src/backend/commands/matview.c b/src/backend/commands/matview.c
index 22b8b820c3..7083fb2350 100644
--- a/src/backend/commands/matview.c
+++ b/src/backend/commands/matview.c
@@ -412,8 +412,13 @@ 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.
+ */
+ (void) ExecutorStart(queryDesc, 0);
/* run the plan */
ExecutorRun(queryDesc, ForwardScanDirection, 0, true);
diff --git a/src/backend/commands/portalcmds.c b/src/backend/commands/portalcmds.c
index 73ed7aa2f0..a1ee5c0acd 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.
+ *
+ * OK to ignore the return value; plan can't become invalid here,
+ * because there's no CachedPlan.
*/
- PortalStart(portal, params, 0, GetActiveSnapshot());
-
+ (void) PortalStart(portal, params, 0, GetActiveSnapshot());
Assert(portal->strategy == PORTAL_ONE_SELECT);
/*
diff --git a/src/backend/commands/prepare.c b/src/backend/commands/prepare.c
index 18f70319fc..f8d0b0ee25 100644
--- a/src/backend/commands/prepare.c
+++ b/src/backend/commands/prepare.c
@@ -183,6 +183,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 */
@@ -251,9 +252,15 @@ 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()))
+ {
+ PortalDrop(portal, false);
+ goto replan;
+ }
(void) PortalRun(portal, count, false, true, dest, dest, qc);
@@ -574,7 +581,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;
@@ -618,6 +625,7 @@ ExplainExecuteQuery(ExecuteStmt *execstmt, IntoClause *into, ExplainState *es,
}
/* Replan if needed, and acquire a transient refcount */
+replan:
cplan = GetCachedPlan(entry->plansource, paramLI,
CurrentResourceOwner, queryEnv);
@@ -639,8 +647,21 @@ 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));
+ {
+ QueryDesc *queryDesc;
+
+ queryDesc = ExplainQueryDesc(pstmt, cplan, queryString,
+ into, es, paramLI, queryEnv);
+ if (queryDesc == NULL)
+ {
+ ExplainResetOutput(es);
+ ReleaseCachedPlan(cplan, CurrentResourceOwner);
+ goto replan;
+ }
+ ExplainOnePlan(queryDesc, into, es, query_string, paramLI,
+ queryEnv, &planduration,
+ (es->buffers ? &bufusage : NULL));
+ }
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 52177759ab..dd139432b9 100644
--- a/src/backend/commands/trigger.c
+++ b/src/backend/commands/trigger.c
@@ -5009,6 +5009,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/execMain.c b/src/backend/executor/execMain.c
index de7bf7ca67..5755336abd 100644
--- a/src/backend/executor/execMain.c
+++ b/src/backend/executor/execMain.c
@@ -119,6 +119,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.
+ *
+ * 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.
*
@@ -128,7 +135,7 @@ static void EvalPlanQualStart(EPQState *epqstate, Plan *planTree);
*
* ----------------------------------------------------------------
*/
-void
+bool
ExecutorStart(QueryDesc *queryDesc, int eflags)
{
/*
@@ -140,14 +147,15 @@ 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, eflags);
+
+ return standard_ExecutorStart(queryDesc, eflags);
}
-void
+bool
standard_ExecutorStart(QueryDesc *queryDesc, int eflags)
{
+ bool plan_valid;
EState *estate;
MemoryContext oldcontext;
@@ -263,9 +271,14 @@ standard_ExecutorStart(QueryDesc *queryDesc, int eflags)
/*
* Initialize the plan state tree
*/
- (void) InitPlan(queryDesc, eflags);
+ plan_valid = InitPlan(queryDesc, eflags);
+
+ /* Mark execution as canceled if plan won't be executed. */
+ estate->es_canceled = !plan_valid;
MemoryContextSwitchTo(oldcontext);
+
+ return plan_valid;
}
/* ----------------------------------------------------------------
@@ -325,6 +338,7 @@ standard_ExecutorRun(QueryDesc *queryDesc,
estate = queryDesc->estate;
Assert(estate != NULL);
+ Assert(!estate->es_canceled);
Assert(!(estate->es_top_eflags & EXEC_FLAG_EXPLAIN_ONLY));
/*
@@ -429,7 +443,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);
@@ -488,11 +502,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));
/*
@@ -506,6 +520,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
*/
diff --git a/src/backend/executor/execParallel.c b/src/backend/executor/execParallel.c
index 457ee46faf..13d2820a41 100644
--- a/src/backend/executor/execParallel.c
+++ b/src/backend/executor/execParallel.c
@@ -1437,7 +1437,11 @@ 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.
+ */
+ (void) ExecutorStart(queryDesc, fpes->eflags);
/* Special executor initialization steps for parallel workers */
queryDesc->planstate->state->es_query_dsa = area;
diff --git a/src/backend/executor/execUtils.c b/src/backend/executor/execUtils.c
index 16704c0c2f..f0f5740c26 100644
--- a/src/backend/executor/execUtils.c
+++ b/src/backend/executor/execUtils.c
@@ -151,6 +151,7 @@ CreateExecutorState(void)
estate->es_top_eflags = 0;
estate->es_instrument = 0;
estate->es_finished = false;
+ estate->es_canceled = false;
estate->es_exprcontexts = NIL;
diff --git a/src/backend/executor/functions.c b/src/backend/executor/functions.c
index 7e452ed743..606da72535 100644
--- a/src/backend/executor/functions.c
+++ b/src/backend/executor/functions.c
@@ -863,7 +863,12 @@ 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.
+ */
+ (void) ExecutorStart(es->qd, eflags);
}
es->status = F_EXEC_RUN;
diff --git a/src/backend/executor/spi.c b/src/backend/executor/spi.c
index f2cca807ef..814ff1390f 100644
--- a/src/backend/executor/spi.c
+++ b/src/backend/executor/spi.c
@@ -71,7 +71,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);
@@ -1582,6 +1582,7 @@ SPI_cursor_open_internal(const char *name, SPIPlanPtr plan,
Snapshot snapshot;
MemoryContext oldcontext;
Portal portal;
+ bool plan_valid;
SPICallbackArg spicallbackarg;
ErrorContextCallback spierrcontext;
@@ -1623,6 +1624,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')
{
@@ -1766,15 +1768,23 @@ 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);
Assert(portal->strategy != PORTAL_MULTI_QUERY);
/* Pop the error context stack */
error_context_stack = spierrcontext.previous;
+ if (!plan_valid)
+ {
+ PortalDrop(portal, false);
+ goto replan;
+ }
+
/* Pop the SPI stack */
_SPI_end_call(true);
@@ -2552,6 +2562,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);
@@ -2661,6 +2672,7 @@ _SPI_execute_plan(SPIPlanPtr plan, const SPIExecuteOptions *options,
{
QueryDesc *qdesc;
Snapshot snap;
+ int eflags;
if (ActiveSnapshotSet())
snap = GetActiveSnapshot();
@@ -2675,8 +2687,23 @@ _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 (!ExecutorStart(qdesc, eflags))
+ {
+ ExecutorEnd(qdesc);
+ FreeQueryDesc(qdesc);
+ Assert(cplan);
+ ReleaseCachedPlan(cplan, plan_owner);
+ goto replan;
+ }
+
+ res = _SPI_pquery(qdesc, canSetTag ? options->tcount : 0);
FreeQueryDesc(qdesc);
}
else
@@ -2851,10 +2878,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)
@@ -2898,14 +2924,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/tcop/postgres.c b/src/backend/tcop/postgres.c
index 21b9763183..4f923bbcae 100644
--- a/src/backend/tcop/postgres.c
+++ b/src/backend/tcop/postgres.c
@@ -1230,7 +1230,12 @@ exec_simple_query(const char *query_string)
/*
* Start the portal. No parameters here.
*/
- PortalStart(portal, NULL, 0, InvalidSnapshot);
+ {
+ bool plan_valid PG_USED_FOR_ASSERTS_ONLY;
+
+ plan_valid = PortalStart(portal, NULL, 0, InvalidSnapshot);
+ Assert(plan_valid);
+ }
/*
* Select the appropriate output format: text unless we are doing a
@@ -1735,6 +1740,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.
@@ -2026,9 +2032,15 @@ 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))
+ {
+ 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 4ef349df8b..fcf9925ed4 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"
@@ -35,12 +36,6 @@
Portal ActivePortal = NULL;
-static void ProcessQuery(PlannedStmt *plan,
- const char *sourceText,
- ParamListInfo params,
- QueryEnvironment *queryEnv,
- DestReceiver *dest,
- QueryCompletion *qc);
static void FillPortalStore(Portal portal, bool isTopLevel);
static uint64 RunFromStore(Portal portal, ScanDirection direction, uint64 count,
DestReceiver *dest);
@@ -118,86 +113,6 @@ FreeQueryDesc(QueryDesc *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
- * sourceText: the source text of the query
- * params: any parameters needed
- * dest: where to send results
- * qc: where to store the command completion status data.
- *
- * qc may be NULL if caller doesn't want a status string.
- *
- * Must be called in a memory context that will be reset or deleted on
- * error; otherwise the executor's memory usage will be leaked.
- */
-static void
-ProcessQuery(PlannedStmt *plan,
- const char *sourceText,
- ParamListInfo params,
- QueryEnvironment *queryEnv,
- DestReceiver *dest,
- QueryCompletion *qc)
-{
- QueryDesc *queryDesc;
-
- /*
- * Create the QueryDesc object
- */
- queryDesc = CreateQueryDesc(plan, NULL, sourceText,
- GetActiveSnapshot(), InvalidSnapshot,
- dest, params, queryEnv, 0);
-
- /*
- * Call ExecutorStart to prepare the plan for execution
- */
- ExecutorStart(queryDesc, 0);
-
- /*
- * Run the plan to completion.
- */
- ExecutorRun(queryDesc, ForwardScanDirection, 0, true);
-
- /*
- * Build command completion status data, if caller wants one.
- */
- if (qc)
- {
- switch (queryDesc->operation)
- {
- case CMD_SELECT:
- SetQueryCompletion(qc, CMDTAG_SELECT, queryDesc->estate->es_processed);
- break;
- case CMD_INSERT:
- SetQueryCompletion(qc, CMDTAG_INSERT, queryDesc->estate->es_processed);
- break;
- case CMD_UPDATE:
- SetQueryCompletion(qc, CMDTAG_UPDATE, queryDesc->estate->es_processed);
- break;
- case CMD_DELETE:
- SetQueryCompletion(qc, CMDTAG_DELETE, queryDesc->estate->es_processed);
- break;
- case CMD_MERGE:
- SetQueryCompletion(qc, CMDTAG_MERGE, queryDesc->estate->es_processed);
- break;
- default:
- SetQueryCompletion(qc, CMDTAG_UNKNOWN, queryDesc->estate->es_processed);
- break;
- }
- }
-
- /*
- * Now, we close down all the scans and free allocated resources.
- */
- ExecutorFinish(queryDesc);
- ExecutorEnd(queryDesc);
-
- FreeQueryDesc(queryDesc);
-}
-
/*
* ChoosePortalStrategy
* Select portal execution strategy given the intended statement list.
@@ -428,19 +343,21 @@ 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)
{
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);
@@ -450,15 +367,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;
@@ -474,6 +389,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)
@@ -491,8 +408,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),
NULL,
@@ -504,30 +421,51 @@ 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 ExecutorStart to prepare the plan for execution. A
+ * cached plan may get invalidated during plan intialization.
*/
- ExecutorStart(queryDesc, myeflags);
+ if (!ExecutorStart(queryDesc, myeflags))
+ {
+ ExecutorEnd(queryDesc);
+ FreeQueryDesc(queryDesc);
+ 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"
@@ -539,29 +477,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:
/*
@@ -584,7 +499,82 @@ 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,
+ NULL,
+ 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, myeflags))
+ {
+ PopActiveSnapshot();
+ ExecutorEnd(queryDesc);
+ FreeQueryDesc(queryDesc);
+ plan_valid = false;
+ goto plan_init_failed;
+ }
+ PopActiveSnapshot();
+ }
+ }
+
portal->tupDesc = NULL;
break;
}
@@ -597,19 +587,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;
}
/*
@@ -1196,7 +1187,7 @@ PortalRunMulti(Portal portal,
QueryCompletion *qc)
{
bool active_snapshot_set = false;
- ListCell *stmtlist_item;
+ ListCell *qdesc_item;
/*
* If the destination is DestRemoteExecute, change to DestNone. The
@@ -1217,9 +1208,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
@@ -1236,33 +1228,26 @@ PortalRunMulti(Portal portal,
if (log_executor_stats)
ResetUsage();
- /*
- * 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.
- */
+ /* Push the snapshot for plannable queries. */
if (!active_snapshot_set)
{
- Snapshot snapshot = GetTransactionSnapshot();
+ Snapshot snapshot = qdesc->snapshot;
- /* If told to, register the snapshot and save in portal */
+ /*
+ * 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 OK that here we
+ * don't likewise update holdSnapshot's command ID.
+ */
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);
+ PushActiveSnapshot(snapshot);
/*
* As for PORTAL_ONE_SELECT portals, it does not seem
@@ -1271,26 +1256,39 @@ PortalRunMulti(Portal portal,
active_snapshot_set = true;
}
- else
- UpdateActiveSnapshotCommandId();
+ /*
+ * Run the plan to completion.
+ */
+ qdesc->dest = dest;
+ ExecutorRun(qdesc, ForwardScanDirection, 0, true);
+
+ /*
+ * Build command completion status data if needed.
+ */
if (pstmt->canSetTag)
{
- /* statement can set tag string */
- ProcessQuery(pstmt,
- portal->sourceText,
- portal->portalParams,
- portal->queryEnv,
- dest, qc);
- }
- else
- {
- /* stmt added by rewrite cannot set tag */
- ProcessQuery(pstmt,
- portal->sourceText,
- portal->portalParams,
- portal->queryEnv,
- altdest, NULL);
+ switch (qdesc->operation)
+ {
+ case CMD_SELECT:
+ SetQueryCompletion(qc, CMDTAG_SELECT, qdesc->estate->es_processed);
+ break;
+ case CMD_INSERT:
+ SetQueryCompletion(qc, CMDTAG_INSERT, qdesc->estate->es_processed);
+ break;
+ case CMD_UPDATE:
+ SetQueryCompletion(qc, CMDTAG_UPDATE, qdesc->estate->es_processed);
+ break;
+ case CMD_DELETE:
+ SetQueryCompletion(qc, CMDTAG_DELETE, qdesc->estate->es_processed);
+ break;
+ case CMD_MERGE:
+ SetQueryCompletion(qc, CMDTAG_MERGE, qdesc->estate->es_processed);
+ break;
+ default:
+ SetQueryCompletion(qc, CMDTAG_UNKNOWN, qdesc->estate->es_processed);
+ break;
+ }
}
if (log_executor_stats)
@@ -1345,12 +1343,12 @@ 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();
+ if (qdesc->estate)
+ {
+ ExecutorFinish(qdesc);
+ ExecutorEnd(qdesc);
+ }
+ FreeQueryDesc(qdesc);
}
/* Pop the snapshot if we pushed one. */
diff --git a/src/backend/utils/mmgr/portalmem.c b/src/backend/utils/mmgr/portalmem.c
index 06dfa85f04..0cad450dcd 100644
--- a/src/backend/utils/mmgr/portalmem.c
+++ b/src/backend/utils/mmgr/portalmem.c
@@ -201,6 +201,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,
@@ -224,6 +231,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;
}
@@ -594,6 +602,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 3d3e632a0c..392abb5150 100644
--- a/src/include/commands/explain.h
+++ b/src/include/commands/explain.h
@@ -88,7 +88,11 @@ extern void ExplainOneUtility(Node *utilityStmt, IntoClause *into,
ExplainState *es, const char *queryString,
ParamListInfo params, QueryEnvironment *queryEnv);
-extern void ExplainOnePlan(PlannedStmt *plannedstmt, IntoClause *into,
+extern QueryDesc *ExplainQueryDesc(PlannedStmt *stmt, struct CachedPlan *cplan,
+ const char *queryString, IntoClause *into, ExplainState *es,
+ ParamListInfo params, QueryEnvironment *queryEnv);
+extern void ExplainOnePlan(QueryDesc *queryDesc,
+ IntoClause *into,
ExplainState *es, const char *queryString,
ParamListInfo params, QueryEnvironment *queryEnv,
const instr_time *planduration,
@@ -104,6 +108,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 430e3ca7dd..d4f7c29301 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 72cbf120c5..10c5cda169 100644
--- a/src/include/executor/executor.h
+++ b/src/include/executor/executor.h
@@ -73,7 +73,7 @@
/* Hook for plugins to get control in ExecutorStart() */
-typedef void (*ExecutorStart_hook_type) (QueryDesc *queryDesc, int eflags);
+typedef bool (*ExecutorStart_hook_type) (QueryDesc *queryDesc, int eflags);
extern PGDLLIMPORT ExecutorStart_hook_type ExecutorStart_hook;
/* Hook for plugins to get control in ExecutorRun() */
@@ -198,8 +198,8 @@ 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, int eflags);
+extern bool standard_ExecutorStart(QueryDesc *queryDesc, int eflags);
extern void ExecutorRun(QueryDesc *queryDesc,
ScanDirection direction, uint64 count, bool execute_once);
extern void standard_ExecutorRun(QueryDesc *queryDesc,
diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h
index 846eb32a1d..bb5734edb5 100644
--- a/src/include/nodes/execnodes.h
+++ b/src/include/nodes/execnodes.h
@@ -670,6 +670,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/tcop/pquery.h b/src/include/tcop/pquery.h
index a5e65b98aa..577b81a9ee 100644
--- a/src/include/tcop/pquery.h
+++ b/src/include/tcop/pquery.h
@@ -29,7 +29,7 @@ extern List *FetchPortalTargetList(Portal portal);
extern List *FetchStatementTargetList(Node *stmt);
-extern void PortalStart(Portal portal, ParamListInfo params,
+extern bool PortalStart(Portal portal, ParamListInfo params,
int eflags, Snapshot snapshot);
extern void PortalSetResultFormat(Portal portal, int nFormats,
diff --git a/src/include/utils/portal.h b/src/include/utils/portal.h
index aa08b1e0fc..af059e30f8 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.35.3
[application/octet-stream] v48-0001-Assorted-tightening-in-various-ExecEnd-routines.patch (31.0K, 8-v48-0001-Assorted-tightening-in-various-ExecEnd-routines.patch)
download | inline diff:
From 96983c4519fe018b10b0b8517d205cdebfcb95a2 Mon Sep 17 00:00:00 2001
From: Amit Langote <[email protected]>
Date: Thu, 28 Sep 2023 16:56:29 +0900
Subject: [PATCH v48 1/8] Assorted tightening in various ExecEnd()* routines
This includes adding NULLness checks on pointers before cleaning them
in up. Many ExecEnd*() routines already perform this check, but a few
instances remain. 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, a forthcoming patch will
modify ExecInit* routines to sometimes exit early, potentially leaving
some pointers in an undetermined state, so it will become crucial to
have these NULLness checks in place.
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 | 47 +++++++++++++++-------
src/backend/executor/nodeBitmapIndexscan.c | 23 +++++------
src/backend/executor/nodeBitmapOr.c | 4 +-
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 | 8 +++-
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 ++++++++++++++-----
37 files changed, 248 insertions(+), 120 deletions(-)
diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c
index 4c5a7bbf62..f7f18d3054 100644
--- a/src/backend/executor/execMain.c
+++ b/src/backend/executor/execMain.c
@@ -3010,6 +3010,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 f154f28902..af22b1676f 100644
--- a/src/backend/executor/nodeAgg.c
+++ b/src/backend/executor/nodeAgg.c
@@ -4304,7 +4304,6 @@ GetAggInitVal(Datum textInitVal, Oid transtype)
void
ExecEndAgg(AggState *node)
{
- PlanState *outerPlan;
int transno;
int numGroupingSets = Max(node->maxsets, 1);
int setno;
@@ -4314,7 +4313,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;
@@ -4327,10 +4326,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);
@@ -4346,19 +4351,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 609df6b9e6..a2af221e05 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 4c5eb2b23b..4abb0609a0 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 2db0acfc76..d3f58c22f9 100644
--- a/src/backend/executor/nodeBitmapHeapscan.c
+++ b/src/backend/executor/nodeBitmapHeapscan.c
@@ -648,40 +648,59 @@ ExecReScanBitmapHeapScan(BitmapHeapScanState *node)
void
ExecEndBitmapHeapScan(BitmapHeapScanState *node)
{
- TableScanDesc scanDesc;
-
- /*
- * 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->vmbuffer != InvalidBuffer)
+ {
ReleaseBuffer(node->vmbuffer);
+ node->vmbuffer = InvalidBuffer;
+ }
if (node->pvmbuffer != InvalidBuffer)
+ {
ReleaseBuffer(node->pvmbuffer);
+ node->pvmbuffer = InvalidBuffer;
+ }
/*
- * close heap scan
+ * close heap scan (no-op if we didn't start it)
*/
- 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 7cf8532bc9..488f11a3ff 100644
--- a/src/backend/executor/nodeBitmapIndexscan.c
+++ b/src/backend/executor/nodeBitmapIndexscan.c
@@ -175,22 +175,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 0bf8af9652..ace18593aa 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/nodeForeignscan.c b/src/backend/executor/nodeForeignscan.c
index 73913ebb18..3aba28285a 100644
--- a/src/backend/executor/nodeForeignscan.c
+++ b/src/backend/executor/nodeForeignscan.c
@@ -301,17 +301,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 bb2500a469..1a3c8abdad 100644
--- a/src/backend/executor/nodeGather.c
+++ b/src/backend/executor/nodeGather.c
@@ -249,6 +249,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 7a71a58509..c6fb45fee0 100644
--- a/src/backend/executor/nodeGatherMerge.c
+++ b/src/backend/executor/nodeGatherMerge.c
@@ -289,6 +289,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 8c650f0e46..6dfe5a1d23 100644
--- a/src/backend/executor/nodeGroup.c
+++ b/src/backend/executor/nodeGroup.c
@@ -226,10 +226,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 e72f0986c2..88ba336882 100644
--- a/src/backend/executor/nodeHash.c
+++ b/src/backend/executor/nodeHash.c
@@ -413,13 +413,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 aea44a9d56..6dc43b9ff2 100644
--- a/src/backend/executor/nodeHashjoin.c
+++ b/src/backend/executor/nodeHashjoin.c
@@ -861,7 +861,7 @@ ExecEndHashJoin(HashJoinState *node)
/*
* Free hash table
*/
- if (node->hj_HashTable)
+ if (node->hj_HashTable != NULL)
{
ExecHashTableDestroy(node->hj_HashTable);
node->hj_HashTable = NULL;
@@ -871,7 +871,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 cd094a190c..28a0e81cb3 100644
--- a/src/backend/executor/nodeIncrementalSort.c
+++ b/src/backend/executor/nodeIncrementalSort.c
@@ -1079,8 +1079,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.
@@ -1100,6 +1108,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 f1db35665c..1f3843abe9 100644
--- a/src/backend/executor/nodeIndexonlyscan.c
+++ b/src/backend/executor/nodeIndexonlyscan.c
@@ -364,15 +364,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)
{
@@ -380,13 +371,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 14b9c00217..32e1714f15 100644
--- a/src/backend/executor/nodeIndexscan.c
+++ b/src/backend/executor/nodeIndexscan.c
@@ -785,22 +785,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 5654158e3e..a97bac9f6d 100644
--- a/src/backend/executor/nodeLimit.c
+++ b/src/backend/executor/nodeLimit.c
@@ -535,6 +535,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 e459971d32..26fbe95c57 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 753ea28915..03c514900b 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 94bf479287..ee4749c852 100644
--- a/src/backend/executor/nodeMemoize.c
+++ b/src/backend/executor/nodeMemoize.c
@@ -1043,6 +1043,7 @@ ExecEndMemoize(MemoizeState *node)
{
#ifdef USE_ASSERT_CHECKING
/* Validate the memory accounting code is correct in assert builds. */
+ if (node->hashtable != NULL)
{
int count;
uint64 mem = 0;
@@ -1089,12 +1090,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 21b5726e6e..0a42a04b19 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 ed3ebe92e5..c84f53e0bd 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 d21a178ad5..ea043c57c1 100644
--- a/src/backend/executor/nodeModifyTable.c
+++ b/src/backend/executor/nodeModifyTable.c
@@ -4430,7 +4430,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;
}
}
@@ -4438,12 +4440,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;
+ }
}
/*
@@ -4455,6 +4461,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 ebd1406843..1211d871ea 100644
--- a/src/backend/executor/nodeNestloop.c
+++ b/src/backend/executor/nodeNestloop.c
@@ -368,7 +368,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 b4bbdc89b1..e9b96416d3 100644
--- a/src/backend/executor/nodeProjectSet.c
+++ b/src/backend/executor/nodeProjectSet.c
@@ -324,6 +324,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 e781003934..f6d60bcd6c 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 e9f5732f33..f15902e840 100644
--- a/src/backend/executor/nodeResult.c
+++ b/src/backend/executor/nodeResult.c
@@ -244,6 +244,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 41c1ea37ad..a6813559e6 100644
--- a/src/backend/executor/nodeSamplescan.c
+++ b/src/backend/executor/nodeSamplescan.c
@@ -185,14 +185,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 49a5933aff..911266da07 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 98c1b84d43..5c2861d243 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 eea7f2ae15..c8a35b64a8 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 1ee6295660..91d7ae82ce 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 a60dcd4943..80ed4b26a8 100644
--- a/src/backend/executor/nodeTableFuncscan.c
+++ b/src/backend/executor/nodeTableFuncscan.c
@@ -217,8 +217,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 da622d3f5f..9147e4afa8 100644
--- a/src/backend/executor/nodeTidrangescan.c
+++ b/src/backend/executor/nodeTidrangescan.c
@@ -327,10 +327,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 15055077d0..74ec6afdcc 100644
--- a/src/backend/executor/nodeTidscan.c
+++ b/src/backend/executor/nodeTidscan.c
@@ -470,8 +470,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 01f951197c..13c556326a 100644
--- a/src/backend/executor/nodeUnique.c
+++ b/src/backend/executor/nodeUnique.c
@@ -169,6 +169,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 77724a6daa..c4c6f009ba 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.
*/
- MemoryContextResetAndDeleteChildren(winstate->partcontext);
- MemoryContextResetAndDeleteChildren(winstate->aggcontext);
+ if (winstate->partcontext != NULL)
+ MemoryContextResetAndDeleteChildren(winstate->partcontext);
+ if (winstate->aggcontext != NULL)
+ MemoryContextResetAndDeleteChildren(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)
MemoryContextResetAndDeleteChildren(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.35.3
[application/octet-stream] v48-0002-Prepare-executor-to-support-detecting-CachedPlan.patch (41.7K, 9-v48-0002-Prepare-executor-to-support-detecting-CachedPlan.patch)
download | inline diff:
From a32d7916a4788d958a3dcd7e59a494916d78ddce Mon Sep 17 00:00:00 2001
From: Amit Langote <[email protected]>
Date: Fri, 22 Sep 2023 18:12:04 +0900
Subject: [PATCH v48 2/8] Prepare executor to support detecting CachedPlan
invalidation
This adds checks at various points during the executor's
initialization of the plan tree to determine whether the originating
CachedPlan has become invalid as a result of taking locks on the
relations referenced in the plan. This includes addding the check
after every call to ExecOpenScanRelation() and to ExecInitNode(),
including the recursive ones to initialize child nodes.
If a given ExecInit*() function detects that the plan has become
invalid, it should return immediately even if the PlanState node
it's building may only be partially valid. That is crucial for
two reasons depending on where the check is:
* The checks following ExecOpenScanRelation() may find the plan
having become invalid because the requested relation was dropped
or had its schema changed concurrently in a manner that risks
unsafe operations in the code that follows. For example, it
might try to dereference a NULL pointer when the check failed
because the relation was dropped.
* For the checks following ExecInitNode(), the returned child
PlanState node might be only partially invalid. The code that
follows may misbehave if it depends on inspecting the child
PlanState. Note that this commit adds the check following all
calls of ExecInitNode() that exist in the code base, even at
sites where there is no code that might misbehave today, because
it might misbehave in the future. It seems like a good idea to
put the guards in place today rather than in the future when the
need arises.
To pass the CachedPlan that the executor will use for these checks,
this adds a new field to QueryDesc and a new parameter to
CreateQueryDesc(). No caller of CreateQueryDesc() is made to pass
an actual CachedPlan though, so there is no functional change.
Reviewed-by: Robert Haas
---
contrib/postgres_fdw/postgres_fdw.c | 10 +++++-
src/backend/commands/copyto.c | 3 +-
src/backend/commands/createas.c | 2 +-
src/backend/commands/explain.c | 2 +-
src/backend/commands/extension.c | 1 +
src/backend/commands/matview.c | 2 +-
src/backend/executor/execMain.c | 39 ++++++++++++++++++----
src/backend/executor/execParallel.c | 9 ++++-
src/backend/executor/execProcnode.c | 4 +++
src/backend/executor/functions.c | 1 +
src/backend/executor/nodeAgg.c | 2 ++
src/backend/executor/nodeAppend.c | 10 +++---
src/backend/executor/nodeBitmapAnd.c | 2 ++
src/backend/executor/nodeBitmapHeapscan.c | 4 +++
src/backend/executor/nodeBitmapOr.c | 2 ++
src/backend/executor/nodeCustom.c | 2 ++
src/backend/executor/nodeForeignscan.c | 4 +++
src/backend/executor/nodeGather.c | 2 ++
src/backend/executor/nodeGatherMerge.c | 2 ++
src/backend/executor/nodeGroup.c | 2 ++
src/backend/executor/nodeHash.c | 2 ++
src/backend/executor/nodeHashjoin.c | 4 +++
src/backend/executor/nodeIncrementalSort.c | 2 ++
src/backend/executor/nodeIndexonlyscan.c | 2 ++
src/backend/executor/nodeIndexscan.c | 2 ++
src/backend/executor/nodeLimit.c | 2 ++
src/backend/executor/nodeLockRows.c | 2 ++
src/backend/executor/nodeMaterial.c | 2 ++
src/backend/executor/nodeMemoize.c | 2 ++
src/backend/executor/nodeMergeAppend.c | 4 ++-
src/backend/executor/nodeMergejoin.c | 4 +++
src/backend/executor/nodeModifyTable.c | 13 ++++++++
src/backend/executor/nodeNestloop.c | 4 +++
src/backend/executor/nodeProjectSet.c | 2 ++
src/backend/executor/nodeRecursiveunion.c | 4 +++
src/backend/executor/nodeResult.c | 2 ++
src/backend/executor/nodeSamplescan.c | 2 ++
src/backend/executor/nodeSeqscan.c | 2 ++
src/backend/executor/nodeSetOp.c | 2 ++
src/backend/executor/nodeSort.c | 2 ++
src/backend/executor/nodeSubqueryscan.c | 2 ++
src/backend/executor/nodeTidrangescan.c | 2 ++
src/backend/executor/nodeTidscan.c | 2 ++
src/backend/executor/nodeUnique.c | 2 ++
src/backend/executor/nodeWindowAgg.c | 2 ++
src/backend/executor/spi.c | 1 +
src/backend/tcop/pquery.c | 5 ++-
src/include/executor/execdesc.h | 4 +++
src/include/executor/executor.h | 10 ++++++
src/include/nodes/execnodes.h | 2 ++
src/include/utils/plancache.h | 14 ++++++++
51 files changed, 194 insertions(+), 18 deletions(-)
diff --git a/contrib/postgres_fdw/postgres_fdw.c b/contrib/postgres_fdw/postgres_fdw.c
index 1393716587..0af60463c2 100644
--- a/contrib/postgres_fdw/postgres_fdw.c
+++ b/contrib/postgres_fdw/postgres_fdw.c
@@ -2126,7 +2126,11 @@ postgresEndForeignModify(EState *estate,
{
PgFdwModifyState *fmstate = (PgFdwModifyState *) resultRelInfo->ri_FdwState;
- /* If fmstate is NULL, we are in EXPLAIN; nothing to do */
+ /*
+ * fmstate could be NULL under two conditions: during an EXPLAIN
+ * operation, or if BeginForeignModify() hasn't been invoked.
+ * In either case, no action is required.
+ */
if (fmstate == NULL)
return;
@@ -2660,7 +2664,11 @@ postgresBeginDirectModify(ForeignScanState *node, int eflags)
/* Get info about foreign table. */
rtindex = node->resultRelInfo->ri_RangeTableIndex;
if (fsplan->scan.scanrelid == 0)
+ {
dmstate->rel = ExecOpenScanRelation(estate, rtindex, eflags);
+ if (unlikely(!ExecPlanStillValid(estate)))
+ return;
+ }
else
dmstate->rel = node->ss.ss_currentRelation;
table = GetForeignTable(RelationGetRelid(dmstate->rel));
diff --git a/src/backend/commands/copyto.c b/src/backend/commands/copyto.c
index eaa3172793..0e3547c35b 100644
--- a/src/backend/commands/copyto.c
+++ b/src/backend/commands/copyto.c
@@ -558,7 +558,8 @@ BeginCopyTo(ParseState *pstate,
((DR_copy *) dest)->cstate = cstate;
/* Create a QueryDesc requesting no output */
- cstate->queryDesc = CreateQueryDesc(plan, pstate->p_sourcetext,
+ cstate->queryDesc = CreateQueryDesc(plan, NULL,
+ pstate->p_sourcetext,
GetActiveSnapshot(),
InvalidSnapshot,
dest, NULL, NULL, 0);
diff --git a/src/backend/commands/createas.c b/src/backend/commands/createas.c
index e91920ca14..18b07c0200 100644
--- a/src/backend/commands/createas.c
+++ b/src/backend/commands/createas.c
@@ -325,7 +325,7 @@ ExecCreateTableAs(ParseState *pstate, CreateTableAsStmt *stmt,
UpdateActiveSnapshotCommandId();
/* Create a QueryDesc, redirecting output to our tuple receiver */
- queryDesc = CreateQueryDesc(plan, pstate->p_sourcetext,
+ queryDesc = CreateQueryDesc(plan, NULL, pstate->p_sourcetext,
GetActiveSnapshot(), InvalidSnapshot,
dest, params, queryEnv, 0);
diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.c
index 13217807ee..281c47b2ee 100644
--- a/src/backend/commands/explain.c
+++ b/src/backend/commands/explain.c
@@ -572,7 +572,7 @@ ExplainOnePlan(PlannedStmt *plannedstmt, IntoClause *into, ExplainState *es,
dest = None_Receiver;
/* Create a QueryDesc for the query */
- queryDesc = CreateQueryDesc(plannedstmt, queryString,
+ queryDesc = CreateQueryDesc(plannedstmt, NULL, queryString,
GetActiveSnapshot(), InvalidSnapshot,
dest, params, queryEnv, instrument_option);
diff --git a/src/backend/commands/extension.c b/src/backend/commands/extension.c
index 535072d181..b287a2e84c 100644
--- a/src/backend/commands/extension.c
+++ b/src/backend/commands/extension.c
@@ -797,6 +797,7 @@ execute_sql_string(const char *sql)
QueryDesc *qdesc;
qdesc = CreateQueryDesc(stmt,
+ NULL,
sql,
GetActiveSnapshot(), NULL,
dest, NULL, NULL, 0);
diff --git a/src/backend/commands/matview.c b/src/backend/commands/matview.c
index ac2e74fa3f..22b8b820c3 100644
--- a/src/backend/commands/matview.c
+++ b/src/backend/commands/matview.c
@@ -408,7 +408,7 @@ refresh_matview_datafill(DestReceiver *dest, Query *query,
UpdateActiveSnapshotCommandId();
/* Create a QueryDesc, redirecting output to our tuple receiver */
- queryDesc = CreateQueryDesc(plan, queryString,
+ queryDesc = CreateQueryDesc(plan, NULL, queryString,
GetActiveSnapshot(), InvalidSnapshot,
dest, NULL, NULL, 0);
diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c
index f7f18d3054..de7bf7ca67 100644
--- a/src/backend/executor/execMain.c
+++ b/src/backend/executor/execMain.c
@@ -79,7 +79,7 @@ ExecutorEnd_hook_type ExecutorEnd_hook = NULL;
ExecutorCheckPerms_hook_type ExecutorCheckPerms_hook = NULL;
/* decls for local routines only used within this module */
-static void InitPlan(QueryDesc *queryDesc, int eflags);
+static bool InitPlan(QueryDesc *queryDesc, int eflags);
static void CheckValidRowMarkRel(Relation rel, RowMarkType markType);
static void ExecPostprocessPlan(EState *estate);
static void ExecEndPlan(PlanState *planstate, EState *estate);
@@ -263,7 +263,7 @@ standard_ExecutorStart(QueryDesc *queryDesc, int eflags)
/*
* Initialize the plan state tree
*/
- InitPlan(queryDesc, eflags);
+ (void) InitPlan(queryDesc, eflags);
MemoryContextSwitchTo(oldcontext);
}
@@ -829,9 +829,13 @@ ExecCheckXactReadOnly(PlannedStmt *plannedstmt)
*
* Initializes the query plan: open files, allocate storage
* and start up the rule manager
+ *
+ * Returns true if the plan tree is successfully initialized for execution,
+ * false otherwise. The latter case may occur if the CachedPlan that provides
+ * the plan tree (queryDesc->cplan) got invalidated during the initialization.
* ----------------------------------------------------------------
*/
-static void
+static bool
InitPlan(QueryDesc *queryDesc, int eflags)
{
CmdType operation = queryDesc->operation;
@@ -839,11 +843,14 @@ InitPlan(QueryDesc *queryDesc, int eflags)
Plan *plan = plannedstmt->planTree;
List *rangeTable = plannedstmt->rtable;
EState *estate = queryDesc->estate;
- PlanState *planstate;
- TupleDesc tupType;
+ PlanState *planstate = NULL;
+ TupleDesc tupType = NULL;
ListCell *l;
int i;
+ Assert(queryDesc->planstate == NULL);
+ Assert(queryDesc->tupDesc == NULL);
+
/*
* Do permissions checks
*/
@@ -855,6 +862,7 @@ InitPlan(QueryDesc *queryDesc, int eflags)
ExecInitRangeTable(estate, rangeTable, plannedstmt->permInfos);
estate->es_plannedstmt = plannedstmt;
+ estate->es_cachedplan = queryDesc->cplan;
/*
* Next, build the ExecRowMark array from the PlanRowMark(s), if any.
@@ -886,6 +894,8 @@ InitPlan(QueryDesc *queryDesc, int eflags)
case ROW_MARK_KEYSHARE:
case ROW_MARK_REFERENCE:
relation = ExecGetRangeTableRelation(estate, rc->rti);
+ if (unlikely(relation == NULL))
+ return false;
break;
case ROW_MARK_COPY:
/* no physical table access is required */
@@ -956,6 +966,8 @@ InitPlan(QueryDesc *queryDesc, int eflags)
estate->es_subplanstates = lappend(estate->es_subplanstates,
subplanstate);
+ if (unlikely(!ExecPlanStillValid(estate)))
+ return false;
i++;
}
@@ -966,6 +978,8 @@ InitPlan(QueryDesc *queryDesc, int eflags)
* processing tuples.
*/
planstate = ExecInitNode(plan, estate, eflags);
+ if (unlikely(!ExecPlanStillValid(estate)))
+ return false;
/*
* Get the tuple descriptor describing the type of tuples to return.
@@ -1009,6 +1023,8 @@ InitPlan(QueryDesc *queryDesc, int eflags)
queryDesc->tupDesc = tupType;
queryDesc->planstate = planstate;
+
+ return true;
}
/*
@@ -2858,7 +2874,8 @@ EvalPlanQualStart(EPQState *epqstate, Plan *planTree)
* Child EPQ EStates share the parent's copy of unchanging state such as
* the snapshot, rangetable, and external Param info. They need their own
* copies of local state, including a tuple table, es_param_exec_vals,
- * result-rel info, etc.
+ * result-rel info, etc. Also, we don't pass the parent't copy of the
+ * CachedPlan, because no new locks will be taken for EvalPlanQual().
*/
rcestate->es_direction = ForwardScanDirection;
rcestate->es_snapshot = parentestate->es_snapshot;
@@ -2947,6 +2964,13 @@ EvalPlanQualStart(EPQState *epqstate, Plan *planTree)
subplanstate = ExecInitNode(subplan, rcestate, 0);
rcestate->es_subplanstates = lappend(rcestate->es_subplanstates,
subplanstate);
+
+ /*
+ * All the necessary locks must already have been taken when
+ * initializing the parent's copy of subplanstate, so the CachedPlan,
+ * if any, should not have become invalid during ExecInitNode().
+ */
+ Assert(ExecPlanStillValid(rcestate));
}
/*
@@ -2988,6 +3012,9 @@ EvalPlanQualStart(EPQState *epqstate, Plan *planTree)
*/
epqstate->recheckplanstate = ExecInitNode(planTree, rcestate, 0);
+ /* See the comment above. */
+ Assert(ExecPlanStillValid(rcestate));
+
MemoryContextSwitchTo(oldcontext);
}
diff --git a/src/backend/executor/execParallel.c b/src/backend/executor/execParallel.c
index cc2b8ccab7..457ee46faf 100644
--- a/src/backend/executor/execParallel.c
+++ b/src/backend/executor/execParallel.c
@@ -1248,8 +1248,15 @@ ExecParallelGetQueryDesc(shm_toc *toc, DestReceiver *receiver,
paramspace = shm_toc_lookup(toc, PARALLEL_KEY_PARAMLISTINFO, false);
paramLI = RestoreParamList(¶mspace);
- /* Create a QueryDesc for the query. */
+ /*
+ * Set up a QueryDesc for the query. While the leader might've sourced
+ * the plan tree from a CachedPlan, we don't have one here. This isn't
+ * an issue since the leader ensured the required locks, making our
+ * plan tree valid. Even as we get our own lock copies in
+ * ExecGetRangeTableRelation(), they're all already held by the leader.
+ */
return CreateQueryDesc(pstmt,
+ NULL,
queryString,
GetActiveSnapshot(), InvalidSnapshot,
receiver, paramLI, NULL, instrument_options);
diff --git a/src/backend/executor/execProcnode.c b/src/backend/executor/execProcnode.c
index b4b5c562c0..febaa194c4 100644
--- a/src/backend/executor/execProcnode.c
+++ b/src/backend/executor/execProcnode.c
@@ -136,6 +136,10 @@ static bool ExecShutdownNode_walker(PlanState *node, void *context);
* 'eflags' is a bitwise OR of flag bits described in executor.h
*
* Returns a PlanState node corresponding to the given Plan node.
+ *
+ * Callers should check upon returning that ExecPlanStillValid(estate)
+ * returns true before continuing further with its processing, because the
+ * returned PlanState might be only partially valid otherwise.
* ------------------------------------------------------------------------
*/
PlanState *
diff --git a/src/backend/executor/functions.c b/src/backend/executor/functions.c
index f55424eb5a..7e452ed743 100644
--- a/src/backend/executor/functions.c
+++ b/src/backend/executor/functions.c
@@ -838,6 +838,7 @@ postquel_start(execution_state *es, SQLFunctionCachePtr fcache)
dest = None_Receiver;
es->qd = CreateQueryDesc(es->stmt,
+ NULL, /* fmgr_sql() doesn't use CachedPlans */
fcache->src,
GetActiveSnapshot(),
InvalidSnapshot,
diff --git a/src/backend/executor/nodeAgg.c b/src/backend/executor/nodeAgg.c
index af22b1676f..597d68139e 100644
--- a/src/backend/executor/nodeAgg.c
+++ b/src/backend/executor/nodeAgg.c
@@ -3304,6 +3304,8 @@ ExecInitAgg(Agg *node, EState *estate, int eflags)
eflags &= ~EXEC_FLAG_REWIND;
outerPlan = outerPlan(node);
outerPlanState(aggstate) = ExecInitNode(outerPlan, estate, eflags);
+ if (unlikely(!ExecPlanStillValid(estate)))
+ return aggstate;
/*
* initialize source tuple type.
diff --git a/src/backend/executor/nodeAppend.c b/src/backend/executor/nodeAppend.c
index a2af221e05..53ca9dc85d 100644
--- a/src/backend/executor/nodeAppend.c
+++ b/src/backend/executor/nodeAppend.c
@@ -185,8 +185,10 @@ ExecInitAppend(Append *node, EState *estate, int eflags)
appendstate->ps.resultopsset = true;
appendstate->ps.resultopsfixed = false;
- appendplanstates = (PlanState **) palloc(nplans *
- sizeof(PlanState *));
+ appendplanstates = (PlanState **) palloc0(nplans *
+ sizeof(PlanState *));
+ appendstate->appendplans = appendplanstates;
+ appendstate->as_nplans = nplans;
/*
* call ExecInitNode on each of the valid plans to be executed and save
@@ -221,11 +223,11 @@ ExecInitAppend(Append *node, EState *estate, int eflags)
firstvalid = j;
appendplanstates[j++] = ExecInitNode(initNode, estate, eflags);
+ if (unlikely(!ExecPlanStillValid(estate)))
+ return appendstate;
}
appendstate->as_first_partial_plan = firstvalid;
- appendstate->appendplans = appendplanstates;
- appendstate->as_nplans = nplans;
/* Initialize async state */
appendstate->as_asyncplans = asyncplans;
diff --git a/src/backend/executor/nodeBitmapAnd.c b/src/backend/executor/nodeBitmapAnd.c
index 4abb0609a0..7556be713c 100644
--- a/src/backend/executor/nodeBitmapAnd.c
+++ b/src/backend/executor/nodeBitmapAnd.c
@@ -89,6 +89,8 @@ ExecInitBitmapAnd(BitmapAnd *node, EState *estate, int eflags)
{
initNode = (Plan *) lfirst(l);
bitmapplanstates[i] = ExecInitNode(initNode, estate, eflags);
+ if (unlikely(!ExecPlanStillValid(estate)))
+ return bitmapandstate;
i++;
}
diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c
index d3f58c22f9..f1f8e16b17 100644
--- a/src/backend/executor/nodeBitmapHeapscan.c
+++ b/src/backend/executor/nodeBitmapHeapscan.c
@@ -770,11 +770,15 @@ ExecInitBitmapHeapScan(BitmapHeapScan *node, EState *estate, int eflags)
* open the scan relation
*/
currentRelation = ExecOpenScanRelation(estate, node->scan.scanrelid, eflags);
+ if (unlikely(!ExecPlanStillValid(estate)))
+ return scanstate;
/*
* initialize child nodes
*/
outerPlanState(scanstate) = ExecInitNode(outerPlan(node), estate, eflags);
+ if (unlikely(!ExecPlanStillValid(estate)))
+ return scanstate;
/*
* get the scan type from the relation descriptor.
diff --git a/src/backend/executor/nodeBitmapOr.c b/src/backend/executor/nodeBitmapOr.c
index ace18593aa..7d2bf45d9c 100644
--- a/src/backend/executor/nodeBitmapOr.c
+++ b/src/backend/executor/nodeBitmapOr.c
@@ -90,6 +90,8 @@ ExecInitBitmapOr(BitmapOr *node, EState *estate, int eflags)
{
initNode = (Plan *) lfirst(l);
bitmapplanstates[i] = ExecInitNode(initNode, estate, eflags);
+ if (unlikely(!ExecPlanStillValid(estate)))
+ return bitmaporstate;
i++;
}
diff --git a/src/backend/executor/nodeCustom.c b/src/backend/executor/nodeCustom.c
index 28b5bb9353..a0befbd0c6 100644
--- a/src/backend/executor/nodeCustom.c
+++ b/src/backend/executor/nodeCustom.c
@@ -61,6 +61,8 @@ ExecInitCustomScan(CustomScan *cscan, EState *estate, int eflags)
if (scanrelid > 0)
{
scan_rel = ExecOpenScanRelation(estate, scanrelid, eflags);
+ if (unlikely(!ExecPlanStillValid(estate)))
+ return css;
css->ss.ss_currentRelation = scan_rel;
}
diff --git a/src/backend/executor/nodeForeignscan.c b/src/backend/executor/nodeForeignscan.c
index 3aba28285a..336acff719 100644
--- a/src/backend/executor/nodeForeignscan.c
+++ b/src/backend/executor/nodeForeignscan.c
@@ -173,6 +173,8 @@ ExecInitForeignScan(ForeignScan *node, EState *estate, int eflags)
if (scanrelid > 0)
{
currentRelation = ExecOpenScanRelation(estate, scanrelid, eflags);
+ if (unlikely(!ExecPlanStillValid(estate)))
+ return scanstate;
scanstate->ss.ss_currentRelation = currentRelation;
fdwroutine = GetFdwRoutineForRelation(currentRelation, true);
}
@@ -264,6 +266,8 @@ ExecInitForeignScan(ForeignScan *node, EState *estate, int eflags)
if (outerPlan(node))
outerPlanState(scanstate) =
ExecInitNode(outerPlan(node), estate, eflags);
+ if (unlikely(!ExecPlanStillValid(estate)))
+ return scanstate;
/*
* Tell the FDW to initialize the scan.
diff --git a/src/backend/executor/nodeGather.c b/src/backend/executor/nodeGather.c
index 1a3c8abdad..c524022c04 100644
--- a/src/backend/executor/nodeGather.c
+++ b/src/backend/executor/nodeGather.c
@@ -89,6 +89,8 @@ ExecInitGather(Gather *node, EState *estate, int eflags)
*/
outerNode = outerPlan(node);
outerPlanState(gatherstate) = ExecInitNode(outerNode, estate, eflags);
+ if (unlikely(!ExecPlanStillValid(estate)))
+ return gatherstate;
tupDesc = ExecGetResultType(outerPlanState(gatherstate));
/*
diff --git a/src/backend/executor/nodeGatherMerge.c b/src/backend/executor/nodeGatherMerge.c
index c6fb45fee0..676faabef5 100644
--- a/src/backend/executor/nodeGatherMerge.c
+++ b/src/backend/executor/nodeGatherMerge.c
@@ -108,6 +108,8 @@ ExecInitGatherMerge(GatherMerge *node, EState *estate, int eflags)
*/
outerNode = outerPlan(node);
outerPlanState(gm_state) = ExecInitNode(outerNode, estate, eflags);
+ if (unlikely(!ExecPlanStillValid(estate)))
+ return gm_state;
/*
* Leader may access ExecProcNode result directly (if
diff --git a/src/backend/executor/nodeGroup.c b/src/backend/executor/nodeGroup.c
index 6dfe5a1d23..efa1c44ab4 100644
--- a/src/backend/executor/nodeGroup.c
+++ b/src/backend/executor/nodeGroup.c
@@ -185,6 +185,8 @@ ExecInitGroup(Group *node, EState *estate, int eflags)
* initialize child nodes
*/
outerPlanState(grpstate) = ExecInitNode(outerPlan(node), estate, eflags);
+ if (unlikely(!ExecPlanStillValid(estate)))
+ return grpstate;
/*
* Initialize scan slot and type.
diff --git a/src/backend/executor/nodeHash.c b/src/backend/executor/nodeHash.c
index 88ba336882..1a4bd5504e 100644
--- a/src/backend/executor/nodeHash.c
+++ b/src/backend/executor/nodeHash.c
@@ -386,6 +386,8 @@ ExecInitHash(Hash *node, EState *estate, int eflags)
* initialize child nodes
*/
outerPlanState(hashstate) = ExecInitNode(outerPlan(node), estate, eflags);
+ if (unlikely(!ExecPlanStillValid(estate)))
+ return hashstate;
/*
* initialize our result slot and type. No need to build projection
diff --git a/src/backend/executor/nodeHashjoin.c b/src/backend/executor/nodeHashjoin.c
index 6dc43b9ff2..c0919074b0 100644
--- a/src/backend/executor/nodeHashjoin.c
+++ b/src/backend/executor/nodeHashjoin.c
@@ -752,8 +752,12 @@ ExecInitHashJoin(HashJoin *node, EState *estate, int eflags)
hashNode = (Hash *) innerPlan(node);
outerPlanState(hjstate) = ExecInitNode(outerNode, estate, eflags);
+ if (unlikely(!ExecPlanStillValid(estate)))
+ return hjstate;
outerDesc = ExecGetResultType(outerPlanState(hjstate));
innerPlanState(hjstate) = ExecInitNode((Plan *) hashNode, estate, eflags);
+ if (unlikely(!ExecPlanStillValid(estate)))
+ return hjstate;
innerDesc = ExecGetResultType(innerPlanState(hjstate));
/*
diff --git a/src/backend/executor/nodeIncrementalSort.c b/src/backend/executor/nodeIncrementalSort.c
index 28a0e81cb3..621ffafe02 100644
--- a/src/backend/executor/nodeIncrementalSort.c
+++ b/src/backend/executor/nodeIncrementalSort.c
@@ -1041,6 +1041,8 @@ ExecInitIncrementalSort(IncrementalSort *node, EState *estate, int eflags)
* nodes may be able to do something more useful.
*/
outerPlanState(incrsortstate) = ExecInitNode(outerPlan(node), estate, eflags);
+ if (unlikely(!ExecPlanStillValid(estate)))
+ return incrsortstate;
/*
* Initialize scan slot and type.
diff --git a/src/backend/executor/nodeIndexonlyscan.c b/src/backend/executor/nodeIndexonlyscan.c
index 1f3843abe9..c555c14888 100644
--- a/src/backend/executor/nodeIndexonlyscan.c
+++ b/src/backend/executor/nodeIndexonlyscan.c
@@ -495,6 +495,8 @@ ExecInitIndexOnlyScan(IndexOnlyScan *node, EState *estate, int eflags)
* open the scan relation
*/
currentRelation = ExecOpenScanRelation(estate, node->scan.scanrelid, eflags);
+ if (unlikely(!ExecPlanStillValid(estate)))
+ return indexstate;
indexstate->ss.ss_currentRelation = currentRelation;
indexstate->ss.ss_currentScanDesc = NULL; /* no heap scan here */
diff --git a/src/backend/executor/nodeIndexscan.c b/src/backend/executor/nodeIndexscan.c
index 32e1714f15..a3bd1f7fb0 100644
--- a/src/backend/executor/nodeIndexscan.c
+++ b/src/backend/executor/nodeIndexscan.c
@@ -908,6 +908,8 @@ ExecInitIndexScan(IndexScan *node, EState *estate, int eflags)
* open the scan relation
*/
currentRelation = ExecOpenScanRelation(estate, node->scan.scanrelid, eflags);
+ if (unlikely(!ExecPlanStillValid(estate)))
+ return indexstate;
indexstate->ss.ss_currentRelation = currentRelation;
indexstate->ss.ss_currentScanDesc = NULL; /* no heap scan here */
diff --git a/src/backend/executor/nodeLimit.c b/src/backend/executor/nodeLimit.c
index a97bac9f6d..ab133f1580 100644
--- a/src/backend/executor/nodeLimit.c
+++ b/src/backend/executor/nodeLimit.c
@@ -476,6 +476,8 @@ ExecInitLimit(Limit *node, EState *estate, int eflags)
*/
outerPlan = outerPlan(node);
outerPlanState(limitstate) = ExecInitNode(outerPlan, estate, eflags);
+ if (unlikely(!ExecPlanStillValid(estate)))
+ return limitstate;
/*
* initialize child expressions
diff --git a/src/backend/executor/nodeLockRows.c b/src/backend/executor/nodeLockRows.c
index 26fbe95c57..e1ef768571 100644
--- a/src/backend/executor/nodeLockRows.c
+++ b/src/backend/executor/nodeLockRows.c
@@ -322,6 +322,8 @@ ExecInitLockRows(LockRows *node, EState *estate, int eflags)
* then initialize outer plan
*/
outerPlanState(lrstate) = ExecInitNode(outerPlan, estate, eflags);
+ if (unlikely(!ExecPlanStillValid(estate)))
+ return lrstate;
/* node returns unmodified slots from the outer plan */
lrstate->ps.resultopsset = true;
diff --git a/src/backend/executor/nodeMaterial.c b/src/backend/executor/nodeMaterial.c
index 03c514900b..c38eef099d 100644
--- a/src/backend/executor/nodeMaterial.c
+++ b/src/backend/executor/nodeMaterial.c
@@ -214,6 +214,8 @@ ExecInitMaterial(Material *node, EState *estate, int eflags)
outerPlan = outerPlan(node);
outerPlanState(matstate) = ExecInitNode(outerPlan, estate, eflags);
+ if (unlikely(!ExecPlanStillValid(estate)))
+ return matstate;
/*
* Initialize result type and slot. No need to initialize projection info
diff --git a/src/backend/executor/nodeMemoize.c b/src/backend/executor/nodeMemoize.c
index ee4749c852..a6bf66029c 100644
--- a/src/backend/executor/nodeMemoize.c
+++ b/src/backend/executor/nodeMemoize.c
@@ -938,6 +938,8 @@ ExecInitMemoize(Memoize *node, EState *estate, int eflags)
outerNode = outerPlan(node);
outerPlanState(mstate) = ExecInitNode(outerNode, estate, eflags);
+ if (unlikely(!ExecPlanStillValid(estate)))
+ return mstate;
/*
* Initialize return slot and type. No need to initialize projection info
diff --git a/src/backend/executor/nodeMergeAppend.c b/src/backend/executor/nodeMergeAppend.c
index 0a42a04b19..52c3edf278 100644
--- a/src/backend/executor/nodeMergeAppend.c
+++ b/src/backend/executor/nodeMergeAppend.c
@@ -120,7 +120,7 @@ ExecInitMergeAppend(MergeAppend *node, EState *estate, int eflags)
mergestate->ms_prune_state = NULL;
}
- mergeplanstates = (PlanState **) palloc(nplans * sizeof(PlanState *));
+ mergeplanstates = (PlanState **) palloc0(nplans * sizeof(PlanState *));
mergestate->mergeplans = mergeplanstates;
mergestate->ms_nplans = nplans;
@@ -151,6 +151,8 @@ ExecInitMergeAppend(MergeAppend *node, EState *estate, int eflags)
Plan *initNode = (Plan *) list_nth(node->mergeplans, i);
mergeplanstates[j++] = ExecInitNode(initNode, estate, eflags);
+ if (unlikely(!ExecPlanStillValid(estate)))
+ return mergestate;
}
mergestate->ps.ps_ProjInfo = NULL;
diff --git a/src/backend/executor/nodeMergejoin.c b/src/backend/executor/nodeMergejoin.c
index c84f53e0bd..887f519b10 100644
--- a/src/backend/executor/nodeMergejoin.c
+++ b/src/backend/executor/nodeMergejoin.c
@@ -1490,11 +1490,15 @@ ExecInitMergeJoin(MergeJoin *node, EState *estate, int eflags)
mergestate->mj_SkipMarkRestore = node->skip_mark_restore;
outerPlanState(mergestate) = ExecInitNode(outerPlan(node), estate, eflags);
+ if (unlikely(!ExecPlanStillValid(estate)))
+ return mergestate;
outerDesc = ExecGetResultType(outerPlanState(mergestate));
innerPlanState(mergestate) = ExecInitNode(innerPlan(node), estate,
mergestate->mj_SkipMarkRestore ?
eflags :
(eflags | EXEC_FLAG_MARK));
+ if (unlikely(!ExecPlanStillValid(estate)))
+ return mergestate;
innerDesc = ExecGetResultType(innerPlanState(mergestate));
/*
diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c
index ea043c57c1..95d909c1d0 100644
--- a/src/backend/executor/nodeModifyTable.c
+++ b/src/backend/executor/nodeModifyTable.c
@@ -3985,6 +3985,13 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags)
linitial_int(node->resultRelations));
}
+ /*
+ * ExecInitResultRelation() may have returned without initializing
+ * rootResultRelInfo if the plan got invalidated, so check.
+ */
+ if (unlikely(!ExecPlanStillValid(estate)))
+ return mtstate;
+
/* set up epqstate with dummy subplan data for the moment */
EvalPlanQualInit(&mtstate->mt_epqstate, estate, NULL, NIL,
node->epqParam, node->resultRelations);
@@ -4013,6 +4020,10 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags)
{
ExecInitResultRelation(estate, resultRelInfo, resultRelation);
+ /* See the comment above. */
+ if (unlikely(!ExecPlanStillValid(estate)))
+ return mtstate;
+
/*
* For child result relations, store the root result relation
* pointer. We do so for the convenience of places that want to
@@ -4039,6 +4050,8 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags)
* Now we may initialize the subplan.
*/
outerPlanState(mtstate) = ExecInitNode(subplan, estate, eflags);
+ if (unlikely(!ExecPlanStillValid(estate)))
+ return mtstate;
/*
* Do additional per-result-relation initialization.
diff --git a/src/backend/executor/nodeNestloop.c b/src/backend/executor/nodeNestloop.c
index 1211d871ea..8d67d17e10 100644
--- a/src/backend/executor/nodeNestloop.c
+++ b/src/backend/executor/nodeNestloop.c
@@ -295,11 +295,15 @@ ExecInitNestLoop(NestLoop *node, EState *estate, int eflags)
* values.
*/
outerPlanState(nlstate) = ExecInitNode(outerPlan(node), estate, eflags);
+ if (unlikely(!ExecPlanStillValid(estate)))
+ return nlstate;
if (node->nestParams == NIL)
eflags |= EXEC_FLAG_REWIND;
else
eflags &= ~EXEC_FLAG_REWIND;
innerPlanState(nlstate) = ExecInitNode(innerPlan(node), estate, eflags);
+ if (unlikely(!ExecPlanStillValid(estate)))
+ return nlstate;
/*
* Initialize result slot, type and projection.
diff --git a/src/backend/executor/nodeProjectSet.c b/src/backend/executor/nodeProjectSet.c
index e9b96416d3..706cc23a21 100644
--- a/src/backend/executor/nodeProjectSet.c
+++ b/src/backend/executor/nodeProjectSet.c
@@ -247,6 +247,8 @@ ExecInitProjectSet(ProjectSet *node, EState *estate, int eflags)
* initialize child nodes
*/
outerPlanState(state) = ExecInitNode(outerPlan(node), estate, eflags);
+ if (unlikely(!ExecPlanStillValid(estate)))
+ return state;
/*
* we don't use inner plan
diff --git a/src/backend/executor/nodeRecursiveunion.c b/src/backend/executor/nodeRecursiveunion.c
index f6d60bcd6c..27dc318acb 100644
--- a/src/backend/executor/nodeRecursiveunion.c
+++ b/src/backend/executor/nodeRecursiveunion.c
@@ -244,7 +244,11 @@ ExecInitRecursiveUnion(RecursiveUnion *node, EState *estate, int eflags)
* initialize child nodes
*/
outerPlanState(rustate) = ExecInitNode(outerPlan(node), estate, eflags);
+ if (unlikely(!ExecPlanStillValid(estate)))
+ return rustate;
innerPlanState(rustate) = ExecInitNode(innerPlan(node), estate, eflags);
+ if (unlikely(!ExecPlanStillValid(estate)))
+ return rustate;
/*
* If hashing, precompute fmgr lookup data for inner loop, and create the
diff --git a/src/backend/executor/nodeResult.c b/src/backend/executor/nodeResult.c
index f15902e840..6820d3bfd5 100644
--- a/src/backend/executor/nodeResult.c
+++ b/src/backend/executor/nodeResult.c
@@ -208,6 +208,8 @@ ExecInitResult(Result *node, EState *estate, int eflags)
* initialize child nodes
*/
outerPlanState(resstate) = ExecInitNode(outerPlan(node), estate, eflags);
+ if (unlikely(!ExecPlanStillValid(estate)))
+ return resstate;
/*
* we don't use inner plan
diff --git a/src/backend/executor/nodeSamplescan.c b/src/backend/executor/nodeSamplescan.c
index a6813559e6..02051fea51 100644
--- a/src/backend/executor/nodeSamplescan.c
+++ b/src/backend/executor/nodeSamplescan.c
@@ -125,6 +125,8 @@ ExecInitSampleScan(SampleScan *node, EState *estate, int eflags)
ExecOpenScanRelation(estate,
node->scan.scanrelid,
eflags);
+ if (unlikely(!ExecPlanStillValid(estate)))
+ return scanstate;
/* we won't set up the HeapScanDesc till later */
scanstate->ss.ss_currentScanDesc = NULL;
diff --git a/src/backend/executor/nodeSeqscan.c b/src/backend/executor/nodeSeqscan.c
index 911266da07..9e3ef94388 100644
--- a/src/backend/executor/nodeSeqscan.c
+++ b/src/backend/executor/nodeSeqscan.c
@@ -153,6 +153,8 @@ ExecInitSeqScan(SeqScan *node, EState *estate, int eflags)
ExecOpenScanRelation(estate,
node->scan.scanrelid,
eflags);
+ if (unlikely(!ExecPlanStillValid(estate)))
+ return scanstate;
/* and create slot with the appropriate rowtype */
ExecInitScanTupleSlot(estate, &scanstate->ss,
diff --git a/src/backend/executor/nodeSetOp.c b/src/backend/executor/nodeSetOp.c
index 5c2861d243..475af4df24 100644
--- a/src/backend/executor/nodeSetOp.c
+++ b/src/backend/executor/nodeSetOp.c
@@ -528,6 +528,8 @@ ExecInitSetOp(SetOp *node, EState *estate, int eflags)
if (node->strategy == SETOP_HASHED)
eflags &= ~EXEC_FLAG_REWIND;
outerPlanState(setopstate) = ExecInitNode(outerPlan(node), estate, eflags);
+ if (unlikely(!ExecPlanStillValid(estate)))
+ return setopstate;
outerDesc = ExecGetResultType(outerPlanState(setopstate));
/*
diff --git a/src/backend/executor/nodeSort.c b/src/backend/executor/nodeSort.c
index c8a35b64a8..9de717aa7c 100644
--- a/src/backend/executor/nodeSort.c
+++ b/src/backend/executor/nodeSort.c
@@ -263,6 +263,8 @@ ExecInitSort(Sort *node, EState *estate, int eflags)
eflags &= ~(EXEC_FLAG_REWIND | EXEC_FLAG_BACKWARD | EXEC_FLAG_MARK);
outerPlanState(sortstate) = ExecInitNode(outerPlan(node), estate, eflags);
+ if (unlikely(!ExecPlanStillValid(estate)))
+ return sortstate;
/*
* Initialize scan slot and type.
diff --git a/src/backend/executor/nodeSubqueryscan.c b/src/backend/executor/nodeSubqueryscan.c
index 91d7ae82ce..d9c10d1f6f 100644
--- a/src/backend/executor/nodeSubqueryscan.c
+++ b/src/backend/executor/nodeSubqueryscan.c
@@ -124,6 +124,8 @@ ExecInitSubqueryScan(SubqueryScan *node, EState *estate, int eflags)
* initialize subquery
*/
subquerystate->subplan = ExecInitNode(node->subplan, estate, eflags);
+ if (unlikely(!ExecPlanStillValid(estate)))
+ return subquerystate;
/*
* Initialize scan slot and type (needed by ExecAssignScanProjectionInfo)
diff --git a/src/backend/executor/nodeTidrangescan.c b/src/backend/executor/nodeTidrangescan.c
index 9147e4afa8..a7482aee50 100644
--- a/src/backend/executor/nodeTidrangescan.c
+++ b/src/backend/executor/nodeTidrangescan.c
@@ -378,6 +378,8 @@ ExecInitTidRangeScan(TidRangeScan *node, EState *estate, int eflags)
* open the scan relation
*/
currentRelation = ExecOpenScanRelation(estate, node->scan.scanrelid, eflags);
+ if (unlikely(!ExecPlanStillValid(estate)))
+ return tidrangestate;
tidrangestate->ss.ss_currentRelation = currentRelation;
tidrangestate->ss.ss_currentScanDesc = NULL; /* no table scan here */
diff --git a/src/backend/executor/nodeTidscan.c b/src/backend/executor/nodeTidscan.c
index 74ec6afdcc..657411ef19 100644
--- a/src/backend/executor/nodeTidscan.c
+++ b/src/backend/executor/nodeTidscan.c
@@ -523,6 +523,8 @@ ExecInitTidScan(TidScan *node, EState *estate, int eflags)
* open the scan relation
*/
currentRelation = ExecOpenScanRelation(estate, node->scan.scanrelid, eflags);
+ if (unlikely(!ExecPlanStillValid(estate)))
+ return tidstate;
tidstate->ss.ss_currentRelation = currentRelation;
tidstate->ss.ss_currentScanDesc = NULL; /* no heap scan here */
diff --git a/src/backend/executor/nodeUnique.c b/src/backend/executor/nodeUnique.c
index 13c556326a..ee30688417 100644
--- a/src/backend/executor/nodeUnique.c
+++ b/src/backend/executor/nodeUnique.c
@@ -136,6 +136,8 @@ ExecInitUnique(Unique *node, EState *estate, int eflags)
* then initialize outer plan
*/
outerPlanState(uniquestate) = ExecInitNode(outerPlan(node), estate, eflags);
+ if (unlikely(!ExecPlanStillValid(estate)))
+ return uniquestate;
/*
* Initialize result slot and type. Unique nodes do no projections, so
diff --git a/src/backend/executor/nodeWindowAgg.c b/src/backend/executor/nodeWindowAgg.c
index c4c6f009ba..1246d7919a 100644
--- a/src/backend/executor/nodeWindowAgg.c
+++ b/src/backend/executor/nodeWindowAgg.c
@@ -2461,6 +2461,8 @@ ExecInitWindowAgg(WindowAgg *node, EState *estate, int eflags)
*/
outerPlan = outerPlan(node);
outerPlanState(winstate) = ExecInitNode(outerPlan, estate, eflags);
+ if (unlikely(!ExecPlanStillValid(estate)))
+ return winstate;
/*
* initialize source tuple type (which is also the tuple type that we'll
diff --git a/src/backend/executor/spi.c b/src/backend/executor/spi.c
index 33975687b3..f2cca807ef 100644
--- a/src/backend/executor/spi.c
+++ b/src/backend/executor/spi.c
@@ -2668,6 +2668,7 @@ _SPI_execute_plan(SPIPlanPtr plan, const SPIExecuteOptions *options,
snap = InvalidSnapshot;
qdesc = CreateQueryDesc(stmt,
+ NULL,
plansource->query_string,
snap, crosscheck_snapshot,
dest,
diff --git a/src/backend/tcop/pquery.c b/src/backend/tcop/pquery.c
index 5565f200c3..4ef349df8b 100644
--- a/src/backend/tcop/pquery.c
+++ b/src/backend/tcop/pquery.c
@@ -65,6 +65,7 @@ static void DoPortalRewind(Portal portal);
*/
QueryDesc *
CreateQueryDesc(PlannedStmt *plannedstmt,
+ CachedPlan *cplan,
const char *sourceText,
Snapshot snapshot,
Snapshot crosscheck_snapshot,
@@ -77,6 +78,7 @@ CreateQueryDesc(PlannedStmt *plannedstmt,
qd->operation = plannedstmt->commandType; /* operation */
qd->plannedstmt = plannedstmt; /* plan */
+ qd->cplan = cplan; /* CachedPlan, if plan is from one */
qd->sourceText = sourceText; /* query text */
qd->snapshot = RegisterSnapshot(snapshot); /* snapshot */
/* RI check snapshot */
@@ -145,7 +147,7 @@ ProcessQuery(PlannedStmt *plan,
/*
* Create the QueryDesc object
*/
- queryDesc = CreateQueryDesc(plan, sourceText,
+ queryDesc = CreateQueryDesc(plan, NULL, sourceText,
GetActiveSnapshot(), InvalidSnapshot,
dest, params, queryEnv, 0);
@@ -493,6 +495,7 @@ PortalStart(Portal portal, ParamListInfo params,
* the destination to DestNone.
*/
queryDesc = CreateQueryDesc(linitial_node(PlannedStmt, portal->stmts),
+ NULL,
portal->sourceText,
GetActiveSnapshot(),
InvalidSnapshot,
diff --git a/src/include/executor/execdesc.h b/src/include/executor/execdesc.h
index af2bf36dfb..4b7368a0dc 100644
--- a/src/include/executor/execdesc.h
+++ b/src/include/executor/execdesc.h
@@ -32,9 +32,12 @@
*/
typedef struct QueryDesc
{
+ NodeTag type;
+
/* These fields are provided by CreateQueryDesc */
CmdType operation; /* CMD_SELECT, CMD_UPDATE, etc. */
PlannedStmt *plannedstmt; /* planner's output (could be utility, too) */
+ struct CachedPlan *cplan; /* CachedPlan, if plannedstmt is from one */
const char *sourceText; /* source text of the query */
Snapshot snapshot; /* snapshot to use for query */
Snapshot crosscheck_snapshot; /* crosscheck for RI update/delete */
@@ -57,6 +60,7 @@ typedef struct QueryDesc
/* in pquery.c */
extern QueryDesc *CreateQueryDesc(PlannedStmt *plannedstmt,
+ struct CachedPlan *cplan,
const char *sourceText,
Snapshot snapshot,
Snapshot crosscheck_snapshot,
diff --git a/src/include/executor/executor.h b/src/include/executor/executor.h
index aeebe0e0ff..72cbf120c5 100644
--- a/src/include/executor/executor.h
+++ b/src/include/executor/executor.h
@@ -19,6 +19,7 @@
#include "nodes/lockoptions.h"
#include "nodes/parsenodes.h"
#include "utils/memutils.h"
+#include "utils/plancache.h"
/*
@@ -256,6 +257,15 @@ extern void ExecEndNode(PlanState *node);
extern void ExecShutdownNode(PlanState *node);
extern void ExecSetTupleBound(int64 tuples_needed, PlanState *child_node);
+/*
+ * Is the CachedPlan in es_cachedplan still valid?
+ */
+static inline bool
+ExecPlanStillValid(EState *estate)
+{
+ return estate->es_cachedplan == NULL ? true :
+ CachedPlanStillValid(estate->es_cachedplan);
+}
/* ----------------------------------------------------------------
* ExecProcNode
diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h
index cb714f4a19..846eb32a1d 100644
--- a/src/include/nodes/execnodes.h
+++ b/src/include/nodes/execnodes.h
@@ -623,6 +623,8 @@ typedef struct EState
* ExecRowMarks, or NULL if none */
List *es_rteperminfos; /* List of RTEPermissionInfo */
PlannedStmt *es_plannedstmt; /* link to top of plan tree */
+ struct CachedPlan *es_cachedplan; /* CachedPlan if plannedstmt is from
+ * one, or NULL if not */
const char *es_sourceText; /* Source text from QueryDesc */
JunkFilter *es_junkFilter; /* top-level junk filter, if any */
diff --git a/src/include/utils/plancache.h b/src/include/utils/plancache.h
index 916e59d9fe..0a9e041d51 100644
--- a/src/include/utils/plancache.h
+++ b/src/include/utils/plancache.h
@@ -221,6 +221,20 @@ extern CachedPlan *GetCachedPlan(CachedPlanSource *plansource,
ParamListInfo boundParams,
ResourceOwner owner,
QueryEnvironment *queryEnv);
+
+/*
+ * CachedPlanStillValid
+ * Returns if a cached generic plan is still valid
+ *
+ * Invoked by the executor for each relation lock acquired during the
+ * initialization of the plan tree within the CachedPlan.
+ */
+static inline bool
+CachedPlanStillValid(CachedPlan *cplan)
+{
+ return cplan->is_valid;
+}
+
extern void ReleaseCachedPlan(CachedPlan *plan, ResourceOwner owner);
extern bool CachedPlanAllowsSimpleValidityCheck(CachedPlanSource *plansource,
--
2.35.3
view thread (31+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: generic plans and "initial" pruning
In-Reply-To: <CA+HiwqG9YF-xxnXRW604LkJbTvFABwfSBgrNcQFMCL5-QG=P0g@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