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>
In-Reply-To: 
 <CACJufxEt5bL-xYojenA7x1Fq=-DfhW3KGH=3Zz9TjJ1k95=uuQ@mail.gmail.com>
From: jian he <jian.universality@gmail.com>
Date: Wed, 25 Jun 2025 13:56:51 +0800
Message-ID: 
 <CACJufxFAaJ3=X7wnGmS0857ia8+-iwDzxhua9X8Qnh_CVB1V1A@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: multipart/mixed; boundary="000000000000ac915906385f1edd"
Archived-At: 
 <https://www.postgresql.org/message-id/CACJufxFAaJ3%3DX7wnGmS0857ia8%2B-iwDzxhua9X8Qnh_CVB1V1A%40mail.gmail.com>
Precedence: bulk

--000000000000ac915906385f1edd
Content-Type: text/plain; charset="UTF-8"

hi.

in src/backend/catalog/sql_features.txt
should we mark any T860, T861, T862, T863, T864
items as YES?


typedef struct SubscriptingRef
{
    /* expressions that evaluate to upper container indexes */
    List       *refupperindexpr;
}
SubscriptingRef.refupperindexpr meaning changed,
So the above comments also need to be changed?


v11-0004-Extract-coerce_jsonpath_subscript.patch
+ /*
+ * We known from can_coerce_type that coercion will succeed, so
+ * coerce_type could be used. Note the implicit coercion context, which is
+ * required to handle subscripts of different types, similar to overloaded
+ * functions.
+ */
+ subExpr = coerce_type(pstate,
+  subExpr, subExprType,
+  targetType, -1,
+  COERCION_IMPLICIT,
+  COERCE_IMPLICIT_CAST,
+  -1);
+ if (subExpr == NULL)
+ ereport(ERROR,
+ (errcode(ERRCODE_DATATYPE_MISMATCH),
+ errmsg("jsonb subscript must have text type"),

the targetType can be "integer", then the error message
errmsg("jsonb subscript must have text type") would be wrong?
Also this error handling is not necessary.
since we can_coerce_type already tell us that coercion will succeed.
Also, do we really put v11-0004 as a separate patch?


in gram.y we have:
                    if (!IsA($5, A_Const) ||
                        castNode(A_Const, $5)->val.node.type != T_String)
                        ereport(ERROR,
                                errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
                                errmsg("only string constants are
supported in JSON_TABLE path specification"),
so simply, in make_jsonpath_item_expr we can

    expr = transformExpr(pstate, expr, pstate->p_expr_kind);
    if (!IsA(expr, Const) || ((Const *) expr)->consttype != INT4OID)
        ereport(ERROR,
                errcode(ERRCODE_DATATYPE_MISMATCH),
                errmsg("only integer constants are supported in jsonb
simplified accessor subscripting"),
                parser_errposition(pstate, exprLocation(expr)));

because I think the current error message "got type: unknown" is not good.
select ('123'::jsonb).a['1'];
ERROR:  jsonb simplified accessor supports subscripting in type: INT4,
got type: unknown
then we don't need two "ereport(ERROR" within make_jsonpath_item_expr
we can also Assert (cnst->constisnull) is false.
see gram.y:16976


I saw you introduce the word "AST", for example
"Converts jsonpath AST into jsonpath value in binary."
I am not sure that is fine.


in jsonb_subscript_make_jsonpath we have:
+ foreach(lc, *indirection)
+ {
+
+        if (IsA(accessor, String))
+ ....
+        else if (IsA(accessor, A_Star))
+ ....
+        else if (IsA(accessor, A_Indices))
+ ....
+        else
+              break;

Is the last else branch unreliable? since indirection only support for
String, A_Star, A_Indices, we already have Assert in jsonb_check_jsonpath_needed
to ensure that.
+ *indirection = list_delete_first_n(*indirection, pathlen);
also this is not necessary,
because pathlen will be the same length as list *indirection in current design.


Please check the attached minor refactoring based on v11-0001 to v11-0006

--000000000000ac915906385f1edd
Content-Type: application/octet-stream;
	name="v11-0001-misc-refactoring-based-on-v11_01_to_06.no-cfbot"
Content-Disposition: attachment;
	filename="v11-0001-misc-refactoring-based-on-v11_01_to_06.no-cfbot"
Content-Transfer-Encoding: base64
Content-ID: <f_mcbjkj0x0>
X-Attachment-Id: f_mcbjkj0x0

RnJvbSBjMDM4Y2M1NDU2YTM2YWY4YmYyMjFhNmRhZDRkODNiZTRhMzk0YzBiIE1vbiBTZXAgMTcg
MDA6MDA6MDAgMjAwMQpGcm9tOiBqaWFuIGhlIDxqaWFuLnVuaXZlcnNhbGl0eUBnbWFpbC5jb20+
CkRhdGU6IFdlZCwgMjUgSnVuIDIwMjUgMTM6MzE6NTYgKzA4MDAKU3ViamVjdDogW1BBVENIIHYx
MSAxLzFdIG1pc2MgcmVmYWN0b3JpbmcgYmFzZWQgb24gdjExXzAxX3RvXzA2CgotLS0KIHNyYy9i
YWNrZW5kL3V0aWxzL2FkdC9qc29uYnN1YnMuYyAgIHwgMzMgKysrKysrKysrKy0tLS0tLS0tLS0t
LS0tLS0tLS0KIHNyYy90ZXN0L3JlZ3Jlc3MvZXhwZWN0ZWQvanNvbmIub3V0IHwgIDQgKystLQog
MiBmaWxlcyBjaGFuZ2VkLCAxMyBpbnNlcnRpb25zKCspLCAyNCBkZWxldGlvbnMoLSkKCmRpZmYg
LS1naXQgYS9zcmMvYmFja2VuZC91dGlscy9hZHQvanNvbmJzdWJzLmMgYi9zcmMvYmFja2VuZC91
dGlscy9hZHQvanNvbmJzdWJzLmMKaW5kZXggMzU4OGExZDA2MmYuLmNmMDRkNmZlMTRhIDEwMDY0
NAotLS0gYS9zcmMvYmFja2VuZC91dGlscy9hZHQvanNvbmJzdWJzLmMKKysrIGIvc3JjL2JhY2tl
bmQvdXRpbHMvYWR0L2pzb25ic3Vicy5jCkBAIC0xMTAsMTIgKzExMCw2IEBAIGNvZXJjZV9qc29u
cGF0aF9zdWJzY3JpcHQoUGFyc2VTdGF0ZSAqcHN0YXRlLCBOb2RlICpzdWJFeHByLCBPaWQgbnVt
dHlwZSkKIAkJCQkJCSAgQ09FUkNJT05fSU1QTElDSVQsCiAJCQkJCQkgIENPRVJDRV9JTVBMSUNJ
VF9DQVNULAogCQkJCQkJICAtMSk7Ci0JaWYgKHN1YkV4cHIgPT0gTlVMTCkKLQkJZXJlcG9ydChF
UlJPUiwKLQkJCQkoZXJyY29kZShFUlJDT0RFX0RBVEFUWVBFX01JU01BVENIKSwKLQkJCQkgZXJy
bXNnKCJqc29uYiBzdWJzY3JpcHQgbXVzdCBoYXZlIHRleHQgdHlwZSIpLAotCQkJCSBwYXJzZXJf
ZXJycG9zaXRpb24ocHN0YXRlLCBleHByTG9jYXRpb24oc3ViRXhwcikpKSk7Ci0KIAlyZXR1cm4g
c3ViRXhwcjsKIH0KIApAQCAtMTk2LDMwICsxOTAsMjIgQEAgc3RhdGljIEpzb25QYXRoUGFyc2VJ
dGVtICoKIG1ha2VfanNvbnBhdGhfaXRlbV9leHByKFBhcnNlU3RhdGUgKnBzdGF0ZSwgTm9kZSAq
ZXhwciwgTGlzdCAqKmV4cHJzKQogewogCUNvbnN0CSAgICpjbnN0OworCWludDMyCQl2YWw7CiAK
IAlleHByID0gdHJhbnNmb3JtRXhwcihwc3RhdGUsIGV4cHIsIHBzdGF0ZS0+cF9leHByX2tpbmQp
OwogCi0JaWYgKCFJc0EoZXhwciwgQ29uc3QpKQorCWlmICgoIUlzQShleHByLCBDb25zdCkpIHx8
ICgoKENvbnN0ICopIGV4cHIpLT5jb25zdHR5cGUgIT0gSU5UNE9JRCkpCiAJCWVyZXBvcnQoRVJS
T1IsCi0JCQkJKGVycmNvZGUoRVJSQ09ERV9EQVRBVFlQRV9NSVNNQVRDSCksCi0JCQkJIGVycm1z
ZygianNvbmIgc2ltcGxpZmllZCBhY2Nlc3NvciBzdXBwb3J0cyBzdWJzY3JpcHRpbmcgaW4gY29u
c3QgaW50NCwgZ290IHR5cGU6ICVzIiwKLQkJCQkJCWZvcm1hdF90eXBlX2JlKGV4cHJUeXBlKGV4
cHIpKSksCi0JCQkJIHBhcnNlcl9lcnJwb3NpdGlvbihwc3RhdGUsIGV4cHJMb2NhdGlvbihleHBy
KSkpKTsKKwkJCQllcnJjb2RlKEVSUkNPREVfREFUQVRZUEVfTUlTTUFUQ0gpLAorCQkJCWVycm1z
Zygib25seSBpbnRlZ2VyIGNvbnN0YW50cyBhcmUgc3VwcG9ydGVkIGluIGpzb25iIHNpbXBsaWZp
ZWQgYWNjZXNzb3Igc3Vic2NyaXB0aW5nIiksCisJCQkJcGFyc2VyX2VycnBvc2l0aW9uKHBzdGF0
ZSwgZXhwckxvY2F0aW9uKGV4cHIpKSk7CiAKIAljbnN0ID0gKENvbnN0ICopIGV4cHI7CisJQXNz
ZXJ0KCFjbnN0LT5jb25zdGlzbnVsbCk7CiAKLQlpZiAoY25zdC0+Y29uc3R0eXBlID09IElOVDRP
SUQgJiYgIWNuc3QtPmNvbnN0aXNudWxsKQotCXsKLQkJaW50MzIJCXZhbCA9IERhdHVtR2V0SW50
MzIoY25zdC0+Y29uc3R2YWx1ZSk7CisJdmFsID0gRGF0dW1HZXRJbnQzMihjbnN0LT5jb25zdHZh
bHVlKTsKIAotCQlyZXR1cm4gbWFrZV9qc29ucGF0aF9pdGVtX2ludCh2YWwsIGV4cHJzKTsKLQl9
Ci0KLQllcmVwb3J0KEVSUk9SLAotCQkJKGVycmNvZGUoRVJSQ09ERV9EQVRBVFlQRV9NSVNNQVRD
SCksCi0JCQkgZXJybXNnKCJqc29uYiBzaW1wbGlmaWVkIGFjY2Vzc29yIHN1cHBvcnRzIHN1YnNj
cmlwdGluZyBpbiB0eXBlOiBJTlQ0LCBnb3QgdHlwZTogJXMiLAotCQkJCQlmb3JtYXRfdHlwZV9i
ZShjbnN0LT5jb25zdHR5cGUpKSwKLQkJCSBwYXJzZXJfZXJycG9zaXRpb24ocHN0YXRlLCBleHBy
TG9jYXRpb24oZXhwcikpKSk7CisJcmV0dXJuIG1ha2VfanNvbnBhdGhfaXRlbV9pbnQodmFsLCBl
eHBycyk7CiB9CiAKIC8qCkBAIC0zMjksNiArMzE1LDkgQEAganNvbmJfc3Vic2NyaXB0X21ha2Vf
anNvbnBhdGgoUGFyc2VTdGF0ZSAqcHN0YXRlLCBMaXN0ICoqaW5kaXJlY3Rpb24sCiAJCQkqbGV4
cHJzID0gbGFwcGVuZCgqbGV4cHJzLCBOVUxMKTsKIAl9CiAKKwlsaXN0X2ZyZWUoKmluZGlyZWN0
aW9uKTsKKwkqaW5kaXJlY3Rpb24gPSBOSUw7CisKIAkqaW5kaXJlY3Rpb24gPSBsaXN0X2RlbGV0
ZV9maXJzdF9uKCppbmRpcmVjdGlvbiwgcGF0aGxlbik7CiAKIAlqc3AgPSBqc29uUGF0aEZyb21Q
YXJzZVJlc3VsdCgmanByZXMsIDAsIE5VTEwpOwpkaWZmIC0tZ2l0IGEvc3JjL3Rlc3QvcmVncmVz
cy9leHBlY3RlZC9qc29uYi5vdXQgYi9zcmMvdGVzdC9yZWdyZXNzL2V4cGVjdGVkL2pzb25iLm91
dAppbmRleCA5MWE3YjgyNTc2NC4uMTE0MjAwZTBlNDMgMTAwNjQ0Ci0tLSBhL3NyYy90ZXN0L3Jl
Z3Jlc3MvZXhwZWN0ZWQvanNvbmIub3V0CisrKyBiL3NyYy90ZXN0L3JlZ3Jlc3MvZXhwZWN0ZWQv
anNvbmIub3V0CkBAIC01MTYzLDcgKzUxNjMsNyBAQCBzZWxlY3QgKCd7ImEiOiAxLCAiYiI6ICJj
IiwgImQiOiBbMSwgMiwgM119Jzo6anNvbmIpWydkJ11bJ2EnXTsKICgxIHJvdykKIAogc2VsZWN0
ICgneyJhIjogMSwgImIiOiAiYyIsICJkIjogWzEsIDIsIDNdfSc6Ompzb25iKS5kWydhJ107Ci1F
UlJPUjogIGpzb25iIHNpbXBsaWZpZWQgYWNjZXNzb3Igc3VwcG9ydHMgc3Vic2NyaXB0aW5nIGlu
IHR5cGU6IElOVDQsIGdvdCB0eXBlOiB1bmtub3duCitFUlJPUjogIG9ubHkgaW50ZWdlciBjb25z
dGFudHMgYXJlIHN1cHBvcnRlZCBpbiBqc29uYiBzaW1wbGlmaWVkIGFjY2Vzc29yIHN1YnNjcmlw
dGluZwogTElORSAxOiBzZWxlY3QgKCd7ImEiOiAxLCAiYiI6ICJjIiwgImQiOiBbMSwgMiwgM119
Jzo6anNvbmIpLmRbJ2EnXTsKICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAg
ICAgICAgICAgICAgICAgICAgICAgICAgIF4KIHNlbGVjdCAoJ3siYSI6IDEsICJiIjogImMiLCAi
ZCI6IFsxLCAyLCAzXX0nOjpqc29uYikuZC5hOwpAQCAtNTIzMyw3ICs1MjMzLDcgQEAgc2VsZWN0
ICgneyJhIjogWyJhMSIsIHsiYjEiOiBbImFhYSIsICJiYmIiLCAiY2NjIl19XSwgImIiOiAiYmIi
fSc6Ompzb25iKS5hWzFdLmIKICgxIHJvdykKIAogc2VsZWN0ICgneyJhIjogMX0nOjpqc29uYilb
J2EnOidiJ107IC0tIGZhaWxzCi1FUlJPUjogIGpzb25iIHNpbXBsaWZpZWQgYWNjZXNzb3Igc3Vw
cG9ydHMgc3Vic2NyaXB0aW5nIGluIHR5cGU6IElOVDQsIGdvdCB0eXBlOiB1bmtub3duCitFUlJP
UjogIG9ubHkgaW50ZWdlciBjb25zdGFudHMgYXJlIHN1cHBvcnRlZCBpbiBqc29uYiBzaW1wbGlm
aWVkIGFjY2Vzc29yIHN1YnNjcmlwdGluZwogTElORSAxOiBzZWxlY3QgKCd7ImEiOiAxfSc6Ompz
b25iKVsnYSc6J2InXTsKICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgXgogc2Vs
ZWN0ICgnWzEsICIyIiwgbnVsbF0nOjpqc29uYilbMToyXTsKLS0gCjIuMzQuMQoK
--000000000000ac915906385f1edd--