From 7423b46ffe28f0def985f49932704504c71ca8e1 Mon Sep 17 00:00:00 2001 From: Amit Langote Date: Fri, 18 Oct 2024 22:06:02 +0900 Subject: [PATCH v57 5/5] Handle CachedPlan invalidation in the executor This commit makes changes to handle cases where a cached plan becomes invalid after deferred locks on prunable relations are taken. InitPlan() now returns immediately without doing anything after finding that the locks taken by ExecDoInitialPruning() have invalidated the CachedPlan. ExecutorStartExt(), a wrapper over ExecutorStart(), is added to handle cases where InitPlan() returns early due to plan invalidation. ExecutorStartExt() updates the CachedPlan to create fresh plans for all queries contained it its owning CachedPlanSource and retries execution with the new plan for the query. This new function is only called by sites that use plancache.c for getting a plan. To update an invalid CachedPlan, ExecutorStartExt() calls the new plancache.c function UpdateCachedPlan(), which creates fresh plans for each query in the CachedPlanSource and replaces the old stale ones in CachedPlan.stmt_list in place. This leads to the old ones leaking into CachedPlan.plan_context, but UpdateCachedPlan() should be called fairly rarely for this to amount to huge amount of leaked memory. This also adds isolation tests using the delay_execution test module to verify scenarios where a CachedPlan becomes invalid before the deferred locks are taken. All ExecutorStart_hook implementations now must add the following block after the ExecutorStart() call to ensure it doesn't work with an invalid plan: /* The plan may have become invalid during ExecutorStart() */ if (!ExecPlanStillValid(queryDesc->estate)) return; Reviewed-by: Robert Haas Discussion: https://postgr.es/m/CA+HiwqFGkMSge6TgC9KQzde0ohpAycLQuV7ooitEEpbKB0O_mg@mail.gmail.comk --- contrib/auto_explain/auto_explain.c | 4 + .../pg_stat_statements/pg_stat_statements.c | 4 + src/backend/commands/explain.c | 8 +- src/backend/commands/portalcmds.c | 1 + src/backend/commands/prepare.c | 10 +- src/backend/commands/trigger.c | 14 + src/backend/executor/README | 35 ++- src/backend/executor/execMain.c | 103 ++++++- src/backend/executor/execUtils.c | 1 + src/backend/executor/spi.c | 19 +- src/backend/tcop/postgres.c | 4 +- src/backend/tcop/pquery.c | 20 +- src/backend/utils/cache/plancache.c | 125 +++++++- src/backend/utils/mmgr/portalmem.c | 4 +- src/include/commands/explain.h | 1 + src/include/commands/trigger.h | 1 + src/include/executor/executor.h | 16 + src/include/nodes/execnodes.h | 1 + src/include/utils/plancache.h | 25 ++ src/include/utils/portal.h | 4 +- src/test/modules/delay_execution/Makefile | 3 +- .../modules/delay_execution/delay_execution.c | 63 +++- .../expected/cached-plan-inval.out | 282 ++++++++++++++++++ src/test/modules/delay_execution/meson.build | 1 + .../specs/cached-plan-inval.spec | 80 +++++ 25 files changed, 787 insertions(+), 42 deletions(-) create mode 100644 src/test/modules/delay_execution/expected/cached-plan-inval.out create mode 100644 src/test/modules/delay_execution/specs/cached-plan-inval.spec diff --git a/contrib/auto_explain/auto_explain.c b/contrib/auto_explain/auto_explain.c index 677c135f59..9eb5e9a619 100644 --- a/contrib/auto_explain/auto_explain.c +++ b/contrib/auto_explain/auto_explain.c @@ -300,6 +300,10 @@ explain_ExecutorStart(QueryDesc *queryDesc, int eflags) else standard_ExecutorStart(queryDesc, eflags); + /* The plan may have become invalid during standard_ExecutorStart() */ + if (!ExecPlanStillValid(queryDesc->estate)) + return; + if (auto_explain_enabled()) { /* diff --git a/contrib/pg_stat_statements/pg_stat_statements.c b/contrib/pg_stat_statements/pg_stat_statements.c index 21b26b7b6e..0bddcf8a48 100644 --- a/contrib/pg_stat_statements/pg_stat_statements.c +++ b/contrib/pg_stat_statements/pg_stat_statements.c @@ -997,6 +997,10 @@ pgss_ExecutorStart(QueryDesc *queryDesc, int eflags) else standard_ExecutorStart(queryDesc, eflags); + /* The plan may have become invalid during standard_ExecutorStart() */ + if (!ExecPlanStillValid(queryDesc->estate)) + return; + /* * If query has queryId zero, don't track it. This prevents double * counting of optimizable statements that are directly contained in diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.c index b699089bd8..07781ce915 100644 --- a/src/backend/commands/explain.c +++ b/src/backend/commands/explain.c @@ -515,7 +515,8 @@ standard_ExplainOneQuery(Query *query, int cursorOptions, } /* run it (if needed) and produce output */ - ExplainOnePlan(plan, NULL, into, es, queryString, params, queryEnv, + ExplainOnePlan(plan, NULL, NULL, -1, into, es, queryString, params, + queryEnv, &planduration, (es->buffers ? &bufusage : NULL), es->memory ? &mem_counters : NULL); } @@ -624,6 +625,7 @@ ExplainOneUtility(Node *utilityStmt, IntoClause *into, ExplainState *es, */ void ExplainOnePlan(PlannedStmt *plannedstmt, CachedPlan *cplan, + CachedPlanSource *plansource, int query_index, IntoClause *into, ExplainState *es, const char *queryString, ParamListInfo params, QueryEnvironment *queryEnv, const instr_time *planduration, @@ -694,8 +696,8 @@ ExplainOnePlan(PlannedStmt *plannedstmt, CachedPlan *cplan, if (into) eflags |= GetIntoRelEFlags(into); - /* call ExecutorStart to prepare the plan for execution */ - ExecutorStart(queryDesc, eflags); + /* Call ExecutorStartExt to prepare the plan for execution. */ + ExecutorStartExt(queryDesc, eflags, plansource, query_index); /* Execute the plan for statistics if asked for */ if (es->analyze) diff --git a/src/backend/commands/portalcmds.c b/src/backend/commands/portalcmds.c index 4f6acf6719..4b1503c05e 100644 --- a/src/backend/commands/portalcmds.c +++ b/src/backend/commands/portalcmds.c @@ -107,6 +107,7 @@ PerformCursorOpen(ParseState *pstate, DeclareCursorStmt *cstmt, ParamListInfo pa queryString, CMDTAG_SELECT, /* cursor's query is always a SELECT */ list_make1(plan), + NULL, NULL); /*---------- diff --git a/src/backend/commands/prepare.c b/src/backend/commands/prepare.c index 311b9ebd5b..4cd79a6e3a 100644 --- a/src/backend/commands/prepare.c +++ b/src/backend/commands/prepare.c @@ -202,7 +202,8 @@ ExecuteQuery(ParseState *pstate, query_string, entry->plansource->commandTag, plan_list, - cplan); + cplan, + entry->plansource); /* * For CREATE TABLE ... AS EXECUTE, we must verify that the prepared @@ -583,6 +584,7 @@ ExplainExecuteQuery(ExecuteStmt *execstmt, IntoClause *into, ExplainState *es, MemoryContextCounters mem_counters; MemoryContext planner_ctx = NULL; MemoryContext saved_ctx = NULL; + int i = 0; if (es->memory) { @@ -655,8 +657,8 @@ ExplainExecuteQuery(ExecuteStmt *execstmt, IntoClause *into, ExplainState *es, PlannedStmt *pstmt = lfirst_node(PlannedStmt, p); if (pstmt->commandType != CMD_UTILITY) - ExplainOnePlan(pstmt, cplan, into, es, query_string, paramLI, - queryEnv, + ExplainOnePlan(pstmt, cplan, entry->plansource, i, + into, es, query_string, paramLI, queryEnv, &planduration, (es->buffers ? &bufusage : NULL), es->memory ? &mem_counters : NULL); else @@ -668,6 +670,8 @@ ExplainExecuteQuery(ExecuteStmt *execstmt, IntoClause *into, ExplainState *es, /* Separate plans with an appropriate separator */ if (lnext(plan_list, p) != NULL) ExplainSeparatePlans(es); + + i++; } if (estate) diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c index 09356e46d1..79572ec8f1 100644 --- a/src/backend/commands/trigger.c +++ b/src/backend/commands/trigger.c @@ -5123,6 +5123,20 @@ AfterTriggerEndQuery(EState *estate) afterTriggers.query_depth--; } +/* ---------- + * AfterTriggerAbortQuery() + * + * Called by ExecutorEnd() if the query execution was aborted due to the + * plan becoming invalid during initialization. + * ---------- + */ +void +AfterTriggerAbortQuery(void) +{ + /* Revert the actions of AfterTriggerBeginQuery(). */ + afterTriggers.query_depth--; +} + /* * AfterTriggerFreeQuery diff --git a/src/backend/executor/README b/src/backend/executor/README index 642d63be61..c76a00b394 100644 --- a/src/backend/executor/README +++ b/src/backend/executor/README @@ -280,6 +280,28 @@ are typically reset to empty once per tuple. Per-tuple contexts are usually associated with ExprContexts, and commonly each PlanState node has its own ExprContext to evaluate its qual and targetlist expressions in. +Relation Locking +---------------- + +Typically, when the executor initializes a plan tree for execution, it doesn't +lock non-index relations if the plan tree is freshly generated and not derived +from a CachedPlan. This is because such locks have already been established +during the query's parsing, rewriting, and planning phases. However, with a +cached plan tree, some relations may remain unlocked. The function +AcquireExecutorLocks() only locks unprunable relations in the plan, deferring +the locking of prunable ones to executor initialization. This avoids +unnecessary locking of relations that will be pruned during "initial" runtime +pruning in ExecDoInitialPruning(). + +This approach creates a window where a cached plan tree with child tables +could become outdated if another backend modifies these tables before +ExecDoInitialPruning() locks them. As a result, the executor has the added duty +to verify the plan tree's validity whenever it locks a child table after +doing initial pruning. This validation is done by checking the CachedPlan.is_valid +attribute. If the plan tree is outdated (is_valid=false), the executor halts +further initialization, cleans up anything in EState that would have been +allocated up to that point, and retries execution after recreating the +invalid plan in the CachedPlan. Query Processing Control Flow ----------------------------- @@ -288,11 +310,13 @@ This is a sketch of control flow for full query processing: CreateQueryDesc - ExecutorStart + ExecutorStart or ExecutorStartExt CreateExecutorState creates per-query context - switch to per-query context to run ExecInitNode + switch to per-query context to run ExecDoInitialPruning and ExecInitNode AfterTriggerBeginQuery + ExecDoInitialPruning + does initial pruning and locks surviving partitions if needed ExecInitNode --- recursively scans plan tree ExecInitNode recurse into subsidiary nodes @@ -316,7 +340,12 @@ 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 after locking partitions in ExecDoInitialPruning(), the control is +immediately returned to ExecutorStartExt(), which will create a new plan tree +and perform the steps starting from CreateExecutorState() again. + +Per above comments, it's not really critical for ExecEndPlan to free any memory; it'll all go away in FreeExecutorState anyway. However, we do need to be careful to close relations, drop buffer pins, etc, so we do need to scan the plan state tree to find these sorts of resources. diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c index ed783236eb..5427bdfd4c 100644 --- a/src/backend/executor/execMain.c +++ b/src/backend/executor/execMain.c @@ -60,6 +60,7 @@ #include "utils/backend_status.h" #include "utils/lsyscache.h" #include "utils/partcache.h" +#include "utils/plancache.h" #include "utils/rls.h" #include "utils/snapmgr.h" @@ -138,6 +139,63 @@ ExecutorStart(QueryDesc *queryDesc, int eflags) standard_ExecutorStart(queryDesc, eflags); } +/* + * ExecutorStartExt + * Start query execution, replanning if the plan is invalidated due to + * locks taken during initialization, which can occur when the plan is + * from a CachedPlan. + * + * This function is a variant of ExecutorStart() that handles cases where + * the CachedPlan might become invalid during initialization, particularly + * when prunable relations are locked. If locks taken during ExecutorStart() + * invalidate the plan, the function calls UpdateCachedPlan() to replan all + * queries in the CachedPlan, including the query at query_index, and then + * retries initialization. + * + * The function repeats the process until ExecutorStart() successfully + * initializes the query at query_index with a valid plan. If invalidation + * occurs, the current execution state is cleaned up by calling ExecutorEnd(), + * and the plan is updated by UpdateCachedPlan(). The loop exits once the + * query is successfully initialized with a valid CachedPlan. + */ +void +ExecutorStartExt(QueryDesc *queryDesc, int eflags, + CachedPlanSource *plansource, + int query_index) +{ + if (queryDesc->cplan == NULL) + { + ExecutorStart(queryDesc, eflags); + return; + } + + /* + * For a CachedPlan, locks acquired during ExecutorStart() may invalidate it. + * Therefore, we must loop and retry with an updated plan until no further + * invalidation occurs. + */ + while (1) + { + ExecutorStart(queryDesc, eflags); + if (!CachedPlanValid(queryDesc->cplan)) + { + /* + * Clean up the current execution state before creating the new + * plan to retry ExecutorStart(). Mark execution as aborted to + * ensure that AFTER trigger state is properly reset. + */ + queryDesc->estate->es_aborted = true; + ExecutorEnd(queryDesc); + + queryDesc->plannedstmt = UpdateCachedPlan(plansource, query_index, + queryDesc->queryEnv); + } + else + /* Exit the loop if the plan is initialized successfully. */ + break; + } +} + void standard_ExecutorStart(QueryDesc *queryDesc, int eflags) { @@ -321,6 +379,7 @@ standard_ExecutorRun(QueryDesc *queryDesc, estate = queryDesc->estate; Assert(estate != NULL); + Assert(!estate->es_aborted); Assert(!(estate->es_top_eflags & EXEC_FLAG_EXPLAIN_ONLY)); /* caller must ensure the query's snapshot is active */ @@ -427,8 +486,11 @@ standard_ExecutorFinish(QueryDesc *queryDesc) Assert(estate != NULL); Assert(!(estate->es_top_eflags & EXEC_FLAG_EXPLAIN_ONLY)); - /* This should be run once and only once per Executor instance */ - Assert(!estate->es_finished); + /* + * This should be run once and only once per Executor instance and never + * if the execution was aborted. + */ + Assert(!estate->es_finished && !estate->es_aborted); /* Switch into per-query memory context */ oldcontext = MemoryContextSwitchTo(estate->es_query_cxt); @@ -487,11 +549,10 @@ standard_ExecutorEnd(QueryDesc *queryDesc) Assert(estate != NULL); /* - * Check that ExecutorFinish was called, unless in EXPLAIN-only mode. This - * Assert is needed because ExecutorFinish is new as of 9.1, and callers - * might forget to call it. + * Check that ExecutorFinish was called, unless in EXPLAIN-only mode or if + * execution was aborted. */ - Assert(estate->es_finished || + Assert(estate->es_finished || estate->es_aborted || (estate->es_top_eflags & EXEC_FLAG_EXPLAIN_ONLY)); /* @@ -505,6 +566,14 @@ standard_ExecutorEnd(QueryDesc *queryDesc) UnregisterSnapshot(estate->es_snapshot); UnregisterSnapshot(estate->es_crosscheck_snapshot); + /* + * Reset AFTER trigger module if the query execution was aborted. + */ + if (estate->es_aborted && + !(estate->es_top_eflags & + (EXEC_FLAG_SKIP_TRIGGERS | EXEC_FLAG_EXPLAIN_ONLY))) + AfterTriggerAbortQuery(); + /* * Must switch out of context before destroying it */ @@ -862,6 +931,7 @@ static void ExecDoInitialPruning(EState *estate) { ListCell *lc; + List *locked_relids = NIL; foreach(lc, estate->es_part_prune_infos) { @@ -897,6 +967,7 @@ ExecDoInitialPruning(EState *estate) Assert(rte->rtekind == RTE_RELATION && rte->rellockmode != NoLock); LockRelationOid(rte->relid, rte->rellockmode); + locked_relids = lappend_int(locked_relids, rtindex); } } estate->es_unpruned_relids = bms_add_members(estate->es_unpruned_relids, @@ -906,6 +977,20 @@ ExecDoInitialPruning(EState *estate) estate->es_part_prune_results = lappend(estate->es_part_prune_results, validsubplans); } + + /* + * Release the useless locks if the plan won't be executed. This is the + * same as what CheckCachedPlan() in plancache.c does. + */ + if (!ExecPlanStillValid(estate)) + { + foreach(lc, locked_relids) + { + RangeTblEntry *rte = exec_rt_fetch(lfirst_int(lc), estate); + + UnlockRelationOid(rte->relid, rte->rellockmode); + } + } } /* @@ -969,6 +1054,9 @@ InitPlan(QueryDesc *queryDesc, int eflags) estate->es_part_prune_infos = plannedstmt->partPruneInfos; ExecDoInitialPruning(estate); + if (!ExecPlanStillValid(estate)) + return; + /* * Next, build the ExecRowMark array from the PlanRowMark(s), if any. */ @@ -2961,6 +3049,9 @@ EvalPlanQualStart(EPQState *epqstate, Plan *planTree) * 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. + * + * es_cachedplan is not copied because EPQ plan execution does not acquire + * any new locks that could invalidate the CachedPlan. */ rcestate->es_direction = ForwardScanDirection; rcestate->es_snapshot = parentestate->es_snapshot; diff --git a/src/backend/executor/execUtils.c b/src/backend/executor/execUtils.c index bc905a0cdc..b7c914d66c 100644 --- a/src/backend/executor/execUtils.c +++ b/src/backend/executor/execUtils.c @@ -147,6 +147,7 @@ CreateExecutorState(void) estate->es_top_eflags = 0; estate->es_instrument = 0; estate->es_finished = false; + estate->es_aborted = false; estate->es_exprcontexts = NIL; diff --git a/src/backend/executor/spi.c b/src/backend/executor/spi.c index e2b781e939..70ab0ece1d 100644 --- a/src/backend/executor/spi.c +++ b/src/backend/executor/spi.c @@ -70,7 +70,8 @@ static int _SPI_execute_plan(SPIPlanPtr plan, const SPIExecuteOptions *options, static ParamListInfo _SPI_convert_params(int nargs, Oid *argtypes, Datum *Values, const char *Nulls); -static int _SPI_pquery(QueryDesc *queryDesc, bool fire_triggers, uint64 tcount); +static int _SPI_pquery(QueryDesc *queryDesc, bool fire_triggers, uint64 tcount, + CachedPlanSource *plansource, int query_index); static void _SPI_error_callback(void *arg); @@ -1685,7 +1686,8 @@ SPI_cursor_open_internal(const char *name, SPIPlanPtr plan, query_string, plansource->commandTag, stmt_list, - cplan); + cplan, + plansource); /* * Set up options for portal. Default SCROLL type is chosen the same way @@ -2500,6 +2502,7 @@ _SPI_execute_plan(SPIPlanPtr plan, const SPIExecuteOptions *options, CachedPlanSource *plansource = (CachedPlanSource *) lfirst(lc1); List *stmt_list; ListCell *lc2; + int i = 0; spicallbackarg.query = plansource->query_string; @@ -2697,8 +2700,9 @@ _SPI_execute_plan(SPIPlanPtr plan, const SPIExecuteOptions *options, options->params, _SPI_current->queryEnv, 0); - res = _SPI_pquery(qdesc, fire_triggers, - canSetTag ? options->tcount : 0); + + res = _SPI_pquery(qdesc, fire_triggers, canSetTag ? options->tcount : 0, + plansource, i); FreeQueryDesc(qdesc); } else @@ -2795,6 +2799,8 @@ _SPI_execute_plan(SPIPlanPtr plan, const SPIExecuteOptions *options, my_res = res; goto fail; } + + i++; } /* Done with this plan, so release refcount */ @@ -2872,7 +2878,8 @@ _SPI_convert_params(int nargs, Oid *argtypes, } static int -_SPI_pquery(QueryDesc *queryDesc, bool fire_triggers, uint64 tcount) +_SPI_pquery(QueryDesc *queryDesc, bool fire_triggers, uint64 tcount, + CachedPlanSource *plansource, int query_index) { int operation = queryDesc->operation; int eflags; @@ -2928,7 +2935,7 @@ _SPI_pquery(QueryDesc *queryDesc, bool fire_triggers, uint64 tcount) else eflags = EXEC_FLAG_SKIP_TRIGGERS; - ExecutorStart(queryDesc, eflags); + ExecutorStartExt(queryDesc, eflags, plansource, query_index); ExecutorRun(queryDesc, ForwardScanDirection, tcount, true); diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c index 7f5eada9d4..3b98248ad4 100644 --- a/src/backend/tcop/postgres.c +++ b/src/backend/tcop/postgres.c @@ -1237,6 +1237,7 @@ exec_simple_query(const char *query_string) query_string, commandTag, plantree_list, + NULL, NULL); /* @@ -2039,7 +2040,8 @@ exec_bind_message(StringInfo input_message) query_string, psrc->commandTag, cplan->stmt_list, - cplan); + cplan, + psrc); /* Done with the snapshot used for parameter I/O and parsing/planning */ if (snapshot_set) diff --git a/src/backend/tcop/pquery.c b/src/backend/tcop/pquery.c index 6e8f6b1b8f..ee5eea4ce1 100644 --- a/src/backend/tcop/pquery.c +++ b/src/backend/tcop/pquery.c @@ -19,6 +19,7 @@ #include "access/xact.h" #include "commands/prepare.h" +#include "executor/execdesc.h" #include "executor/tstoreReceiver.h" #include "miscadmin.h" #include "pg_trace.h" @@ -37,6 +38,8 @@ Portal ActivePortal = NULL; static void ProcessQuery(PlannedStmt *plan, CachedPlan *cplan, + CachedPlanSource *plansource, + int query_index, const char *sourceText, ParamListInfo params, QueryEnvironment *queryEnv, @@ -126,6 +129,8 @@ FreeQueryDesc(QueryDesc *qdesc) * * plan: the plan tree for the query * cplan: CachedPlan supplying the plan + * plansource: CachedPlanSource supplying the cplan + * query_index: index of the query in plansource->query_list * sourceText: the source text of the query * params: any parameters needed * dest: where to send results @@ -139,6 +144,8 @@ FreeQueryDesc(QueryDesc *qdesc) static void ProcessQuery(PlannedStmt *plan, CachedPlan *cplan, + CachedPlanSource *plansource, + int query_index, const char *sourceText, ParamListInfo params, QueryEnvironment *queryEnv, @@ -157,7 +164,7 @@ ProcessQuery(PlannedStmt *plan, /* * Call ExecutorStart to prepare the plan for execution */ - ExecutorStart(queryDesc, 0); + ExecutorStartExt(queryDesc, 0, plansource, query_index); /* * Run the plan to completion. @@ -518,9 +525,9 @@ PortalStart(Portal portal, ParamListInfo params, myeflags = eflags; /* - * Call ExecutorStart to prepare the plan for execution + * Call ExecutorStartExt() to prepare the plan for execution. */ - ExecutorStart(queryDesc, myeflags); + ExecutorStartExt(queryDesc, myeflags, portal->plansource, 0); /* * This tells PortalCleanup to shut down the executor @@ -1201,6 +1208,7 @@ PortalRunMulti(Portal portal, { bool active_snapshot_set = false; ListCell *stmtlist_item; + int i = 0; /* * If the destination is DestRemoteExecute, change to DestNone. The @@ -1283,6 +1291,8 @@ PortalRunMulti(Portal portal, /* statement can set tag string */ ProcessQuery(pstmt, portal->cplan, + portal->plansource, + i, portal->sourceText, portal->portalParams, portal->queryEnv, @@ -1293,6 +1303,8 @@ PortalRunMulti(Portal portal, /* stmt added by rewrite cannot set tag */ ProcessQuery(pstmt, portal->cplan, + portal->plansource, + i, portal->sourceText, portal->portalParams, portal->queryEnv, @@ -1357,6 +1369,8 @@ PortalRunMulti(Portal portal, */ if (lnext(portal->stmts, stmtlist_item) != NULL) CommandCounterIncrement(); + + i++; } /* Pop the snapshot if we pushed one. */ diff --git a/src/backend/utils/cache/plancache.c b/src/backend/utils/cache/plancache.c index 449fb8f4e2..d3e78afd97 100644 --- a/src/backend/utils/cache/plancache.c +++ b/src/backend/utils/cache/plancache.c @@ -101,7 +101,8 @@ static dlist_head cached_expression_list = DLIST_STATIC_INIT(cached_expression_l static void ReleaseGenericPlan(CachedPlanSource *plansource); static List *RevalidateCachedQuery(CachedPlanSource *plansource, - QueryEnvironment *queryEnv); + QueryEnvironment *queryEnv, + bool release_generic); static bool CheckCachedPlan(CachedPlanSource *plansource); static CachedPlan *BuildCachedPlan(CachedPlanSource *plansource, List *qlist, ParamListInfo boundParams, QueryEnvironment *queryEnv, @@ -579,10 +580,17 @@ ReleaseGenericPlan(CachedPlanSource *plansource) * The result value is the transient analyzed-and-rewritten query tree if we * had to do re-analysis, and NIL otherwise. (This is returned just to save * a tree copying step in a subsequent BuildCachedPlan call.) + * + * This also releases and drops the generic plan (plansource->gplan), if any, + * as most callers will typically build a new CachedPlan for the plansource + * right after this. However, when called from UpdateCachedPlan(), the + * function does not release the generic plan, as UpdateCachedPlan() updates + * an existing CachedPlan in place. */ static List * RevalidateCachedQuery(CachedPlanSource *plansource, - QueryEnvironment *queryEnv) + QueryEnvironment *queryEnv, + bool release_generic) { bool snapshot_set; RawStmt *rawtree; @@ -679,8 +687,9 @@ RevalidateCachedQuery(CachedPlanSource *plansource, MemoryContextDelete(qcxt); } - /* Drop the generic plan reference if any */ - ReleaseGenericPlan(plansource); + /* Drop the generic plan reference, if any, and if requested */ + if (release_generic) + ReleaseGenericPlan(plansource); /* * Now re-do parse analysis and rewrite. This not incidentally acquires @@ -905,6 +914,8 @@ CheckCachedPlan(CachedPlanSource *plansource) * Planning work is done in the caller's memory context. The finished plan * is in a child memory context, which typically should get reparented * (unless this is a one-shot plan, in which case we don't copy the plan). + * + * Note: When changing this, you should also look at UpdateCachedPlan(). */ static CachedPlan * BuildCachedPlan(CachedPlanSource *plansource, List *qlist, @@ -933,7 +944,7 @@ BuildCachedPlan(CachedPlanSource *plansource, List *qlist, * let's treat it as real and redo the RevalidateCachedQuery call. */ if (!plansource->is_valid) - qlist = RevalidateCachedQuery(plansource, queryEnv); + qlist = RevalidateCachedQuery(plansource, queryEnv, true); /* * If we don't already have a copy of the querytree list that can be @@ -1188,7 +1199,7 @@ GetCachedPlan(CachedPlanSource *plansource, ParamListInfo boundParams, elog(ERROR, "cannot apply ResourceOwner to non-saved cached plan"); /* Make sure the querytree list is valid and we have parse-time locks */ - qlist = RevalidateCachedQuery(plansource, queryEnv); + qlist = RevalidateCachedQuery(plansource, queryEnv, true); /* Decide whether to use a custom plan */ customplan = choose_custom_plan(plansource, boundParams); @@ -1284,6 +1295,106 @@ GetCachedPlan(CachedPlanSource *plansource, ParamListInfo boundParams, return plan; } +/* + * UpdateCachedPlan + * Create fresh plans for all the queries in the plansource, replacing + * those in the generic plan's stmt_list, and return the plan for the + * query_index'th query. + * + * This function is primarily intended for ExecutorStartExt(), which handles + * cases where the original generic CachedPlan becomes invalid when prunable + * relations in the old plan for the query_index'th query are locked for + * execution. + * + * Note that even though this function is called due to invalidations received + * during the execution of the query_index'th query, they might affect both + * queries that have already finished execution (e.g., due to concurrent + * modifications on prunable relations that were not locked during their + * execution) and those that have not yet executed. Therefore, we must update + * all plans to safely set CachedPlan.is_valid to true. + */ + +PlannedStmt * +UpdateCachedPlan(CachedPlanSource *plansource, int query_index, + QueryEnvironment *queryEnv) +{ + List *query_list = plansource->query_list, + *plan_list; + ListCell *l1, + *l2; + CachedPlan *plan = plansource->gplan; + MemoryContext oldcxt; + + Assert(ActiveSnapshotSet()); + + /* Sanity checks */ + if (plan == NULL) + elog(ERROR, "UpdateCachedPlan() called in the wrong context: plansource->gplan is NULL"); + else if (plan->is_valid) + elog(ERROR, "UpdateCachedPlan() called in the wrong context: plansource->gplan->is_valid is true"); + + /* + * The plansource might have become invalid since GetCachedPlan(). See the + * comment in BuildCachedPlan() for details on why this might happen. + * + * The risk of invalidation is higher here than when BuildCachedPlan() + * is called from GetCachedPlan(), because this function is called + * within the executor, where much more processing could have occurred + * since GetCachedPlan() initially returned the CachedPlan. + * + * Although invalidation is likely a false positive, we make the + * plan valid to ensure the query list used for planning is up to date. + * + * However, plansource->gplan must not be released, as the upstream + * callers (such as the callers of ExecutorStartExt()) still reference it. + * The freshly created plans will replace any potentially invalid ones in + * plansource->gplan->stmt_list. + */ + if (!plansource->is_valid) + query_list = RevalidateCachedQuery(plansource, queryEnv, false); + Assert(query_list != NIL); + + /* + * Build a new generic plan for all the queries after make a copy + * to be scribbled on by the planner. + */ + query_list = copyObject(query_list); + + /* + * Planning work is done in the caller's memory context. The resulting + * PlannedStmt is then copied into plan->context. + */ + plan_list = pg_plan_queries(query_list, plansource->query_string, + plansource->cursor_options, NULL); + Assert(list_length(plan_list) == list_length(plan->stmt_list)); + + oldcxt = MemoryContextSwitchTo(plan->context); + forboth (l1, plan_list, l2, plan->stmt_list) + { + PlannedStmt *plannedstmt = lfirst(l1); + + lfirst(l2) = copyObject(plannedstmt); + } + MemoryContextSwitchTo(oldcxt); + + /* + * XXX Should this also (re)set the properties of the CachedPlan that are + * set in BuildCachedPlan() after creating the fresh plans such as + * planRoleId, dependsOnRole, and save_xmin? + */ + + /* + * We've updated all the plans that might have been invalidated, so mark + * the CachedPlan as valid. + */ + plan->is_valid = true; + + /* Also update generic_cost because we just created a new generic plan. */ + plansource->generic_cost = cached_plan_cost(plan, false); + + return list_nth_node(PlannedStmt, plan->stmt_list, query_index); +} + /* * ReleaseCachedPlan: release active use of a cached plan. * @@ -1662,7 +1773,7 @@ CachedPlanGetTargetList(CachedPlanSource *plansource, return NIL; /* Make sure the querytree list is valid and we have parse-time locks */ - RevalidateCachedQuery(plansource, queryEnv); + RevalidateCachedQuery(plansource, queryEnv, true); /* Get the primary statement and find out what it returns */ pstmt = QueryListGetPrimaryStmt(plansource->query_list); diff --git a/src/backend/utils/mmgr/portalmem.c b/src/backend/utils/mmgr/portalmem.c index 93137820ac..ef4791bf65 100644 --- a/src/backend/utils/mmgr/portalmem.c +++ b/src/backend/utils/mmgr/portalmem.c @@ -284,7 +284,8 @@ PortalDefineQuery(Portal portal, const char *sourceText, CommandTag commandTag, List *stmts, - CachedPlan *cplan) + CachedPlan *cplan, + CachedPlanSource *plansource) { Assert(PortalIsValid(portal)); Assert(portal->status == PORTAL_NEW); @@ -299,6 +300,7 @@ PortalDefineQuery(Portal portal, portal->commandTag = commandTag; portal->stmts = stmts; portal->cplan = cplan; + portal->plansource = plansource; portal->status = PORTAL_DEFINED; } diff --git a/src/include/commands/explain.h b/src/include/commands/explain.h index 21c71e0d53..a39989a950 100644 --- a/src/include/commands/explain.h +++ b/src/include/commands/explain.h @@ -104,6 +104,7 @@ extern void ExplainOneUtility(Node *utilityStmt, IntoClause *into, ParamListInfo params, QueryEnvironment *queryEnv); extern void ExplainOnePlan(PlannedStmt *plannedstmt, CachedPlan *cplan, + CachedPlanSource *plansource, int plan_index, IntoClause *into, ExplainState *es, const char *queryString, ParamListInfo params, QueryEnvironment *queryEnv, diff --git a/src/include/commands/trigger.h b/src/include/commands/trigger.h index 8a5a9fe642..db21561c8c 100644 --- a/src/include/commands/trigger.h +++ b/src/include/commands/trigger.h @@ -258,6 +258,7 @@ extern void ExecASTruncateTriggers(EState *estate, extern void AfterTriggerBeginXact(void); extern void AfterTriggerBeginQuery(void); extern void AfterTriggerEndQuery(EState *estate); +extern void AfterTriggerAbortQuery(void); extern void AfterTriggerFireDeferred(void); extern void AfterTriggerEndXact(bool isCommit); extern void AfterTriggerBeginSubXact(void); diff --git a/src/include/executor/executor.h b/src/include/executor/executor.h index 69c3ebff00..1270af3be5 100644 --- a/src/include/executor/executor.h +++ b/src/include/executor/executor.h @@ -19,6 +19,7 @@ #include "nodes/lockoptions.h" #include "nodes/parsenodes.h" #include "utils/memutils.h" +#include "utils/plancache.h" /* @@ -198,6 +199,8 @@ ExecGetJunkAttribute(TupleTableSlot *slot, AttrNumber attno, bool *isNull) * prototypes from functions in execMain.c */ extern void ExecutorStart(QueryDesc *queryDesc, int eflags); +extern void ExecutorStartExt(QueryDesc *queryDesc, int eflags, + CachedPlanSource *plansource, int query_index); extern void standard_ExecutorStart(QueryDesc *queryDesc, int eflags); extern void ExecutorRun(QueryDesc *queryDesc, ScanDirection direction, uint64 count, bool execute_once); @@ -261,6 +264,19 @@ extern void ExecEndNode(PlanState *node); extern void ExecShutdownNode(PlanState *node); extern void ExecSetTupleBound(int64 tuples_needed, PlanState *child_node); +/* + * Is the CachedPlan in es_cachedplan still valid? + * + * Called from InitPlan() because invalidation messages that affect the plan + * might be received after locks have been taken on runtime-prunable relations. + * The caller should take appropriate action if the plan has become invalid. + */ +static inline bool +ExecPlanStillValid(EState *estate) +{ + return estate->es_cachedplan == NULL ? true : + CachedPlanValid(estate->es_cachedplan); +} /* ---------------------------------------------------------------- * ExecProcNode diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h index ac9be82e19..1ec1021808 100644 --- a/src/include/nodes/execnodes.h +++ b/src/include/nodes/execnodes.h @@ -693,6 +693,7 @@ typedef struct EState int es_top_eflags; /* eflags passed to ExecutorStart */ int es_instrument; /* OR of InstrumentOption flags */ bool es_finished; /* true when ExecutorFinish is done */ + bool es_aborted; /* true when execution was aborted */ List *es_exprcontexts; /* List of ExprContexts within EState */ diff --git a/src/include/utils/plancache.h b/src/include/utils/plancache.h index e227c4f11b..7b2f3ced26 100644 --- a/src/include/utils/plancache.h +++ b/src/include/utils/plancache.h @@ -18,6 +18,8 @@ #include "access/tupdesc.h" #include "lib/ilist.h" #include "nodes/params.h" +#include "nodes/parsenodes.h" +#include "nodes/plannodes.h" #include "tcop/cmdtag.h" #include "utils/queryenvironment.h" #include "utils/resowner.h" @@ -159,6 +161,12 @@ typedef struct CachedPlan int generation; /* parent's generation number for this plan */ int refcount; /* count of live references to this struct */ MemoryContext context; /* context containing this CachedPlan */ + + /* + * If the plan is not associated with a CachedPlanSource, it is saved in + * a separate global list. + */ + dlist_node node; /* list link, if is_standalone */ } CachedPlan; /* @@ -224,6 +232,10 @@ extern CachedPlan *GetCachedPlan(CachedPlanSource *plansource, ParamListInfo boundParams, ResourceOwner owner, QueryEnvironment *queryEnv); +extern PlannedStmt *UpdateCachedPlan(CachedPlanSource *plansource, + int query_index, + QueryEnvironment *queryEnv); + extern void ReleaseCachedPlan(CachedPlan *plan, ResourceOwner owner); extern bool CachedPlanAllowsSimpleValidityCheck(CachedPlanSource *plansource, @@ -253,4 +265,17 @@ CachedPlanRequiresLocking(CachedPlan *cplan) return !cplan->is_oneshot && cplan->is_generic; } +/* + * CachedPlanValid + * Returns whether a cached generic plan is still valid. + * + * Invoked by the executor to check if the plan has not been invalidated after + * taking locks during the initialization of the plan. + */ +static inline bool +CachedPlanValid(CachedPlan *cplan) +{ + return cplan->is_valid; +} + #endif /* PLANCACHE_H */ diff --git a/src/include/utils/portal.h b/src/include/utils/portal.h index 29f49829f2..58c3828d2c 100644 --- a/src/include/utils/portal.h +++ b/src/include/utils/portal.h @@ -138,6 +138,7 @@ typedef struct PortalData QueryCompletion qc; /* command completion data for executed query */ List *stmts; /* list of PlannedStmts */ CachedPlan *cplan; /* CachedPlan, if stmts are from one */ + CachedPlanSource *plansource; /* CachedPlanSource, for cplan */ ParamListInfo portalParams; /* params to pass to query */ QueryEnvironment *queryEnv; /* environment for query */ @@ -241,7 +242,8 @@ extern void PortalDefineQuery(Portal portal, const char *sourceText, CommandTag commandTag, List *stmts, - CachedPlan *cplan); + CachedPlan *cplan, + CachedPlanSource *plansource); extern PlannedStmt *PortalGetPrimaryStmt(Portal portal); extern void PortalCreateHoldStore(Portal portal); extern void PortalHashTableDeleteAll(void); diff --git a/src/test/modules/delay_execution/Makefile b/src/test/modules/delay_execution/Makefile index 70f24e846d..3eeb097fde 100644 --- a/src/test/modules/delay_execution/Makefile +++ b/src/test/modules/delay_execution/Makefile @@ -8,7 +8,8 @@ OBJS = \ delay_execution.o ISOLATION = partition-addition \ - partition-removal-1 + partition-removal-1 \ + cached-plan-inval ifdef USE_PGXS PG_CONFIG = pg_config diff --git a/src/test/modules/delay_execution/delay_execution.c b/src/test/modules/delay_execution/delay_execution.c index 155c8a8d55..304ca77f7b 100644 --- a/src/test/modules/delay_execution/delay_execution.c +++ b/src/test/modules/delay_execution/delay_execution.c @@ -1,14 +1,18 @@ /*------------------------------------------------------------------------- * * delay_execution.c - * Test module to allow delay between parsing and execution of a query. + * Test module to introduce delay at various points during execution of a + * query to test that execution proceeds safely in light of concurrent + * changes. * * The delay is implemented by taking and immediately releasing a specified * advisory lock. If another process has previously taken that lock, the * current process will be blocked until the lock is released; otherwise, * there's no effect. This allows an isolationtester script to reliably - * test behaviors where some specified action happens in another backend - * between parsing and execution of any desired query. + * test behaviors where some specified action happens in another backend in + * a couple of cases: 1) between parsing and execution of any desired query + * when using the planner_hook, 2) between RevalidateCachedQuery() and + * ExecutorStart() when using the ExecutorStart_hook. * * Copyright (c) 2020-2024, PostgreSQL Global Development Group * @@ -22,6 +26,7 @@ #include +#include "executor/executor.h" #include "optimizer/planner.h" #include "utils/builtins.h" #include "utils/guc.h" @@ -32,9 +37,11 @@ PG_MODULE_MAGIC; /* GUC: advisory lock ID to use. Zero disables the feature. */ static int post_planning_lock_id = 0; +static int executor_start_lock_id = 0; -/* Save previous planner hook user to be a good citizen */ +/* Save previous hook users to be a good citizen */ static planner_hook_type prev_planner_hook = NULL; +static ExecutorStart_hook_type prev_ExecutorStart_hook = NULL; /* planner_hook function to provide the desired delay */ @@ -70,11 +77,41 @@ delay_execution_planner(Query *parse, const char *query_string, return result; } +/* ExecutorStart_hook function to provide the desired delay */ +static void +delay_execution_ExecutorStart(QueryDesc *queryDesc, int eflags) +{ + /* If enabled, delay by taking and releasing the specified lock */ + if (executor_start_lock_id != 0) + { + DirectFunctionCall1(pg_advisory_lock_int8, + Int64GetDatum((int64) executor_start_lock_id)); + DirectFunctionCall1(pg_advisory_unlock_int8, + Int64GetDatum((int64) executor_start_lock_id)); + + /* + * Ensure that we notice any pending invalidations, since the advisory + * lock functions don't do this. + */ + AcceptInvalidationMessages(); + } + + /* Now start the executor, possibly via a previous hook user */ + if (prev_ExecutorStart_hook) + prev_ExecutorStart_hook(queryDesc, eflags); + else + standard_ExecutorStart(queryDesc, eflags); + + if (executor_start_lock_id != 0) + elog(NOTICE, "Finished ExecutorStart(): CachedPlan is %s", + CachedPlanValid(queryDesc->cplan) ? "valid" : "not valid"); +} + /* Module load function */ void _PG_init(void) { - /* Set up the GUC to control which lock is used */ + /* Set up GUCs to control which lock is used */ DefineCustomIntVariable("delay_execution.post_planning_lock_id", "Sets the advisory lock ID to be locked/unlocked after planning.", "Zero disables the delay.", @@ -86,10 +123,22 @@ _PG_init(void) NULL, NULL, NULL); - + DefineCustomIntVariable("delay_execution.executor_start_lock_id", + "Sets the advisory lock ID to be locked/unlocked before starting execution.", + "Zero disables the delay.", + &executor_start_lock_id, + 0, + 0, INT_MAX, + PGC_USERSET, + 0, + NULL, + NULL, + NULL); MarkGUCPrefixReserved("delay_execution"); - /* Install our hook */ + /* Install our hooks. */ prev_planner_hook = planner_hook; planner_hook = delay_execution_planner; + prev_ExecutorStart_hook = ExecutorStart_hook; + ExecutorStart_hook = delay_execution_ExecutorStart; } diff --git a/src/test/modules/delay_execution/expected/cached-plan-inval.out b/src/test/modules/delay_execution/expected/cached-plan-inval.out new file mode 100644 index 0000000000..5bfb2b33b3 --- /dev/null +++ b/src/test/modules/delay_execution/expected/cached-plan-inval.out @@ -0,0 +1,282 @@ +Parsed test spec with 2 sessions + +starting permutation: s1prep s2lock s1exec s2dropi s2unlock +step s1prep: SET plan_cache_mode = force_generic_plan; + PREPARE q AS SELECT * FROM foov WHERE a = $1 FOR UPDATE; + EXPLAIN (COSTS OFF) EXECUTE q (1); +QUERY PLAN +------------------------------------------------ +LockRows + -> Append + Subplans Removed: 2 + -> Bitmap Heap Scan on foo12_1 foo_1 + Recheck Cond: (a = $1) + -> Bitmap Index Scan on foo12_1_a + Index Cond: (a = $1) +(7 rows) + +step s2lock: SELECT pg_advisory_lock(12345); +pg_advisory_lock +---------------- + +(1 row) + +step s1exec: LOAD 'delay_execution'; + SET delay_execution.executor_start_lock_id = 12345; + EXPLAIN (COSTS OFF) EXECUTE q (1); +step s2dropi: DROP INDEX foo12_1_a; +step s2unlock: SELECT pg_advisory_unlock(12345); +pg_advisory_unlock +------------------ +t +(1 row) + +step s1exec: <... completed> +s1: NOTICE: Finished ExecutorStart(): CachedPlan is not valid +s1: NOTICE: Finished ExecutorStart(): CachedPlan is valid +QUERY PLAN +------------------------------------- +LockRows + -> Append + Subplans Removed: 2 + -> Seq Scan on foo12_1 foo_1 + Filter: (a = $1) +(5 rows) + + +starting permutation: s1prep2 s2lock s1exec2 s2dropi s2unlock +step s1prep2: SET plan_cache_mode = force_generic_plan; + PREPARE q2 AS SELECT * FROM foov WHERE a = one() or a = two(); + EXPLAIN (COSTS OFF) EXECUTE q2; +s1: NOTICE: Finished ExecutorStart(): CachedPlan is valid +QUERY PLAN +-------------------------------------------------- +Append + Subplans Removed: 1 + -> Bitmap Heap Scan on foo12_1 foo_1 + Recheck Cond: ((a = one()) OR (a = two())) + -> BitmapOr + -> Bitmap Index Scan on foo12_1_a + Index Cond: (a = one()) + -> Bitmap Index Scan on foo12_1_a + Index Cond: (a = two()) + -> Seq Scan on foo12_2 foo_2 + Filter: ((a = one()) OR (a = two())) +(11 rows) + +step s2lock: SELECT pg_advisory_lock(12345); +pg_advisory_lock +---------------- + +(1 row) + +step s1exec2: LOAD 'delay_execution'; + SET delay_execution.executor_start_lock_id = 12345; + EXPLAIN (COSTS OFF) EXECUTE q2; +step s2dropi: DROP INDEX foo12_1_a; +step s2unlock: SELECT pg_advisory_unlock(12345); +pg_advisory_unlock +------------------ +t +(1 row) + +step s1exec2: <... completed> +s1: NOTICE: Finished ExecutorStart(): CachedPlan is not valid +s1: NOTICE: Finished ExecutorStart(): CachedPlan is valid +QUERY PLAN +-------------------------------------------- +Append + Subplans Removed: 1 + -> Seq Scan on foo12_1 foo_1 + Filter: ((a = one()) OR (a = two())) + -> Seq Scan on foo12_2 foo_2 + Filter: ((a = one()) OR (a = two())) +(6 rows) + + +starting permutation: s1prep3 s2lock s1exec3 s2dropi s2unlock +step s1prep3: SET plan_cache_mode = force_generic_plan; + PREPARE q3 AS UPDATE foov SET a = a WHERE a = one() or a = two(); + EXPLAIN (COSTS OFF) EXECUTE q3; +s1: NOTICE: Finished ExecutorStart(): CachedPlan is valid +s1: NOTICE: Finished ExecutorStart(): CachedPlan is valid +s1: NOTICE: Finished ExecutorStart(): CachedPlan is valid +QUERY PLAN +-------------------------------------------------------------- +Nested Loop + -> Append + Subplans Removed: 1 + -> Bitmap Heap Scan on foo12_1 foo_1 + Recheck Cond: ((a = one()) OR (a = two())) + -> BitmapOr + -> Bitmap Index Scan on foo12_1_a + Index Cond: (a = one()) + -> Bitmap Index Scan on foo12_1_a + Index Cond: (a = two()) + -> Seq Scan on foo12_2 foo_2 + Filter: ((a = one()) OR (a = two())) + -> Materialize + -> Append + Subplans Removed: 1 + -> Bitmap Heap Scan on bar1 bar_1 + Recheck Cond: (a = one()) + -> Bitmap Index Scan on bar1_a_idx + Index Cond: (a = one()) + +Update on bar + Update on bar1 bar_1 + -> Nested Loop + -> Append + Subplans Removed: 1 + -> Bitmap Heap Scan on foo12_1 foo_1 + Recheck Cond: ((a = one()) OR (a = two())) + -> BitmapOr + -> Bitmap Index Scan on foo12_1_a + Index Cond: (a = one()) + -> Bitmap Index Scan on foo12_1_a + Index Cond: (a = two()) + -> Seq Scan on foo12_2 foo_2 + Filter: ((a = one()) OR (a = two())) + -> Materialize + -> Append + Subplans Removed: 1 + -> Bitmap Heap Scan on bar1 bar_1 + Recheck Cond: (a = one()) + -> Bitmap Index Scan on bar1_a_idx + Index Cond: (a = one()) + +Update on foo + Update on foo12_1 foo_1 + Update on foo12_2 foo_2 + -> Append + Subplans Removed: 1 + -> Bitmap Heap Scan on foo12_1 foo_1 + Recheck Cond: ((a = one()) OR (a = two())) + -> BitmapOr + -> Bitmap Index Scan on foo12_1_a + Index Cond: (a = one()) + -> Bitmap Index Scan on foo12_1_a + Index Cond: (a = two()) + -> Seq Scan on foo12_2 foo_2 + Filter: ((a = one()) OR (a = two())) +(56 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; +step s2dropi: DROP INDEX foo12_1_a; +step s2unlock: SELECT pg_advisory_unlock(12345); +pg_advisory_unlock +------------------ +t +(1 row) + +step s1exec3: <... completed> +s1: NOTICE: Finished ExecutorStart(): CachedPlan is not valid +s1: NOTICE: Finished ExecutorStart(): CachedPlan is valid +s1: NOTICE: Finished ExecutorStart(): CachedPlan is valid +s1: NOTICE: Finished ExecutorStart(): CachedPlan is valid +QUERY PLAN +------------------------------------------------------------- +Nested Loop + -> Append + Subplans Removed: 1 + -> Seq Scan on foo12_1 foo_1 + Filter: ((a = one()) OR (a = two())) + -> Seq Scan on foo12_2 foo_2 + Filter: ((a = one()) OR (a = two())) + -> Materialize + -> Append + Subplans Removed: 1 + -> Bitmap Heap Scan on bar1 bar_1 + Recheck Cond: (a = one()) + -> Bitmap Index Scan on bar1_a_idx + Index Cond: (a = one()) + +Update on bar + Update on bar1 bar_1 + -> Nested Loop + -> Append + Subplans Removed: 1 + -> Seq Scan on foo12_1 foo_1 + Filter: ((a = one()) OR (a = two())) + -> Seq Scan on foo12_2 foo_2 + Filter: ((a = one()) OR (a = two())) + -> Materialize + -> Append + Subplans Removed: 1 + -> Bitmap Heap Scan on bar1 bar_1 + Recheck Cond: (a = one()) + -> Bitmap Index Scan on bar1_a_idx + Index Cond: (a = one()) + +Update on foo + Update on foo12_1 foo_1 + Update on foo12_2 foo_2 + -> Append + Subplans Removed: 1 + -> Seq Scan on foo12_1 foo_1 + Filter: ((a = one()) OR (a = two())) + -> Seq Scan on foo12_2 foo_2 + Filter: ((a = one()) OR (a = two())) +(41 rows) + + +starting permutation: s1prep4 s2lock s1exec4 s2dropi s2unlock +step s1prep4: SET plan_cache_mode = force_generic_plan; + SET enable_seqscan TO off; + PREPARE q4 AS SELECT * FROM generate_series(1, 1) WHERE EXISTS (SELECT * FROM foov WHERE a = $1 FOR UPDATE); + EXPLAIN (COSTS OFF) EXECUTE q4 (1); +s1: NOTICE: Finished ExecutorStart(): CachedPlan is valid +QUERY PLAN +--------------------------------------------------------------- +Result + One-Time Filter: (InitPlan 1).col1 + InitPlan 1 + -> LockRows + -> Append + Subplans Removed: 2 + -> Index Scan using foo12_1_a on foo12_1 foo_1 + Index Cond: (a = $1) + -> Function Scan on generate_series +(9 rows) + +step s2lock: SELECT pg_advisory_lock(12345); +pg_advisory_lock +---------------- + +(1 row) + +step s1exec4: LOAD 'delay_execution'; + SET delay_execution.executor_start_lock_id = 12345; + EXPLAIN (COSTS OFF) EXECUTE q4 (1); +step s2dropi: DROP INDEX foo12_1_a; +step s2unlock: SELECT pg_advisory_unlock(12345); +pg_advisory_unlock +------------------ +t +(1 row) + +step s1exec4: <... completed> +s1: NOTICE: Finished ExecutorStart(): CachedPlan is not valid +s1: NOTICE: Finished ExecutorStart(): CachedPlan is valid +QUERY PLAN +--------------------------------------------- +Result + One-Time Filter: (InitPlan 1).col1 + InitPlan 1 + -> LockRows + -> Append + Subplans Removed: 2 + -> Seq Scan on foo12_1 foo_1 + Disabled: true + Filter: (a = $1) + -> Function Scan on generate_series +(10 rows) + diff --git a/src/test/modules/delay_execution/meson.build b/src/test/modules/delay_execution/meson.build index 41f3ac0b89..5a70b183d0 100644 --- a/src/test/modules/delay_execution/meson.build +++ b/src/test/modules/delay_execution/meson.build @@ -24,6 +24,7 @@ tests += { 'specs': [ 'partition-addition', 'partition-removal-1', + 'cached-plan-inval', ], }, } diff --git a/src/test/modules/delay_execution/specs/cached-plan-inval.spec b/src/test/modules/delay_execution/specs/cached-plan-inval.spec new file mode 100644 index 0000000000..f27e8fb521 --- /dev/null +++ b/src/test/modules/delay_execution/specs/cached-plan-inval.spec @@ -0,0 +1,80 @@ +# Test to check that invalidation of cached generic plans during ExecutorStart +# correctly triggers replanning and re-execution. + +setup +{ + CREATE TABLE foo (a int, b text) PARTITION BY LIST(a); + CREATE TABLE foo12 PARTITION OF foo FOR VALUES IN (1, 2) PARTITION BY LIST (a); + CREATE TABLE foo12_1 PARTITION OF foo12 FOR VALUES IN (1); + CREATE TABLE foo12_2 PARTITION OF foo12 FOR VALUES IN (2); + CREATE INDEX foo12_1_a ON foo12_1 (a); + CREATE TABLE foo3 PARTITION OF foo FOR VALUES IN (3); + CREATE VIEW foov AS SELECT * FROM foo; + CREATE FUNCTION one () RETURNS int AS $$ BEGIN RETURN 1; END; $$ LANGUAGE PLPGSQL STABLE; + CREATE FUNCTION two () RETURNS int AS $$ BEGIN RETURN 2; END; $$ LANGUAGE PLPGSQL STABLE; + CREATE TABLE bar (a int, b text) PARTITION BY LIST(a); + CREATE TABLE bar1 PARTITION OF bar FOR VALUES IN (1); + CREATE INDEX ON bar1(a); + CREATE TABLE bar2 PARTITION OF bar FOR VALUES IN (2); + CREATE RULE update_foo AS ON UPDATE TO foo DO ALSO UPDATE bar SET a = a WHERE a = one(); + CREATE RULE update_bar AS ON UPDATE TO bar DO ALSO SELECT 1; +} + +teardown +{ + DROP VIEW foov; + DROP RULE update_foo ON foo; + DROP TABLE foo, bar; + DROP FUNCTION one(), two(); +} + +session "s1" +# Append with run-time pruning +step "s1prep" { SET plan_cache_mode = force_generic_plan; + PREPARE q AS SELECT * FROM foov WHERE a = $1 FOR UPDATE; + EXPLAIN (COSTS OFF) EXECUTE q (1); } + +# Another case with Append with run-time pruning +step "s1prep2" { SET plan_cache_mode = force_generic_plan; + PREPARE q2 AS SELECT * FROM foov WHERE a = one() or a = two(); + EXPLAIN (COSTS OFF) EXECUTE q2; } + +# Case with a rule adding another query +step "s1prep3" { SET plan_cache_mode = force_generic_plan; + PREPARE q3 AS UPDATE foov SET a = a WHERE a = one() or a = two(); + EXPLAIN (COSTS OFF) EXECUTE q3; } + +# Another case with Append with run-time pruning in a subquery +step "s1prep4" { SET plan_cache_mode = force_generic_plan; + SET enable_seqscan TO off; + PREPARE q4 AS SELECT * FROM generate_series(1, 1) WHERE EXISTS (SELECT * FROM foov WHERE a = $1 FOR UPDATE); + EXPLAIN (COSTS OFF) EXECUTE q4 (1); } + +# 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; } +step "s1exec4" { LOAD 'delay_execution'; + SET delay_execution.executor_start_lock_id = 12345; + EXPLAIN (COSTS OFF) EXECUTE q4 (1); } + +session "s2" +step "s2lock" { SELECT pg_advisory_lock(12345); } +step "s2unlock" { SELECT pg_advisory_unlock(12345); } +step "s2dropi" { DROP INDEX foo12_1_a; } + +# While "s1exec", etc. wait to acquire the advisory lock, "s2drop" is able to +# drop the index being used in the cached plan. When "s1exec" is then +# unblocked and initializes the cached plan for execution, it detects the +# concurrent index drop and causes the cached plan to be discarded and +# recreated without the index. +permutation "s1prep" "s2lock" "s1exec" "s2dropi" "s2unlock" +permutation "s1prep2" "s2lock" "s1exec2" "s2dropi" "s2unlock" +permutation "s1prep3" "s2lock" "s1exec3" "s2dropi" "s2unlock" +permutation "s1prep4" "s2lock" "s1exec4" "s2dropi" "s2unlock" -- 2.43.0