From: Tom Lane <tgl@sss.pgh.pa.us>
To: Richard Guo <guofenglinux@gmail.com>
cc: David Rowley <dgrowleyml@gmail.com>, gourlaouen.mikael@gmail.com,
        pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #18627: Regression (15 -> 16) - Join removal not performed
 when join condition spans multiple tables
In-reply-to: 
 <CAMbWs49f1+Dv7afcw4ngFw2cHLFRiqpzu-GypRVk_Ryd-gVRJw@mail.gmail.com>
References: <18627-44f950eb6a8416c2@postgresql.org>
 <CAApHDvpUiPCefrOL2id3LbF6MC0oiz8pJcDcUw3qZ=aVHjViHQ@mail.gmail.com>
 <2899426.1727024160@sss.pgh.pa.us>
 <CAMbWs49f1+Dv7afcw4ngFw2cHLFRiqpzu-GypRVk_Ryd-gVRJw@mail.gmail.com>
Comments: In-reply-to Richard Guo <guofenglinux@gmail.com>
	message dated "Mon, 23 Sep 2024 15:55:59 +0800"
MIME-Version: 1.0
Content-Type: text/plain; charset="UTF-8"
Content-ID: <3239683.1727129515.1@sss.pgh.pa.us>
Content-Transfer-Encoding: 8bit
Date: Mon, 23 Sep 2024 18:11:55 -0400
Message-ID: <3239684.1727129515@sss.pgh.pa.us>
Archived-At: 
 <https://www.postgresql.org/message-id/3239684.1727129515%40sss.pgh.pa.us>
Precedence: bulk

Richard Guo <guofenglinux@gmail.com> writes:
> On Mon, Sep 23, 2024 at 12:56 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> That's ugly for sure, and it would only revert to the status
>> quo ante.  But I'm not seeing a better way right now.

> It seems to me that this approach basically reverts the changes in
> 8538519db.  I'm afraid this would re-introduce the bug fixed by that
> commit.

Ugh, you're probably right.

I'm taking a second look at the idea of regenerating the attr_needed
values altogether.  It doesn't look that bad, especially if we cheat
to the extent of preserving the existing "relation 0" bits (that is,
tlist and HAVING references).  That's fine since we could never
remove a rel that supplies Vars used in those places.  Then we
basically have to reconstruct surviving references from

   * EquivalenceClasses
   * joinclauses (available from the rels' joinlists)
   * lateral references (available from the rels' lateral_vars lists)
   * surviving placeholders (much like fix_placeholder_input_needed_levels)

The need to again call pull_var_clause on these clauses is mildly
annoying but I think it won't be that bad runtime-wise.  Slightly
more annoying is that a joinclause mentioning N relations would be
visited N times.  Perhaps we could de-duplicate using rinfo
serial numbers?

While I've not written any code yet, it seems like this route
would lead to a bigger patch than we'd want to back-patch into
v16 or v17.  So that's not great; but on the other hand it took
a year for anyone to notice this regression, so maybe we can get
away with not fixing it in those branches.

			regards, tom lane