public inbox for [email protected]
help / color / mirror / Atom feedFrom: Chao Li <[email protected]>
To: Alexandra Wang <[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: Nikita Glukhov <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Cc: David E. Wheeler <[email protected]>
Subject: Re: SQL:2023 JSON simplified accessor support
Date: Wed, 3 Sep 2025 11:56:10 +0800
Message-ID: <[email protected]> (raw)
In-Reply-To: <CAK98qZ1XFee_x1uQFcwJkZCSVcPBy-A1ib3N5Ls2uDdZviKVOw@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]>
<CAK98qZ1XFee_x1uQFcwJkZCSVcPBy-A1ib3N5Ls2uDdZviKVOw@mail.gmail.com>
Hi Alex,
Thanks for addressing my comments. I have a few follow up comments:
> On Sep 3, 2025, at 10:16, Alexandra Wang <[email protected]> wrote:
>
>
>
> I don't like the idea of removing the "bool isSlice" argument from the
> *SubscriptTransform() function pointer. This patch series should make
> only minimal changes to the subscripting API. While it's true that
> each implementation can easily determine the boolean value of isSlice
> itself, I don't see a clear benefit to changing the interface. As you
> noted, arrsubs cares about isSlice; and users can also define custom
> data types that support subscripting, so the interface is not limited
> to built-in types.
>
> As the comment for *SubscriptTransform() explains:
>> (Of course, if the transform method
>> * does not care to support slicing, it can just throw an error if isSlice.)
>
> It's possible that in the end the "isSlice" argument isn't needed at
> all, but I don’t think this patch set is the right place for that
> refactor.
>
I agree we should keep the change minimum and tightly related to the core feature.
My original suggestion was to move
/*
* Detect whether any of the indirection items are slice specifiers.
*
* A list containing only simple subscripts refers to a single container
* element. If any of the items are slice specifiers (lower:upper), then
* the subscript expression means a container slice operation.
*/
foreach(idx, *indirection)
{
Node *ai = lfirst(idx);
if (IsA(ai, A_Indices) && castNode(A_Indices, ai)->is_slice)
{
isSlice = true;
break;
}
}
To out of transformContainerSubscripts(). Because the function was called only once under “if”, now this patch change it to be called under “while”, which brings two issues:
* Original it was O(n) as it was under “if”, now it is O(n2) as it is under “while”.
* Logically it feels wrong now. Because this loops over the entire indirection list to check is_slice, while the subsequent sbsroutines->transform() may only process a portion (prefix) of indirection list. Say, the 5th element is slice, but the first sbsroutines-transform() call will only process the first 3 elements of indirection list, then pass true to the first transform() call sounds wrong.
if we pull the loop out of transformContainerSubscripts(), then we have to add a new parameter “bool is_slice” to it. But after some researching, I found that making that change is more complicated than removing “is_slice” parameter from SubscriptTransform(). That’s why I ended up suggesting removing “is_slice” from SubscriptTransform().
Does that sound reasonable?
--
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