public inbox for [email protected]
help / color / mirror / Atom feedFrom: Peter Eisentraut <[email protected]>
To: pgsql-hackers <[email protected]>
Subject: Re: json_query conditional wrapper bug
Date: Wed, 4 Sep 2024 12:16:31 +0200
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
On 28.08.24 11:21, Peter Eisentraut wrote:
> These are ok:
>
> select json_query('{"a": 1, "b": 42}'::jsonb, 'lax $.b' without wrapper);
> json_query
> ------------
> 42
>
> select json_query('{"a": 1, "b": 42}'::jsonb, 'lax $.b' with
> unconditional wrapper);
> json_query
> ------------
> [42]
>
> But this appears to be wrong:
>
> select json_query('{"a": 1, "b": 42}'::jsonb, 'lax $.b' with conditional
> wrapper);
> json_query
> ------------
> [42]
>
> This should return an unwrapped 42.
If I make the code change illustrated in the attached patch, then I get
the correct result here. And various regression test results change,
which, to me, all look more correct after this patch. I don't know what
the code I removed was supposed to accomplish, but it seems to be wrong
somehow. In the current implementation, the WITH CONDITIONAL WRAPPER
clause doesn't appear to work correctly in any case I could identify.
From ea8a36584abf6cea0a05ac43ad8d101012b14313 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <[email protected]>
Date: Wed, 4 Sep 2024 12:09:35 +0200
Subject: [PATCH] WIP: Fix JSON_QUERY WITH CONDITIONAL WRAPPER
---
src/backend/utils/adt/jsonpath_exec.c | 5 +----
src/test/regress/sql/sqljson_queryfuncs.sql | 10 +++++-----
2 files changed, 6 insertions(+), 9 deletions(-)
diff --git a/src/backend/utils/adt/jsonpath_exec.c b/src/backend/utils/adt/jsonpath_exec.c
index e3ee0093d4d..a55a34685f0 100644
--- a/src/backend/utils/adt/jsonpath_exec.c
+++ b/src/backend/utils/adt/jsonpath_exec.c
@@ -3957,10 +3957,7 @@ JsonPathQuery(Datum jb, JsonPath *jp, JsonWrapper wrapper, bool *empty,
else if (wrapper == JSW_UNCONDITIONAL)
wrap = true;
else if (wrapper == JSW_CONDITIONAL)
- wrap = count > 1 ||
- IsAJsonbScalar(singleton) ||
- (singleton->type == jbvBinary &&
- JsonContainerIsScalar(singleton->val.binary.data));
+ wrap = count > 1;
else
{
elog(ERROR, "unrecognized json wrapper %d", (int) wrapper);
diff --git a/src/test/regress/sql/sqljson_queryfuncs.sql b/src/test/regress/sql/sqljson_queryfuncs.sql
index 21ff7787a28..a7342416a9c 100644
--- a/src/test/regress/sql/sqljson_queryfuncs.sql
+++ b/src/test/regress/sql/sqljson_queryfuncs.sql
@@ -146,11 +146,11 @@ CREATE DOMAIN rgb AS rainbow CHECK (VALUE IN ('red', 'green', 'blue'));
SELECT JSON_VALUE(NULL::jsonb, '$');
SELECT
- JSON_QUERY(js, '$'),
- JSON_QUERY(js, '$' WITHOUT WRAPPER),
- JSON_QUERY(js, '$' WITH CONDITIONAL WRAPPER),
- JSON_QUERY(js, '$' WITH UNCONDITIONAL ARRAY WRAPPER),
- JSON_QUERY(js, '$' WITH ARRAY WRAPPER)
+ JSON_QUERY(js, '$') AS "unspec",
+ JSON_QUERY(js, '$' WITHOUT WRAPPER) AS "without",
+ JSON_QUERY(js, '$' WITH CONDITIONAL WRAPPER) AS "with cond",
+ JSON_QUERY(js, '$' WITH UNCONDITIONAL ARRAY WRAPPER) AS "with uncond",
+ JSON_QUERY(js, '$' WITH ARRAY WRAPPER) AS "with"
FROM
(VALUES
(jsonb 'null'),
--
2.46.0
Attachments:
[text/plain] 0001-WIP-Fix-JSON_QUERY-WITH-CONDITIONAL-WRAPPER.patch (1.9K, ../[email protected]/2-0001-WIP-Fix-JSON_QUERY-WITH-CONDITIONAL-WRAPPER.patch)
download | inline diff:
From ea8a36584abf6cea0a05ac43ad8d101012b14313 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <[email protected]>
Date: Wed, 4 Sep 2024 12:09:35 +0200
Subject: [PATCH] WIP: Fix JSON_QUERY WITH CONDITIONAL WRAPPER
---
src/backend/utils/adt/jsonpath_exec.c | 5 +----
src/test/regress/sql/sqljson_queryfuncs.sql | 10 +++++-----
2 files changed, 6 insertions(+), 9 deletions(-)
diff --git a/src/backend/utils/adt/jsonpath_exec.c b/src/backend/utils/adt/jsonpath_exec.c
index e3ee0093d4d..a55a34685f0 100644
--- a/src/backend/utils/adt/jsonpath_exec.c
+++ b/src/backend/utils/adt/jsonpath_exec.c
@@ -3957,10 +3957,7 @@ JsonPathQuery(Datum jb, JsonPath *jp, JsonWrapper wrapper, bool *empty,
else if (wrapper == JSW_UNCONDITIONAL)
wrap = true;
else if (wrapper == JSW_CONDITIONAL)
- wrap = count > 1 ||
- IsAJsonbScalar(singleton) ||
- (singleton->type == jbvBinary &&
- JsonContainerIsScalar(singleton->val.binary.data));
+ wrap = count > 1;
else
{
elog(ERROR, "unrecognized json wrapper %d", (int) wrapper);
diff --git a/src/test/regress/sql/sqljson_queryfuncs.sql b/src/test/regress/sql/sqljson_queryfuncs.sql
index 21ff7787a28..a7342416a9c 100644
--- a/src/test/regress/sql/sqljson_queryfuncs.sql
+++ b/src/test/regress/sql/sqljson_queryfuncs.sql
@@ -146,11 +146,11 @@ CREATE DOMAIN rgb AS rainbow CHECK (VALUE IN ('red', 'green', 'blue'));
SELECT JSON_VALUE(NULL::jsonb, '$');
SELECT
- JSON_QUERY(js, '$'),
- JSON_QUERY(js, '$' WITHOUT WRAPPER),
- JSON_QUERY(js, '$' WITH CONDITIONAL WRAPPER),
- JSON_QUERY(js, '$' WITH UNCONDITIONAL ARRAY WRAPPER),
- JSON_QUERY(js, '$' WITH ARRAY WRAPPER)
+ JSON_QUERY(js, '$') AS "unspec",
+ JSON_QUERY(js, '$' WITHOUT WRAPPER) AS "without",
+ JSON_QUERY(js, '$' WITH CONDITIONAL WRAPPER) AS "with cond",
+ JSON_QUERY(js, '$' WITH UNCONDITIONAL ARRAY WRAPPER) AS "with uncond",
+ JSON_QUERY(js, '$' WITH ARRAY WRAPPER) AS "with"
FROM
(VALUES
(jsonb 'null'),
--
2.46.0
view thread (5+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected]
Subject: Re: json_query conditional wrapper bug
In-Reply-To: <[email protected]>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox