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 1v0vtQ-002Yuq-0S for pgsql-hackers@arkaria.postgresql.org; Tue, 23 Sep 2025 05:48:48 +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 1v0vtO-003tq1-9f for pgsql-hackers@arkaria.postgresql.org; Tue, 23 Sep 2025 05:48:46 +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 1v0vtN-003tpt-Nz for pgsql-hackers@lists.postgresql.org; Tue, 23 Sep 2025 05:48:45 +0000 Received: from mail-pl1-x634.google.com ([2607:f8b0:4864:20::634]) by makus.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256 (Exim 4.96) (envelope-from ) id 1v0vtL-001t6b-0Z for pgsql-hackers@postgresql.org; Tue, 23 Sep 2025 05:48:44 +0000 Received: by mail-pl1-x634.google.com with SMTP id d9443c01a7336-26a0a694ea8so32793815ad.3 for ; Mon, 22 Sep 2025 22:48:43 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1758606522; x=1759211322; 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=aAB6ZSGaaNZEUdK8l+ZRHwuPWTDyy7niC5MHTKEYfmU=; b=KejAL0JsRLqtgF8DY5+1TVGsBPrfWWQXyIncfR12RVGJh25w5vfcHpeagiWZEIAu/U mm750veTnzmNU+Ys0z20jqHt2Dx1+HeD+k7Vy0ivgMa+bRx8MiY43IYadeCaVnPWXBuS WK9FWVcbQwSO+Es12npDLD1TkNuUOxu84DQPAmGvAuyvfS8O7Zn7ITh/wBMBqknFFcm0 M+Pqnp9tP9P/VdRzp+1QH+78UP8s981an5AqrM+L7qU2Y10TSt3tSkZ7weDiWrE3gCF7 QgYItYNpZ+lDBUSHhuVsrdUzFDddEB1izRdkzQ0Z0sAozm+SfVdzfamwQoatjMzw82S8 EMPQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1758606522; x=1759211322; 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=aAB6ZSGaaNZEUdK8l+ZRHwuPWTDyy7niC5MHTKEYfmU=; b=S13Kq6A4tPd1G8ZRpiDBiSDsoDk7KKOzReLlcf5aAnwLmyPrW7vyhRfUplBV4ue+0K HxcOez+Nb9yIfECnfayIWDUUmFvx8LHWcEiAB4ZbmgLM9R5x1XGYbP2CPrSwpgemI1Lg 1sOVoAAZGfHhIPKBzvzyn6EqXVlfc6VIZ42P6skZD+ay28D86Ueo849Tr/HvMzpgRGqH urHwN1Cbklr2GguL5aVJS5GWs/HC7swJRzu5ZALxFRXRMYTvAklDgGWNOGj3xAJrkeBq iyb33SEMSALWMq1/ROvJPSilaDxtPc34ixGIz7xCs3Dbk/ZO/YgIKZz2jNIORmWJNBEd Oe5A== X-Gm-Message-State: AOJu0YxDJ6HNFZoJWITqhRWKhcqpx96U9TUVbtjZKQleDOxczvBTcv1d 2PKC/Zb+9337DtKFFUgIcJ3ex+X/GPaIj2lklHLMJIoGAtNr2zGU/tj+ X-Gm-Gg: ASbGnctcXoW9vKOdcY0wc3LRXjHBndFJInyCklWfcAYd86tOoyRK478dMA8HgTFUGBa 5/1XlWCnNVdj88q4rYYuxqUjlQysKRciTfJiUW4KIcWV84UHDB22ZwEBn0OVWSM/9zMyzH8Qe45 QEpBt72T5HQJWQ7cG+NIbRQQ+Dte5/Wog8oIUoPg1xdxV+kKFnipKRwu7cPqWbUaZV0ljm9c79T i8hqG5RDT2xyqCI/wqS1LLIvF4DDegjXbFXpiGyQy/+3bGCIjmO4HyzOWxgrwEocEz2fGroImA6 qVl63kbzkRjaiRxTJcCljhCVfsjc9r5mfUlYszEgw2bc1aYfb//1JinAlxModQKzachbROeRRpX dd2XWXM4gHvvoM0AkTPC/7zI//VTWs9Zl X-Google-Smtp-Source: AGHT+IG9PLJ/99sWqwvsKg6xkfIk5XeUF4Bn9mnV9KXKeVXp4uTK6miHAfDUZEkw0vBLa25r6bmRxQ== X-Received: by 2002:a17:902:fc4b:b0:24e:3cf2:2450 with SMTP id d9443c01a7336-27cc07ef72amr19236915ad.2.1758606522286; Mon, 22 Sep 2025 22:48:42 -0700 (PDT) Received: from smtpclient.apple ([142.171.105.12]) by smtp.gmail.com with ESMTPSA id d9443c01a7336-269802deb53sm151807545ad.65.2025.09.22.22.48.38 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Mon, 22 Sep 2025 22:48:41 -0700 (PDT) From: Chao Li Message-Id: Content-Type: multipart/alternative; boundary="Apple-Mail=_D299B2FC-0084-4AB0-81B8-223A77F340A3" Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3826.700.81\)) Subject: Re: SQL:2023 JSON simplified accessor support Date: Tue, 23 Sep 2025 13:48:24 +0800 In-Reply-To: Cc: PostgreSQL Hackers , Nikita Glukhov , jian he , Nikita Malakhov , Vik Fearing , Mark Dilger , Matheus Alcantara , Peter Eisentraut , Andrew Dunstan , "David E. Wheeler" To: Alexandra Wang References: <21fa36b4-6d56-4556-b524-9e3955b922c8@eisentraut.org> <290418A2-3A9B-4304-AAAD-4A40CFF1CB93@gmail.com> <98C6F445-773B-4C82-8198-7F5CD56D5252@gmail.com> <052E941B-C905-448F-AE3A-753C12AAC938@gmail.com> <8CB9A1EC-2D14-44FF-A600-6669353A14C2@gmail.com> <826F61C4-AE01-4070-8F78-CEE8781EACEB@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=_D299B2FC-0084-4AB0-81B8-223A77F340A3 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=utf-8 The new approach of introducing =E2=80=9Ctransform_partial=E2=80=9D = 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=3D# select (p).meta[1]->'a' from people; ?column? ---------- 30 (1 row) # dot notation also works evantest=3D# select (p).meta[1].a from people; a ---- 30 (1 row) # but index accessor doesn=E2=80=99t work evantest=3D# 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 = wrote: >=20 >=20 > There were trailing whitespaces in the documentation I added, I=E2=80=99= ve fixed them now. >=20 > = 1 - 0001 - overall looks good 2 - 0002 ``` + /* Collect leading A_Indices subscripts */ + foreach(lc, indirection) + { + Node *n =3D lfirst(lc); + + if (IsA(n, A_Indices)) + { + A_Indices *ai =3D (A_Indices *) n; + + subscriptlist =3D lappend(subscriptlist, = n); + if (ai->is_slice) + isSlice =3D true; + } + else + break; ``` We can break after =E2=80=9CisSlice=3Dtrue=E2=80=9D. 3 - 0002 ``` + * list, and handle the + * remaining indirections differently or to raise an error as needed. ``` Not well formatted, =E2=80=9Cremaining=E2=80=9D can go to the previous = line.=20 4 - 0002 ``` + if (sbsroutines->transform_partial !=3D NULL) + { ``` Do we want to assert that one of transform and transform_partial should = not be NULL before =E2=80=9Cif"? 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 ``` =E2=80=9CIf there is no partial transform function=E2=80=9D sounds = redundant, I think we can just go with =E2=80=9CFull transform function = only accepts =E2=80=A6=E2=80=9D. 6 - 0002 ``` + /* This should not happen with well-behaved transform = functions */ + elog(ERROR, "subscripting transform function failed to = consume any indirection elements=E2=80=9D); ``` I don=E2=80=99t see an existing error message uses =E2=80=9Cindirection=E2= =80=9D and =E2=80=9Ctransform=E2=80=9D. 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=20 select ('{"a": 1}'::jsonb)[0]['a'];=20 and=20 select ('{"a": 1}'::jsonb)[0].a; Will return different results. Maybe that part needs more discussion, = but we at least don=E2=80=99t 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/ --Apple-Mail=_D299B2FC-0084-4AB0-81B8-223A77F340A3 Content-Transfer-Encoding: quoted-printable Content-Type: text/html; charset=utf-8 The new = approach of introducing =E2=80=9Ctransform_partial=E2=80=9D 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=3D# select (p).meta[1]->'a' from = people;
 ?column?
----------
 30
(1 row)

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

# but index accessor doesn=E2=80=99t = work
evantest=3D# 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.wang.oss@gmail.com> wrote:


There were trailing = whitespaces in the documentation I added, I=E2=80=99ve fixed them now. =

<v21-0004-Extract-coerce_jsonpath_subscript.patc= h><v21-0005-Implement-read-only-dot-notation-for-j= sonb.patch><v21-0001-Add-test-coverage-for-indirection-tran= sformation.patch><v21-0002-Add-an-alternative-transform-function-= in-Subscri.patch><v21-0003-Export-jsonPathFromParseResult.patch&g= t;

1 - 0001 - overall = looks good

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

We can break = after =E2=80=9CisSlice=3Dtrue=E2=80=9D.

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

Not well formatted, =  =E2=80=9Cremaining=E2=80=9D can go to the previous = line. 

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

Do we want to = assert that one of transform and transform_partial should not be NULL = before =E2=80=9Cif"?

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
```

=E2=80=9CIf = there is no partial transform function=E2=80=9D sounds redundant, I = think we can just go with =E2=80=9CFull transform function only accepts = =E2=80=A6=E2=80=9D.

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

I = don=E2=80=99t see an existing error message uses =E2=80=9Cindirection=E2=80= =9D and =E2=80=9Ctransform=E2=80=9D. 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=E2=80=99t 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/




= --Apple-Mail=_D299B2FC-0084-4AB0-81B8-223A77F340A3--