Re: SQL/JSON revisited

Amit Langote <amitlangote09@gmail.com>

From: Amit Langote <amitlangote09@gmail.com>
To: Justin Pryzby <pryzby@telsasoft.com>
Cc: Alvaro Herrera <alvherre@alvh.no-ip.org>, Andrew Dunstan <andrew@dunslane.net>, Peter Eisentraut <peter.eisentraut@enterprisedb.com>, Andres Freund <andres@anarazel.de>, vignesh C <vignesh21@gmail.com>, pgsql-hackers@postgresql.org, e.indrupskaya@postgrespro.ru
Date: 2023-03-28T03:29:23Z
Lists: pgsql-hackers

Attachments

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

Commits

  1. Add SQL/JSON query functions

  2. Add soft error handling to some expression nodes

  3. Adjust populate_record_field() to handle errors softly

  4. Refactor code used by jsonpath executor to fetch variables

  5. Code review for recent SQL/JSON commits

  6. Fix inconsistencies and style issues in new SQL/JSON code

  7. Don't install postmaster symlink anymore

  8. Revert SQL/JSON features

  9. Doc: standardize markup a bit more.