On Thu, Jul 10, 2025 at 9:34 PM jian he <[email protected]> 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).
v12-0001-minor-refactor-based-on-v12_0001_to_0006.no-cfbot
Description: Binary data
