From: Tom Lane <tgl@sss.pgh.pa.us>
To: pgsql-hackers@lists.postgresql.org
Subject: Re: Weird special case in jsonb_concat()
Date: Sat, 19 Dec 2020 15:35:28 -0500
Message-ID: <333852.1608410128@sss.pgh.pa.us> (raw)
In-Reply-To: <163099.1608312033@sss.pgh.pa.us>
References: <163099.1608312033@sss.pgh.pa.us>
I wrote:
> However, further experimentation found a case that fails:
> regression=# select '3'::jsonb || '{}'::jsonb;
> ERROR: invalid concatenation of jsonb objects
> I wonder what is the point of this weird exception, and whether
> whoever devised it can provide a concise explanation of what
> they think the full behavior of "jsonb || jsonb" is. Why isn't
> '[3, {}]' a reasonable result here, if the cases above are OK?
Here is a proposed patch for that. It turns out that the third
else-branch in IteratorConcat() already does the right thing, if
we just remove its restrictive else-condition and let it handle
everything except the two-objects and two-arrays cases. But it
seemed to me that trying to handle both the object || array
and array || object cases in that one else-branch was poorly
thought out: only one line of code can actually be shared, and it
took several extra lines of infrastructure to support the sharing.
So I split those cases into separate else-branches.
This also addresses the inadequate documentation that was the
original complaint.
Thoughts? Should we back-patch this? The existing behavior
seems to me to be inconsistent enough to be arguably a bug,
but we've not had field complaints saying "this should work".
regards, tom lane
Attachments:fix-jsonb-concatenation-special-cases.patch (text/x-diff, 7.0K) downloaddiff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgmlindex df29af6371..5f3c393a25 100644--- a/doc/src/sgml/func.sgml+++ b/doc/src/sgml/func.sgml@@ -14715,8 +14715,12 @@ table2-mapping
</para>
<para>
Concatenates two <type>jsonb</type> values.
- Concatenating two objects generates an object with the union of their+ Concatenating two arrays generates an array containing all the+ elements of each input. Concatenating two objects generates an+ object containing the union of their
keys, taking the second object's value when there are duplicate keys.
+ All other cases are treated by converting a non-array input into a+ single-element array, and then proceeding as for two arrays.
Does not operate recursively: only the top-level array or object
structure is merged.
</para>
@@ -14727,6 +14731,14 @@ table2-mapping
<para>
<literal>'{"a": "b"}'::jsonb || '{"c": "d"}'::jsonb</literal>
<returnvalue>{"a": "b", "c": "d"}</returnvalue>
+ </para>+ <para>+ <literal>'[1, 2]'::jsonb || '3'::jsonb</literal>+ <returnvalue>[1, 2, 3]</returnvalue>+ </para>+ <para>+ <literal>'{"a": "b"}'::jsonb || '42'::jsonb</literal>+ <returnvalue>[{"a": "b"}, 42]</returnvalue>
</para></entry>
</row>
diff --git a/src/backend/utils/adt/jsonfuncs.c b/src/backend/utils/adt/jsonfuncs.cindex 7a25415078..5beade00f4 100644--- a/src/backend/utils/adt/jsonfuncs.c+++ b/src/backend/utils/adt/jsonfuncs.c@@ -4690,11 +4690,14 @@ IteratorConcat(JsonbIterator **it1, JsonbIterator **it2,
rk2 = JsonbIteratorNext(it2, &v2, false);
/*
- * Both elements are objects.+ * JsonbIteratorNext reports raw scalars as if they were single-element+ * arrays; hence we only need consider "object" and "array" cases here.
*/
if (rk1 == WJB_BEGIN_OBJECT && rk2 == WJB_BEGIN_OBJECT)
{
/*
+ * Both elements are objects.+ *
* Append all the tokens from v1 to res, except last WJB_END_OBJECT
* (because res will not be finished yet).
*/
@@ -4703,18 +4706,18 @@ IteratorConcat(JsonbIterator **it1, JsonbIterator **it2,
pushJsonbValue(state, r1, &v1);
/*
- * Append all the tokens from v2 to res, include last WJB_END_OBJECT- * (the concatenation will be completed).+ * Append all the tokens from v2 to res, including last WJB_END_OBJECT+ * (the concatenation will be completed). Any duplicate keys will+ * automatically override the value from the first object.
*/
while ((r2 = JsonbIteratorNext(it2, &v2, true)) != WJB_DONE)
res = pushJsonbValue(state, r2, r2 != WJB_END_OBJECT ? &v2 : NULL);
}
-- /*- * Both elements are arrays (either can be scalar).- */
else if (rk1 == WJB_BEGIN_ARRAY && rk2 == WJB_BEGIN_ARRAY)
{
+ /*+ * Both elements are arrays.+ */
pushJsonbValue(state, rk1, NULL);
while ((r1 = JsonbIteratorNext(it1, &v1, true)) != WJB_END_ARRAY)
@@ -4731,46 +4734,40 @@ IteratorConcat(JsonbIterator **it1, JsonbIterator **it2,
res = pushJsonbValue(state, WJB_END_ARRAY, NULL /* signal to sort */ );
}
- /* have we got array || object or object || array? */- else if (((rk1 == WJB_BEGIN_ARRAY && !(*it1)->isScalar) && rk2 == WJB_BEGIN_OBJECT) ||- (rk1 == WJB_BEGIN_OBJECT && (rk2 == WJB_BEGIN_ARRAY && !(*it2)->isScalar)))+ else if (rk1 == WJB_BEGIN_OBJECT)
{
- JsonbIterator **it_array = rk1 == WJB_BEGIN_ARRAY ? it1 : it2;- JsonbIterator **it_object = rk1 == WJB_BEGIN_OBJECT ? it1 : it2;- bool prepend = (rk1 == WJB_BEGIN_OBJECT);+ /*+ * We have object || array.+ */+ Assert(rk2 == WJB_BEGIN_ARRAY);
pushJsonbValue(state, WJB_BEGIN_ARRAY, NULL);
- if (prepend)- {- pushJsonbValue(state, WJB_BEGIN_OBJECT, NULL);- while ((r1 = JsonbIteratorNext(it_object, &v1, true)) != WJB_DONE)- pushJsonbValue(state, r1, r1 != WJB_END_OBJECT ? &v1 : NULL);-- while ((r2 = JsonbIteratorNext(it_array, &v2, true)) != WJB_DONE)- res = pushJsonbValue(state, r2, r2 != WJB_END_ARRAY ? &v2 : NULL);- }- else- {- while ((r1 = JsonbIteratorNext(it_array, &v1, true)) != WJB_END_ARRAY)- pushJsonbValue(state, r1, &v1);-- pushJsonbValue(state, WJB_BEGIN_OBJECT, NULL);- while ((r2 = JsonbIteratorNext(it_object, &v2, true)) != WJB_DONE)- pushJsonbValue(state, r2, r2 != WJB_END_OBJECT ? &v2 : NULL);+ pushJsonbValue(state, WJB_BEGIN_OBJECT, NULL);+ while ((r1 = JsonbIteratorNext(it1, &v1, true)) != WJB_DONE)+ pushJsonbValue(state, r1, r1 != WJB_END_OBJECT ? &v1 : NULL);- res = pushJsonbValue(state, WJB_END_ARRAY, NULL);- }+ while ((r2 = JsonbIteratorNext(it2, &v2, true)) != WJB_DONE)+ res = pushJsonbValue(state, r2, r2 != WJB_END_ARRAY ? &v2 : NULL);
}
else
{
/*
- * This must be scalar || object or object || scalar, as that's all- * that's left. Both of these make no sense, so error out.+ * We have array || object.
*/
- ereport(ERROR,- (errcode(ERRCODE_INVALID_PARAMETER_VALUE),- errmsg("invalid concatenation of jsonb objects")));+ Assert(rk1 == WJB_BEGIN_ARRAY);+ Assert(rk2 == WJB_BEGIN_OBJECT);++ pushJsonbValue(state, WJB_BEGIN_ARRAY, NULL);++ while ((r1 = JsonbIteratorNext(it1, &v1, true)) != WJB_END_ARRAY)+ pushJsonbValue(state, r1, &v1);++ pushJsonbValue(state, WJB_BEGIN_OBJECT, NULL);+ while ((r2 = JsonbIteratorNext(it2, &v2, true)) != WJB_DONE)+ pushJsonbValue(state, r2, r2 != WJB_END_OBJECT ? &v2 : NULL);++ res = pushJsonbValue(state, WJB_END_ARRAY, NULL);
}
return res;
diff --git a/src/test/regress/expected/jsonb.out b/src/test/regress/expected/jsonb.outindex a70cd0b7c1..1e6c6ef200 100644--- a/src/test/regress/expected/jsonb.out+++ b/src/test/regress/expected/jsonb.out@@ -4111,9 +4111,41 @@ select '{"a":"b"}'::jsonb || '[]'::jsonb;
(1 row)
select '"a"'::jsonb || '{"a":1}';
-ERROR: invalid concatenation of jsonb objects+ ?column? +-----------------+ ["a", {"a": 1}]+(1 row)+
select '{"a":1}' || '"a"'::jsonb;
-ERROR: invalid concatenation of jsonb objects+ ?column? +-----------------+ [{"a": 1}, "a"]+(1 row)++select '[3]'::jsonb || '{}'::jsonb;+ ?column? +----------+ [3, {}]+(1 row)++select '3'::jsonb || '[]'::jsonb;+ ?column? +----------+ [3]+(1 row)++select '3'::jsonb || '4'::jsonb;+ ?column? +----------+ [3, 4]+(1 row)++select '3'::jsonb || '{}'::jsonb;+ ?column? +----------+ [3, {}]+(1 row)+
select '["a", "b"]'::jsonb || '{"c":1}';
?column?
----------------------
diff --git a/src/test/regress/sql/jsonb.sql b/src/test/regress/sql/jsonb.sqlindex 3e2b8f66df..b6409767f6 100644--- a/src/test/regress/sql/jsonb.sql+++ b/src/test/regress/sql/jsonb.sql@@ -1056,6 +1056,11 @@ select '{"a":"b"}'::jsonb || '[]'::jsonb;
select '"a"'::jsonb || '{"a":1}';
select '{"a":1}' || '"a"'::jsonb;
+select '[3]'::jsonb || '{}'::jsonb;+select '3'::jsonb || '[]'::jsonb;+select '3'::jsonb || '4'::jsonb;+select '3'::jsonb || '{}'::jsonb;+
select '["a", "b"]'::jsonb || '{"c":1}';
select '{"c": 1}'::jsonb || '["a", "b"]';
=
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: pgsql-hackers@postgresql.org
Cc: tgl@sss.pgh.pa.us, pgsql-hackers@lists.postgresql.org
Subject: Re: Weird special case in jsonb_concat()
In-Reply-To: <333852.1608410128@sss.pgh.pa.us>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox