On Wed, Dec 14, 2022 at 5:35 PM Amit Langote <amitlangot...@gmail.com> wrote: > I have moved the original functionality of GetCachedPlan() to > GetCachedPlanInternal(), turning the former into a sort of controller > as described shortly. The latter's CheckCachedPlan() part now only > locks the "minimal" set of, non-prunable, relations, making a note of > whether the plan contains any prunable subnodes and thus prunable > relations whose locking is deferred to the caller, GetCachedPlan(). > GetCachedPlan(), as a sort of controller as mentioned before, does the > pruning if needed on the minimally valid plan returned by > GetCachedPlanInternal(), locks the partitions that survive, and redoes > the whole thing if the locking of partitions invalidates the plan.
After sleeping on it, I realized this doesn't have to be that complicated. Rather than turn GetCachedPlan() into a wrapper for handling deferred partition locking as outlined above, I could have changed it more simply as follows to get the same thing done: if (!customplan) { - if (CheckCachedPlan(plansource)) + bool hasUnlockedParts = false; + + if (CheckCachedPlan(plansource, &hasUnlockedParts) && + hasUnlockedParts && + CachedPlanLockPartitions(plansource, boundParams, owner, extra)) { /* We want a generic plan, and we already have a valid one */ plan = plansource->gplan; Attached updated patch does it like that. -- Thanks, Amit Langote EDB: http://www.enterprisedb.com
v30-0002-In-GetCachedPlan-only-lock-unpruned-partitions.patch
Description: Binary data
v30-0001-Preparatory-refactoring-before-reworking-CachedP.patch
Description: Binary data