Horton
Menu

Thread

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Fix var_is_nonnullable() to handle invalid NOT NULL constraints

  1. var_is_nonnullable() fails to handle invalid NOT NULL constraints

    Richard Guo <guofenglinux@gmail.com> — 2026-04-10T08:48:26Z

    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().

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?

- Richard

  2. Re: var_is_nonnullable() fails to handle invalid NOT NULL constraints

    SATYANARAYANA NARLAPURAM <satyanarlapuram@gmail.com> — 2026-04-12T09:33:19Z

    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

  3. Re: var_is_nonnullable() fails to handle invalid NOT NULL constraints

    Richard Guo <guofenglinux@gmail.com> — 2026-04-15T00:51:03Z

    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