From: Andrey Rachitskiy Date: Sun, 16 Aug 2026 19:24:00 +0500 Subject: [PATCH] Reset JsonExpr empty/error flags before NULL short-circuit SQL NULL context/path skips EEOP_JSONEXPR_PATH (JUMP_IF_NULL to CONST NULL) so jsonpath is not evaluated, then falls through to domain coercion and ON EMPTY / ON ERROR checks. empty/error were cleared only inside PATH, so a previous row's EMPTY/ERROR leaked onto the NULL row when DEFAULT was not NULL. Clear the flags in a new EEOP_JSONEXPR_RESET at the start of each JsonExpr evaluation. BUG #19621 Reported-by: Suyang Zhong Author: Andrey Rachitskiy Discussion: https://www.postgresql.org/message-id/19621-0a480d2dc74e6bd5%40postgresql.org Backpatch-through: 17 --- diff --git a/src/backend/executor/execExpr.c b/src/backend/executor/execExpr.c index cfea7e160c2..a3050576cb3 100644 --- a/src/backend/executor/execExpr.c +++ b/src/backend/executor/execExpr.c @@ -4759,6 +4759,11 @@ ExecInitJsonExpr(JsonExpr *jsexpr, ExprState *state, jsestate->jsexpr = jsexpr; + /* Clear empty/error here. SQL NULL skips PATH. */ + scratch->opcode = EEOP_JSONEXPR_RESET; + scratch->d.jsonexpr.jsestate = jsestate; + ExprEvalPushStep(state, scratch); + /* * Evaluate formatted_expr storing the result into * jsestate->formatted_expr. diff --git a/src/backend/executor/execExprInterp.c b/src/backend/executor/execExprInterp.c index 9bc23cb16fa..a82f9ee11a5 100644 --- a/src/backend/executor/execExprInterp.c +++ b/src/backend/executor/execExprInterp.c @@ -578,6 +578,7 @@ ExecInterpExpr(ExprState *state, ExprContext *econtext, bool *isnull) &&CASE_EEOP_XMLEXPR, &&CASE_EEOP_JSON_CONSTRUCTOR, &&CASE_EEOP_IS_JSON, + &&CASE_EEOP_JSONEXPR_RESET, &&CASE_EEOP_JSONEXPR_PATH, &&CASE_EEOP_JSONEXPR_COERCION, &&CASE_EEOP_JSONEXPR_COERCION_FINISH, @@ -1936,6 +1937,13 @@ ExecInterpExpr(ExprState *state, ExprContext *econtext, bool *isnull) EEO_NEXT(); } + EEO_CASE(EEOP_JSONEXPR_RESET) + { + ExecEvalJsonExprReset(state, op); + + EEO_NEXT(); + } + EEO_CASE(EEOP_JSONEXPR_PATH) { /* too complex for an inline implementation */ @@ -4890,6 +4898,28 @@ ExecEvalJsonIsPredicate(ExprState *state, ExprEvalStep *op) *op->resvalue = BoolGetDatum(res); } +/* + * ExecEvalJsonExprReset + * Clear empty/error and ErrorSaveContext for this evaluation. + * + * SQL NULL skips EEOP_JSONEXPR_PATH, so this cannot live there. + */ +void +ExecEvalJsonExprReset(ExprState *state, ExprEvalStep *op) +{ + JsonExprState *jsestate = op->d.jsonexpr.jsestate; + + memset(&jsestate->error, 0, sizeof(NullableDatum)); + memset(&jsestate->empty, 0, sizeof(NullableDatum)); + + if (jsestate->escontext.details_wanted) + { + jsestate->escontext.error_data = NULL; + jsestate->escontext.details_wanted = false; + } + jsestate->escontext.error_occurred = false; +} + /* * Evaluate a jsonpath against a document, both of which must have been * evaluated and their values saved in op->d.jsonexpr.jsestate. @@ -4922,18 +4952,6 @@ ExecEvalJsonExprPath(ExprState *state, ExprEvalStep *op, item = jsestate->formatted_expr.value; path = DatumGetJsonPathP(jsestate->pathspec.value); - /* Set error/empty to false. */ - memset(&jsestate->error, 0, sizeof(NullableDatum)); - memset(&jsestate->empty, 0, sizeof(NullableDatum)); - - /* Also reset ErrorSaveContext contents for the next row. */ - if (jsestate->escontext.details_wanted) - { - jsestate->escontext.error_data = NULL; - jsestate->escontext.details_wanted = false; - } - jsestate->escontext.error_occurred = false; - switch (jsexpr->op) { case JSON_EXISTS_OP: diff --git a/src/backend/jit/llvm/llvmjit_expr.c b/src/backend/jit/llvm/llvmjit_expr.c index 29617437477..a96c09afe23 100644 --- a/src/backend/jit/llvm/llvmjit_expr.c +++ b/src/backend/jit/llvm/llvmjit_expr.c @@ -2259,6 +2259,12 @@ llvm_compile_expr(ExprState *state) LLVMBuildBr(b, opblocks[opno + 1]); break; + case EEOP_JSONEXPR_RESET: + build_EvalXFunc(b, mod, "ExecEvalJsonExprReset", + v_state, op); + LLVMBuildBr(b, opblocks[opno + 1]); + break; + case EEOP_JSONEXPR_PATH: { JsonExprState *jsestate = op->d.jsonexpr.jsestate; diff --git a/src/backend/jit/llvm/llvmjit_types.c b/src/backend/jit/llvm/llvmjit_types.c index c8a1f841293..654600da5b0 100644 --- a/src/backend/jit/llvm/llvmjit_types.c +++ b/src/backend/jit/llvm/llvmjit_types.c @@ -173,6 +173,7 @@ void *referenced_functions[] = ExecEvalXmlExpr, ExecEvalJsonConstructor, ExecEvalJsonIsPredicate, + ExecEvalJsonExprReset, ExecEvalJsonCoercion, ExecEvalJsonCoercionFinish, ExecEvalJsonExprPath, diff --git a/src/include/executor/execExpr.h b/src/include/executor/execExpr.h index c61b3d624d5..8afc09d5dfa 100644 --- a/src/include/executor/execExpr.h +++ b/src/include/executor/execExpr.h @@ -265,6 +265,7 @@ typedef enum ExprEvalOp EEOP_XMLEXPR, EEOP_JSON_CONSTRUCTOR, EEOP_IS_JSON, + EEOP_JSONEXPR_RESET, EEOP_JSONEXPR_PATH, EEOP_JSONEXPR_COERCION, EEOP_JSONEXPR_COERCION_FINISH, @@ -754,7 +755,7 @@ typedef struct ExprEvalStep JsonIsPredicate *pred; /* original expression node */ } is_json; - /* for EEOP_JSONEXPR_PATH */ + /* for EEOP_JSONEXPR_RESET, EEOP_JSONEXPR_PATH, EEOP_JSONEXPR_COERCION_FINISH */ struct { struct JsonExprState *jsestate; @@ -892,6 +893,7 @@ extern void ExecEvalXmlExpr(ExprState *state, ExprEvalStep *op); extern void ExecEvalJsonConstructor(ExprState *state, ExprEvalStep *op, ExprContext *econtext); extern void ExecEvalJsonIsPredicate(ExprState *state, ExprEvalStep *op); +extern void ExecEvalJsonExprReset(ExprState *state, ExprEvalStep *op); extern int ExecEvalJsonExprPath(ExprState *state, ExprEvalStep *op, ExprContext *econtext); extern void ExecEvalJsonCoercion(ExprState *state, ExprEvalStep *op, diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h index e95ac3eda35..fc8c6e21ad2 100644 --- a/src/include/nodes/execnodes.h +++ b/src/include/nodes/execnodes.h @@ -1099,7 +1099,7 @@ typedef struct DomainConstraintState * State for JsonExpr evaluation, too big to inline. * * This contains the information going into and coming out of the - * EEOP_JSONEXPR_PATH eval step. + * EEOP_JSONEXPR_RESET / EEOP_JSONEXPR_PATH eval steps. */ typedef struct JsonExprState { @@ -1119,7 +1119,7 @@ typedef struct JsonExprState * Output variables that drive the EEOP_JUMP_IF_NOT_TRUE steps that are * added for ON ERROR and ON EMPTY expressions, if any. * - * Reset for each evaluation of EEOP_JSONEXPR_PATH. + * Cleared by EEOP_JSONEXPR_RESET at the start of each evaluation. */ /* Set to true if jsonpath evaluation cause an error. */ @@ -1161,7 +1161,7 @@ typedef struct JsonExprState * not ERROR, a pointer to this is passed to ExecInitExprRec() when * initializing the coercion expressions or to ExecInitJsonCoercion(). * - * Reset for each evaluation of EEOP_JSONEXPR_PATH. + * Reset by EEOP_JSONEXPR_RESET at the start of each evaluation. */ ErrorSaveContext escontext; } JsonExprState; diff --git a/src/test/regress/expected/sqljson_queryfuncs.out b/src/test/regress/expected/sqljson_queryfuncs.out index ff64dce0c59..b5209bae5e7 100644 --- a/src/test/regress/expected/sqljson_queryfuncs.out +++ b/src/test/regress/expected/sqljson_queryfuncs.out @@ -475,6 +475,57 @@ FROM 2 | -1 (3 rows) +-- SQL NULL must not reuse a previous row's ON EMPTY / ON ERROR result. +SELECT x IS NULL AS is_null, + json_value(x, '$.a' RETURNING int DEFAULT 42 ON EMPTY) AS jv +FROM (VALUES ('{}'), (NULL), ('{}')) v(x); + is_null | jv +---------+---- + f | 42 + t | + f | 42 +(3 rows) + +SELECT x IS NULL AS is_null, + json_value(x, 'strict $.a' RETURNING int DEFAULT 42 ON ERROR) AS jv +FROM (VALUES ('1'), (NULL), ('1')) v(x); + is_null | jv +---------+---- + f | 42 + t | + f | 42 +(3 rows) + +SELECT p IS NULL AS is_null, + json_value('{}', p RETURNING int DEFAULT 42 ON EMPTY) AS jv +FROM (VALUES ('$.a'::jsonpath), (NULL), ('$.a'::jsonpath)) v(p); + is_null | jv +---------+---- + f | 42 + t | + f | 42 +(3 rows) + +SELECT x IS NULL AS is_null, + json_query(x, '$.a' DEFAULT '"empty"' ON EMPTY) AS jq +FROM (VALUES ('{}'::jsonb), (NULL), ('{}'::jsonb)) v(x); + is_null | jq +---------+--------- + f | "empty" + t | + f | "empty" +(3 rows) + +SELECT x IS NULL AS is_null, + json_exists(x, 'strict $.a' TRUE ON ERROR) AS je +FROM (VALUES ('1'::jsonb), (NULL), ('1'::jsonb)) v(x); + is_null | je +---------+---- + f | t + t | + f | t +(3 rows) + SELECT JSON_VALUE(jsonb 'null', '$a' PASSING point ' (1, 2 )' AS a); json_value ------------ diff --git a/src/test/regress/sql/sqljson_queryfuncs.sql b/src/test/regress/sql/sqljson_queryfuncs.sql index a69ef253f66..03f6a932b9c 100644 --- a/src/test/regress/sql/sqljson_queryfuncs.sql +++ b/src/test/regress/sql/sqljson_queryfuncs.sql @@ -128,6 +128,27 @@ SELECT FROM generate_series(0, 2) x; +-- SQL NULL must not reuse a previous row's ON EMPTY / ON ERROR result. +SELECT x IS NULL AS is_null, + json_value(x, '$.a' RETURNING int DEFAULT 42 ON EMPTY) AS jv +FROM (VALUES ('{}'), (NULL), ('{}')) v(x); + +SELECT x IS NULL AS is_null, + json_value(x, 'strict $.a' RETURNING int DEFAULT 42 ON ERROR) AS jv +FROM (VALUES ('1'), (NULL), ('1')) v(x); + +SELECT p IS NULL AS is_null, + json_value('{}', p RETURNING int DEFAULT 42 ON EMPTY) AS jv +FROM (VALUES ('$.a'::jsonpath), (NULL), ('$.a'::jsonpath)) v(p); + +SELECT x IS NULL AS is_null, + json_query(x, '$.a' DEFAULT '"empty"' ON EMPTY) AS jq +FROM (VALUES ('{}'::jsonb), (NULL), ('{}'::jsonb)) v(x); + +SELECT x IS NULL AS is_null, + json_exists(x, 'strict $.a' TRUE ON ERROR) AS je +FROM (VALUES ('1'::jsonb), (NULL), ('1'::jsonb)) v(x); + SELECT JSON_VALUE(jsonb 'null', '$a' PASSING point ' (1, 2 )' AS a); SELECT JSON_VALUE(jsonb 'null', '$a' PASSING point ' (1, 2 )' AS a RETURNING point); SELECT JSON_VALUE(jsonb 'null', '$a' PASSING point ' (1, 2 )' AS a RETURNING point ERROR ON ERROR);