Re: generic plans and "initial" pruning

On Fri, Oct 11, 2024 at 4:30 PM Amit Langote <amitlangote09(at)gmail(dot)com> wrote:
> On Fri, Oct 11, 2024 at 5:15 AM Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
> Ok, I'm updating 0005 to change how the CachedPlan is handled when it
> becomes invalid during InitPlan(). Currently (v56), a separate
> transient CachedPlan is created for the query being initialized when
> invalidation occurs. However, it seems better to update the original
> CachedPlan in place to avoid extra bookkeeping for transient plans—an
> approach Robert suggested in an off-list discussion.
>
> Will post a new version next week.

Sorry for the delay.

I've completed hacking on the approach to update the existing
CachedPlan in-place when it’s invalidated during plan initialization
in its stmt_list. Previously, we created transient (living for that
execution) CachedPlans for each query/plan, tracked separately from
the original CachedPlan, so that invalidation callbacks could
reference them. This meant that the original CachedPlan would continue
to hold invalid plans until the next GetCachedPlan() call.

With the new approach, the original CachedPlan is updated directly:
new PlannedStmts are installed into the existing stmt_list, allowing
any callers iterating over that list to continue unaffected. The new
UpdateCachedPlan() function now creates new plans for all queries in
the CachedPlan’s owning CachedPlanSource, replacing the previous
plans, and marks it valid. So the CachedPlan becomes valid
immediately instead of in the next GetCachedPlan().

One caveat is that, without a dedicated memory context for the
PlannedStmts in stmt_list, the old ones leak into CacheMemoryContext.
However, since UpdateCachedPlan() is rarely invoked, I haven’t focused
on addressing this leak. If needed, we could introduce an additional
memory context next to CachedPlan.context, which would allow freeing
the PlannedStmts without affecting the stmt_list. For now, I’ve
ensured that stmt_list itself is not overwritten in
UpdateCachedPlan().

UpdateCachedPlan() is added in 0005.

I've kept 0005, the patch to retry execution with an updated plan if
the plan becomes invalid after taking locks on prunable relations
(deferred until initial pruning), separate for now. However, I plan to
eventually merge it into 0004, the patch implementing deferred
locking.

I've also fixed the comment in 0003 about PartitionPruneInfo.relid as
Tom pointed out, which now reads:

* relids RelOptInfo.relids of the parent plan node (e.g. Append
* or MergeAppend) to which this PartitionPruneInfo node
* belongs. The pruning logic ensures that this matches
* the parent plan node's apprelids.

I've stared at the refactoring patches 0001-0003 long enough at this
point to think that they are good for committing.

--
Thanks, Amit Langote