Re: generic plans and "initial" pruning

On Sat, Mar 7, 2026 at 6:54 PM Amit Langote <amitlangote09(at)gmail(dot)com> wrote:
> Attached is v6 of the patch series. I've been working toward
> committing this, so I wanted to lay out the ExecutorPrep() design and
> the key trade-offs before doing so.
>
> When a cached generic plan references a partitioned table,
> GetCachedPlan() locks all partitions upfront via
> AcquireExecutorLocks(), even those that initial pruning will
> eliminate. But initial partition pruning only runs later during
> ExecutorStart(). Moving pruning earlier requires some executor setup
> (range table, permissions, pruning state), and ExecutorPrep() is the
> vehicle for that. Unlike the approach reverted in last May, this
> keeps the CachedPlan itself unchanged -- all per-execution state flows
> through a separate CachedPlanPrepData that the caller provides.
>
> The approach also keeps GetCachedPlan()'s interface
> backward-compatible: the new CachedPlanPrepData argument is optional.
> If a caller passes NULL, all partitions are locked as before and
> nothing changes. This means existing callers and any new code that
> calls GetCachedPlan() without caring about pruning-aware locking just
> works.
>
> The risk is on the other side: if a caller does pass a
> CachedPlanPrepData, GetCachedPlan() will lock only the surviving
> partitions and populate prep_estates with the EStates that
> ExecutorPrep() created. The caller then must make those EStates
> available to ExecutorStart() -- via QueryDesc->estate,
> portal->prep_estates, or the equivalent path for SPI and SQL
> functions. If it fails to do so, ExecutorStart() will call
> ExecutorPrep() again, which may compute different pruning results than
> the original call, potentially expecting locks on relations that were
> never acquired. The executor would then operate on relations it
> doesn't hold locks on.
>
> So the contract is: if you opt in to pruning-aware locking by passing
> CachedPlanPrepData, you must complete the pipeline by delivering the
> prep EStates to the executor. In the current patch, all the call sites
> that pass a CachedPlanPrepData (portals, SPI, EXECUTE, SQL functions,
> EXPLAIN) do thread the EStates through correctly, and I've tried to
> make the plumbing straightforward enough that it's hard to get wrong.
> But it is a new invariant that didn't exist before, and a caller that
> gets it wrong would fail silently rather than with an obvious error.
>
> To catch such violations, I've added a debug-only check in
> standard_ExecutorStart() that fires when no prep EState was provided.
> It iterates over the plan's rtable and verifies that every lockable
> relation is actually locked. It should always be true if
> AcquireExecutorLocks() locked everything, but would fail if
> pruning-aware locking happened upstream and the caller dropped the
> prep EState. The check is skipped in parallel workers, which acquire
> relation locks lazily in ExecGetRangeTableRelation().
>
> + if (queryDesc->estate == NULL)
> + {
> +#ifdef USE_ASSERT_CHECKING
> + if (!IsParallelWorker())
> + {
> + ListCell *lc;
> +
> + foreach(lc, queryDesc->plannedstmt->rtable)
> + {
> + RangeTblEntry *rte = lfirst_node(RangeTblEntry, lc);
> +
> + if (rte->rtekind == RTE_RELATION ||
> + (rte->rtekind == RTE_SUBQUERY && rte->relid != InvalidOid))
> + Assert(CheckRelationOidLockedByMe(rte->relid,
> + rte->rellockmode,
> + true));
> + }
> + }
> +#endif
> + queryDesc->estate = ExecutorPrep(queryDesc->plannedstmt,
> + queryDesc->params,
> + CurrentResourceOwner,
> + true,
> + eflags);
> + }
> +#ifdef USE_ASSERT_CHECKING
> + else
> + {
> + /*
> + * A prep EState was provided, meaning pruning-aware locking
> + * should have locked at least the unpruned relations.
> + */
> + if (!IsParallelWorker())
> + {
> + int rtindex = -1;
> +
> + while ((rtindex =
> bms_next_member(queryDesc->estate->es_unpruned_relids,
> + rtindex)) >= 0)
> + {
> + RangeTblEntry *rte = exec_rt_fetch(rtindex, queryDesc->estate);
> +
> + Assert(rte->rtekind == RTE_RELATION ||
> + (rte->rtekind == RTE_SUBQUERY &&
> + rte->relid != InvalidOid));
> + Assert(CheckRelationOidLockedByMe(rte->relid,
> + rte->rellockmode, true));
> + }
> + }
> + }
> +#endif
>
> So the invariant is: if no prep EState was provided, every relation in
> the plan is locked; if one was provided, at least the unpruned
> relations are locked. Both are checked in assert builds.
>
> I think this covers the main concerns, but I may be missing something.
> If anyone sees a problem with this approach, I'd like to hear about
> it.

Here's v7. Some plancache.c changes that I'd made were in the wrong
patch in v6; this version puts them where they belong.

--
Thanks, Amit Langote