Re: SQL:2023 JSON simplified accessor support

From: Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com>
To: Alexandra Wang <alexandra(dot)wang(dot)oss(at)gmail(dot)com>
Cc: PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>, Nikita Glukhov <glukhov(dot)n(dot)a(at)gmail(dot)com>, jian he <jian(dot)universality(at)gmail(dot)com>, Nikita Malakhov <hukutoc(at)gmail(dot)com>, Vik Fearing <vik(at)postgresfriends(dot)org>, Mark Dilger <mark(dot)dilger(at)enterprisedb(dot)com>, Matheus Alcantara <matheusssilv97(at)gmail(dot)com>, Peter Eisentraut <peter(at)eisentraut(dot)org>, Andrew Dunstan <andrew(at)dunslane(dot)net>, "David E(dot) Wheeler" <david(at)justatheory(dot)com>
Subject: Re: SQL:2023 JSON simplified accessor support
Date: 2025-09-23 05:48:24
Message-ID: F4BF37C7-E219-40B2-B230-C9892F68E5BA@gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

The new approach of introducing “transform_partial” looks like a better solution, which leads to less code change to hstore_subs and arraysubs. However, when I tested the v21, I encountered errors when combine composite type, array and jsonb together.

Prepare test data:
```
drop table if exists people;
drop type if exists person;
CREATE TYPE person AS (
name text,
size int[],
meta jsonb[]
);

CREATE TABLE people (
p person
);

INSERT INTO people VALUES (ROW('Alice', array[10, 20], array['{"a": 30}'::jsonb, '{"a": 40}'::jsonb]));
```

Then run the test:
```
# old jsonb accessor works to extract a jsonb field from an array item of a composite field
evantest=# select (p).meta[1]->'a' from people;
?column?
----------
30
(1 row)

# dot notation also works
evantest=# select (p).meta[1].a from people;
a
----
30
(1 row)

# but index accessor doesn’t work
evantest=# select (p).meta[1]['a'] from people;
ERROR: invalid input syntax for type integer: "a"
LINE 1: select (p).meta[1]['a'] from people;
^
```

Other than that, I got a few new comments:

> On Sep 23, 2025, at 01:31, Alexandra Wang <alexandra(dot)wang(dot)oss(at)gmail(dot)com> wrote:
>
>
> There were trailing whitespaces in the documentation I added, I’ve fixed them now.
>
> <v21-0004-Extract-coerce_jsonpath_subscript.patch><v21-0005-Implement-read-only-dot-notation-for-jsonb.patch><v21-0001-Add-test-coverage-for-indirection-transformation.patch><v21-0002-Add-an-alternative-transform-function-in-Subscri.patch><v21-0003-Export-jsonPathFromParseResult.patch>

1 - 0001 - overall looks good

2 - 0002
```
+ /* Collect leading A_Indices subscripts */
+ foreach(lc, indirection)
+ {
+ Node *n = lfirst(lc);
+
+ if (IsA(n, A_Indices))
+ {
+ A_Indices *ai = (A_Indices *) n;
+
+ subscriptlist = lappend(subscriptlist, n);
+ if (ai->is_slice)
+ isSlice = true;
+ }
+ else
+ break;
```

We can break after “isSlice=true”.

3 - 0002
```
+ * list, and handle the
+ * remaining indirections differently or to raise an error as needed.
```

Not well formatted, “remaining” can go to the previous line.

4 - 0002
```
+ if (sbsroutines->transform_partial != NULL)
+ {
```

Do we want to assert that one of transform and transform_partial should not be NULL before “if"?

5 - 0002
```
+ /*
+ * If there is no partial transform function, use the full transform
+ * function, which only accepts bracket subscripts (A_Indices nodes).
+ * We pre-collect the leading A_Indices nodes from the indirection
```

“If there is no partial transform function” sounds redundant, I think we can just go with “Full transform function only accepts …”.

6 - 0002
```
+ /* This should not happen with well-behaved transform functions */
+ elog(ERROR, "subscripting transform function failed to consume any indirection elements”);
```

I don’t see an existing error message uses “indirection” and “transform”. This error message looks more like a log message rather than a message to show to end users.

7 - 0002
```
--- a/src/backend/utils/adt/jsonbsubs.c
+++ b/src/backend/utils/adt/jsonbsubs.c
@@ -39,11 +39,10 @@ typedef struct JsonbSubWorkspace
* Transform the subscript expressions, coerce them to text,
* and determine the result type of the SubscriptingRef node.
*/
-static void
+static int
jsonb_subscript_transform(SubscriptingRef *sbsref,
List *indirection,
ParseState *pstate,
- bool isSlice,
bool isAssignment)
```

As return type is changed, function comment should be updated accordingly.

8 - 0005 - jsonb.sql

As we discussed earlier, now

select ('{"a": 1}'::jsonb)[0]['a'];

and

select ('{"a": 1}'::jsonb)[0].a;

Will return different results. Maybe that part needs more discussion, but we at least don’t want random behavior. So I would suggest add the two cases into the test script, so that other reviewers may easily notice that, thus gets more inputs from more people.

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

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Masahiko Sawada 2025-09-23 05:59:44 Re: Report reorder buffer size
Previous Message Tom Lane 2025-09-23 05:43:47 Re: [PATCH] Add tests for Bitmapset