On Tue, 18 Jul 2023, 08:26 Amit Langote, <amitlangot...@gmail.com> wrote:
> Hi Thom, > > On Tue, Jul 18, 2023 at 1:33 AM Thom Brown <t...@linux.com> wrote: > > On Thu, 13 Jul 2023 at 13:59, Amit Langote <amitlangot...@gmail.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 >
