Re: generic plans and "initial" pruning

On Tue, 18 Jul 2023, 08:26 Amit Langote, <amitlangote09(at)gmail(dot)com> wrote:

> 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?
>

I did.

For full disclosure, I also had max_locks_per_transaction set to 10000.

>
> > 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.

Thom

>