Re: generic plans and "initial" pruning - Mailing list pgsql-hackers

From Amit Langote
Subject Re: generic plans and "initial" pruning
Date
Msg-id CA+HiwqE7_YpU--EsrhvNqcZ+10+92EGFaX5609AUJb9ENLntnQ@mail.gmail.com
Whole thread Raw
In response to Re: generic plans and "initial" pruning  (Amit Langote <amitlangote09@gmail.com>)
List pgsql-hackers
On Thu, Jul 17, 2025 at 9:11 PM Amit Langote <amitlangote09@gmail.com> wrote:
> The refinements I described in my email above might help mitigate some
> of those executor-related issues. However, I'm starting to wonder if
> it's worth reconsidering our decision to handle pruning, locking, and
> validation entirely at executor startup, which was the approach taken
> in the reverted patch.
>
> The alternative approach, doing initial pruning and locking within
> plancache.c itself (which I floated a while ago), might be worth
> revisiting. It avoids the complications we've discussed around the
> executor API and preserves the clear separation of concerns that
> plancache.c provides, though it does introduce some new layering
> concerns, which I describe further below.
>
> To support this, we'd need a mechanism to pass pruning results to the
> executor alongside each PlannedStmt. For each PartitionPruneInfo in
> the plan, that would include the corresponding PartitionPruneState and
> the bitmapset of surviving relids determined by initial pruning. Given
> that a CachedPlan can contain multiple PlannedStmts, this would
> effectively be a list of pruning results, one per statement. One
> reasonable way to handle that might be to define a parallel data
> structure, separate from PlannedStmt, constructed by plancache.c and
> carried via QueryDesc. The memory and lifetime management would mirror
> how ParamListInfo is handled today, leaving the executor API unchanged
> and avoiding intrusive changes to PlannedStmt.
>
> However, one potentially problematic aspect of this design is managing
> the lifecycle of the relations referenced by PartitionPruneState.
> Currently, partitioned table relations are opened by the executor
> after entering ExecutorStart() and closed automatically by
> ExecEndPlan(), allowing cleanup of pruning states implicitly. If we
> perform initial pruning earlier, we'd need to keep these relations
> open longer, necessitating explicit cleanup calls (e.g., a new
> FinishPartitionPruneState()) invoked by the caller of the executor,
> such as from ExecutorEnd() or even higher-level callers. This
> introduces some questionable layering by shifting responsibility for
> relation management tasks, which ideally belong within the executor,
> into its callers.
>
> My sense is that the complexity involved in carrying pruning results
> via this parallel data structure was one of the concerns Tom raised
> previously, alongside the significant pruning code refactoring that
> the earlier patch required. The latter, at least, should no longer be
> necessary given recent code improvements.

One point I forgot to mention about this approach is that we'd also
need to ensure permissions on parent relations are checked before
performing initial pruning in plancache.c, since pruning may involve
evaluating user-provided expressions. So in effect, we'd need to
invoke not just ExecDoInitialPruning(), but also
ExecCheckPermissions(), or some variant of it, prior to executor
startup. While manageable, it does add slightly to the complexity.

--
Thanks, Amit Langote



pgsql-hackers by date:

Previous
From: Fujii Masao
Date:
Subject: Re: Log prefix missing for subscriber log messages received from publisher
Next
From: Masahiko Sawada
Date:
Subject: Support getrandom() for pg_strong_random() source