Re: Fix bogus Asserts in calc_non_nestloop_required_outer

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: Richard Guo <guofenglinux(at)gmail(dot)com>, Aleksander Alekseev <aleksander(at)timescale(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Fix bogus Asserts in calc_non_nestloop_required_outer
Date: 2024-01-10 04:22:25
Message-ID: 2331001.1704860545@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

I wrote:
> Robert Haas <robertmhaas(at)gmail(dot)com> writes:
>> But we could also find some way to assert that the parameterization
>> sets contain only top-most rels.

> Hmm ... perhaps worth doing. I think bms_is_subset against
> all_baserels would work.

[ tries that ... ] Nope, it needs to be all_query_rels. I plead
ENOCAFFEINE. However, while we could easily add

+ Assert(bms_is_subset(inner_paramrels, root->all_query_rels));
+ Assert(bms_is_subset(outer_paramrels, root->all_query_rels));

to try_nestloop_path, that doesn't work as-is in
calc_non_nestloop_required_outer because we don't pass "root" to it.
We could add that, or perhaps contemplate some more-extensive
rearrangement to make the division of labor between
calc_nestloop_required_outer and its caller more like that between
calc_non_nestloop_required_outer and its callers. But I'm feeling
kind of unenthused about that, because

(1) I don't see how to rearrange things without duplicating code:
try_nestloop_path needs access to some of these values, while moving
any work out of calc_non_nestloop_required_outer would mean two
copies in its two callers. So there are reasons why that's not
perfectly symmetric. (But I still maintain that we should try to
make the code look similar between these two code paths, when
considering both the calc_xxx functions and their callers.)

(2) joinpath.c already knows that parameterization sets mention
only top-level relids, specifically at the definition and uses of
PATH_PARAM_BY_PARENT, and I bet there are more dependencies elsewhere.
So I'm not sure about the value of asserting it only here.

In short I'm inclined to apply v3 as-is.

regards, tom lane

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Michael Paquier 2024-01-10 04:26:14 Re: Test slots invalidations in 035_standby_logical_decoding.pl only if dead rows are removed
Previous Message Thomas Munro 2024-01-10 04:13:37 Re: Streaming I/O, vectored I/O (WIP)