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