Horton

Re: SQL:2023 JSON simplified accessor support

Chao Li <li.evan.chao@gmail.com>

View thread
From: Chao Li <li.evan.chao@gmail.com>
To: Alexandra Wang <alexandra.wang.oss@gmail.com>
Cc: jian he <jian.universality@gmail.com>, Nikita Malakhov <hukutoc@gmail.com>, Vik Fearing <vik@postgresfriends.org>, Mark Dilger <mark.dilger@enterprisedb.com>, Matheus Alcantara <matheusssilv97@gmail.com>, Peter Eisentraut <peter@eisentraut.org>, Andrew Dunstan <andrew@dunslane.net>, Nikita Glukhov <glukhov.n.a@gmail.com>, PostgreSQL Hackers <pgsql-hackers@postgresql.org>, "David E. Wheeler" <david@justatheory.com>
Date: 2025-09-02T02:59:39Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Add test coverage for indirection transformation

  2. Fix typo in comment

  3. Implementation of subscripting for jsonb


> On Aug 29, 2025, at 15:22, Chao Li <li.evan.chao@gmail.com> wrote:
> 
> +static bool
> +jsonb_check_jsonpath_needed(List *indirection)
> +{
> +       ListCell   *lc;
> +
> +       foreach(lc, indirection)
> +       {
> +               Node       *accessor = lfirst(lc);
> +
> +               if (IsA(accessor, String))
> +                       return true;
> 
> Like I mentioned in my previous comment, if we convert String (dot notation) to Indices in rewritten phase, then jsonpath might be no longer needed, and the code logic is much simplified.
> 

Taking back this comment. As Alex explained in the other email, dot notation and indices accessor are different.

> 
> --- a/src/backend/parser/parse_node.c
> +++ b/src/backend/parser/parse_node.c
> @@ -244,7 +244,7 @@ transformContainerSubscripts(ParseState *pstate,
>                                                          Node *containerBase,
>                                                          Oid containerType,
>                                                          int32 containerTypMod,
> -                                                        List *indirection,
> +                                                        List **indirection,
>                                                          bool isAssignment)
>  {
>         SubscriptingRef *sbsref;
> @@ -280,7 +280,7 @@ transformContainerSubscripts(ParseState *pstate,
>          * element.  If any of the items are slice specifiers (lower:upper), then
>          * the subscript expression means a container slice operation.
>          */
> -       foreach(idx, indirection)
> +       foreach(idx, *indirection)
>         {
>                 A_Indices  *ai = lfirst_node(A_Indices, idx);
> 
> Should this foreach be pull up to out of transformContainerSubscripts()? Originally transformContainerSubscripts() runs only once, but now it’s placed in a while loop, and every time this foreach needs to go through the entire indirection list, which are duplicated computation.


After revisiting the code, I think this loop of checking is_slice should be removed here.

It seems only arrsubs cares about this is_slice flag.

hstore_subs can only take a single indirection, so it can do a simple check like:

```
ai = initial_node(A_Indices, *indirection);
If (list_length(*indirection) != 1 || ai->is_slice)
{
    ereport(…)
}
```

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

So that, “bool isSlace” can be removed from SubscriptTransform, and let every individual subs check is_slice.

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