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: