From: Tom Lane <tgl@sss.pgh.pa.us>
To: Richard Guo <guofenglinux@gmail.com>
cc: Robert Haas <robertmhaas@gmail.com>,
        Andrey Lepikhov <a.lepikhov@postgrespro.ru>,
        PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>
Subject: Re: Clause accidentally pushed down ( Possible bug in Making Vars
 outer-join aware)
In-reply-to: 
 <CAMbWs48iJprH69xQ27nigwODu1m_4U3Y2WMTxk47sTVdkW0Seg@mail.gmail.com>
References: <0b819232-4b50-f245-1c7d-c8c61bf41827@postgrespro.ru>
 <CAMbWs4_HJNuLv9HDReEBDrQrCPV-uqpVOecoJkurprnY+Do9Fg@mail.gmail.com>
 <CAMbWs4_8EZU4DetHyZGm1CwCxUhBdWpRnA9dtM-Z82i=bVKA6A@mail.gmail.com>
 <CAMbWs4_XcPHqc=A=36cxvJRhkjAgEeJtOFLOnBPEfQYGqnGOCw@mail.gmail.com>
 <efb33bee-145d-95a8-1048-2e7b8f282e2c@postgrespro.ru>
 <CAMbWs48FHw_tTXSbA-_MPaeu7S7JCt6Xs-gxwkx=pvLiwC6eTg@mail.gmail.com>
 <c69bd07e-1175-10da-9ac0-4bfe59235ac2@postgrespro.ru>
 <402433.1677524538@sss.pgh.pa.us>
 <2663884e-769a-b298-55b0-014c5a1ee55c@postgrespro.ru>
 <524303.1677538463@sss.pgh.pa.us> <999171.1677611407@sss.pgh.pa.us>
 <CAMbWs4-Pi5cM=V8B-g5Qfnr0t3EgAbhus0YNQN=XquJ8fNoatw@mail.gmail.com>
 <CAMbWs4-Pmh9umsUbfs8Lay=k6wFrsuLdLJ7ci=Af6DqAwsN1Hg@mail.gmail.com>
 <CA+TgmoZ9yQLYziHAbt_X3hpU8FKkPXGLV=OhoD-GejAUckQbrA@mail.gmail.com>
 <CA+TgmoaeccFHN9ouXiSQmMJVh6uF-ANbbDjrwopQ+icYyzJSJw@mail.gmail.com>
 <CAMbWs4_nudsuzd-bLLgu1WCzeV1pLv3SG1Ub8vo-ETJzvgc0Bg@mail.gmail.com>
 <1526312.1683912310@sss.pgh.pa.us> <24!
 89296.1684195396@sss.pgh.pa.us>
 <CAMbWs4-Kx3ZHUByn6D2ZUTeF4Fm7uudkzgHNguDutpV8tepxtg@mail.gmail.com>
 <2766442.1684265014@sss.pgh.pa.us> <3014965.1684293045@sss.pgh.pa.us>
 <CAMbWs48iJprH69xQ27nigwODu1m_4U3Y2WMTxk47sTVdkW0Seg@mail.gmail.com>
Comments: In-reply-to Richard Guo <guofenglinux@gmail.com>
	message dated "Wed, 17 May 2023 16:29:54 +0800"
MIME-Version: 1.0
Content-Type: text/plain; charset="UTF-8"
Content-ID: <3344780.1684337332.1@sss.pgh.pa.us>
Content-Transfer-Encoding: quoted-printable
Date: Wed, 17 May 2023 11:28:52 -0400
Message-ID: <3344781.1684337332@sss.pgh.pa.us>
Archived-At: 
 <https://www.postgresql.org/message-id/3344781.1684337332%40sss.pgh.pa.us>
Precedence: bulk

Richard Guo <guofenglinux@gmail.com> writes:
> On Wed, May 17, 2023 at 11:10=E2=80=AFAM Tom Lane <tgl@sss.pgh.pa.us> wr=
ote:
>> * I'm not terribly happy with 0004 as it stands, specifically the
>> business of making build_join_rel recalculate pushed_down_ojrelids
>> from scratch.  Again, that's adding cycles to the mainline case to
>> support an unusual case.  It'd be better to make
>> add_outer_joins_to_relids have an extra output that is the
>> pushed_down_ojrelids set, and pass that forward.  I ran out of time
>> to make that happen today, though.

> Actually I did that in this way before I invented the join_info_array
> thing.  See patch 'v2-0001-Adjust-outer-join-s-target-list.patch' in
> https://www.postgresql.org/message-id/CAMbWs49CB3txv_s3SwC1L2h1DiUPAn%2B=
v0-O4UFfg33b%2BHXNnTg%40mail.gmail.com
> A little difference is that that patch makes add_outer_joins_to_relids
> collect pushed down joins rather than pushed down ojrelids, so that in
> build_joinrel_tlist we can just loop over pushed down joins rather than
> the whole join_info_list.

Ah, good idea.  I made some cosmetic adjustments and pushed the lot.

We still have a couple of loose ends in this thread:

1. Do we need the change in add_nulling_relids that I postulated in [1]?

-			bms_overlap(phv->phrels, context->target_relids))
+			bms_is_subset(phv->phrels, context->target_relids))

The test case that made that fall over no longer does so, but I'm
suspicious that this is still needed.  I'm not quite sure though,
and I'm even less sure about the corresponding change in
remove_nulling_relids:

-			!bms_overlap(phv->phrels, context->except_relids))
+			!bms_is_subset(phv->phrels, context->except_relids))

which I made up pretty much out of whole cloth.

2. Can we do anything to tighten make_outerjoininfo's computation of
potential commutator pairs, as I speculated in [2]?  This isn't a bug
hazard anymore (I think), but adding useless bits there clearly does
cause us to waste cycles later.  If we can tighten that cheaply, it
should be a win.

3. In general, it seems like what your fixes are doing is computing
transitive closures of the commute_above relationships.  I wonder if
we can save anything by pre-calculating the closure sets.  Again,
I don't want to add cycles in cases where the whole thing doesn't
apply, but maybe we'd not have to.

			regards, tom lane

[1] https://www.postgresql.org/message-id/2489296.1684195396%40sss.pgh.pa.=
us
[2] https://www.postgresql.org/message-id/2766442.1684265014%40sss.pgh.pa.=
us