Re: SQL/JSON revisited - Mailing list pgsql-hackers

From Amit Langote
Subject Re: SQL/JSON revisited
Date
Msg-id CA+HiwqGAJb+mQKFPf6Vov9HsdzCSq21Guy4uwerZDAyWWZqNaQ@mail.gmail.com
Whole thread Raw
In response to Re: SQL/JSON revisited  (Justin Pryzby <pryzby@telsasoft.com>)
List pgsql-hackers
On Tue, Mar 28, 2023 at 6:18 AM Justin Pryzby <pryzby@telsasoft.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

pgsql-hackers by date:

Previous
From: Peter Geoghegan
Date:
Subject: Re: Why mark empty pages all visible?
Next
From: Kyotaro Horiguchi
Date:
Subject: Re: Reconcile stats in find_tabstat_entry() and get rid of PgStat_BackendFunctionEntry