Hi,

Here is v5 of this patch series.

Changes since v4:

* Removed the ExecPrep struct. ExecutorPrep() now returns an EState
directly, which is carried through QueryDesc->estate and prep_estates
lists. This simplifies the interface and avoids the ownership tracking
(owns_estate) that ExecPrep required.

* Moved the PARAM_EXEC values setup from
ExecCreatePartitionPruneState() to InitExecPartitionPruneContexts().
This allows ExecutorPrep() to create pruning state before the executor
has set up PARAM_EXEC slots.

* Added a standalone test (0003) that inspects pg_locks to show
baseline locking behavior: currently both pruning-on and pruning-off
lock all child partitions. The pruning-aware locking patch (0004) then
updates the expected output, making the behavioral change visible as a
diff.

* Absorbed the plan invalidation test (v4's 0005) into the locking
patch (0004), since that's where CachedPlanPrepCleanup lives.

* Moved the parallel worker reuse patch to the end of the series
(0006), since the core functionality doesn't depend on it.

* Fixed the missing i++ in CheckInitialPruningResultsInWorker(),
renamed it from ExecCheckInitialPruningResults(), and made it a
debug-only check (#ifdef USE_ASSERT_CHECKING) that verifies the
worker's pruning result matches the leader's.

The series is:

0001: Refactor partition pruning initialization for clarity
0002: Introduce ExecutorPrep and refactor executor startup
0003: Add test for partition lock behavior with generic cached plans
0004: Use pruning-aware locking in cached plans
0005: Make SQL function executor track ExecutorPrep state
0006: Reuse partition pruning results in parallel workers

The core of this series is ExecutorPrep() in 0002 and its use by
GetCachedPlan() in 0004. This is where I'd appreciate the most
scrutiny.

ExecutorPrep() factors out permission checks, range table
initialization, and initial partition pruning from InitPlan() into a
callable helper that can run before ExecutorStart(). This is what
makes pruning-aware locking possible: GetCachedPlan() calls
ExecutorPrep() during plan validation to determine which partitions
survive initial pruning, then locks only those (plus firstResultRel
targets). The resulting EStates are passed through CachedPlanPrepData
and eventually adopted by ExecutorStart(), so the pruning work isn't
repeated.

The risk is that this pulls part of what we traditionally consider
"execution" into a phase that runs during plan cache validation.
Specifically:

* Snapshot use: ExecutorPrep() requires an active snapshot for pruning
expressions that may call PL functions. During plan cache validation,
we use a transaction snapshot rather than the portal's execution
snapshot. I believe this is safe because initial pruning has always
used whatever snapshot happens to be active at the time
ExecDoInitialPruning() runs, but it's a subtle change in when that
happens.

* EState memory lifetime: The EState created by ExecutorPrep() lives
longer than it used to, since it's created during GetCachedPlan() but
consumed later during ExecutorStart(). CachedPlanPrepData manages
this: it tracks the EStates and their resource owner, and
CachedPlanPrepCleanup() frees them if the plan is invalidated before
execution reaches ExecutorStart(). I've tried to ensure that the
EState's memory context hierarchy stays consistent with the current
system through the reparenting done in portal/SPI/function paths.

* Plan invalidation: If the plan is invalidated between ExecutorPrep()
and ExecutorStart() (e.g., DDL on a partition during pruning), the
prep state is discarded by CachedPlanPrepCleanup() and GetCachedPlan()
retries. The invalidation regression test in 0004 exercises this path.

As of 0002, ExecutorStart() is the only caller of ExecutorPrep(), so
there is no behavioral change until 0004 wires it into
GetCachedPlan(). This makes it possible to review the refactoring in
isolation.

I would really appreciate review from folks familiar with the executor
lifecycle and plan cache internals. The approach touches the boundary
between plan caching and execution, and getting the lifetime and
snapshot semantics right is important.

A note on 0006: it is intentionally last and independently droppable.
It passes the leader's pruning results to parallel workers via
nodeToString/stringToNode, which is the simplest approach but adds
serialization overhead to all parallel queries regardless of whe

--
Thanks, Amit Langote

Attachment: v5-0001-Refactor-partition-pruning-initialization-for-cla.patch
Description: Binary data

Attachment: v5-0002-Introduce-ExecutorPrep-and-refactor-executor-star.patch
Description: Binary data

Attachment: v5-0003-Add-test-for-partition-lock-behavior-with-generic.patch
Description: Binary data

Attachment: v5-0004-Use-pruning-aware-locking-in-cached-plans.patch
Description: Binary data

Attachment: v5-0005-Make-SQL-function-executor-track-ExecutorPrep-sta.patch
Description: Binary data

Attachment: v5-0006-Reuse-partition-pruning-results-in-parallel-worke.patch
Description: Binary data

Reply via email to