MIME-Version: 1.0
References: <19460-5625143cef66012f@postgresql.org> <53936.1776633020@sss.pgh.pa.us>
In-Reply-To: <53936.1776633020@sss.pgh.pa.us>
From: Richard Guo <guofenglinux@gmail.com>
Date: Mon, 20 Apr 2026 10:12:11 +0900
Message-ID: <CAMbWs48E2qqd4B6AdvJDyPBi685-_bDEvoe0aJnYzAPL1v30zg@mail.gmail.com>
Subject: Re: BUG #19460: FULL JOIN rewriting issue on empty queries
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: francois.jehl@pigment.com, pgsql-bugs@lists.postgresql.org, 
	Robert Haas <robertmhaas@gmail.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Archived-At: <https://www.postgresql.org/message-id/CAMbWs48E2qqd4B6AdvJDyPBi685-_bDEvoe0aJnYzAPL1v30zg%40mail.gmail.com>
Precedence: bulk

On Mon, Apr 20, 2026 at 6:10=E2=80=AFAM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> This turns out to be because somebody long ago thought that outer join
> removal could be lazy about how much of the planner's data structures
> it needs to update.  Specifically, when the lower LEFT OUTER JOIN
> gets removed, we failed to remove the associated relids from the
> left_relids and right_relids of the upper "ON rhs.id =3D lhs.id" clause,
> and that blocks recognition of the applicability of a hash or merge
> join, because clause_sides_match_join() fails.

I came to the same conclusion.

> The fix seems pretty trivial, as attached.  (While I'm only certain
> that we have to fix left_relids and right_relids, this discovery
> makes it seem like it'd be pretty foolish not to fix all the relid
> sets of a RestrictInfo.)  I didn't make a regression test case yet,
> but we need one since no existing test results change (!?).

This fix LGTM.  I think it'd be better to have a regression test case.
How about this one:

create table t (id int unique);

explain (costs off)
select t1.*
from t t1 full join
  (select 1 as x
   from t t2 left join t t3 on t2.id =3D t3.id
  ) sub on t1.id =3D sub.x;
ERROR:  FULL JOIN is only supported with merge-joinable or
hash-joinable join conditions

> I'm feeling a tad nervous about pushing this into released branches.
> It seems likely that it might enable quite a few join plans that were
> previously not considered, and people tend not to like plan changes in
> stable branches.  However, (a) it's hard to argue that this isn't a
> regression from pre-v16, and (b) since this change affects no existing
> test, maybe the blast radius isn't as big as I fear.

Fair points on both sides.  I'd lean slightly toward back-patching
this fix, mostly because of your points (a) and (b).  Without a
back-patch, users like Fran=C3=A7ois would need to adjust affected queries
when upgrading from pre-v16 to v16=E2=80=93v18, which feels a bit unfortuna=
te.

- Richard