Re: generic plans and "initial" pruning

From: Amit Langote <amitlangote09(at)gmail(dot)com>
To: Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>
Cc: Robert Haas <robertmhaas(at)gmail(dot)com>, Jacob Champion <jchampion(at)timescale(dot)com>, David Rowley <dgrowleyml(at)gmail(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: generic plans and "initial" pruning
Date: 2022-12-16 02:33:53
Message-ID: CA+HiwqGKXwx_TmciQURVj26eWM0Qp=WrT3n6T51+AmrcqZV3mw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Wed, Dec 14, 2022 at 5:35 PM Amit Langote <amitlangote09(at)gmail(dot)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 Content-Type Size
v30-0002-In-GetCachedPlan-only-lock-unpruned-partitions.patch application/octet-stream 66.2 KB
v30-0001-Preparatory-refactoring-before-reworking-CachedP.patch application/octet-stream 17.2 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Amit Kapila 2022-12-16 02:46:05 Re: [PATCH] Reuse Workers and Replication Slots during Logical Replication
Previous Message David Rowley 2022-12-16 02:26:20 Re: Add proper planner support for ORDER BY / DISTINCT aggregates