Re: Reuse child_relids in try_partitionwise_join was Re: Assert failure on bms_equal(child_joinrel->relids, child_joinrelids)

On Wed, Jun 12, 2024 at 4:22 AM David Christensen <david+pg(at)pgguru(dot)net>
wrote:

> On Mon, Jun 10, 2024 at 8:15 AM Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
> >
> > On Mon, Jun 10, 2024 at 3:09 AM Ashutosh Bapat
> > <ashutosh(dot)bapat(dot)oss(at)gmail(dot)com> wrote:
> > > This is just one instance of measurements. If I run the experiment
> multiple times the results and the patterns will vary. Usually I have found
> planning time to vary within 5% for regular tables and within 9% for
> partitioned tables with a large number of partitions. Below are
> measurements with the experiment repeated multiple times. For a given
> number of partitioned tables (each with 1000 partitions) being joined,
> planning time is measured 10 consecutive times. For this set of 10 runs we
> calculate average and standard deviation of planning time. Such 10 sets are
> sampled. This means planning time is sampled 100 times in total with and
> without patch respectively. Measurements with master and patched are
> reported in the attached excel sheet.
> >
> > Well, this is fine then I guess, but if the original results weren't
> > stable enough for people to draw conclusions from, then it's better
> > not to post them, and instead do this work to get results that are
> > stable before posting.
>
> Just doing a quick code review of the structure and the caller, I'd
> agree that this is properly hoisting the invariant, so don't see that
> it should contribute to any performance regressions. To the extent
> that it's called multiple times I can see that it's an improvement,
> with minimal code shuffling it seems like a sensible change (even in
> the single-caller case).
>
> In short +1 from me.
>

Hi David,
Do you think it needs more review or we can change it to "ready for
committer"?

--
Best Wishes,
Ashutosh Bapat