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