transformJsonFuncExpr pathspec cache lookup failed
hi.
in transformJsonFuncExpr:
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)
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))));
There is no test for this, if you try it, you can easily reach "cache
lookup failed".
SELECT JSON_VALUE(jsonb 'null', NULL::date);
ERROR: cache lookup failed for type 0
because we first call ``format_type_be(exprType(path_spec))),`` then ereport.
format_type_be can not code with InvalidOid.
A patch is attached.
-----------
Also, note that we allow:
SELECT * FROM JSON_TABLE(jsonb'"1.23"', '$' COLUMNS (js2 int PATH '$'));
but don't allow
SELECT * FROM JSON_TABLE(jsonb'"1.23"', '$'::jsonpath COLUMNS (js2 int
PATH '$'));
Maybe we should support this.
since every A_Const should have a type for it. Allowing something like:
JSON_TABLE(jsonb '"1.23"', '$'::some_jsonpath_type ... )
seems consistent.
I guess that's a separate issue, so I didn't touch it.
Attachments:
v1-0001-fix-transformJsonFuncExpr-pathspec-cache-lookup-failure.patchtext/x-patch; charset=US-ASCII; name=v1-0001-fix-transformJsonFuncExpr-pathspec-cache-lookup-failure.patchDownload
From c3aa2acd0cbbb0af05530d5076d4dd55b3e2e31f Mon Sep 17 00:00:00 2001
From: jian he <jian.universality@gmail.com>
Date: Fri, 7 Nov 2025 10:48:27 +0800
Subject: [PATCH v1 1/1] fix transformJsonFuncExpr pathspec cache lookup
failure
discussion: https://postgr.es/m/
---
src/backend/parser/parse_expr.c | 13 +++++++++----
src/test/regress/expected/sqljson_queryfuncs.out | 8 ++++++++
src/test/regress/sql/sqljson_queryfuncs.sql | 2 ++
3 files changed, 19 insertions(+), 4 deletions(-)
diff --git a/src/backend/parser/parse_expr.c b/src/backend/parser/parse_expr.c
index 12119f147fc..bdee31eb22f 100644
--- a/src/backend/parser/parse_expr.c
+++ b/src/backend/parser/parse_expr.c
@@ -4287,6 +4287,8 @@ transformJsonFuncExpr(ParseState *pstate, JsonFuncExpr *func)
Node *path_spec;
const char *func_name = NULL;
JsonFormatType default_format;
+ Oid pathspec_type;
+ ParseLoc pathspec_loc;
switch (func->op)
{
@@ -4500,16 +4502,19 @@ 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),
+ pathspec_type = exprType(path_spec);
+ pathspec_loc = exprLocation(path_spec);
+
+ path_spec = coerce_to_target_type(pstate, path_spec, pathspec_type,
JSONPATHOID, -1,
COERCION_EXPLICIT, COERCE_IMPLICIT_CAST,
- exprLocation(path_spec));
+ pathspec_loc);
if (path_spec == 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))));
+ "jsonpath", format_type_be(pathspec_type)),
+ parser_errposition(pstate, pathspec_loc)));
jsexpr->path_spec = path_spec;
/* Transform and coerce the PASSING arguments to jsonb. */
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
On Fri, 7 Nov 2025 at 07:50, jian he <jian.universality@gmail.com> wrote:
hi.
in transformJsonFuncExpr:
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)
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))));There is no test for this, if you try it, you can easily reach "cache
lookup failed".
SELECT JSON_VALUE(jsonb 'null', NULL::date);
ERROR: cache lookup failed for type 0because we first call ``format_type_be(exprType(path_spec))),`` then ereport.
format_type_be can not code with InvalidOid.A patch is attached.
-----------
Also, note that we allow:
SELECT * FROM JSON_TABLE(jsonb'"1.23"', '$' COLUMNS (js2 int PATH '$'));
but don't allow
SELECT * FROM JSON_TABLE(jsonb'"1.23"', '$'::jsonpath COLUMNS (js2 int
PATH '$'));Maybe we should support this.
since every A_Const should have a type for it. Allowing something like:
JSON_TABLE(jsonb '"1.23"', '$'::some_jsonpath_type ... )
seems consistent.
I guess that's a separate issue, so I didn't touch it.
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?
Second, about allowing JSON_TABLE(jsonb '"1.23"',
'$'::some_jsonpath_type ... ). The way we check input for JSON_TABLE
is we simply check for node type in the parser.
So, I think it may be too difficult to change that, because we do not
work with catalog in parser, so we cannot resolve if the type is
indeed some_json_type?
What we can do is we can change parser rules, to allow more
user-friendly error here:
```
reshke=# SELECT * FROM JSON_TABLE(jsonb '{"d1": "H"}', '$'
COLUMNS (js1 text PATH '$.d1'::jsonpath ));
ERROR: syntax error at or near "::"
LINE 2: COLUMNS (js1 text PATH '$.d1'::jsonpath ));
^
```
The way we can achieve it is to change the second token in
json_table_column_path_clause_opt to a_expr and give a more clear
error.
Thoughts?
--
Best regards,
Kirill Reshke
On Fri, Nov 7, 2025 at 2:26 PM Kirill Reshke <reshkekirill@gmail.com> 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.
Attachments:
v2-0001-fix-transformJsonFuncExpr-pathspec-cache-lookup-failure.patchtext/x-patch; charset=US-ASCII; name=v2-0001-fix-transformJsonFuncExpr-pathspec-cache-lookup-failure.patchDownload
From b1fe8643aa1ab8a351ab0269d5b7037a071bd669 Mon Sep 17 00:00:00 2001
From: jian he <jian.universality@gmail.com>
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_hJD0DicgaVN2Wteu8aJbVJPBjZA@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
Hi,
On Wed, Nov 19, 2025 at 11:55 AM jian he <jian.universality@gmail.com> wrote:
On Fri, Nov 7, 2025 at 2:26 PM Kirill Reshke <reshkekirill@gmail.com> 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.
I agree, though I prefer a different name for that coerced_* variable.
Also, let’s define the _type and _loc variables inside the error
block. Updated patch attached.
Please feel free to post a patch for the 2nd issue.
--
Thanks, Amit Langote
Attachments:
v2-0001-Fix-error-reporting-for-SQL-JSON-path-type-mismat.patchapplication/octet-stream; name=v2-0001-Fix-error-reporting-for-SQL-JSON-path-type-mismat.patchDownload
From 32e378962e71cc399d442564b91ab165d4dc3772 Mon Sep 17 00:00:00 2001
From: Amit Langote <amitlan@postgresql.org>
Date: Wed, 26 Nov 2025 16:22:06 +0900
Subject: [PATCH v2] Fix error reporting for SQL/JSON path type mismatches
transformJsonFuncExpr() used exprType()/exprLocation() on the
possibly coerced path expression, which could be NULL when
coercion to jsonpath failed. Preserve the original expression
node so that type and location in the "must be of type jsonpath"
error are reported correctly. Add regression tests to cover
these cases.
Reported-by: Jian He <jian.universality@gmail.com>
Author: Jian He <jian.universality@gmail.com>
Reviewed-by: Kirill Reshke <reshkekirill@gmail.com>
Discussion: https://postgr.es/m/CACJufxHunVg81JMuNo8Yvv_hJD0DicgaVN2Wteu8aJbVJPBjZA@mail.gmail.com
Backpatch-through: 17
---
src/backend/parser/parse_expr.c | 24 ++++++++++++-------
.../regress/expected/sqljson_queryfuncs.out | 8 +++++++
src/test/regress/sql/sqljson_queryfuncs.sql | 2 ++
3 files changed, 26 insertions(+), 8 deletions(-)
diff --git a/src/backend/parser/parse_expr.c b/src/backend/parser/parse_expr.c
index 32d6ae918ca..87b93166163 100644
--- a/src/backend/parser/parse_expr.c
+++ b/src/backend/parser/parse_expr.c
@@ -4285,6 +4285,7 @@ transformJsonFuncExpr(ParseState *pstate, JsonFuncExpr *func)
{
JsonExpr *jsexpr;
Node *path_spec;
+ Node *coerced_path_spec;
const char *func_name = NULL;
JsonFormatType default_format;
@@ -4500,17 +4501,24 @@ 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)
+ coerced_path_spec = coerce_to_target_type(pstate, path_spec,
+ exprType(path_spec),
+ JSONPATHOID, -1,
+ COERCION_EXPLICIT,
+ COERCE_IMPLICIT_CAST,
+ exprLocation(path_spec));
+ if (coerced_path_spec == NULL)
+ {
+ Oid pathspec_type = exprType(path_spec);
+ int pathspec_loc = exprLocation(path_spec);
+
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_path_spec;
/* 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.47.3
On Wed, Nov 26, 2025 at 3:32 PM Amit Langote <amitlangote09@gmail.com> wrote:
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.
I agree, though I prefer a different name for that coerced_* variable.
Also, let’s define the _type and _loc variables inside the error
block. Updated patch attached.
v2-0001 looks good to me.
On Wed, 26 Nov 2025 at 10:32, Amit Langote <amitlangote09@gmail.com> wrote:
Hi,
On Wed, Nov 19, 2025 at 11:55 AM jian he <jian.universality@gmail.com> wrote:
On Fri, Nov 7, 2025 at 2:26 PM Kirill Reshke <reshkekirill@gmail.com> 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.
I agree, though I prefer a different name for that coerced_* variable.
Also, let’s define the _type and _loc variables inside the error
block. Updated patch attached.Please feel free to post a patch for the 2nd issue.
--
Thanks, Amit Langote
v2 lgtm
--
Best regards,
Kirill Reshke
On Wed, Nov 26, 2025 at 8:57 PM Kirill Reshke <reshkekirill@gmail.com> wrote:
On Wed, 26 Nov 2025 at 10:32, Amit Langote <amitlangote09@gmail.com> wrote:
On Wed, Nov 19, 2025 at 11:55 AM jian he <jian.universality@gmail.com> wrote:
On Fri, Nov 7, 2025 at 2:26 PM Kirill Reshke <reshkekirill@gmail.com> 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.
I agree, though I prefer a different name for that coerced_* variable.
Also, let’s define the _type and _loc variables inside the error
block. Updated patch attached.Please feel free to post a patch for the 2nd issue.
v2 lgtm
Pushed after moving the _type, _loc variable declarations outside
after all, because I had missed that they are passed to
coerce_to_target_type() as well in Jian's patch.
--
Thanks, Amit Langote