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).
