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

Attachment: v30-0002-In-GetCachedPlan-only-lock-unpruned-partitions.patch
Description: Binary data

Attachment: v30-0001-Preparatory-refactoring-before-reworking-CachedP.patch
Description: Binary data

Reply via email to