From ab0e6db5fe8aa5aac591ed2c04a17023dad712f2 Mon Sep 17 00:00:00 2001 From: jian he Date: Sun, 27 Sep 2026 00:04:56 +0800 Subject: [PATCH v3 1/1] Handle SQL NULL inside EEOP_JSONEXPR_PATH SQL NULL context/path used JUMP_IF_NULL to skip EEOP_JSONEXPR_PATH and land on a CONST NULL step. empty/error were cleared only inside PATH, so a previous row's EMPTY/ERROR leaked onto the NULL row when DEFAULT was not NULL, because execution then fell through the ON EMPTY / ON ERROR checks. Point those JUMP_IF_NULL steps at EEOP_JSONEXPR_PATH instead, drop the CONST NULL landing pad, and return NULL from ExecEvalJsonExprPath after clearing the flags (still jumping to domain coercion when needed). Jsonpath is still not evaluated for NULL input. ON EMPTY / ON ERROR are skipped as intended. BUG #19621 Reported-by: Suyang Zhong Author: jian he Discussion: https://www.postgresql.org/message-id/19621-0a480d2dc74e6bd5@postgresql.org Backpatch-through: 17 --- src/backend/executor/execExpr.c | 29 ++++----- src/backend/executor/execExprInterp.c | 23 ++++++- .../regress/expected/sqljson_queryfuncs.out | 61 +++++++++++++++++++ src/test/regress/sql/sqljson_queryfuncs.sql | 24 ++++++++ 4 files changed, 116 insertions(+), 21 deletions(-) diff --git a/src/backend/executor/execExpr.c b/src/backend/executor/execExpr.c index 82e846a1f4f..18b33b24282 100644 --- a/src/backend/executor/execExpr.c +++ b/src/backend/executor/execExpr.c @@ -4807,6 +4807,17 @@ ExecInitJsonExpr(JsonExpr *jsexpr, ExprState *state, jsestate->args = lappend(jsestate->args, var); } + /* + * Adjust jump target addresses of JUMPs that we added above to point to + * the EEOP_JSONEXPR_PATH step, which returns NULL when either + * formatted_expr or pathspec is NULL. + */ + foreach(lc, jumps_return_null) + { + ExprEvalStep *as = &state->steps[lfirst_int(lc)]; + + as->d.jump.jumpdone = state->steps_len; + } /* Step for jsonpath evaluation; see ExecEvalJsonExprPath(). */ scratch->opcode = EEOP_JSONEXPR_PATH; scratch->resvalue = resv; @@ -4814,24 +4825,6 @@ ExecInitJsonExpr(JsonExpr *jsexpr, ExprState *state, scratch->d.jsonexpr.jsestate = jsestate; ExprEvalPushStep(state, scratch); - /* - * Step to return NULL after jumping to skip the EEOP_JSONEXPR_PATH step - * when either formatted_expr or pathspec is NULL. Adjust jump target - * addresses of JUMPs that we added above. - */ - foreach(lc, jumps_return_null) - { - ExprEvalStep *as = &state->steps[lfirst_int(lc)]; - - as->d.jump.jumpdone = state->steps_len; - } - scratch->opcode = EEOP_CONST; - scratch->resvalue = resv; - scratch->resnull = resnull; - scratch->d.constval.value = (Datum) 0; - scratch->d.constval.isnull = true; - ExprEvalPushStep(state, scratch); - escontext = jsexpr->on_error->btype != JSON_BEHAVIOR_ERROR ? &jsestate->escontext : NULL; diff --git a/src/backend/executor/execExprInterp.c b/src/backend/executor/execExprInterp.c index 397219f7a3a..900946ae32e 100644 --- a/src/backend/executor/execExprInterp.c +++ b/src/backend/executor/execExprInterp.c @@ -4893,6 +4893,7 @@ ExecEvalJsonIsPredicate(ExprState *state, ExprEvalStep *op) /* * Evaluate a jsonpath against a document, both of which must have been * evaluated and their values saved in op->d.jsonexpr.jsestate. + * (If the document is NULL, the jsonpath evaluation is skipped) * * If an error occurs during JsonPath* evaluation or when coercing its result * to the RETURNING type, JsonExprState.error is set to true, provided the @@ -4919,9 +4920,6 @@ ExecEvalJsonExprPath(ExprState *state, ExprEvalStep *op, int jump_eval_coercion = jsestate->jump_eval_coercion; char *val_string = NULL; - 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)); @@ -4934,6 +4932,25 @@ ExecEvalJsonExprPath(ExprState *state, ExprEvalStep *op, } jsestate->escontext.error_occurred = false; + /* + * Return NULL if formatted_expr or pathspec is NULL, skipping ON ERROR + * and ON EMPTY. If the RETURNING type is a domain with constraints, the + * NULL must still be coerced to it so that the constraints are checked. + */ + if (jsestate->formatted_expr.isnull || jsestate->pathspec.isnull) + { + *op->resvalue = (Datum) 0; + *op->resnull = true; + + if (jump_eval_coercion >= 0) + return jump_eval_coercion; + else + return jsestate->jump_end; + } + + item = jsestate->formatted_expr.value; + path = DatumGetJsonPathP(jsestate->pathspec.value); + switch (jsexpr->op) { case JSON_EXISTS_OP: diff --git a/src/test/regress/expected/sqljson_queryfuncs.out b/src/test/regress/expected/sqljson_queryfuncs.out index fe8aa927b5e..c24d86fc35f 100644 --- a/src/test/regress/expected/sqljson_queryfuncs.out +++ b/src/test/regress/expected/sqljson_queryfuncs.out @@ -337,6 +337,16 @@ SELECT JSON_VALUE('"purple"'::jsonb, 'lax $[*]' RETURNING rgb); SELECT JSON_VALUE('"purple"'::jsonb, 'lax $[*]' RETURNING rgb ERROR ON ERROR); ERROR: value for domain rgb violates check constraint "rgb_check" +SELECT x IS NULL AS is_null, + json_value(x, 'strict $.a' RETURNING sqljsonb_int_not_null DEFAULT 1 ON ERROR) AS jv +FROM (VALUES ('1'::jsonb), (NULL), ('1'::jsonb)) v(x); + is_null | jv +---------+---- + f | 1 + t | 1 + f | 1 +(3 rows) + SELECT JSON_VALUE(jsonb '[]', '$'); json_value ------------ @@ -475,6 +485,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 0767db9cfff..80fdb805f0f 100644 --- a/src/test/regress/sql/sqljson_queryfuncs.sql +++ b/src/test/regress/sql/sqljson_queryfuncs.sql @@ -80,6 +80,9 @@ CREATE TYPE rainbow AS ENUM ('red', 'orange', 'yellow', 'green', 'blue', 'purple CREATE DOMAIN rgb AS rainbow CHECK (VALUE IN ('red', 'green', 'blue')); SELECT JSON_VALUE('"purple"'::jsonb, 'lax $[*]' RETURNING rgb); SELECT JSON_VALUE('"purple"'::jsonb, 'lax $[*]' RETURNING rgb ERROR ON ERROR); +SELECT x IS NULL AS is_null, + json_value(x, 'strict $.a' RETURNING sqljsonb_int_not_null DEFAULT 1 ON ERROR) AS jv +FROM (VALUES ('1'::jsonb), (NULL), ('1'::jsonb)) v(x); SELECT JSON_VALUE(jsonb '[]', '$'); SELECT JSON_VALUE(jsonb '[]', '$' ERROR ON ERROR); @@ -128,6 +131,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); -- 2.34.1