On Thu, Aug 21, 2025 at 12:54 PM Alexandra Wang
<alexandra.wang.oss@gmail.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?
