Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1utHlQ-004A7K-Qq for pgsql-hackers@arkaria.postgresql.org; Tue, 02 Sep 2025 03:32:58 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.94.2) (envelope-from ) id 1utHlP-001nDE-Pq for pgsql-hackers@arkaria.postgresql.org; Tue, 02 Sep 2025 03:32:56 +0000 Received: from makus.postgresql.org ([2001:4800:3e1:1::229]) by malur.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1utHlP-001nD6-4i for pgsql-hackers@lists.postgresql.org; Tue, 02 Sep 2025 03:32:55 +0000 Received: from mail-pf1-x42a.google.com ([2607:f8b0:4864:20::42a]) by makus.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256 (Exim 4.96) (envelope-from ) id 1utHlN-0005ws-0y for pgsql-hackers@postgresql.org; Tue, 02 Sep 2025 03:32:54 +0000 Received: by mail-pf1-x42a.google.com with SMTP id d2e1a72fcca58-77264a94031so631870b3a.2 for ; Mon, 01 Sep 2025 20:32:53 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1756783972; x=1757388772; darn=postgresql.org; h=references:to:cc:in-reply-to:date:subject:mime-version:message-id :from:from:to:cc:subject:date:message-id:reply-to; bh=s2p06AbsK75hBTbope/D+XckjVnoruEuAtOXFmSTPxs=; b=JxzdE2Hn0GM4WL0i+BT/sCnUW+pyLbsn9VB4cntuUjdoS9TG1t5uFdE9ZajcfFmWfy oKKuZgHafEX65ChgDMOJ+GfLJFgJ/ki4b8s8ga+hktZBRBpzb7WD7XiWSwgQobw/6sUZ k97W+FT46IN5FBYkaA8TTTIAd9PK73SbvP52a50e3c7YmJBO7rPvpRI9cRmPkarTehu8 nHKZuHwg+QrsiK/rmmWIRZbI+PpZutUwEQEHp9sbeCGtD7+sQFaE2Lq/FjIGaQRuyWkr DLTdzjryRxQiv2MtMHjeuan5jn14Bl53K1jcngAzcqXP092CVUnrjP+Jq3RfoLH93iS4 Secg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1756783972; x=1757388772; h=references:to:cc:in-reply-to:date:subject:mime-version:message-id :from:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=s2p06AbsK75hBTbope/D+XckjVnoruEuAtOXFmSTPxs=; b=WSi/hsYdr8rrjQgbaMcaoowqLO9nkX3m7eUP+WOqtUtA8mx4ZRxrrsrjV4hdIEPKjL AYSUHDX04xN8ROYmhBEGMs7eXCPDEDRhNjyg4NZV0m/IkCdN3Qi9/rEKF4tnRrWV6if3 ddFExwgWi6KNwElFvJq2LaBagkMWC7DoouE1PQlB56pFflSdR+gpm+XDrK2VPYbmwZPv N/HCpPMoBwePBCdCGTqeQbU7lhKMmbdr/kgDhhAanhMCwdJLvE08vhESHvXDwNNg+Mvy Q/8pZDamj+17hmEL3+j7dVl4ZUbLgzZhBKX6p0zzZzPkTiYusPgatw+LGeDQoXvNuEes LepQ== X-Forwarded-Encrypted: i=1; AJvYcCVpcfUBD1h1HlTC5/E5HxKoA7VpHi4cwcD6FNeIeczCVQBD8FKVQkUU4xqvVrZIzCFKq+HVouizjrShPrYZ@postgresql.org X-Gm-Message-State: AOJu0YxheFip4pJLKdFrRKCdocK92NBNV7xgf21USocEirOCvIR91X59 EEq0hDFKSEJibtVXqrx0BqEZ1be31H5dlk4nqIsxLS3NHf7eRxijjxnR X-Gm-Gg: ASbGncsSspG0Di3fkFZAoxFmx+F3DYa4gpwjWMqH5OR70n7Ru/I5x2yMEkpamNKfccf JDg07L0IlpezVjKKcef9LKDVeHGh3PmlN59DDuoD980Mt6qdvxWUPmpoPsY4XOp7qsDmziJzcAk cE1ynLap+lx/WHW918DGl9l/HYw1pv9r5DTPhQn9u8IezgIqUHdfHB7+sWSQ9/iPZ/jKw5kSlLZ xI4zv7gf7VjbFie1EgUxIBCEi0RP5TfFf1ok6kp+0ITQfJq3lXQxVIkNnoQwEGDceHkVwAjPAoV Fz1wRJIwx0z/fmkXlO0Fw0MyR6S0z4CQTcztJbAPrvDFHnAKLwABp0YWLs/lbCHHLkNtMkxxKxP t4mdzTrlhkmJJ5RSaAlFtDwsGEnGFnqbmhMda2RyUud0= X-Google-Smtp-Source: AGHT+IF9HmPwtdinecpLKzYGa1vfmtP7PG9AoTyxZXnWegT5fl9d/X+mAW2YyUt27+wYh5iRniwRXg== X-Received: by 2002:a05:6a00:2314:b0:76b:e805:30e4 with SMTP id d2e1a72fcca58-7723e388591mr12574180b3a.24.1756783971959; Mon, 01 Sep 2025 20:32:51 -0700 (PDT) Received: from smtpclient.apple ([142.171.105.12]) by smtp.gmail.com with ESMTPSA id d2e1a72fcca58-7722a2af911sm11817845b3a.23.2025.09.01.20.32.47 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Mon, 01 Sep 2025 20:32:51 -0700 (PDT) From: Chao Li Message-Id: Content-Type: multipart/alternative; boundary="Apple-Mail=_2DAF94D3-1518-473F-8801-F4AB0999AA85" Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3826.700.81\)) Subject: Re: SQL:2023 JSON simplified accessor support Date: Tue, 2 Sep 2025 11:32:33 +0800 In-Reply-To: <98C6F445-773B-4C82-8198-7F5CD56D5252@gmail.com> Cc: jian he , Nikita Malakhov , Vik Fearing , Mark Dilger , Matheus Alcantara , Peter Eisentraut , Andrew Dunstan , PostgreSQL Hackers , "David E. Wheeler" To: Alexandra Wang , Nikita Glukhov References: <21fa36b4-6d56-4556-b524-9e3955b922c8@eisentraut.org> <290418A2-3A9B-4304-AAAD-4A40CFF1CB93@gmail.com> <98C6F445-773B-4C82-8198-7F5CD56D5252@gmail.com> X-Mailer: Apple Mail (2.3826.700.81) List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Archived-At: Precedence: bulk --Apple-Mail=_2DAF94D3-1518-473F-8801-F4AB0999AA85 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=utf-8 > On Sep 2, 2025, at 10:59, Chao Li wrote: >=20 > jsonbsubs doesn=E2=80=99t need to check is_slice flag as well, I will = explain that in my next email tougher with my new comments. Now jsonb_check_json_needed() loops over all indirections: static bool jsonb_check_jsonpath_needed(List *indirection) { ListCell *lc; foreach(lc, indirection) { Node *accessor =3D lfirst(lc); if (IsA(accessor, String)) return true; else { Assert(IsA(accessor, A_Indices)); if (castNode(A_Indices, accessor)->is_slice) return true; } } return false; } Let=E2=80=99s consider this statement: select data[=E2=80=98a=E2=80=99][=E2= =80=98b=E2=80=99].c from jsonb_table. For this indirection list, first two elements are non-slice indices, and = the third is a string accessor. So json_check_jsonpath_needed() will = return true. Then: static void jsonb_subscript_transform(SubscriptingRef *sbsref, List **indirection, ParseState *pstate, bool isSlice, bool isAssignment) { List *upperIndexpr =3D NIL; ListCell *idx; /* Determine the result type of the subscripting operation; always = jsonb */ sbsref->refrestype =3D JSONBOID; sbsref->reftypmod =3D -1; if (isSlice || jsonb_check_jsonpath_needed(*indirection)) { jsonb_subscript_make_jsonpath(pstate, indirection, sbsref); if (sbsref->refjsonbpath) return; } It will fall into the call to json_subscript_make_jsonpath(), and it = cannot process [=E2=80=98a=E2=80=99], so sbsref->refjsonbpath will be = NULL, then it will fall through down to the logic of processing = [=E2=80=98a=E2=80=99]. That=E2=80=99s actually a waste of looping over the entire indirection = list and making an unnecessary call to jsonb_subscript_make_jsonpath(). In json_check_jsonpath_needed(), we can only check the first item. Also, = as jsonb_check_jsonpath_needed() already has the logic of checking = is_slice, here in jsonb_subscript_transform(), we don=E2=80=99t need to = check =E2=80=9CisSlice=E2=80=9D in the =E2=80=9Cif=E2=80=9D condition at = all. I made the change locally, and =E2=80=9Cmake check=E2=80=9D passed. My dirty change is like: ``` @@ -118,11 +118,11 @@ = coerce_jsonpath_subscript_to_int4_or_text(ParseState *pstate, Node = *subExpr) static bool jsonb_check_jsonpath_needed(List *indirection) { - ListCell *lc; + //ListCell *lc; - foreach(lc, indirection) - { - Node *accessor =3D lfirst(lc); + //foreach(lc, indirection) + //{ + Node *accessor =3D lfirst(list_head(indirection)); if (IsA(accessor, String) || IsA(accessor, A_Star)) return true; @@ -133,7 +133,8 @@ jsonb_check_jsonpath_needed(List *indirection) if (castNode(A_Indices, accessor)->is_slice) return true; } - } + //break; + //} return false; } @@ -401,7 +435,7 @@ jsonb_subscript_transform(SubscriptingRef *sbsref, sbsref->refrestype =3D JSONBOID; sbsref->reftypmod =3D -1; - if (isSlice || jsonb_check_jsonpath_needed(*indirection)) + if (jsonb_check_jsonpath_needed(*indirection)) { jsonb_subscript_make_jsonpath(pstate, indirection, = sbsref); if (sbsref->refjsonbpath) ``` The other comment is that jsonb_subscript_make_jsonpath() can be = optimized a little bit. When indices and dot notations are mixed, it = will always hit the clause =E2=80=9Cif (api_from =3D=3D NULL)=E2=80=9D. = In this case, memory of jpi has been allocated, and with the =E2=80=9Cif=E2= =80=9D we need to free the memory. So, we can pull up calculating = jpi_from, if it is NULL, then we don=E2=80=99t need to allocate and free = memory. My dirty change is like: ``` else if (IsA(accessor, A_Indices)) { + JsonPathParseItem *jpi_from =3D NULL; A_Indices *ai =3D castNode(A_Indices, = accessor); + if (!ai->is_slice) + { + Assert(ai->uidx && !ai->lidx); + jpi_from =3D = make_jsonpath_item_expr(pstate, ai->uidx, &sbsref->refupperindexpr, = true); + if (jpi_from =3D=3D NULL) + { + /* + * Break out of the loop if the = subscript is not a + * non-null integer constant, so = that we can fall back to + * jsonb subscripting logic. + * + * This is needed to handle = cases with mixed usage of SQL + * standard json simplified = accessor syntax and PostgreSQL + * jsonb subscripting syntax, = e.g: + * + * select (jb).a['b'].c from = jsonb_table; + * + * where dot-notation (.a and = .c) is the SQL standard json + * simplified accessor syntax, = and the ['b'] subscript is + * the PostgreSQL jsonb = subscripting syntax, because 'b' + * is not a non-null constant = integer and cannot be used + * for json array access. + * + * In this case, we cannot = create a JsonPath item, so we + * break out of the loop and let + * jsonb_subscript_transform() = handle this indirection as + * a PostgreSQL jsonb subscript. + */ + break; + } + } + jpi =3D make_jsonpath_item(jpiIndexArray); jpi->value.array.nelems =3D 1; jpi->value.array.elems =3D = palloc(sizeof(jpi->value.array.elems[0])); @@ -303,12 +337,12 @@ jsonb_subscript_make_jsonpath(ParseState *pstate, = List **indirection, Subscripti } else { - JsonPathParseItem *jpi_from =3D NULL; + //JsonPathParseItem *jpi_from =3D NULL; - Assert(ai->uidx && !ai->lidx); - jpi_from =3D = make_jsonpath_item_expr(pstate, ai->uidx, &sbsref->refupperindexpr, = true); - if (jpi_from =3D=3D NULL) - { + //Assert(ai->uidx && !ai->lidx); + //jpi_from =3D = make_jsonpath_item_expr(pstate, ai->uidx, &sbsref->refupperindexpr, = true); + //if (jpi_from =3D=3D NULL) + //{ /* * Break out of the loop if the = subscript is not a * non-null integer constant, so = that we can fall back to @@ -331,10 +365,10 @@ jsonb_subscript_make_jsonpath(ParseState *pstate, = List **indirection, Subscripti * jsonb_subscript_transform() = handle this indirection as * a PostgreSQL jsonb subscript. */ - pfree(jpi->value.array.elems); - pfree(jpi); - break; - } + // pfree(jpi->value.array.elems); + // pfree(jpi); + // break; + //} jpi->value.array.elems[0].from =3D = jpi_from; jpi->value.array.elems[0].to =3D NULL; ``` With this change, =E2=80=9Cmake check=E2=80=9D also passed. The third comment is about test cases. I only see tests for indices = following dot notation, like data.a[=E2=80=98b=E2=80=99], but not the = reverse. Let=E2=80=99s add a few test cases for dot notation following = indices, like data[=E2=80=98a=E2=80=99].b. The last comment is about error message: ``` evantest=3D# select data.a from test_jsonb_types; ERROR: missing FROM-clause entry for table "data" LINE 1: select data.a from test_jsonb_types; ``` =E2=80=9CMissing FROM-clause entry=E2=80=9D is quite confusing and not = providing much useful hint to resolve the problem. When I first saw this = error, I was stuck. Only after read through the commit comments, I = figured out the problem. For end users, we should provide some more = meaningful error messages. Best regards, -- Chao Li (Evan) HighGo Software Co., Ltd. https://www.highgo.com/ --Apple-Mail=_2DAF94D3-1518-473F-8801-F4AB0999AA85 Content-Transfer-Encoding: quoted-printable Content-Type: text/html; charset=utf-8

On Sep 2, 2025, at 10:59, Chao Li = <li.evan.chao@gmail.com> wrote:

jsonbsubs doesn=E2=80=99t need to check = is_slice flag as well, I will explain that in my next email tougher with = my new = comments.


Now jsonb_check_json_needed() loops over all = indirections:

static bool
jsonb_check_jsonpath_needed(List *indirection)
{
ListCell *lc;

foreach(lc, = indirection)
{
Node *accessor =3D lfirst(lc);

if (IsA(accessor, String))
return true;
else
{
Assert(IsA(accessor, A_Indices));

if (castNode(A_Indices, accessor)->is_slice)
return true;
}
}

return false;
}

Let=E2=80=99s = consider this statement: select data[=E2=80=98a=E2=80=99][=E2=80=98b=E2=80= =99].c from jsonb_table.

For this indirection = list, first two elements are non-slice indices, and the third is a = string accessor. So json_check_jsonpath_needed() will return = true.

Then:

static void
jsonb_subscript_transform(SubscriptingRef *sbsref,
List **indirection,
ParseState = *pstate,
bool isSlice,
bool isAssignment)
{
List *upperIndexpr =3D NIL;
ListCell *idx;

= /* Determine the result type of the subscripting operation; always = jsonb */
sbsref->refrestype =3D JSONBOID;
sbsref->reftypmod =3D -1;

if (isSlice || jsonb_check_jsonpath_needed(*indirection))
{
jsonb_subscript_make_jsonpath(pstate, indirection, sbsref);
if (sbsref->refjsonbpath)
return;
}

It will fall into = the call to json_subscript_make_jsonpath(), and it cannot process = [=E2=80=98a=E2=80=99], so sbsref->refjsonbpath will be NULL, then it = will fall through down to the logic of processing = [=E2=80=98a=E2=80=99].

That=E2=80=99s actually = a waste of looping over the entire indirection list and making an = unnecessary call to = jsonb_subscript_make_jsonpath().

In = json_check_jsonpath_needed(), we can only check the first item. Also, as = jsonb_check_jsonpath_needed() already has the logic of checking = is_slice, here in jsonb_subscript_transform(), we don=E2=80=99t need to = check =E2=80=9CisSlice=E2=80=9D in the =E2=80=9Cif=E2=80=9D condition at = all.

I made the change locally, and =E2=80=9Cmake= check=E2=80=9D passed.

My dirty change is = like:
```
@@ -118,11 +118,11 @@ = coerce_jsonpath_subscript_to_int4_or_text(ParseState *pstate, Node = *subExpr)
 static = bool
 jsonb_check_jsonpath_needed(List = *indirection)
 {
-       = ListCell   *lc;
+       //ListCell   = *lc;

-       foreach(lc, = indirection)
-       {
-   =             Node       = *accessor =3D lfirst(lc);
+       //foreach(lc, = indirection)
+       //{
+   =             Node       = *accessor =3D = lfirst(list_head(indirection));

    =             if (IsA(accessor, String) || = IsA(accessor, A_Star))
          =               return = true;
@@ -133,7 +133,8 @@ jsonb_check_jsonpath_needed(List = *indirection)
              =           if (castNode(A_Indices, = accessor)->is_slice)
          =                     =   return true;
            =     }
-       }
+   =             //break;
+   =     //}

        = return false;
 }

@@ = -401,7 +435,7 @@ jsonb_subscript_transform(SubscriptingRef = *sbsref,
        sbsref->refrestype =3D = JSONBOID;
        sbsref->reftypmod =3D = -1;

-       if (isSlice || = jsonb_check_jsonpath_needed(*indirection))
+     =   if (jsonb_check_jsonpath_needed(*indirection))
  =       {
          =       jsonb_subscript_make_jsonpath(pstate, indirection, = sbsref);
              =   if = (sbsref->refjsonbpath)
```

The= other comment is that jsonb_subscript_make_jsonpath() can be optimized = a little bit. When indices and dot notations are mixed, it will always = hit the clause =E2=80=9Cif (api_from =3D=3D NULL)=E2=80=9D. In this = case, memory of jpi has been allocated, and with the =E2=80=9Cif=E2=80=9D = we need to free the memory. So, we can pull up calculating jpi_from, if = it is NULL, then we don=E2=80=99t need to allocate and free memory. My = dirty change is like:

```
  =               else if (IsA(accessor, = A_Indices))
              =   {
+               =         JsonPathParseItem *jpi_from =3D = NULL;
                =         A_Indices  *ai =3D castNode(A_Indices, = accessor);

+           =             if = (!ai->is_slice)
+             =           {
+       =                     =     Assert(ai->uidx && !ai->lidx);
+ =                     =           jpi_from =3D = make_jsonpath_item_expr(pstate, ai->uidx, = &sbsref->refupperindexpr, true);
+       =                     =     if (jpi_from =3D=3D NULL)
+       =                     =     {
+             =                     =       /*
+           =                     =          * Break out of the loop if the = subscript is not a
+             =                     =        * non-null integer constant, so that we can = fall back to
+             =                     =        * jsonb subscripting logic.
+ =                     =                   =  *
+               =                     =      * This is needed to handle cases with mixed usage of = SQL
+                 =                     =    * standard json simplified accessor syntax and = PostgreSQL
+               =                     =      * jsonb subscripting syntax, e.g:
+   =                     =                 =  *
+               =                     =      * select (jb).a['b'].c from jsonb_table;
+ =                     =                   =  *
+               =                     =      * where dot-notation (.a and .c) is the SQL standard = json
+                 =                     =    * simplified accessor syntax, and the ['b'] subscript = is
+                 =                     =    * the PostgreSQL jsonb subscripting syntax, because = 'b'
+                 =                     =    * is not a non-null constant integer and cannot be = used
+                 =                     =    * for json array access.
+       =                     =              *
+   =                     =                  * In this = case, we cannot create a JsonPath item, so we
+     =                     =                * break out of = the loop and let
+             =                     =        * jsonb_subscript_transform() handle this = indirection as
+             =                     =        * a PostgreSQL jsonb subscript.
+ =                     =                   =  */
+               =                     =     break;
+           =                     = }
+                 =       }
+
        =                 jpi =3D = make_jsonpath_item(jpiIndexArray);
        =                 = jpi->value.array.nelems =3D 1;
        =                 = jpi->value.array.elems =3D = palloc(sizeof(jpi->value.array.elems[0]));
@@ -303,12 = +337,12 @@ jsonb_subscript_make_jsonpath(ParseState *pstate, List = **indirection, Subscripti
          =               }
  =                     =   else
              =           {
-       =                     =     JsonPathParseItem *jpi_from =3D NULL;
+   =                     =         //JsonPathParseItem *jpi_from =3D = NULL;

-           =                     = Assert(ai->uidx && !ai->lidx);
-     =                     =       jpi_from =3D make_jsonpath_item_expr(pstate, = ai->uidx, &sbsref->refupperindexpr, true);
-   =                     =         if (jpi_from =3D=3D NULL)
-   =                     =         {
+         =                     =   //Assert(ai->uidx && !ai->lidx);
+   =                     =         //jpi_from =3D = make_jsonpath_item_expr(pstate, ai->uidx, = &sbsref->refupperindexpr, true);
+       =                     =     //if (jpi_from =3D=3D NULL)
+     =                     =       //{
          =                     =           /*
      =                     =                * Break out of = the loop if the subscript is not a
        =                     =              * non-null integer = constant, so that we can fall back to
@@ -331,10 +365,10 @@ = jsonb_subscript_make_jsonpath(ParseState *pstate, List **indirection, = Subscripti
              =                     =        * jsonb_subscript_transform() handle this = indirection as
            =                     =          * a PostgreSQL jsonb = subscript.
              =                     =        */
-         =                     =           = pfree(jpi->value.array.elems);
-       =                     =             pfree(jpi);
-   =                     =                 = break;
-               =                 }
+ =                     =           //     =  pfree(jpi->value.array.elems);
+       =                     =     //      pfree(jpi);
+   =                     =         //      break;
+ =                     =           //}

  =                     =           jpi->value.array.elems[0].from =3D = jpi_from;
              =                   = jpi->value.array.elems[0].to =3D = NULL;
```

With this change, = =E2=80=9Cmake check=E2=80=9D also passed.

The = third comment is about test cases. I only see tests for indices = following dot notation, like data.a[=E2=80=98b=E2=80=99], but not the = reverse. Let=E2=80=99s add a few test cases for dot notation following = indices, like data[=E2=80=98a=E2=80=99].b.

The = last comment is about error = message:

```
evantest=3D# select = data.a from test_jsonb_types;
ERROR:  missing FROM-clause = entry for table "data"
LINE 1: select data.a from = test_jsonb_types;
```

=E2=80=9CMi= ssing FROM-clause entry=E2=80=9D is quite confusing and not providing = much useful hint to resolve the problem. When I first saw this error, I = was stuck. Only after read through the commit comments, I figured out = the problem. For end users, we should provide some more meaningful error = messages.


Best regards,
--
Chao Li (Evan)
HighGo Software = Co., Ltd.
https://www.highgo.com/




= --Apple-Mail=_2DAF94D3-1518-473F-8801-F4AB0999AA85--