Re: generic plans and "initial" pruning

From: Amit Langote <amitlangote09(at)gmail(dot)com>
To: Thom Brown <thom(at)linux(dot)com>
Cc: Daniel Gustafsson <daniel(at)yesql(dot)se>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>, Andres Freund <andres(at)anarazel(dot)de>, David Rowley <dgrowleyml(at)gmail(dot)com>, Jacob Champion <jchampion(at)timescale(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>, Robert Haas <robertmhaas(at)gmail(dot)com>
Subject: Re: generic plans and "initial" pruning
Date: 2023-07-18 07:26:35
Message-ID: CA+HiwqHBbptyxjQx7964DjitA8FVNs1MN=uwrzRy=oOD0Hy3ag@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi Thom,

On Tue, Jul 18, 2023 at 1:33 AM Thom Brown <thom(at)linux(dot)com> wrote:
> On Thu, 13 Jul 2023 at 13:59, Amit Langote <amitlangote09(at)gmail(dot)com> wrote:
> > In an absolutely brown-paper-bag moment, I realized that I had not
> > updated src/backend/executor/README to reflect the changes to the
> > executor's control flow that this patch makes. That is, after
> > scrapping the old design back in January whose details *were*
> > reflected in the patches before that redesign.
> >
> > Anyway, the attached fixes that.
> >
> > Tom, do you think you have bandwidth in the near future to give this
> > another look? I think I've addressed the comments that you had given
> > back in April, though as mentioned in the previous message, there may
> > still be some funny-looking aspects still remaining. In any case, I
> > have no intention of pressing ahead with the patch without another
> > committer having had a chance to sign off on it.
>
> I've only just started taking a look at this, and my first test drive
> yields very impressive results:
>
> 8192 partitions (3 runs, 10000 rows)
> Head 391.294989 382.622481 379.252236
> Patched 13088.145995 13406.135531 13431.828051

Just to be sure, did you use pgbench --Mprepared with plan_cache_mode
= force_generic_plan in postgresql.conf?

> Looking at your changes to README, I would like to suggest rewording
> the following:
>
> +table during planning. This means that inheritance child tables, which are
> +added to the query's range table during planning, if they are present in a
> +cached plan tree would not have been locked.
>
> To:
>
> This means that inheritance child tables present in a cached plan
> tree, which are added to the query's range table during planning,
> would not have been locked.
>
> Also, further down:
>
> s/intiatialize/initialize/
>
> I'll carry on taking a closer look and see if I can break it.

Thanks for looking. I've fixed these issues in the attached updated
patch. I've also changed the position of a newly added paragraph in
src/backend/executor/README so that it doesn't break the flow of the
existing text.

--
Thanks, Amit Langote
EDB: http://www.enterprisedb.com

Attachment Content-Type Size
v42-0001-Add-field-to-store-parent-relids-to-Append-Merge.patch application/octet-stream 21.2 KB
v42-0004-Track-opened-range-table-relations-in-a-List-in-.patch application/octet-stream 2.4 KB
v42-0002-Set-inFromCl-to-false-in-child-table-RTEs.patch application/octet-stream 3.7 KB
v42-0003-Delay-locking-of-child-tables-in-cached-plans-un.patch application/octet-stream 128.6 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Joan 2023-07-18 07:53:25 There should be a way to use the force flag when restoring databases
Previous Message Richard Guo 2023-07-18 07:17:15 Re: An inefficient query caused by unnecessary PlaceHolderVar