Hi, While looking at ScalarArrayOpExpr strictness, I came across what looks like a wrong-result case involving SQL-function inlining.
Here is a small example:
CREATE TEMP TABLE t (x int);
INSERT INTO t VALUES (NULL), (1);
CREATE FUNCTION strict_any(int) RETURNS bool
LANGUAGE SQL STRICT IMMUTABLE AS
$$ SELECT $1 = ANY ('{}'::int[]) $$;
SELECT x IS NULL AS null_input,
strict_any(x) IS NULL AS result_is_null
FROM t
ORDER BY x NULLS FIRST;
Since the function is declared STRICT, I would expect result_is_null to
be true for the NULL input. On current master it is false. EXPLAIN
shows that the function has been inlined as:
x = ANY ('{}'::int[])
For an empty array, NULL = ANY(array) is FALSE rather than NULL, so the
inlined expression seems to bypass the function's NULL-on-NULL-input
behavior. Empty-array ALL expressions appear to have the same issue,
returning TRUE for a NULL scalar input after inlining.
It looks like contain_nonstrict_functions() now relies on
check_functions_in_node() for ScalarArrayOpExpr. That verifies that the
operator function is strict, but it does not account for whether the
array can be empty. Before 2f153ddfdd3, this code used
is_strict_saop() with falseOK = false.
That commit was a mechanical refactor to reduce duplication by routing
the per-node checks through check_functions_in_node(). For the mutable,
volatile and parallel-hazard walkers the SAOP case is a pure operator-
property check, so the conversion was behavior-preserving. The nonstrict
case is different: is_strict_saop(expr, false) also depends on whether
the array can be empty, which a function-OID check cannot express, so the
special case looks like it was dropped inadvertently.
Would restoring that SAOP-specific check be the right approach?
This is a separate issue from, but in the same is_strict_saop() area as,
the nullability question I raised earlier in [1].
Regards,
Ayush
[1]
https://www.postgresql.org/message-id/cajtyswv3vqrjmst-gv1nsxeef-zonjvjpys910abaiumij4...@mail.gmail.com
0001-Fix-SAOP-strictness-check-for-SQL-function-inlining.patch
Description: Binary data
