public inbox for [email protected]  

help / color / mirror / Atom feed

From: Tom Lane <[email protected]>
To: Richard Guo <[email protected]>
Cc: Sergey Shinderuk <[email protected]>
Cc: [email protected]
Cc: David Rowley <[email protected]>
Subject: Re: BUG #19412: Wrong query result with not null constraint
Date: Thu, 19 Feb 2026 13:25:48 -0500
Message-ID: <[email protected]> (raw)
In-Reply-To: <CAMbWs481y0EmS7fFJE9DEjjHGJO4mh9nPmusPDQ77Psxgo96ww@mail.gmail.com>
References: <[email protected]>
	<CAApHDvpXarz-D6tP-7y4ZrtnhXtdbRY0kQxRDQTVc3Z9DckYVw@mail.gmail.com>
	<CAMbWs49kVgb_wS4VFuQpfZ+Re4tWVn7vmheR6HqKppnNuEaH-g@mail.gmail.com>
	<CAMbWs49NVUNpQPwGb56SCSQL-bkjtxnkCb6Ot7dBxGndJoA3Pw@mail.gmail.com>
	<[email protected]>
	<CAMbWs481y0EmS7fFJE9DEjjHGJO4mh9nPmusPDQ77Psxgo96ww@mail.gmail.com>

Richard Guo <[email protected]> writes:
> On Wed, Feb 18, 2026 at 9:03 PM Sergey Shinderuk
> <[email protected]> wrote:
>> I'm not familiar with the code, just curios. There is a long comment
>> above saying "You might think we need to adjust var->varnullingrels, but
>> that shouldn't need any changes." Doesn't it need an update?

> No, I don't think we need to update it.  That comment explains why
> varnullingrels do not require translation (since they are outer join
> relids, not baserel relids).  It's unrelated to what this patch does,
> which is about propagating varnullingrels into the translated Var.

I agree with this fix: I think the code is like it is simply because
it didn't occur to me that the child Vars could have any nullingrel
bits yet.  However, I don't agree that that comment needs no updates.
I suggest something like

-         * Below, we just propagate var->varnullingrels into the translated
-         * Var.
+         * Below, we just merge var->varnullingrels into the translated
+         * Var.  (We must merge not just copy: the child Var could have
+         * some nullingrel bits set already, and we mustn't drop those.)

Also, I think I'd then drop the comment you added adjacent to the
actual update; it seems redundant if the earlier comment says this.

I agree with back-patching to v16.  This particular example doesn't
misbehave in versions that don't have the drop-allegedly-redundant-
NOT-NULL-tests logic, but the varnullingrels are certainly wrong
all the way back, so possibly there are other examples that do
misbehave in v16.

			regards, tom lane

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: Re: BUG #19412: Wrong query result with not null constraint
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox