GREATEST/LEAST ignores comparison operator volatility in contain_volatile_functions_walker
PostgreSQL version: 18.4 (also reproduced on 18.3)
Platform: aarch64-apple-darwin, Apple clang 17
GREATEST and LEAST (MinMaxExpr nodes) are missing from
contain_volatile_functions_walker in src/backend/optimizer/util/clauses.c.
OpExpr nodes are correctly handled -- the walker resolves the oprcode via
set_opfuncid and checks func_volatile. But there is no MinMaxExpr case, so
GREATEST/LEAST always appears function-free to the volatility checker
regardless of the < or > operator's provolatile.
The practical consequence: a GREATEST/LEAST expression whose comparison
operator is STABLE or VOLATILE is incorrectly treated as IMMUTABLE. The
reproducer below demonstrates this via a generated column, which requires
an IMMUTABLE expression. PostgreSQL accepts the column definition when it
should reject it.
-- Reproducer (self-contained, tested on 18.4):
DROP TABLE IF EXISTS t CASCADE;
DROP TYPE IF EXISTS myval CASCADE;
CREATE TYPE myval;
CREATE FUNCTION myval_in(cstring) RETURNS myval
LANGUAGE internal STRICT IMMUTABLE AS 'textin';
CREATE FUNCTION myval_out(myval) RETURNS cstring
LANGUAGE internal STRICT IMMUTABLE AS 'textout';
CREATE TYPE myval (INPUT = myval_in, OUTPUT = myval_out, LIKE = text);
-- < is STABLE: result depends on the myval.reverse session GUC
CREATE FUNCTION myval_lt(a myval, b myval) RETURNS boolean
LANGUAGE sql STABLE STRICT AS $$
SELECT CASE current_setting('myval.reverse', true)
WHEN 'on' THEN a::text > b::text
ELSE a::text < b::text
END
$$;
CREATE OPERATOR < (
leftarg = myval, rightarg = myval,
procedure = myval_lt, negator = >=
);
-- btree opclass so GREATEST can resolve a sort operator for myval
CREATE FUNCTION myval_cmp(a myval, b myval) RETURNS integer
LANGUAGE sql IMMUTABLE STRICT AS $$
SELECT CASE WHEN a::text < b::text THEN -1
WHEN a::text > b::text THEN 1
ELSE 0 END
$$;
CREATE OPERATOR CLASS myval_ops DEFAULT FOR TYPE myval USING btree AS
OPERATOR 1 <,
FUNCTION 1 myval_cmp(myval, myval);
CREATE TABLE t (x myval, y myval);
-- Control: a direct call to the Stable function is correctly rejected.
-- ALTER TABLE t ADD COLUMN ctrl boolean
-- GENERATED ALWAYS AS (myval_lt(x, y)) STORED;
-- => ERROR: generation expression is not immutable (correct)
-- Bug: GREATEST uses the same Stable < operator but the missing
-- MinMaxExpr case in contain_volatile_functions_walker means
-- PostgreSQL treats the expression as immutable.
ALTER TABLE t ADD COLUMN z myval
GENERATED ALWAYS AS (GREATEST(x, y)) STORED;
-- Expected: ERROR: generation expression is not immutable
-- Actual: ALTER TABLE (bug: accepted without error)
The fix would be to add a MinMaxExpr case to
contain_volatile_functions_walker that resolves the comparison operator's
oprcode (analogous to the existing OpExpr handling) and checks its
provolatile.
Mats Rydberg <mats@planetscale.com> writes:
GREATEST and LEAST (MinMaxExpr nodes) are missing from
contain_volatile_functions_walker in src/backend/optimizer/util/clauses.c.
OpExpr nodes are correctly handled -- the walker resolves the oprcode via
set_opfuncid and checks func_volatile. But there is no MinMaxExpr case, so
GREATEST/LEAST always appears function-free to the volatility checker
regardless of the < or > operator's provolatile.
This is intentional, per the comment in
contain_mutable_functions_walker (which
contain_volatile_functions_walker refers to):
* It should be safe to treat MinMaxExpr as immutable, because it will
* depend on a non-cross-type btree comparison function, and those should
* always be immutable.
A non-cross-type btree comparison function directly determines the
ordering of an index for its data type, so if it isn't immutable then
you can't rely on the index to be consistent.
The practical consequence: a GREATEST/LEAST expression whose comparison
operator is STABLE or VOLATILE is incorrectly treated as IMMUTABLE. The
reproducer below demonstrates this via a generated column, which requires
an IMMUTABLE expression. PostgreSQL accepts the column definition when it
should reject it.
This reproducer depends on an invalid operator class. The reason
why you can't make an operator class without superuser privilege
is that the system depends on them behaving per spec. We'd try
to enforce that rather than just assume it, were it not for the
halting problem.
regards, tom lane