MIME-Version: 1.0
References: 
 <CAMbWs48ALW=mR0ydQ62dGS-Q+3D7WdDSh=EWDezcKp19xi=TUA@mail.gmail.com>
In-Reply-To: 
 <CAMbWs48ALW=mR0ydQ62dGS-Q+3D7WdDSh=EWDezcKp19xi=TUA@mail.gmail.com>
From: SATYANARAYANA NARLAPURAM <satyanarlapuram@gmail.com>
Date: Sun, 12 Apr 2026 02:33:19 -0700
Message-ID: 
 <CAHg+QDcXStdF1m4ZbgT5qnRL_BruLSyBDGgRsibEFutKM2Jqxw@mail.gmail.com>
Subject: Re: var_is_nonnullable() fails to handle invalid NOT NULL constraints
To: Richard Guo <guofenglinux@gmail.com>
Cc: Pg Hackers <pgsql-hackers@lists.postgresql.org>
Content-Type: multipart/alternative; boundary="0000000000001cdc7d064f400f01"
Archived-At: 
 <https://www.postgresql.org/message-id/CAHg%2BQDcXStdF1m4ZbgT5qnRL_BruLSyBDGgRsibEFutKM2Jqxw%40mail.gmail.com>
Precedence: bulk

--0000000000001cdc7d064f400f01
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

Hi RIchard,

On Fri, Apr 10, 2026 at 1:48=E2=80=AFAM Richard Guo <guofenglinux@gmail.com=
> wrote:

> While fixing another bug in var_is_nonnullable(), I noticed $subject.
> The NOTNULL_SOURCE_SYSCACHE code path (newly added for the NOT IN to
> anti-join transformation) checks pg_attribute.attnotnull, which can be
> true even for invalid (NOT VALID) NOT NULL constraints.
>
> The consequence is that query_outputs_are_not_nullable() could wrongly
> conclude that a subquery's output is non-nullable, causing NOT IN to
> be incorrectly converted to an anti-join.
>
> The attached fix checks the attnullability field in the relation's
> tuple descriptor instead, which correctly distinguishes valid from
> invalid constraints.  This is also consistent with what we do in
> get_relation_notnullatts().
>

I tested this patch against the current HEAD (155c03ee) and it looks good.
Build & tests: Applies cleanly, compiles without warnings, all 247
regression tests
pass including the new subselect test case. Reproduced the bug before the
patch
and verified it is fixed after the patch.


> It could be argued that the added table_open/table_close call is a
> performance concern, but I don't think so:
>
> 1. The relation is already locked by the rewriter, so
> table_open(rte->relid, NoLock) is just a relcache lookup.
>
> 2. This code path is only reached when converting NOT IN to an
> anti-join, and only after the outer side of the test expression has
> already been proved non-nullable.
>
> 3. It is only called for relation RTEs in the subquery.
>
> Thoughts?
>

Looks like it needs to perform table_open/table_close multiple times
depending upon
the number of output columns? I don't see it as a major concern but let
others comment.

Thanks,
Satya

--0000000000001cdc7d064f400f01
Content-Type: text/html; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

<div dir=3D"ltr"><div dir=3D"ltr">Hi RIchard,</div><br><div class=3D"gmail_=
quote gmail_quote_container"><div dir=3D"ltr" class=3D"gmail_attr">On Fri, =
Apr 10, 2026 at 1:48=E2=80=AFAM Richard Guo &lt;<a href=3D"mailto:guofengli=
nux@gmail.com">guofenglinux@gmail.com</a>&gt; wrote:<br></div><blockquote c=
lass=3D"gmail_quote" style=3D"margin:0px 0px 0px 0.8ex;border-left:1px soli=
d rgb(204,204,204);padding-left:1ex">While fixing another bug in var_is_non=
nullable(), I noticed $subject.<br>
The NOTNULL_SOURCE_SYSCACHE code path (newly added for the NOT IN to<br>
anti-join transformation) checks pg_attribute.attnotnull, which can be<br>
true even for invalid (NOT VALID) NOT NULL constraints.<br>
<br>
The consequence is that query_outputs_are_not_nullable() could wrongly<br>
conclude that a subquery&#39;s output is non-nullable, causing NOT IN to<br=
>
be incorrectly converted to an anti-join.<br>
<br>
The attached fix checks the attnullability field in the relation&#39;s<br>
tuple descriptor instead, which correctly distinguishes valid from<br>
invalid constraints.=C2=A0 This is also consistent with what we do in<br>
get_relation_notnullatts().<br></blockquote><div><br></div><div>I tested th=
is patch against the current HEAD (155c03ee) and it looks good.</div><div>B=
uild &amp; tests: Applies cleanly, compiles without warnings, all 247 regre=
ssion tests=C2=A0</div><div>pass including the new subselect test case. Rep=
roduced the bug before the patch</div><div>and verified it is fixed after t=
he patch.</div><div><br></div><blockquote class=3D"gmail_quote" style=3D"ma=
rgin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:=
1ex">
<br>
It could be argued that the added table_open/table_close call is a<br>
performance concern, but I don&#39;t think so:<br>
<br>
1. The relation is already locked by the rewriter, so<br>
table_open(rte-&gt;relid, NoLock) is just a relcache lookup.<br>
<br>
2. This code path is only reached when converting NOT IN to an<br>
anti-join, and only after the outer side of the test expression has<br>
already been proved non-nullable.<br>
<br>
3. It is only called for relation RTEs in the subquery.<br>
<br>
Thoughts?<br></blockquote><div><br></div><div>Looks like it needs to perfor=
m table_open/table_close multiple times depending upon</div><div>the number=
 of output columns? I don&#39;t see it as a major concern but let others co=
mment.</div><div><br></div><div>Thanks,</div><div>Satya</div></div></div>

--0000000000001cdc7d064f400f01--