public inbox for [email protected]
help / color / mirror / Atom feedFrom: Chao Li <[email protected]>
To: Alexandra Wang <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Cc: Nikita Glukhov <[email protected]>
Cc: jian he <[email protected]>
Cc: Nikita Malakhov <[email protected]>
Cc: Vik Fearing <[email protected]>
Cc: Mark Dilger <[email protected]>
Cc: Matheus Alcantara <[email protected]>
Cc: Peter Eisentraut <[email protected]>
Cc: Andrew Dunstan <[email protected]>
Cc: David E. Wheeler <[email protected]>
Subject: Re: SQL:2023 JSON simplified accessor support
Date: Tue, 23 Sep 2025 13:48:24 +0800
Message-ID: <[email protected]> (raw)
In-Reply-To: <CAK98qZ3fATHtD8n471AjniZ2KHFE59MfJzjo+P0SR3_oUT8Jbw@mail.gmail.com>
References: <[email protected]>
<[email protected]>
<CAK98qZ2QGcyJrJAFv9wjY6S8yP9dUVnmG9Gb4OXuzuMMuM1Z5Q@mail.gmail.com>
<[email protected]>
<[email protected]>
<CAK98qZ3Ly6PhRwCVmMKJBba5oHVF9k370MMT2b_gep-SuQfRtg@mail.gmail.com>
<CAK98qZ1za8XgOLY+2hQMPGoxVYFyh=dDkM2dZPVeJK4J6poyvA@mail.gmail.com>
<CAK98qZ0EfrPcv3ZwGdeDiLPEpXYfHOEc8S2D=OCPJGcyWy5d-g@mail.gmail.com>
<CAK98qZ1rZaVNy6ViangQom4iinZVH7=ebqhTsPxMQbN5ZtE9XQ@mail.gmail.com>
<CAFY6G8cY5SUG5L-0ryVpom6HynE49p8-XQ59qkaEgnZZJ-c4Rg@mail.gmail.com>
<CAFY6G8eUGcU3A4AHprgYbSOAOj7+WGhGxS_YP0wd2+aCpZTiNg@mail.gmail.com>
<CAK98qZ14uKKRVFg4ibzMfReYaZD6Byxq8nYnvNNtXNjCfcd8kQ@mail.gmail.com>
<CAHgHdKtL9nNaKXGCLt9gWugVzYWKhoBDQ7NESUwdCBty8kFK-A@mail.gmail.com>
<CAK98qZ1nz6ZZhQqTOCNwRguZE5GsBLW5BQT_k=s7AA6gc2CN_g@mail.gmail.com>
<[email protected]>
<CAN-LCVM6A9z6AzxsEX-vx9=3XgEUU6+zjUqejQw8LfjaRY5P1A@mail.gmail.com>
<CAK98qZ2Pmf6ZSChLq+CEPKJ_8jSa0gFTNJWJTcWbCziDpqa=CA@mail.gmail.com>
<CACJufxEt5bL-xYojenA7x1Fq=-DfhW3KGH=3Zz9TjJ1k95=uuQ@mail.gmail.com>
<CAK98qZ0whQ=c+JGXbGSEBxCtLgy6sf-YGYqsKTAGsS-wt0wj+A@mail.gmail.com>
<CACJufxHqiKbh1RN4-rquYdnS8qK9kEQq=bpt6ED_yo1+OkU8jg@mail.gmail.com>
<CAK98qZ19bC=Qw9rWGOFKyX4B-fg1XQWEbV2OWAawqWC62fx79A@mail.gmail.com>
<CACJufxGW0Uq2Xx2NLexYR410pvLx2+QKBvBjgM+5qHsqvT5BFQ@mail.gmail.com>
<CAK98qZ3CvSfKS5yV3FAtOpWhbWHkB5aFWrngV_wYnwUbmHF4SQ@mail.gmail.com>
<CACJufxG34m9BGnfD9RD5OEohkV3Oh-+7Xf=3epXyHfQj4DPiOw@mail.gmail.com>
<CAK98qZ35eF+9MZuqR4HNrmebyBFdNiNLiLZHpQPB7S7OUk-DDQ@mail.gmail.com>
<[email protected]>
<[email protected]>
<[email protected]>
<CAK98qZ0SUBjETvUT31U01pSKYCuQPznCtb2kf3-jB5+Dh2V03w@mail.gmail.com>
<[email protected]>
<CAK98qZ1GwW_dCHSi7wMV_H+eQ9qdWajUNXmntufHF4Md=xRSsw@mail.gmail.com>
<[email protected]>
<CAK98qZ0v5dkgMZNK-ogC8Cio-6OPSnNVM-4BWa=Jh=JnvZpB-Q@mail.gmail.com>
<CAK98qZ2nezKeQFqPV9GX80mDiLqS2Fh6MRBGz6ox_HkQF86asA@mail.gmail.com>
<CAK98qZ2EhKC=Z23jNbMX=aGPGi9+n42a8Eiz1rKYmF388UCHUQ@mail.gmail.com>
<[email protected]>
<CAK98qZ3fATHtD8n471AjniZ2KHFE59MfJzjo+P0SR3_oUT8Jbw@mail.gmail.com>
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 <[email protected]> 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/
view thread (68+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: SQL:2023 JSON simplified accessor support
In-Reply-To: <[email protected]>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox