Shouldn't jsonpath .string() Unwrap?
Hackers,
Most of the jsonpath methods auto-unwrap in lax mode:
david=# select jsonb_path_query('[-2,5]', '$.abs()');
jsonb_path_query
------------------
2
5
(2 rows)
The obvious exceptions are size() and type(), which apply directly to arrays, so no need to unwrap:
david=# select jsonb_path_query('[-2,5]', '$.size()');
jsonb_path_query
------------------
2
(1 row)
david=# select jsonb_path_query('[-2,5]', '$.type()');
jsonb_path_query
------------------
"array"
But what about string()? Is there some reason it doesn’t unwrap?
david=# select jsonb_path_query('[-2,5]', '$.string()');
ERROR: jsonpath item method .string() can only be applied to a bool, string, numeric, or datetime value
What I expect:
david=# select jsonb_path_query('[-2,5]', '$.string()');
jsonb_path_query
—————————
"2"
"5"
(2 rows)
However, I do see a test[1]https://github.com/postgres/postgres/blob/REL_17_BETA1/src/test/regress/expected/jsonb_jsonpath.out#L2527-L2530 for this behavior, so maybe there’s a reason for it?
Best,
David
On Sat, Jun 8, 2024 at 3:50 PM David E. Wheeler <david@justatheory.com>
wrote:
Hackers,
Most of the jsonpath methods auto-unwrap in lax mode:
david=# select jsonb_path_query('[-2,5]', '$.abs()');
jsonb_path_query
------------------
2
5
(2 rows)The obvious exceptions are size() and type(), which apply directly to
arrays, so no need to unwrap:david=# select jsonb_path_query('[-2,5]', '$.size()');
jsonb_path_query
------------------
2
(1 row)david=# select jsonb_path_query('[-2,5]', '$.type()');
jsonb_path_query
------------------
"array"But what about string()? Is there some reason it doesn’t unwrap?
david=# select jsonb_path_query('[-2,5]', '$.string()');
ERROR: jsonpath item method .string() can only be applied to a bool,
string, numeric, or datetime valueWhat I expect:
david=# select jsonb_path_query('[-2,5]', '$.string()');
jsonb_path_query
—————————
"2"
"5"
(2 rows)However, I do see a test[1] for this behavior, so maybe there’s a reason
for it?
Adding Andrew.
I'm willing to call this an open item against this feature as I don't see
any documentation explaining that string() behaves differently than the
others.
David J.
On Jun 12, 2024, at 4:02 PM, David G. Johnston <david.g.johnston@gmail.com> wrote:
Adding Andrew.
Thank you.
I'm willing to call this an open item against this feature as I don't see any documentation explaining that string() behaves differently than the others.
Maybe there’s some wording in the standard on this topic?
I’m happy to provide a patch to auto-unwrap .string() in lax mode. Seems pretty straightforward.
D
On 2024-06-12 We 16:02, David G. Johnston wrote:
On Sat, Jun 8, 2024 at 3:50 PM David E. Wheeler
<david@justatheory.com> wrote:Hackers,
Most of the jsonpath methods auto-unwrap in lax mode:
david=# select jsonb_path_query('[-2,5]', '$.abs()');
jsonb_path_query
------------------
2
5
(2 rows)The obvious exceptions are size() and type(), which apply directly
to arrays, so no need to unwrap:david=# select jsonb_path_query('[-2,5]', '$.size()');
jsonb_path_query
------------------
2
(1 row)david=# select jsonb_path_query('[-2,5]', '$.type()');
jsonb_path_query
------------------
"array"But what about string()? Is there some reason it doesn’t unwrap?
david=# select jsonb_path_query('[-2,5]', '$.string()');
ERROR: jsonpath item method .string() can only be applied to a
bool, string, numeric, or datetime valueWhat I expect:
david=# select jsonb_path_query('[-2,5]', '$.string()');
jsonb_path_query
—————————
"2"
"5"
(2 rows)However, I do see a test[1] for this behavior, so maybe there’s a
reason for it?Adding Andrew.
I'm willing to call this an open item against this feature as I don't
see any documentation explaining that string() behaves differently
than the others.
Hmm. You might be right. Many of these items have this code, but the
string() branch does not:
if (unwrap && JsonbType(jb) == jbvArray)
return executeItemUnwrapTargetArray(cxt, jsp, jb, found,
false);
cheers
andrew
--
Andrew Dunstan
EDB:https://www.enterprisedb.com
On Jun 13, 2024, at 3:53 PM, Andrew Dunstan <andrew@dunslane.net> wrote:
Hmm. You might be right. Many of these items have this code, but the string() branch does not:
if (unwrap && JsonbType(jb) == jbvArray)
return executeItemUnwrapTargetArray(cxt, jsp, jb, found,
false);
Exactly, would be pretty easy to add. I can work up a patch this weekend.
D
On 06/13/24 18:45, David E. Wheeler wrote:
On Jun 13, 2024, at 3:53 PM, Andrew Dunstan <andrew@dunslane.net> wrote:
Hmm. You might be right. Many of these items have this code, but the string() branch does not:
if (unwrap && JsonbType(jb) == jbvArray)
return executeItemUnwrapTargetArray(cxt, jsp, jb, found,
false);Exactly, would be pretty easy to add. I can work up a patch this weekend.
My opinion is yes, that should be done. 9.46, umm, General
Rule 11 g ii 6) A) says just "if MODE is lax and <JSON method> is not
type or size, then let BASE be Unwrap(BASE)." No special exemption
there for string(), nor further below at C) XV) for the operation
of string().
Regards,
-Chap
On Jun 13, 2024, at 21:55, Chapman Flack <jcflack@acm.org> wrote:
My opinion is yes, that should be done. 9.46, umm, General
Rule 11 g ii 6) A) says just "if MODE is lax and <JSON method> is not
type or size, then let BASE be Unwrap(BASE)." No special exemption
there for string(), nor further below at C) XV) for the operation
of string().
Thank you! Cited that bit in the commit message in the attached patch (also available as a GitHub PR[1]https://github.com/theory/postgres/pull/5).
D
Attachments:
v1-0001-Teach-jsonpath-string-to-unwrap-in-lax-mode.patchapplication/octet-stream; name=v1-0001-Teach-jsonpath-string-to-unwrap-in-lax-mode.patch; x-unix-mode=0644Download
From 702b5564d3fb04a52ca0551ed15f093794ce640a Mon Sep 17 00:00:00 2001
From: "David E. Wheeler" <david@justatheory.com>
Date: Fri, 14 Jun 2024 10:31:39 -0400
Subject: [PATCH v1] Teach jsonpath string() to unwrap in lax mode
All other jsonpath methods, aside from type() and .size(), will unwrap
an array in lax mode, but string(), added in 66ea94e, overlooked this
behavior. A discussion on pgsql-hackers[1] cites the SQL standard:
> General Rule 11 g ii 6) A) says just "if MODE is lax and <JSON method>
> is not type or size, then let BASE be Unwrap(BASE)." No special
> exemption there for string(), nor further below at C) XV) for the
> operation of string().
So teach string() to also unwrap in lax mode, update the test for this
behavior, and add a line to the docs about the behavior of methods in
lax mode.
[1]: https://www.postgresql.org/message-id/flat/A64AE04F-4410-42B7-A141-7A7349260F4D%40justatheory.com
---
doc/src/sgml/func.sgml | 5 ++++-
src/backend/utils/adt/jsonpath_exec.c | 3 +++
src/test/regress/expected/jsonb_jsonpath.out | 9 +++++++--
src/test/regress/sql/jsonb_jsonpath.sql | 2 +-
4 files changed, 15 insertions(+), 4 deletions(-)
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index 17c44bc338..7114b077b9 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -17792,7 +17792,10 @@ ERROR: jsonpath member accessor can only be applied to an object
methods available in <type>jsonpath</type>. Note that while the unary
operators and methods can be applied to multiple values resulting from a
preceding path step, the binary operators (addition etc.) can only be
- applied to single values.
+ applied to single values. In lax mode, methods applied to an array will be
+ executed for each value in the array. The extepsions are
+ <literal>.type()</literal> and <literal>.size()</literal>, which apply to
+ the array itself.
</para>
<table id="functions-sqljson-op-table">
diff --git a/src/backend/utils/adt/jsonpath_exec.c b/src/backend/utils/adt/jsonpath_exec.c
index ceb30033e1..c30d059a76 100644
--- a/src/backend/utils/adt/jsonpath_exec.c
+++ b/src/backend/utils/adt/jsonpath_exec.c
@@ -1606,6 +1606,9 @@ executeItemOptUnwrapTarget(JsonPathExecContext *cxt, JsonPathItem *jsp,
JsonbValue jbv;
char *tmp = NULL;
+ if (unwrap && JsonbType(jb) == jbvArray)
+ return executeItemUnwrapTargetArray(cxt, jsp, jb, found, false);
+
switch (JsonbType(jb))
{
case jbvString:
diff --git a/src/test/regress/expected/jsonb_jsonpath.out b/src/test/regress/expected/jsonb_jsonpath.out
index c3f8e8249d..a13eee24fb 100644
--- a/src/test/regress/expected/jsonb_jsonpath.out
+++ b/src/test/regress/expected/jsonb_jsonpath.out
@@ -2524,8 +2524,13 @@ select jsonb_path_query('null', '$.string()', silent => true);
------------------
(0 rows)
-select jsonb_path_query('[]', '$.string()');
-ERROR: jsonpath item method .string() can only be applied to a bool, string, numeric, or datetime value
+select jsonb_path_query('[2, true]', '$.string()');
+ jsonb_path_query
+------------------
+ "2"
+ "true"
+(2 rows)
+
select jsonb_path_query('[]', 'strict $.string()');
ERROR: jsonpath item method .string() can only be applied to a bool, string, numeric, or datetime value
select jsonb_path_query('{}', '$.string()');
diff --git a/src/test/regress/sql/jsonb_jsonpath.sql b/src/test/regress/sql/jsonb_jsonpath.sql
index cbd2db533d..9c9d327140 100644
--- a/src/test/regress/sql/jsonb_jsonpath.sql
+++ b/src/test/regress/sql/jsonb_jsonpath.sql
@@ -575,7 +575,7 @@ select jsonb_path_query('12.3', '$.number() * 2');
-- Test .string()
select jsonb_path_query('null', '$.string()');
select jsonb_path_query('null', '$.string()', silent => true);
-select jsonb_path_query('[]', '$.string()');
+select jsonb_path_query('[2, true]', '$.string()');
select jsonb_path_query('[]', 'strict $.string()');
select jsonb_path_query('{}', '$.string()');
select jsonb_path_query('[]', 'strict $.string()', silent => true);
--
2.45.2
On 06/14/24 10:39, David E. Wheeler wrote:
Cited that bit in the commit message in the attached patch (also available as a GitHub PR[1]).
I would s/extepsions/exceptions/ in the added documentation. :)
Offhand (as GitHub PRs aren't really The PG Way), if they were The Way,
I would find this one a little hard to follow, being based at a point
28 unrelated commits ahead of the ref it's opened against. I suspect
'master' on theory/postgres could be fast-forwarded to f1affb6 and then
the PR would look much more approachable.
Regards,
-Chap
On Jun 14, 2024, at 11:25, Chapman Flack <jcflack@acm.org> wrote:
I would s/extepsions/exceptions/ in the added documentation. :)
Bah, fixed and attached, thanks.
Offhand (as GitHub PRs aren't really The PG Way), if they were The Way,
I would find this one a little hard to follow, being based at a point
28 unrelated commits ahead of the ref it's opened against. I suspect
'master' on theory/postgres could be fast-forwarded to f1affb6 and then
the PR would look much more approachable.
Yeah, I pushed the PR and branch before I synced master, and GitHub was taking a while to notice and update the PR. I fixed it with `git commit --all --amend --date now --reedit-message HEAD` and force-pushed (then fixed the typo and fixed again).
D
Attachments:
v2-0001-Teach-jsonpath-string-to-unwrap-in-lax-mode.patchapplication/octet-stream; name=v2-0001-Teach-jsonpath-string-to-unwrap-in-lax-mode.patch; x-unix-mode=0644Download
From 62fd2860abdaaad97741670e80dcd84cc7222925 Mon Sep 17 00:00:00 2001
From: "David E. Wheeler" <david@justatheory.com>
Date: Fri, 14 Jun 2024 12:02:05 -0400
Subject: [PATCH v2] Teach jsonpath string() to unwrap in lax mode
All other jsonpath methods, aside from type() and .size(), will unwrap
an array in lax mode, but string(), added in 66ea94e, overlooked this
behavior. A discussion on pgsql-hackers[1] cites the SQL standard:
> General Rule 11 g ii 6) A) says just "if MODE is lax and <JSON method>
> is not type or size, then let BASE be Unwrap(BASE)." No special
> exemption there for string(), nor further below at C) XV) for the
> operation of string().
So teach string() to also unwrap in lax mode, update the test for this
behavior, and add a line to the docs about the behavior of methods in
lax mode.
[1]: https://www.postgresql.org/message-id/flat/A64AE04F-4410-42B7-A141-7A7349260F4D%40justatheory.com
---
doc/src/sgml/func.sgml | 5 ++++-
src/backend/utils/adt/jsonpath_exec.c | 3 +++
src/test/regress/expected/jsonb_jsonpath.out | 9 +++++++--
src/test/regress/sql/jsonb_jsonpath.sql | 2 +-
4 files changed, 15 insertions(+), 4 deletions(-)
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index 17c44bc338..ededec932b 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -17792,7 +17792,10 @@ ERROR: jsonpath member accessor can only be applied to an object
methods available in <type>jsonpath</type>. Note that while the unary
operators and methods can be applied to multiple values resulting from a
preceding path step, the binary operators (addition etc.) can only be
- applied to single values.
+ applied to single values. In lax mode, methods applied to an array will be
+ executed for each value in the array. The exceptions are
+ <literal>.type()</literal> and <literal>.size()</literal>, which apply to
+ the array itself.
</para>
<table id="functions-sqljson-op-table">
diff --git a/src/backend/utils/adt/jsonpath_exec.c b/src/backend/utils/adt/jsonpath_exec.c
index ceb30033e1..c30d059a76 100644
--- a/src/backend/utils/adt/jsonpath_exec.c
+++ b/src/backend/utils/adt/jsonpath_exec.c
@@ -1606,6 +1606,9 @@ executeItemOptUnwrapTarget(JsonPathExecContext *cxt, JsonPathItem *jsp,
JsonbValue jbv;
char *tmp = NULL;
+ if (unwrap && JsonbType(jb) == jbvArray)
+ return executeItemUnwrapTargetArray(cxt, jsp, jb, found, false);
+
switch (JsonbType(jb))
{
case jbvString:
diff --git a/src/test/regress/expected/jsonb_jsonpath.out b/src/test/regress/expected/jsonb_jsonpath.out
index c3f8e8249d..a13eee24fb 100644
--- a/src/test/regress/expected/jsonb_jsonpath.out
+++ b/src/test/regress/expected/jsonb_jsonpath.out
@@ -2524,8 +2524,13 @@ select jsonb_path_query('null', '$.string()', silent => true);
------------------
(0 rows)
-select jsonb_path_query('[]', '$.string()');
-ERROR: jsonpath item method .string() can only be applied to a bool, string, numeric, or datetime value
+select jsonb_path_query('[2, true]', '$.string()');
+ jsonb_path_query
+------------------
+ "2"
+ "true"
+(2 rows)
+
select jsonb_path_query('[]', 'strict $.string()');
ERROR: jsonpath item method .string() can only be applied to a bool, string, numeric, or datetime value
select jsonb_path_query('{}', '$.string()');
diff --git a/src/test/regress/sql/jsonb_jsonpath.sql b/src/test/regress/sql/jsonb_jsonpath.sql
index cbd2db533d..9c9d327140 100644
--- a/src/test/regress/sql/jsonb_jsonpath.sql
+++ b/src/test/regress/sql/jsonb_jsonpath.sql
@@ -575,7 +575,7 @@ select jsonb_path_query('12.3', '$.number() * 2');
-- Test .string()
select jsonb_path_query('null', '$.string()');
select jsonb_path_query('null', '$.string()', silent => true);
-select jsonb_path_query('[]', '$.string()');
+select jsonb_path_query('[2, true]', '$.string()');
select jsonb_path_query('[]', 'strict $.string()');
select jsonb_path_query('{}', '$.string()');
select jsonb_path_query('[]', 'strict $.string()', silent => true);
--
2.45.2
Hi,
Sorry, I have missed this in the original patch. I am surprised how that
happened. But thanks for catching the same and fixing it.
The changes are straightforward and look good to me. However, I have kept
the existing test of an empty array as is, assuming that it is one of the
valid tests. It now returns zero rows instead of an error. Your added test
case covers this issue.
Thanks
On Fri, Jun 14, 2024 at 9:34 PM David E. Wheeler <david@justatheory.com>
wrote:
On Jun 14, 2024, at 11:25, Chapman Flack <jcflack@acm.org> wrote:
I would s/extepsions/exceptions/ in the added documentation. :)
Bah, fixed and attached, thanks.
Offhand (as GitHub PRs aren't really The PG Way), if they were The Way,
I would find this one a little hard to follow, being based at a point
28 unrelated commits ahead of the ref it's opened against. I suspect
'master' on theory/postgres could be fast-forwarded to f1affb6 and then
the PR would look much more approachable.Yeah, I pushed the PR and branch before I synced master, and GitHub was
taking a while to notice and update the PR. I fixed it with `git commit
--all --amend --date now --reedit-message HEAD` and force-pushed (then
fixed the typo and fixed again).D
--
*Jeevan Chalke*
*Principal, ManagerProduct Development*
enterprisedb.com <https://www.enterprisedb.com>
Attachments:
v3-0001-Teach-jsonpath-string-to-unwrap-in-lax-mode.patchapplication/octet-stream; name=v3-0001-Teach-jsonpath-string-to-unwrap-in-lax-mode.patchDownload
From a8b386df15fa4dba9c80ebfaab8d2a732d85ac82 Mon Sep 17 00:00:00 2001
From: "David E. Wheeler" <david@justatheory.com>
Date: Sat, 15 Jun 2024 19:44:21 +0530
Subject: [PATCH v3] Teach jsonpath string() to unwrap in lax mode
All other jsonpath methods, aside from type() and .size(), will unwrap
an array in lax mode, but string(), added in 66ea94e, overlooked this
behavior. A discussion on pgsql-hackers[1] cites the SQL standard:
> General Rule 11 g ii 6) A) says just "if MODE is lax and <JSON method>
> is not type or size, then let BASE be Unwrap(BASE)." No special
> exemption there for string(), nor further below at C) XV) for the
> operation of string().
So teach string() to also unwrap in lax mode, update the test for this
behavior, and add a line to the docs about the behavior of methods in
lax mode.
[1]: https://www.postgresql.org/message-id/flat/A64AE04F-4410-42B7-A141-7A7349260F4D%40justatheory.com
---
doc/src/sgml/func.sgml | 5 ++++-
src/backend/utils/adt/jsonpath_exec.c | 3 +++
src/test/regress/expected/jsonb_jsonpath.out | 12 +++++++++++-
src/test/regress/sql/jsonb_jsonpath.sql | 1 +
4 files changed, 19 insertions(+), 2 deletions(-)
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index 17c44bc..2609269 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -17792,7 +17792,10 @@ ERROR: jsonpath member accessor can only be applied to an object
methods available in <type>jsonpath</type>. Note that while the unary
operators and methods can be applied to multiple values resulting from a
preceding path step, the binary operators (addition etc.) can only be
- applied to single values.
+ applied to single values. In lax mode, methods applied to an array will be
+ executed for each value in the array. The exceptions are
+ <literal>.type()</literal> and <literal>.size()</literal>, which apply to
+ the array itself.
</para>
<table id="functions-sqljson-op-table">
diff --git a/src/backend/utils/adt/jsonpath_exec.c b/src/backend/utils/adt/jsonpath_exec.c
index ceb3003..c30d059 100644
--- a/src/backend/utils/adt/jsonpath_exec.c
+++ b/src/backend/utils/adt/jsonpath_exec.c
@@ -1606,6 +1606,9 @@ executeItemOptUnwrapTarget(JsonPathExecContext *cxt, JsonPathItem *jsp,
JsonbValue jbv;
char *tmp = NULL;
+ if (unwrap && JsonbType(jb) == jbvArray)
+ return executeItemUnwrapTargetArray(cxt, jsp, jb, found, false);
+
switch (JsonbType(jb))
{
case jbvString:
diff --git a/src/test/regress/expected/jsonb_jsonpath.out b/src/test/regress/expected/jsonb_jsonpath.out
index c3f8e82..a6112e8 100644
--- a/src/test/regress/expected/jsonb_jsonpath.out
+++ b/src/test/regress/expected/jsonb_jsonpath.out
@@ -2525,7 +2525,10 @@ select jsonb_path_query('null', '$.string()', silent => true);
(0 rows)
select jsonb_path_query('[]', '$.string()');
-ERROR: jsonpath item method .string() can only be applied to a bool, string, numeric, or datetime value
+ jsonb_path_query
+------------------
+(0 rows)
+
select jsonb_path_query('[]', 'strict $.string()');
ERROR: jsonpath item method .string() can only be applied to a bool, string, numeric, or datetime value
select jsonb_path_query('{}', '$.string()');
@@ -2576,6 +2579,13 @@ select jsonb_path_query('1234', '$.string().type()');
"string"
(1 row)
+select jsonb_path_query('[2, true]', '$.string()');
+ jsonb_path_query
+------------------
+ "2"
+ "true"
+(2 rows)
+
select jsonb_path_query('"2023-08-15 12:34:56 +5:30"', '$.timestamp().string()');
ERROR: cannot convert value from timestamptz to timestamp without time zone usage
HINT: Use *_tz() function for time zone support.
diff --git a/src/test/regress/sql/jsonb_jsonpath.sql b/src/test/regress/sql/jsonb_jsonpath.sql
index cbd2db5..5e14f77 100644
--- a/src/test/regress/sql/jsonb_jsonpath.sql
+++ b/src/test/regress/sql/jsonb_jsonpath.sql
@@ -586,6 +586,7 @@ select jsonb_path_query('"1.23aaa"', '$.string()');
select jsonb_path_query('1234', '$.string()');
select jsonb_path_query('true', '$.string()');
select jsonb_path_query('1234', '$.string().type()');
+select jsonb_path_query('[2, true]', '$.string()');
select jsonb_path_query('"2023-08-15 12:34:56 +5:30"', '$.timestamp().string()');
select jsonb_path_query_tz('"2023-08-15 12:34:56 +5:30"', '$.timestamp().string()'); -- should work
select jsonb_path_query_array('[1.23, "yes", false]', '$[*].string()');
--
1.8.3.1
On Jun 15, 2024, at 10:27, Jeevan Chalke <jeevan.chalke@enterprisedb.com> wrote:
Sorry, I have missed this in the original patch. I am surprised how that happened. But thanks for catching the same and fixing it.
No worries. :-)
The changes are straightforward and look good to me. However, I have kept the existing test of an empty array as is, assuming that it is one of the valid tests. It now returns zero rows instead of an error. Your added test case covers this issue.
Makes sense, thank you.
D
On Jun 15, 2024, at 10:39, David E. Wheeler <david@justatheory.com> wrote:
The changes are straightforward and look good to me. However, I have kept the existing test of an empty array as is, assuming that it is one of the valid tests. It now returns zero rows instead of an error. Your added test case covers this issue.
Makes sense, thank you.
Added https://commitfest.postgresql.org/48/5039/.
D
On 2024-06-15 Sa 10:51, David E. Wheeler wrote:
On Jun 15, 2024, at 10:39, David E. Wheeler <david@justatheory.com> wrote:
The changes are straightforward and look good to me. However, I have kept the existing test of an empty array as is, assuming that it is one of the valid tests. It now returns zero rows instead of an error. Your added test case covers this issue.
Makes sense, thank you.
Not really needed, I will commit shortly.
cheers
andrew
--
Andrew Dunstan
EDB: https://www.enterprisedb.com