public inbox for [email protected]
help / color / mirror / Atom feedFrom: Alexandra Wang <[email protected]>
To: 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: Fri, 22 Aug 2025 12:33:22 -0700
Message-ID: <CAK98qZ3CvSfKS5yV3FAtOpWhbWHkB5aFWrngV_wYnwUbmHF4SQ@mail.gmail.com> (raw)
In-Reply-To: <CACJufxGW0Uq2Xx2NLexYR410pvLx2+QKBvBjgM+5qHsqvT5BFQ@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>
Hi Jian,
On Fri, Aug 22, 2025 at 1:20 AM jian he <[email protected]> wrote:
> On Thu, Aug 21, 2025 at 12:54 PM Alexandra Wang
> <[email protected]> wrote:
> >
> > Hi Jian,
> >
> > Thanks for reviewing! I’ve attached v13, which addresses your
> > feedback.
> >
>
> hi.
> in the context of v13-0001 and v13-0002.
>
> In transformIndirection:
> while (subscripts)
> {
> Node *newresult = (Node *) =
> transformContainerSubscripts(....&subscripts...)
> }
>
> In transformContainerSubscripts:
> sbsroutines->transform(sbsref, indirection, pstate,
> isSlice, isAssignment);
> /*
> * Error out, if datatype failed to consume any indirection elements.
> */
> if (list_length(*indirection) == indirection_length)
> {
> }
> As you can see from the above code pattern, "sbsroutines->transform" is
> responsible for consuming the "indirection".
> If it doesn’t consume it, the function should either return NULL or raise
> an
> error.
> But what happens if the elements consumed by "sbsroutines->transform"
> are not contiguous?
>
> jsonb_subscript_transform below changes in v13-0002
> + if (!IsA(lfirst(idx), A_Indices))
> + break;
> may cause elements consumed not contiguous.
>
>
> diff --git a/src/backend/utils/adt/jsonbsubs.c
> b/src/backend/utils/adt/jsonbsubs.c
> index 8ad6aa1ad4f..a0d38a0fd80 100644
> --- a/src/backend/utils/adt/jsonbsubs.c
> +++ b/src/backend/utils/adt/jsonbsubs.c
> @@ -55,9 +55,14 @@ jsonb_subscript_transform(SubscriptingRef *sbsref,
> */
> foreach(idx, *indirection)
> {
> - A_Indices *ai = lfirst_node(A_Indices, idx);
> + A_Indices *ai;
> Node *subExpr;
>
> + if (!IsA(lfirst(idx), A_Indices))
> + break;
> +
> + ai = lfirst_node(A_Indices, idx);
> +
> if (isSlice)
> {
> Node *expr = ai->uidx ? ai->uidx : ai->lidx;
> @@ -160,7 +165,9 @@ jsonb_subscript_transform(SubscriptingRef *sbsref,
> sbsref->refrestype = JSONBOID;
> sbsref->reftypmod = -1;
>
> - *indirection = NIL;
> + /* Remove processed elements */
> + if (upperIndexpr)
> + *indirection = list_delete_first_n(*indirection,
> list_length(upperIndexpr));
> }
>
> If the branch ``if (!IsA(lfirst(idx), A_Indices))`` is ever reached, then
> ``*indirection = list_delete_first_n(*indirection,
> list_length(upperIndexpr));``
> might produce an incorrect result?
>
> However, it appears that this branch is currently unreachable,
> at least regress tests not coverage for
> + if (!IsA(lfirst(idx), A_Indices))
> + break;
>
> Later patch we may have resolved this issue, but in the context of
> 0001 and 0002,
> this seem inconsistent?
>
I don’t understand the question. In the case of an unsupported Node
type (not an Indices in patch 0001 or 0002), we break out of the loop
to stop transforming the remaining subscripts. So there won’t be any
‘not contiguous’ indirection elements.
Best,
Alex
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]
Subject: Re: SQL:2023 JSON simplified accessor support
In-Reply-To: <CAK98qZ3CvSfKS5yV3FAtOpWhbWHkB5aFWrngV_wYnwUbmHF4SQ@mail.gmail.com>
* 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