MIME-Version: 1.0
References: <d2d872c3-84f9-48f6-9963-f4537f02bc91@eisentraut.org>
 <adb15d59-2e6b-4d2e-ac5b-7aaea41f738f@gmail.com>
 <CAK98qZ2QGcyJrJAFv9wjY6S8yP9dUVnmG9Gb4OXuzuMMuM1Z5Q@mail.gmail.com>
 <b24dc997-d78f-47bd-b323-14e68232d127@dunslane.net>
 <21fa36b4-6d56-4556-b524-9e3955b922c8@eisentraut.org>
 <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>
 <c7dc6735-57cb-4287-871f-4c92a7205357@postgresfriends.org>
 <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>
In-Reply-To: 
 <CAK98qZ3CvSfKS5yV3FAtOpWhbWHkB5aFWrngV_wYnwUbmHF4SQ@mail.gmail.com>
From: jian he <jian.universality@gmail.com>
Date: Mon, 25 Aug 2025 16:09:47 +0800
Message-ID: 
 <CACJufxG34m9BGnfD9RD5OEohkV3Oh-+7Xf=3epXyHfQj4DPiOw@mail.gmail.com>
Subject: Re: SQL:2023 JSON simplified accessor support
To: Alexandra Wang <alexandra.wang.oss@gmail.com>
Cc: 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>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Archived-At: 
 <https://www.postgresql.org/message-id/CACJufxG34m9BGnfD9RD5OEohkV3Oh-%2B7Xf%3D3epXyHfQj4DPiOw%40mail.gmail.com>
Precedence: bulk

On Sat, Aug 23, 2025 at 3:34=E2=80=AFAM Alexandra Wang
<alexandra.wang.oss@gmail.com> wrote:
>
> I don=E2=80=99t understand the question. In the case of an unsupported No=
de
> type (not an Indices in patch 0001 or 0002), we break out of the loop
> to stop transforming the remaining subscripts. So there won=E2=80=99t be =
any
> =E2=80=98not contiguous=E2=80=99 indirection elements.
>

Sorry, my last message is wrong.
this time, I applied v13-0001 to v13-0005.
and I found some minor issues.....

v13-0002-Allow-Generic-Type-Subscripting-to-Accept-Dot-No.patch
+ /*
+ * Error out, if datatype failed to consume any indirection elements.
+ */
+ if (list_length(*indirection) =3D=3D indirection_length)
+ {
+ Node   *ind =3D linitial(*indirection);
+
+ if (noError)
+ return NULL;
+
+ if (IsA(ind, String))
+ ereport(ERROR,
+ (errcode(ERRCODE_DATATYPE_MISMATCH),
+ errmsg("type %s does not support dot notation",
+ format_type_be(containerType)),
+ parser_errposition(pstate, exprLocation(containerBase))));
+ else if (IsA(ind, A_Indices))
+ ereport(ERROR,
+ (errcode(ERRCODE_DATATYPE_MISMATCH),
+ errmsg("type %s does not support array subscripting",
+ format_type_be(containerType)),
+ parser_errposition(pstate, exprLocation(containerBase))));
+ else
+ elog(ERROR, "invalid indirection operation: %d", nodeTag(ind));
+ }

these error message still not reached after I apply
v13-0005-Implement-read-only-dot-notation-for-jsonb.patch
maybe we can simply do

+ if (noError)
+ return NULL;
+
+ ereport(ERROR,
+ errcode(ERRCODE_DATATYPE_MISMATCH),
+ errmsg("type %s does not support subscripting",
+ format_type_be(containerType)),
+ parser_errposition(pstate, exprLocation(containerBase)));
?

for V13-0004.
in master we have
            if (subExpr =3D=3D NULL)
                ereport(ERROR,
                        (errcode(ERRCODE_DATATYPE_MISMATCH),
                         errmsg("jsonb subscript must have text type"),
                         parser_errposition(pstate, exprLocation(subExpr)))=
);
but this part is removed from v13-0004-Extract-coerce_jsonpath_subscript
?

coerce_jsonpath_subscript_to_int4_or_text
maybe we can just rename to
coerce_jsonpath_subscript,
then add some comments explaining why currently only support result node co=
erce
to text or int4 data type.

for v13-0005-Implement-read-only-dot-notation-for-jsonb.patch

+ i =3D 0;
+ foreach(lc, sbsref->refupperindexpr) {
+ Expr *e =3D (Expr *) lfirst(lc);
+
+ /* When slicing, individual subscript bounds can be omitted */
+ if (!e) {
+ sbsrefstate->upperprovided[i] =3D false;
+ sbsrefstate->upperindexnull[i] =3D true;
+ } else {
+ sbsrefstate->upperprovided[i] =3D true;
+ /* Each subscript is evaluated into appropriate array entry */
+ ExecInitExprRec(e, state,
+ &sbsrefstate->upperindex[i],
+ &sbsrefstate->upperindexnull[i]);
+ }
+ i++;
  }
curly braces should be put into the next new line.

there are two
+ if (!sbsref->refjsonbpath)
+{
+}
maybe we can put these two together.


+SELECT (jb)['b']['x'].z FROM test_jsonb_dot_notation; -- returns NULL
with warnings
+ z
+---
+
+(1 row)
there is no warning, "returns NULL with warnings" should be removed.


+-- clean up
+DROP TABLE test_jsonb_dot_notation;
\ No newline at end of file

to remove "\ No newline at end of file"
you need to add a newline to jsonb.sql, you may see other regress sql
test files.


+ foreach(lc, *indirection)
+ if()
+ {
+ }
+ else
+ {
+ /*
+ * Unexpected node type in indirection list. This should not
+ * happen with current grammar, but we handle it defensively by
+ * breaking out of the loop rather than crashing. In case of
+ * future grammar changes that might introduce new node types,
+ * this allows us to create a jsonpath from as many indirection
+ * elements as we can and let transformIndirection() fallback to
+ * alternative logic to handle the remaining indirection elements.
+ */
+ Assert(false); /* not reachable */
+ break;
+ }
+
+ /* append path item */
+ path->next =3D jpi;
+ path =3D jpi;
+ pathlen++;

If the above else branch is reached, it will crash due to `Assert(false);`,
which contradicts the preceding comments.
to make the comments accurate, we need remove
" Assert(false); /* not reachable */"
?


  /*
- * Transform and convert the subscript expressions. Jsonb subscripting
- * does not support slices, look only at the upper index.
+ * We would only reach here if json simplified accessor is not needed, or
+ * if jsonb_subscript_make_jsonpath() didn't consume any indirection
+ * element =E2=80=94 either way, the first indirection element could not b=
e
+ * converted into a JsonPath component. This happens when it's a non-slice
+ * A_Indices with a non-integer upper index. The code below falls back to
+ * traditional jsonb subscripting for such cases.
  */
the above comments, "non-integer" part is wrong?
For example, the below two SELECTs both use "traditional" jsonb
subscripting logic.
CREATE TABLE ts1 AS SELECT '[1, [2]]' ::jsonb jb;
SELECT (jb)[1] FROM ts1;
SELECT (jb)['a'] FROM ts1;