Horton

Re: generic plans and "initial" pruning

Tender Wang <tndrwang@gmail.com>

View thread
From: Tender Wang <tndrwang@gmail.com>
To: Amit Langote <amitlangote09@gmail.com>
Cc: Tom Lane <tgl@sss.pgh.pa.us>, Alexander Lakhin <exclusion@gmail.com>, Tomas Vondra <tomas@vondra.me>, Robert Haas <robertmhaas@gmail.com>, Alvaro Herrera <alvherre@alvh.no-ip.org>, Andres Freund <andres@anarazel.de>, Daniel Gustafsson <daniel@yesql.se>, David Rowley <dgrowleyml@gmail.com>, PostgreSQL Hackers <pgsql-hackers@postgresql.org>, Thom Brown <thom@linux.com>
Date: 2025-11-23T12:17:11Z
Lists: pgsql-hackers 
Amit Langote <amitlangote09@gmail.com> 于2025年11月20日周四 15:30写道：

> On Mon, Nov 17, 2025 at 9:50 PM Amit Langote <amitlangote09@gmail.com>
> wrote:
> > On Wed, Nov 12, 2025 at 11:17 PM Amit Langote <amitlangote09@gmail.com>
> wrote:
> > > * Enable pruning-aware locking in cached / generic plan reuse (0004):
> > > extends GetCachedPlan() and CheckCachedPlan() to call ExecutorPrep()
> > > on each PlannedStmt in the CachedPlan, locking only surviving
> > > partitions. Adds CachedPlanPrepData to pass this through plan cache
> > > APIs and down to execution via QueryDesc. Also reinstates the
> > > firstResultRel locking rule added in 28317de72 but later lost due to
> > > revert of the earlier pruning patch, to ensure correctness when all
> > > target partitions are pruned.
> >
> > Looking at the changes to executor/function.c, I also noticed that I
> > had mistakenly allocated the ExecutorPrep state in
> > SQLFunctionCache.fcontext whereas the correct context for execution
> > related state is SQLFunctionCache.subcontext.  In the updated patch,
> > I've made postquel_start() reparent the prep EState's es_query_cxt to
> > subcontext from fcontext. I also did not have a test case that
> > exercised cached plan reuse for SQL functions, so I added one. I split
> > the function.c's GetCachedPlan() + CachedPlanPrepData plumbing into a
> > new patch 0005 so it can be reviewed separately, since it is the only
> > non-mechanical call-site change.
>
> I also noticed a bug in the prep cleanup logic that runs when a cached
> plan becomes invalid during the prep phase. Patch 0005 fixes that and
> adds a regression test that exercises the invalidation path. This will
> be folded into 0004 later.
>

I spent time looking at these patches.

I search all places that call GetCachedPlan(), and we always pass
&cprep(CachedPlanPrepData) to GetCachedPlan().
In PrepAndCheckCachedPlan(), if the plan_cache_mode is force_generic_plan,
the LockPolicy is always LOCK_UNPRUNED. Because *cprep has never been NULL.
It seems that the LockPolicy has no chance to be LOCK_ALL. Do I miss
something here?
-- 
Thanks,
Tender Wang