var_is_nonnullable() fails to handle invalid NOT NULL constraints

Hi RIchard,

On Fri, Apr 10, 2026 at 1:48 AM 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

On Sun, Apr 12, 2026 at 6:33 PM SATYANARAYANA NARLAPURAM
<satyanarlapuram@gmail.com> wrote:

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.

Thanks for testing.

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.

I don't see it as a major concern either performance-wise. table_open
here is just a hash lookup and refcount increment, and table_close is
just a refcount decrement. And the call only happens in NOT IN cases,
and only after sublink_testexpr_is_not_nullable has already returned
true.

So I've committed this patch.

- Richard