MIME-Version: 1.0
References: <eaa2598c-5356-4e1e-9ec3-5fd6eb1cd704@tantorlabs.com>
 <52596e08-0902-475b-ad0f-265ec6918f19@gmail.com>
 <7791ec74-369d-48c7-969c-cf4ad18024f2@tantorlabs.com>
 <ed61316c-2e48-45d1-9cca-dc539bfd93b9@tantorlabs.com>
 <dd5e91c1-5e59-471c-bd9d-96a11c7928f4@tantorlabs.com>
 <6b3aebb5-f26c-4f50-abd6-e733d452af26@gmail.com>
 <f43547c8-1a21-4337-98f9-f6a1d2b8fd2b@tantorlabs.com>
 <CAN4CZFNJHro_pEVTHggBX43f+AebwKLZdp39+V4pV0EB4BZuZw@mail.gmail.com>
 <52f3c637-465d-4f0e-9546-732ecf61ab87@tantorlabs.com>
 <3fc74079-f429-48c8-ad8c-688b21d4d1c1@gmail.com>
 <4c761b02-5a60-4076-aa0c-9c6fef06e2c1@tantorlabs.com>
 <CAN4CZFOTAyVk9srBp560049Xu655xUDZv5NBTvj7RYND-xKymA@mail.gmail.com>
 <2bdf4a7e-f5b9-4e75-b5a3-1e2608fd3297@tantorlabs.com>
In-Reply-To: <2bdf4a7e-f5b9-4e75-b5a3-1e2608fd3297@tantorlabs.com>
From: David Rowley <dgrowleyml@gmail.com>
Date: Tue, 3 Mar 2026 14:08:19 +1300
Message-ID: 
 <CAApHDvphShGABn-3AoE36dTvGHW7gUpFSw0_ZZnH84wGCW3hHw@mail.gmail.com>
Subject: Re: Reduce planning time for large NOT IN lists containing NULL
To: Ilia Evdokimov <ilya.evdokimov@tantorlabs.com>
Cc: Zsolt Parragi <zsolt.parragi@percona.com>,
 David Geier <geidav.pg@gmail.com>,
	PostgreSQL Developers <pgsql-hackers@lists.postgresql.org>
Content-Type: text/plain; charset="UTF-8"
Archived-At: 
 <https://www.postgresql.org/message-id/CAApHDvphShGABn-3AoE36dTvGHW7gUpFSw0_ZZnH84wGCW3hHw%40mail.gmail.com>
Precedence: bulk

On Tue, 3 Mar 2026 at 04:04, Ilia Evdokimov
<ilya.evdokimov@tantorlabs.com> wrote:
> I've fixed this in v5-patch.

I had a look at this and wondered if we guarantee that no rows will
match, then why can't we perform constant folding on the
ScalarArrayOpExpr when !useOr and the array contains a NULL element
and the operator is strict. Seemingly, one of the reasons for that is
down to the expression returning NULL vs false. Take the following two
tests from expressions.out:

select return_int_input(1) not in (10, 9, 2, 8, 3, 7, 4, 6, 5, 2, null);
 ?column?
----------

(1 row)

select return_int_input(1) not in (10, 9, 2, 8, 3, 7, 4, 6, 5, 1, null);
 ?column?
----------
 f
(1 row)

Here we see that we return false when we find the left operand in the
array, but NULL when we don't find it and the array contains NULL. So,
unless the left operand is a const, we wouldn't know how to simplify
the ScalarArrayOpExpr during planning as the false or NULL would only
be known during evaluation of the expression.

However, when the expression being simplified is an EXPRKIND_QUAL, it
shouldn't matter if the result is false or NULL as both mean the same
and there shouldn't be any code that cares about the difference.
Currently, we don't pass the "kind" down into
eval_const_expressions(), but I don't really see why we couldn't. It
would be a fair bit of work figuring out with confidence what the
extra arg should be passed as in all the existing call sites of that
function. We'd have to document in the header comment for
eval_const_expressions() that constant-folding on EXPRKIND_QUAL
expressions can enable additional optimisations which disregard the
difference between NULL and false.

For the patch, I imagine it's still a useful optimisation as the
ScalarArrayOpExpr might not be in an EXPRKIND_QUAL.

There are a couple of things I don't like:

1) The new test is in expressions.sql. The comment at the top of that
file reads: "expression evaluation tests that don't fit into a more
specific file". The new test isn't anything to do with expression
evaluation. It's about planner estimation. I see that
misc_function.sql has the explain_mask_costs() function. I'm not sure
that's the right place either, as the usages of that function are for
testing SupportRequestRows prosupport functions. I wonder if we need a
dedicated row_estimate.sql or selectivity_est.sql file. The
explain_mask_costs() wouldn't be out of place if they were moved into
a new test like that. It was me that started putting those in
misc_function.sql, and I don't object to them being moved to a new
test. I'd be as a separate commit, however.

2) The new test creates a new table and inserts 1000 rows. There does
not seem to be anything special about the new table. Why don't you use
one of the ones from test_setup.sql?

3) Looking at var_eq_const(), it seems like it's coded to assume the
operator is always strict, per "If the constant is NULL, assume
operator is strict and return zero". If that's good enough for
var_eq_const(), then it should be good enough for the new code. I
think it would be good if you wrote that or something similar in the
new code so that the reader knows taking the short-circuit with
non-strict functions is on purpose.

David