Re: SQL:2023 JSON simplified accessor support

Hi Jian,

On Fri, Aug 22, 2025 at 1:20 AM jian he <jian(dot)universality(at)gmail(dot)com> wrote:

> On Thu, Aug 21, 2025 at 12:54 PM Alexandra Wang
> <alexandra(dot)wang(dot)oss(at)gmail(dot)com> 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