Re: SQL:2023 JSON simplified accessor support

On Thu, Jul 10, 2025 at 9:34 PM jian he <jian.universality@gmail.com> wrote:
>
> ------------------------------------------
> in jsonb_subscript_make_jsonpath we have
>     foreach(lc, *indirection)
> {
>         if (IsA(accessor, String))
>             ....
>         else if (IsA(accessor, A_Indices))
>         else
>             /*
>              * Unsupported node type for creating jsonpath. Instead of
>              * throwing an ERROR, break here so that we create a jsonpath from
>              * as many indirection elements as we can and let
>              * transformIndirection() fallback to alternative logic to handle
>              * the remaining indirection elements.
>              */
>               break;
> }
> the above ELSE branch comments look suspicious to me.
> transformIndirection->transformContainerSubscripts->jsonb_subscript_transform->jsonb_subscript_make_jsonpath
> As you can see, transformIndirection have a long distance from
> jsonb_subscript_make_jsonpath,
> let transformIndirection handle remaining indirection elements seems not good.
>
context v12-0001 to v12-0006.
this ELSE branch comments is wrong, because
+ if (jsonb_check_jsonpath_needed(*indirection))
+ {
+ jsonb_subscript_make_jsonpath(pstate, indirection, sbsref);
+ if (sbsref->refjsonbpath)
+ return;
+ }
in jsonb_check_jsonpath_needed we already use Assert to confirm that
list "indirection"
is either String or A_Indices Node.

in transformContainerSubscripts we have
sbsroutines->transform(sbsref, indirection, pstate,
                           isSlice, isAssignment);
    /*
     * Error out, if datatype failed to consume any indirection elements.
     */
    if (list_length(*indirection) == indirection_length)
    {
        Node       *ind = 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));
    }
sbsroutines->transform currently will call
array_subscript_transform, hstore_subscript_transform, jsonb_subscript_transform

in jsonb_subscript_transform callee we unconditionally do:
    *indirection = list_delete_first_n(*indirection, pathlen);
   *indirection = list_delete_first_n(*indirection, list_length(upperIndexpr));
in array_subscript_transform, we do
*indirection = list_delete_first_n(*indirection, ndim);

That means, if sbsroutines->transform not error out and indirection is
not NIL (which is unlikely)
then sbsroutines->transform will consume some induction elements.
instead of the above verbose ereport(ERROR, error handling, we can use Assert
    Assert(indirection_length > list_length(*indirection));

for the above comments, i did a refactoring based on v12 (0001 to 0006).