Re: SQL/JSON revisited

From: Amit Langote <amitlangote09(at)gmail(dot)com>
To: Justin Pryzby <pryzby(at)telsasoft(dot)com>
Cc: Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>, Andrew Dunstan <andrew(at)dunslane(dot)net>, Peter Eisentraut <peter(dot)eisentraut(at)enterprisedb(dot)com>, Andres Freund <andres(at)anarazel(dot)de>, vignesh C <vignesh21(at)gmail(dot)com>, pgsql-hackers(at)postgresql(dot)org, e(dot)indrupskaya(at)postgrespro(dot)ru
Subject: Re: SQL/JSON revisited
Date: 2023-03-28 03:29:23
Message-ID: CA+HiwqGAJb+mQKFPf6Vov9HsdzCSq21Guy4uwerZDAyWWZqNaQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tue, Mar 28, 2023 at 6:18 AM Justin Pryzby <pryzby(at)telsasoft(dot)com> wrote:
> I ran sqlsmith on this patch for a short while, and reduced one of its
> appalling queries to this:
>
> postgres=# SELECT jsonb_object_agg_unique_strict('', null::xid8);
> ERROR: unexpected jsonb type as object key

I think this may have to do with the following changes to
uniqueifyJsonbObject() that the patch makes:

@@ -1936,7 +1942,7 @@ lengthCompareJsonbPair(const void *a, const void
*b, void *binequal)
* Sort and unique-ify pairs in JsonbValue object
*/
static void
-uniqueifyJsonbObject(JsonbValue *object)
+uniqueifyJsonbObject(JsonbValue *object, bool unique_keys, bool skip_nulls)
{
bool hasNonUniq = false;

@@ -1946,15 +1952,32 @@ uniqueifyJsonbObject(JsonbValue *object)
qsort_arg(object->val.object.pairs, object->val.object.nPairs,
sizeof(JsonbPair),
lengthCompareJsonbPair, &hasNonUniq);

- if (hasNonUniq)
+ if (hasNonUniq && unique_keys)
+ ereport(ERROR,
+ errcode(ERRCODE_DUPLICATE_JSON_OBJECT_KEY_VALUE),
+ errmsg("duplicate JSON object key value"));
+
+ if (hasNonUniq || skip_nulls)
{
- JsonbPair *ptr = object->val.object.pairs + 1,
- *res = object->val.object.pairs;
+ JsonbPair *ptr,
+ *res;
+
+ while (skip_nulls && object->val.object.nPairs > 0 &&
+ object->val.object.pairs->value.type == jbvNull)
+ {
+ /* If skip_nulls is true, remove leading items with null */
+ object->val.object.pairs++;
+ object->val.object.nPairs--;
+ }
+
+ ptr = object->val.object.pairs + 1;
+ res = object->val.object.pairs;

The code below the while loop does not take into account the
possibility that object->val.object.pairs would be pointing to garbage
when object->val.object.nPairs is 0.

Attached delta patch that applies on top of Alvaro's v12-0001 fixes
the case for me:

postgres=# SELECT jsonb_object_agg_unique_strict('', null::xid8);
jsonb_object_agg_unique_strict
--------------------------------
{}
(1 row)

postgres=# SELECT jsonb_object_agg_unique_strict('1', null::xid8);
jsonb_object_agg_unique_strict
--------------------------------
{}
(1 row)

SELECT jsonb_object_agg_unique_strict('1', '1'::xid8);
jsonb_object_agg_unique_strict
--------------------------------
{"1": "1"}
(1 row)

--
Thanks, Amit Langote
EDB: http://www.enterprisedb.com

Attachment Content-Type Size
v12-0001-delta-uniqueifyJsonbObject-bugfix.patch application/octet-stream 1.3 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Kyotaro Horiguchi 2023-03-28 03:36:15 Re: Reconcile stats in find_tabstat_entry() and get rid of PgStat_BackendFunctionEntry
Previous Message Peter Geoghegan 2023-03-28 03:12:11 Re: Why mark empty pages all visible?