Wrong results: NOT IN to anti-join with an upper-level Var in the sub-select's output
Hi,
383eb21eb converts a NOT IN to an anti-join once it can prove both the outer
expressions and the sub-select's output columns non-null. The second proof goes
wrong when an output column is an upper-level Var, and the query then keeps a
row it should have dropped. Column order matters here -- o.b and i.x have to
land on the same attnum:
\pset null '<NULL>'
CREATE TABLE o (b int, a int NOT NULL);
CREATE TABLE i (x int);
INSERT INTO o VALUES (NULL, 1);
INSERT INTO i VALUES (5);
SELECT * FROM o WHERE o.a NOT IN (SELECT o.b FROM i WHERE i.x IS NOT NULL);
b | a
--------+---
<NULL> | 1
(1 row)
The sub-select returns one row and its value is NULL, so the NOT IN is NULL and
that row should not come back:
SELECT o.a, o.b,
(SELECT count(*) FROM i WHERE i.x IS NOT NULL) AS subquery_rows,
(o.a NOT IN (SELECT o.b FROM i WHERE i.x IS NOT NULL)) AS
not_in_value
FROM o;
a | b | subquery_rows | not_in_value
---+--------+---------------+--------------
1 | <NULL> | 1 | <NULL>
(1 row)
and writing the same predicate so that it can't be pulled up agrees:
SELECT * FROM o
WHERE (o.a NOT IN (SELECT o.b FROM i WHERE i.x IS NOT NULL)) OR false;
b | a
---+---
(0 rows)
EXPLAIN (COSTS OFF) on the first query gets
Nested Loop Anti Join
Join Filter: (o.a = o.b)
-> Seq Scan on o
-> Materialize
-> Seq Scan on i
Filter: (x IS NOT NULL)
When expr_is_nonnullable() can't prove an output column non-null,
query_outputs_are_not_nullable() falls back to find_nonnullable_vars() over the
quals find_subquery_safe_quals() collected. find_nonnullable_vars() only
reports Vars of the current level, and the multibitmapset it returns identifies
them by varno and varattno alone, so the upper-level o.b -- varno 1, varattno 1
in the outer rangetable -- matches i.x, varno 1, varattno 1 in the sub-select's.
var_is_nonnullable() turns upper-level Vars away; this fallback doesn't:
- if (IsA(expr, Var))
+ if (IsA(expr, Var) && ((Var *) expr)->varlevelsup == 0)There is nothing else to try for an upper-level Var there anyway, since
query_outputs_are_not_nullable() only has the sub-Query to work with. Patch
attached, with a test; the test fails without the one-line change, and make
check is green with it.
383eb21eb is in 19beta1, so this wants fixing on that branch too.
The same weakness has a second instance in the follow-up patch on [1]/messages/by-id/CAMbWs4_TNUs1jn7q0J-=Esz7ziiFdjDAtW4x2u6tv6H5hhmhDA@mail.gmail.com, which
adds an outer-side proof with its own find_nonnullable_vars() lookup; Ayush
reported that one there [2]/messages/by-id/CAJTYsWWH6Pm2xiq=OM3vnBEBz5=Jp93G5_VUSKKGdkXinbXzZw@mail.gmail.com. reduce_outer_joins() also calls
find_nonnullable_vars(), but it only compares two such sets, both built at the
same level, so it isn't affected -- those two are the places that match a single
Var against the set.
[1]: /messages/by-id/CAMbWs4_TNUs1jn7q0J-=Esz7ziiFdjDAtW4x2u6tv6H5hhmhDA@mail.gmail.com
[2]: /messages/by-id/CAJTYsWWH6Pm2xiq=OM3vnBEBz5=Jp93G5_VUSKKGdkXinbXzZw@mail.gmail.com
Thanks,
Rui
Attachments:
v1-0001-notin-subselect-upper-level-output-var.patch.txttext/plain; charset=US-ASCII; name=v1-0001-notin-subselect-upper-level-output-var.patch.txtDownload+53-7
Rui Zhao <zhaorui126@gmail.com> 于2026年7月31日周五 14:02写道：
Hi,
\pset null '<NULL>'
CREATE TABLE o (b int, a int NOT NULL);
CREATE TABLE i (x int);
INSERT INTO o VALUES (NULL, 1);
INSERT INTO i VALUES (5);SELECT * FROM o WHERE o.a NOT IN (SELECT o.b FROM i WHERE i.x IS NOT NULL);
b | a
--------+---
<NULL> | 1
(1 row)When expr_is_nonnullable() can't prove an output column non-null,
query_outputs_are_not_nullable() falls back to find_nonnullable_vars() over the
quals find_subquery_safe_quals() collected. find_nonnullable_vars() only
reports Vars of the current level, and the multibitmapset it returns identifies
them by varno and varattno alone, so the upper-level o.b -- varno 1, varattno 1
in the outer rangetable -- matches i.x, varno 1, varattno 1 in the sub-select's.
var_is_nonnullable() turns upper-level Vars away; this fallback doesn't:- if (IsA(expr, Var)) + if (IsA(expr, Var) && ((Var *) expr)->varlevelsup == 0)There is nothing else to try for an upper-level Var there anyway, since
query_outputs_are_not_nullable() only has the sub-Query to work with. Patch
attached, with a test; the test fails without the one-line change, and make
check is green with it.
query_outputs_are_not_nullable() forgot to process the upper-level reference.
I think it is rare for the output columns of a subquery to include
columns from the parent query.
So we just add ((Var *) expr)->varlevelsup == 0 here; it is ok for
v19. We can support more kinds of NOT IN pull-up in future versions.
In my opinion, it's better to add comments before "if (IsA(expr, Var)
&& ((Var *) expr)->varlevelsup == 0)". The original comment inside the
if can remain unchanged.
--
Thanks,
Tender Wang
Hi Tender,
Thanks for looking. Moved the comment ahead of the test and left the one
inside the if alone, as you suggested; v2 attached. make check is still green
and the test still fails without the code change.
Just to be clear about what the check costs us: nothing that works today.
expr_is_nonnullable() already declines upper-level Vars, so the only way the
fallback was reaching one was by matching it against a local Var of an
unrelated relation -- there was never a correct conversion there to lose. And
proving an upper-level output non-null would take the outer query's range
table, which query_outputs_are_not_nullable() isn't given, so that would want a
different mechanism rather than a relaxation of this test.
Thanks,
Rui
Attachments:
v2-0001-Don-t-prove-a-sub-select-s-upper-level-output-Var.patchapplication/octet-stream; name=v2-0001-Don-t-prove-a-sub-select-s-upper-level-output-Var.patchDownload+48-2
On Sat, Aug 1, 2026 at 12:26 AM Rui Zhao <zhaorui126@gmail.com> wrote:
Thanks for looking. Moved the comment ahead of the test and left the one
inside the if alone, as you suggested; v2 attached. make check is still green
and the test still fails without the code change.
Thanks for the report and the patch!
The patch LGTM, except that the comment is overly verbose. Also I'd
prefer to reuse existing tables instead of creating new ones in the
test case.
I've made some tweaks and then pushed the patch.
- Richard