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

On Jul 1, 2024, at 12:56 AM, Ashutosh Bapat <ashutosh(dot)bapat(dot)oss(at)gmail(dot)com> wrote:

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

I think it’s ready from my standpoint. 

David