Re: apply_scanjoin_target_to_paths and partitionwise join

From: Ashutosh Bapat <ashutosh(dot)bapat(dot)oss(at)gmail(dot)com>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: Richard Guo <guofenglinux(at)gmail(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Andrei Lepikhov <lepihov(at)gmail(dot)com>, Jakub Wartak <jakub(dot)wartak(at)enterprisedb(dot)com>, arne(dot)roland(at)malkut(dot)net, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, 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: 2025-12-24 14:20:25
Message-ID: CAExHW5v9J6KB811Coz-aaC2BLgDmimDAsFS2Jv=3q-dsKRQn-A@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Wed, Dec 24, 2025 at 6:58 PM Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
>
> Thanks for writing back. I only just noticed this thread again.
>
> On Wed, Dec 17, 2025 at 5:53 AM Ashutosh Bapat
> <ashutosh(dot)bapat(dot)oss(at)gmail(dot)com> wrote:
> > My patch removed a redundant SET enable_partitionwise_join = on. That
> > change is not included in your commit, I believe, because it's
> > irrelevant to the fix. However, it's better to avoid the redundancy to
> > avoid confusion. PFA patch for the same.
>
> Yeah, I couldn't really understand your test case changes, so I
> basically redid those from scratch. I personally think that the
> regression tests are kind of horrible about how they use SET enable_*
> commands. It's often extremely difficult to figure out what the
> current values are at any given point in what may be a very long test
> case file. I don't have an opinion at the present time on whether
> changing this makes it more or less confusing.

I think there's an unwritten convention that we re/set GUCs nearer the
queries which require/exercise those. That way they are visible. The
test file is about testing partitionwise join, so it's expected that
most of the queries will require PWJ enabled. Seeing
enable_partitionwise_join = true in the middle of the file made me
think that we are disabling PWJ somewhere before to test disabled PWJ
and re-enabling it. But I couldn't find a statement disabling it.
After spending some time and going through the original commit which
added enable_partitionwise_join = true, I realised that it was not
required there. I did that exercise twice, once when writing the patch
and once while comparing my patch and your commit. Removing that
statement will save somebody the same exercise. But I am ok, if we
don't want to remove it.

>
> > [3] https://www.postgresql.org/message-id/786.1565541557%40sss.pgh.pa.us
>
> This is a really interesting link. Your email contained no reference
> to this footnote (unless I missed something) but it seems quite
> relevant to this discussion.

This link was included in one of my very first emails. I included its
reference in some sentence I removed while finalizing the response;
leaving the link behind (no bibtex for emails). Thanks for noticing
it. It is interesting indeed.

--
Best Wishes,
Ashutosh Bapat

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Jelte Fennema-Nio 2025-12-24 14:31:47 Re: cleanup: Split long Makefile lists across lines and sort them
Previous Message zengman 2025-12-24 14:18:10 Re:cleanup: Split long Makefile lists across lines and sort them