From: Tom Lane <tgl@sss.pgh.pa.us>
To: Dean Rasheed <dean.a.rasheed@gmail.com>
cc: Richard Guo <guofenglinux@gmail.com>, exclusion@gmail.com,
        pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #18634: Wrong varnullingrels with merge ... when not matched
 by source
In-reply-to: 
 <CAEZATCWcks92b7YXukLWN8FOOx_Di7sj4_qao=q==6MT_oAoOA@mail.gmail.com>
References: <18634-db5299c937877f2b@postgresql.org>
 <374420.1727390489@sss.pgh.pa.us>
 <CAMbWs4_GGgGaU=oT0zt0rR0uosdjWVZ0Ts5js5dgM21aQSdM5w@mail.gmail.com>
 <CAEZATCWcks92b7YXukLWN8FOOx_Di7sj4_qao=q==6MT_oAoOA@mail.gmail.com>
Comments: In-reply-to Dean Rasheed <dean.a.rasheed@gmail.com>
	message dated "Fri, 27 Sep 2024 16:06:12 +0100"
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <491921.1727451801.1@sss.pgh.pa.us>
Date: Fri, 27 Sep 2024 11:43:21 -0400
Message-ID: <491922.1727451801@sss.pgh.pa.us>
Archived-At: 
 <https://www.postgresql.org/message-id/491922.1727451801%40sss.pgh.pa.us>
Precedence: bulk

Dean Rasheed <dean.a.rasheed@gmail.com> writes:
> On Fri, 27 Sept 2024 at 13:52, Richard Guo <guofenglinux@gmail.com> wrote:
>> I think we can check the jointype of the join between the target and
>> the source relation when adding the vars in mergeJoinCondition.  If it
>> is JOIN_LEFT, we mark the vars that belong to source as nullable by
>> this join.

> Another option is to do it in transform_MERGE_to_join(). That feels
> safer, because the jointree might have been modified by the time we
> reach preprocess_targetlist().

Yeah, I think it's critical that these Vars be already correctly
marked before we engage in all the slicing-and-dicing that
prepjointree et al will do.  As an example, it seems not impossible
for join removal to make wrong decisions if they aren't.

> Something like the attached.

Could use some comments ... but actually, now I'm confused about why
any of this is the right thing at all:

+	 * Similarly, any non-target Vars in the join condition will be added to
+	 * the targetlist by preprocess_targetlist(), and so must be marked as
+	 * nullable by the join, for LEFT and FULL joins.

Why do we need these Vars in the tlist?  If they're for re-evaluating
the join condition, isn't the already-nulled form of them the wrong
thing?

			regards, tom lane