Re: Hash a ScalarArrayOpExpr whose array is fixed for one execution

[Ayoub Kazar]

Hello Linden, David,

On 29/10/2026 10:46, David Geier wrote:

A comment saying why we cannot unify the Const and non-Const case in
execExprInterp.c would be useful. One could think that it might be a
good idea to now always go the non-Const route for code simplicity /
consistency.

Done, above saop_hash_eval_array(). The non-Const route copies the
array, and a Const array needs no copy, because it is part of the plan.

But we could copy it, couldn't we? I'm more looking for a reason like
performance. As in, if we unified the code, what would happen that we
don't want to happen? Because we would get rid of a bunch of if-else and
harder to understand code if also Const arrays went through array_expr.

+1 to simplifying the code, but we should be careful about hot-path overhead. In master, Const arrays are pre-validated at plan time, so ExecEvalHashedScalarArrayOp runs without checking for NULL, empty, or short arrays on every tuple (aren't those for the whole execution? so why check every time?). In v4, having this in the inner loop for every row is not insignificant.

Doesn't saop_array_may_vary_walker() misses T_CaseWhen? T_CaseExpr's
args is a list of CaseWhen nodes. When recursing it doesn't match
against T_CaseWhen and returns true while CaseWhen doesn't vary. I think
that would only occur in this form:

x = ANY(CASE WHEN $2 THEN $1 ELSE '{...}' END

This form is hashed. The walker never gets a CaseWhen node, so
T_CaseWhen is not needed. I went through expression_tree_walker() and
ran a test to confirm this.

Ah, this is because raw_expression_tree_walker_impl() skips T_WhenExpr
and directly recurses into its contents. You could add the case with a
comment to be on the safe side, if the code ever gets changed.

AI suggests also NamedArtExpr and CollateExpr, i think they are safe as well to allow.

Other findings:




>> Open question
>> ------------
>> cost_qual_eval charges the hashed cost for an opaque stable array using
>> estimate_array_length()'s default guess, which slightly over-estimates
>> startup for an array that turns out short at run time. I have left that
>> for a follow-up; happy to fold in a fix if reviewers prefer.
> What are your ideas for improving on that? You cannot know the runtime
> array length at planning time.


Here's a case:

CREATE TABLE large_bench (id bigint, cat int, val text);

CREATE INDEX idx_large_bench_cat ON large_bench(cat);
ANALYZE large_bench;
PREPARE p_query(int[]) AS SELECT * FROM large_bench WHERE cat = ANY($1);
SET plan_cache_mode = force_generic_plan;

Optimizer Costs:
Master: Bitmap Index Scan (187,282) < Seq Scan (203,410)


test executing with |ARRAY[1, 2]|:
Cold Cache:
Patched Seq Scan: 1,519 ms (reads 168k buffer pages, filters 4.09M rows)
Unpatched Index Scan: 129 ms (reads 24k buffer pages)
Impact: 11.7x slowdown
Warm Cache (5-run average):
Patched Seq Scan: 233.8 ms (touches all 90,910 table blocks)
Unpatched Index Scan: 84.1 ms (touches only 22,730 blocks)
Impact: 2.8x slowdown and 4x more buffer I/O



Regards,
Ayoub

While it is true that val <> ALL(array) == NOT(val = ANY (array)) (in Logic), however for user defined types and operators that are not strict, comparing NULL with a value can return false for both = and <> ... So this makes the equivalence in that case not always true. (Attached is a reprod of the bug) We should be cautious about charging an O(1) per-tuple cost during generic planning when the array length is unknown, given that the executor can unilaterally decide to abandon hashing and execute an O(N) linear scan instead. INSERT INTO large_bench SELECT g, (g / 100) % 11, repeat('x', 100) FROM generate_series(1, 5000000) g; Patched v4: Seq Scan (165,910) < Bitmap Index Scan (187,282) (because of the cheap cost per tuple) I think costing both cases (array length known or unknown) as unhashed linear scans isn't a bad idea maybe, i admit i haven't thought a lot about it yet.

non-strict-reprod.sql
Description: application/sql