Re: apply_scanjoin_target_to_paths and partitionwise join

From: Ashutosh Bapat <ashutosh(dot)bapat(dot)oss(at)gmail(dot)com>
To: Jakub Wartak <jakub(dot)wartak(at)enterprisedb(dot)com>, Tomas Vondra <tomas(dot)vondra(at)enterprisedb(dot)com>, Zhihong Yu <zyu(at)yugabyte(dot)com>, arne(dot)roland(at)malkut(dot)net
Cc: PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Julien Rouhaud <rjuju123(at)gmail(dot)com>, Etsuro Fujita <etsuro(dot)fujita(at)gmail(dot)com>
Subject: Re: apply_scanjoin_target_to_paths and partitionwise join
Date: 2024-05-22 07:57:07
Message-ID: CAExHW5uZh5fJ2siyYpwVWUq+r5EW_gatE1WRZ9H56x+115CUCg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Mon, May 6, 2024 at 6:28 PM Ashutosh Bapat <ashutosh(dot)bapat(dot)oss(at)gmail(dot)com>
wrote:

>
>
> On Mon, May 6, 2024 at 4:26 PM Jakub Wartak <jakub(dot)wartak(at)enterprisedb(dot)com>
> wrote:
>
>> Hi Ashutosh & hackers,
>>
>> On Mon, Apr 15, 2024 at 9:00 AM Ashutosh Bapat
>> <ashutosh(dot)bapat(dot)oss(at)gmail(dot)com> wrote:
>> >
>> > Here's patch with
>> >
>> [..]
>> > Adding to the next commitfest but better to consider this for the next
>> set of minor releases.
>>
>> 1. The patch does not pass cfbot -
>> https://cirrus-ci.com/task/5486258451906560 on master due to test
>> failure "not ok 206 + partition_join"
>>
>
> So I need to create a patch for master first. I thought CFBot somehow knew
> that the patch was created for PG 15. :)
>

PFA patch for master. That should fix CfBot.

>
>> 4. meson test ends up with failures like below:
>>
>> 4/290 postgresql:regress / regress/regress
>> ERROR 32.67s
>> 6/290 postgresql:pg_upgrade / pg_upgrade/002_pg_upgrade
>> ERROR 56.96s
>> 35/290 postgresql:recovery / recovery/027_stream_regress
>> ERROR 40.20s
>>
>> (all due to "regression tests pass" failures)
>> [...]
>
>
>> So with the patch that SQL does not use partitionwise join as it finds
>> it more optimal to stick to a plan with cost of "1.10..2.21" instead
>> of "1.11..2.22" (w/ partition_join), nitpicking but still a failure
>> technically. Perhaps it could be even removed? (it's pretty close to
>> noise?).
>>
>
>
The test was added by 6b94e7a6da2f1c6df1a42efe64251f32a444d174 and later
modified by 3c569049b7b502bb4952483d19ce622ff0af5fd6. The modification just
avoided eliminating the join, so that change can be ignored.
6b94e7a6da2f1c6df1a42efe64251f32a444d174 added the tests to test fractional
paths being considered when creating ordered append paths. Reading the
commit message, I was expecting a test which did not use a join as well and
also which used inheritance. But it seems that the queries added by that
commit, test all the required scenarios and hence we see two queries
involving join between partitioned tables. As the comment there says the
intention is to verify index only scans and not exactly partitionwise join.
So just fixing the expected output of one query looks fine. The other query
will test partitionwise join and fractional paths anyway. I am including
Tomas, Arne and Zhihong, who worked on the first commit, to comment on
expected output changes.

I will create patches for the back-branches once the patch for master is in
a committable state.

--
Best Wishes,
Ashutosh Bapat

Attachment Content-Type Size
0001-Fix-partitioned-join-case-in-apply_scanjoin-20240522.patch text/x-patch 12.9 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Shlok Kyal 2024-05-22 09:15:19 Re: State of pg_createsubscriber
Previous Message Laurenz Albe 2024-05-22 07:21:10 Re: Ambiguous description on new columns