On Aug 26, 2025, at 11:52, Alexandra Wang <alexandra.wang.oss@gmail.com> wrote:
<v14-0002-Allow-Generic-Type-Subscripting-to-Accept-Dot-No.patch><v14-0003-Export-jsonPathFromParseResult.patch><v14-0001-Allow-transformation-of-only-a-sublist-of-subscr.patch><v14-0005-Implement-read-only-dot-notation-for-jsonb.patch><v14-0007-Implement-jsonb-wildcard-member-accessor.patch><v14-0006-Implement-Jsonb-subscripting-with-slicing.patch><v14-0004-Extract-coerce_jsonpath_subscript.patch>
I just started with 0001, I just got a comment:
diff --git a/src/include/parser/parse_node.h b/src/include/parser/parse_node.h
index f7d07c84542..58a4b9df157 100644
--- a/src/include/parser/parse_node.h
+++ b/src/include/parser/parse_node.h
@@ -361,7 +361,7 @@ extern SubscriptingRef *transformContainerSubscripts(ParseState *pstate,
Node *containerBase,
Oid containerType,
int32 containerTypMod,
- List *indirection,
+ List **indirection,
bool isAssignment);
Here we change “indirection” from * to **, I guess the reason is to indicate if the indirection list is fully processed. In case of fully processed, set it to NULL, then caller gets NULL via **.
As “indirection” is of type “List *”, can we just set “indirection->length = 0”? I checked pg_list.h, there is not a function or macro to cleanup a list (free elements and reset length to 0). There is a “list_free(List *list)”, but it will free “list” itself as well. Maybe we can add a new function, say “list_reset(List *list)” or “list_cleanup(List *list)”. This way will keep the function interface unchanged, and less changes would be involved.
diff --git a/src/backend/parser/parse_expr.c b/src/backend/parser/parse_expr.c
index d66276801c6..e1565e11d09 100644
--- a/src/backend/parser/parse_expr.c
+++ b/src/backend/parser/parse_expr.c
@@ -466,14 +466,13 @@ transformIndirection(ParseState *pstate, A_Indirection *ind)
Assert(IsA(n, String));
/* process subscripts before this field selection */
- if (subscripts)
+ while (subscripts)
result = (Node *) transformContainerSubscripts(pstate,
Changing “if” to “while” in 0001 is a little confusing. Because the impression is that, transform will stop when it cannot handle next indirection, so that transform again will also fail. Maybe my impression is wrong, can you add some comments here to explain why change “if” to “while”.
Regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
