diff options
author | Amit Langote <amitlan@postgresql.org> | 2025-05-22 14:17:24 +0900 |
---|---|---|
committer | Amit Langote <amitlan@postgresql.org> | 2025-05-22 17:02:35 +0900 |
commit | 1722d5eb05d8e5d2e064cd1798abcae4f296ca9d (patch) | |
tree | 6661dfcd476b8e355f4f05d38badbe1c6de2ed36 /src/include/utils | |
parent | f3622b64762bb5ee5242937f0fadcacb1a10f30e (diff) | |
download | postgresql-1722d5eb05d8e5d2e064cd1798abcae4f296ca9d.tar.gz postgresql-1722d5eb05d8e5d2e064cd1798abcae4f296ca9d.zip |
Revert "Don't lock partitions pruned by initial pruning"
As pointed out by Tom Lane, the patch introduced fragile and invasive
design around plan invalidation handling when locking of prunable
partitions was deferred from plancache.c to the executor. In
particular, it violated assumptions about CachedPlan immutability and
altered executor APIs in ways that are difficult to justify given the
added complexity and overhead.
This also removes the firstResultRels field added to PlannedStmt in
commit 28317de72, which was intended to support deferred locking of
certain ModifyTable result relations.
Reported-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/605328.1747710381@sss.pgh.pa.us
Diffstat (limited to 'src/include/utils')
-rw-r--r-- | src/include/utils/plancache.h | 46 | ||||
-rw-r--r-- | src/include/utils/portal.h | 4 |
2 files changed, 5 insertions, 45 deletions
diff --git a/src/include/utils/plancache.h b/src/include/utils/plancache.h index 07ec5318db7..1baa6d50bfd 100644 --- a/src/include/utils/plancache.h +++ b/src/include/utils/plancache.h @@ -18,8 +18,6 @@ #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" @@ -153,11 +151,10 @@ typedef struct CachedPlanSource * The reference count includes both the link from the parent CachedPlanSource * (if any), and any active plan executions, so the plan can be discarded * exactly when refcount goes to zero. Both the struct itself and the - * subsidiary data, except the PlannedStmts in stmt_list live in the context - * denoted by the context field; the PlannedStmts live in the context denoted - * by stmt_context. Separate contexts makes it easy to free a no-longer-needed - * cached plan. (However, if is_oneshot is true, the context does not belong - * solely to the CachedPlan so no freeing is possible.) + * subsidiary data live in the context denoted by the context field. + * This makes it easy to free a no-longer-needed cached plan. (However, + * if is_oneshot is true, the context does not belong solely to the CachedPlan + * so no freeing is possible.) */ typedef struct CachedPlan { @@ -165,7 +162,6 @@ typedef struct CachedPlan List *stmt_list; /* list of PlannedStmts */ bool is_oneshot; /* is it a "oneshot" plan? */ bool is_saved; /* is CachedPlan in a long-lived context? */ - bool is_reused; /* is it a reused generic plan? */ bool is_valid; /* is the stmt_list currently valid? */ Oid planRoleId; /* Role ID the plan was created for */ bool dependsOnRole; /* is plan specific to that role? */ @@ -174,10 +170,6 @@ 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 */ - MemoryContext stmt_context; /* context containing the PlannedStmts in - * stmt_list, but not the List itself which is - * in the above context; NULL if is_oneshot is - * true. */ } CachedPlan; /* @@ -249,10 +241,6 @@ 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, @@ -265,30 +253,4 @@ extern bool CachedPlanIsSimplyValid(CachedPlanSource *plansource, extern CachedExpression *GetCachedExpression(Node *expr); extern void FreeCachedExpression(CachedExpression *cexpr); -/* - * CachedPlanRequiresLocking: should the executor acquire additional locks? - * - * If the plan is a saved generic plan, the executor must acquire locks for - * relations that are not covered by AcquireExecutorLocks(), such as partitions - * that are subject to initial runtime pruning. - */ -static inline bool -CachedPlanRequiresLocking(CachedPlan *cplan) -{ - return !cplan->is_oneshot && cplan->is_reused; -} - -/* - * 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 ddee031f551..0b62143af8b 100644 --- a/src/include/utils/portal.h +++ b/src/include/utils/portal.h @@ -138,7 +138,6 @@ 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,8 +240,7 @@ extern void PortalDefineQuery(Portal portal, const char *sourceText, CommandTag commandTag, List *stmts, - CachedPlan *cplan, - CachedPlanSource *plansource); + CachedPlan *cplan); extern PlannedStmt *PortalGetPrimaryStmt(Portal portal); extern void PortalCreateHoldStore(Portal portal); extern void PortalHashTableDeleteAll(void); |