Volatile expressions in SQL/JSON functions are evaluated twice

Hi,

When an SQL/JSON function wraps its input in a JsonValueExpr, the
input expression is evaluated twice. On 18.6:

CREATE TABLE log (i int);
CREATE FUNCTION f() RETURNS text LANGUAGE sql VOLATILE
AS $$ INSERT INTO log VALUES (1) RETURNING '{"a": 1}' $$;
SELECT JSON_QUERY(f(), '$.a');
SELECT count(*) FROM log;
count
-------
2

Counting calls of nextval() instead, each of these calls it twice on
18.6 (master, as of aa7b9cf13, has the same code):

JSON_QUERY(nextval('s')::text, '$') -- non-jsonb context item
SELECT * FROM JSON_TABLE(nextval('s')::text, '$' COLUMNS (v int PATH '$'))
JSON_VALUE(jsonb 'null', '$x' PASSING nextval('s')::dint AS x)
-- dint is a domain over int
JSON_OBJECT('a': nextval('s')::text FORMAT JSON)
JSON_ARRAY(nextval('s')::text FORMAT JSON)
JSON_ARRAYAGG(nextval('s')::text FORMAT JSON) -- twice per row
JSON_OBJECTAGG('k' || g: nextval('s')::text FORMAT JSON)
JSON(nextval('s')::text::bytea FORMAT JSON)

It's also visible with a subquery: in EXPLAIN ANALYZE of
SELECT JSON_QUERY((SELECT f()), '$.a'), there are two identical
InitPlans, and both are executed.

The cause is the T_JsonValueExpr case in ExecInitExprRec(). Since
11c87216d, the fix for bug #18657, it recurses into both raw_expr and
formatted_expr. Initializing raw_expr is needed there, so that Aggrefs
in it are linked into the parent Agg node. But the steps it emits are
also executed at runtime, although, as that commit message says, the
value of raw_expr is not used: formatted_expr contains the same
expression, possibly coerced, and its value overwrites that of
raw_expr. 11c87216d was back-patched to 16, so this is a regression in
16.5 and 17.1; before that, only formatted_expr was evaluated.

The attached patch still emits the steps for raw_expr, so that its
initialization is unchanged, but jumps over them. It also updates the
JsonValueExpr comment in primnodes.h, which said that formatted_expr
"takes precedence" over raw_expr. With the patch, each of the above
calls nextval() once, and in the EXPLAIN ANALYZE above the InitPlan
from raw_expr is shown as never executed. I also checked Aggrefs,
window functions, GROUPING() and correlated subqueries in raw_expr.

Tests: the test from bug #18657 still passes, the new tests fail
without the fix, and make check also passes with jit_above_cost = 0 on
an LLVM build. The patch applies to REL_17_STABLE and REL_18_STABLE.
For REL_16_STABLE, the sqljson_queryfuncs hunks have to be dropped,
since JSON_QUERY() and JSON_TABLE() don't exist there. With that, it
applies, fixes JSON_OBJECT() and JSON_ARRAY() on 16 too (I checked on
16.15), and make check passes.

One case the patch doesn't change: in the query from bug #18657,

SELECT JSON_OBJECT('a': JSON_OBJECTAGG('b': volatile_one()
RETURNING text) FORMAT JSON);

volatile_one() is still called twice per row, before and after the
patch. There the planner computes the Aggref in raw_expr and the one in
formatted_expr as two separate aggregates. Fixing that would mean not
duplicating the expression in the parse tree, as Tom suggested in the
#18657 thread [1]. That can't be back-patched because of stored rules,
but I could look into it for master if that seems worthwhile.

[1] https://postgr.es/m/569213.1729138329@sss.pgh.pa.us

--
Ilya Staroverov