On Fri, Nov 7, 2025 at 2:26 PM Kirill Reshke <[email protected]> wrote: > > Hi! > I tried your fix and this indeed fixes an issue. Two minor comments: > > First, > in the `src/backend/parser/parse_expr.c` fil there are multiple > examples of working with `coerce_to_target_type`, they all share > different coding practice: > > ``` > coerced_expr = coerce_to_target_type(.., expr, ..) > > if (coerced_expr == NULL) > ereport(); > > > expr = coerced_expr; > ``` > > Instead of > ``` > expr = coerce_to_target_type(.., expr, ..) > > if (expr == NULL) > ereport(); > ``` > > Let's be consistent? >
IMHO, > coerced_expr = coerce_to_target_type(.., expr, ..) is better than > expr = coerce_to_target_type(.., expr, ..) I changed accordingly.
From b1fe8643aa1ab8a351ab0269d5b7037a071bd669 Mon Sep 17 00:00:00 2001 From: jian he <[email protected]> Date: Tue, 18 Nov 2025 16:13:50 +0800 Subject: [PATCH v2 1/1] fix transformJsonFuncExpr pathspec cache lookup failure discussion: https://postgr.es/m/cacjufxhunvg81jmuno8yvv_hjd0dicgavn2wteu8ajbvjpb...@mail.gmail.com --- src/backend/parser/parse_expr.c | 22 ++++++++++++------- .../regress/expected/sqljson_queryfuncs.out | 8 +++++++ src/test/regress/sql/sqljson_queryfuncs.sql | 2 ++ 3 files changed, 24 insertions(+), 8 deletions(-) diff --git a/src/backend/parser/parse_expr.c b/src/backend/parser/parse_expr.c index 32d6ae918ca..67fb2fb485d 100644 --- a/src/backend/parser/parse_expr.c +++ b/src/backend/parser/parse_expr.c @@ -4285,8 +4285,11 @@ transformJsonFuncExpr(ParseState *pstate, JsonFuncExpr *func) { JsonExpr *jsexpr; Node *path_spec; + Node *coerced_expr; const char *func_name = NULL; JsonFormatType default_format; + Oid pathspec_type; + ParseLoc pathspec_loc; switch (func->op) { @@ -4500,17 +4503,20 @@ transformJsonFuncExpr(ParseState *pstate, JsonFuncExpr *func) jsexpr->format = func->context_item->format; path_spec = transformExprRecurse(pstate, func->pathspec); - path_spec = coerce_to_target_type(pstate, path_spec, exprType(path_spec), - JSONPATHOID, -1, - COERCION_EXPLICIT, COERCE_IMPLICIT_CAST, - exprLocation(path_spec)); - if (path_spec == NULL) + pathspec_type = exprType(path_spec); + pathspec_loc = exprLocation(path_spec); + + coerced_expr = coerce_to_target_type(pstate, path_spec, pathspec_type, + JSONPATHOID, -1, + COERCION_EXPLICIT, COERCE_IMPLICIT_CAST, + pathspec_loc); + if (coerced_expr == NULL) ereport(ERROR, (errcode(ERRCODE_DATATYPE_MISMATCH), errmsg("JSON path expression must be of type %s, not of type %s", - "jsonpath", format_type_be(exprType(path_spec))), - parser_errposition(pstate, exprLocation(path_spec)))); - jsexpr->path_spec = path_spec; + "jsonpath", format_type_be(pathspec_type)), + parser_errposition(pstate, pathspec_loc))); + jsexpr->path_spec = coerced_expr; /* Transform and coerce the PASSING arguments to jsonb. */ transformJsonPassingArgs(pstate, func_name, diff --git a/src/test/regress/expected/sqljson_queryfuncs.out b/src/test/regress/expected/sqljson_queryfuncs.out index 5a35aeb7bba..53145f50f18 100644 --- a/src/test/regress/expected/sqljson_queryfuncs.out +++ b/src/test/regress/expected/sqljson_queryfuncs.out @@ -1331,6 +1331,10 @@ SELECT JSON_QUERY(jsonb '{"a": 123}', '$' || '.' || 'a' WITH WRAPPER); [123] (1 row) +SELECT JSON_QUERY(jsonb '{"a": 123}', ('$' || '.' || 'a' || NULL)::date WITH WRAPPER); +ERROR: JSON path expression must be of type jsonpath, not of type date +LINE 1: SELECT JSON_QUERY(jsonb '{"a": 123}', ('$' || '.' || 'a' || ... + ^ -- Should fail (invalid path) SELECT JSON_QUERY(jsonb '{"a": 123}', 'error' || ' ' || 'error'); ERROR: syntax error at or near " " of jsonpath input @@ -1355,6 +1359,10 @@ SELECT json_value('"aaa"', path RETURNING json) FROM jsonpaths; "aaa" (1 row) +SELECT json_value('"aaa"', jsonpaths RETURNING json) FROM jsonpaths; +ERROR: JSON path expression must be of type jsonpath, not of type jsonpaths +LINE 1: SELECT json_value('"aaa"', jsonpaths RETURNING json) FROM js... + ^ -- Test PASSING argument parsing SELECT JSON_QUERY(jsonb 'null', '$xyz' PASSING 1 AS xy); ERROR: could not find jsonpath variable "xyz" diff --git a/src/test/regress/sql/sqljson_queryfuncs.sql b/src/test/regress/sql/sqljson_queryfuncs.sql index 8d7b225b612..a5d5e256d7f 100644 --- a/src/test/regress/sql/sqljson_queryfuncs.sql +++ b/src/test/regress/sql/sqljson_queryfuncs.sql @@ -450,6 +450,7 @@ SELECT JSON_VALUE(jsonb '{"a": 123}', '$' || '.' || 'a'); SELECT JSON_VALUE(jsonb '{"a": 123}', '$' || '.' || 'b' DEFAULT 'foo' ON EMPTY); SELECT JSON_QUERY(jsonb '{"a": 123}', '$' || '.' || 'a'); SELECT JSON_QUERY(jsonb '{"a": 123}', '$' || '.' || 'a' WITH WRAPPER); +SELECT JSON_QUERY(jsonb '{"a": 123}', ('$' || '.' || 'a' || NULL)::date WITH WRAPPER); -- Should fail (invalid path) SELECT JSON_QUERY(jsonb '{"a": 123}', 'error' || ' ' || 'error'); @@ -460,6 +461,7 @@ SELECT JSON_QUERY(NULL FORMAT JSON, '$'); -- Test non-const jsonpath CREATE TEMP TABLE jsonpaths (path) AS SELECT '$'; SELECT json_value('"aaa"', path RETURNING json) FROM jsonpaths; +SELECT json_value('"aaa"', jsonpaths RETURNING json) FROM jsonpaths; -- Test PASSING argument parsing SELECT JSON_QUERY(jsonb 'null', '$xyz' PASSING 1 AS xy); -- 2.34.1
