Re: SQL:2023 JSON simplified accessor support

Chao Li <li.evan.chao@gmail.com>

From: Chao Li <li.evan.chao@gmail.com>
To: Alexandra Wang <alexandra.wang.oss@gmail.com>
Cc: jian he <jian.universality@gmail.com>, Nikita Malakhov <hukutoc@gmail.com>, Vik Fearing <vik@postgresfriends.org>, Mark Dilger <mark.dilger@enterprisedb.com>, Matheus Alcantara <matheusssilv97@gmail.com>, Peter Eisentraut <peter@eisentraut.org>, Andrew Dunstan <andrew@dunslane.net>, Nikita Glukhov <glukhov.n.a@gmail.com>, PostgreSQL Hackers <pgsql-hackers@postgresql.org>, "David E. Wheeler" <david@justatheory.com>
Date: 2025-09-03T07:11:03Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →
  1. Add test coverage for indirection transformation

  2. Fix typo in comment

  3. Implementation of subscripting for jsonb


> On Sep 3, 2025, at 10:16, Alexandra Wang <alexandra.wang.oss@gmail.com> wrote:
> 
> 
> <v15-0007-Implement-jsonb-wildcard-member-accessor.patch><v15-0001-Allow-transformation-of-only-a-sublist-of-subscr.patch><v15-0004-Extract-coerce_jsonpath_subscript.patch><v15-0006-Implement-Jsonb-subscripting-with-slicing.patch><v15-0005-Implement-read-only-dot-notation-for-jsonb.patch><v15-0003-Export-jsonPathFromParseResult.patch><v15-0002-Allow-Generic-Type-Subscripting-to-Accept-Dot-No.patch>



I have a few more other small comments:

1 - 0002
```
diff --git a/src/backend/parser/parse_expr.c b/src/backend/parser/parse_expr.c
index 6e8fd42c612..ff104c95311 100644
--- a/src/backend/parser/parse_expr.c
+++ b/src/backend/parser/parse_expr.c
@@ -441,8 +441,9 @@ transformIndirection(ParseState *pstate, A_Indirection *ind)
 	ListCell   *i;
 
 	/*
-	 * We have to split any field-selection operations apart from
-	 * subscripting.  Adjacent A_Indices nodes have to be treated as a single
+	 * Combine field names and subscripts into a single indirection list, as
+	 * some subscripting containers, such as jsonb, support field access using
+	 * dot notation. Adjacent A_Indices nodes have to be treated as a single
 	 * multidimensional subscript operation.
 	 */
 	foreach(i, ind->indirection)
@@ -460,19 +461,43 @@ transformIndirection(ParseState *pstate, A_Indirection *ind)
 		}
 		else
 		{
-			Node	   *newresult;
-
 			Assert(IsA(n, String));
+			subscripts = lappend(subscripts, n);
+		}
+	}
```

I raised this comment in my previous email, I guess you missed this one.

With this change, the 3 clauses are quite similar, the if-elseif-else can be simplified as:

     If (!IsA(n, A_Indices) && !Is_A(n, A_Start))
          Assert(IsA(n, String));
     subscripts = lappend(subscripts, n)

2 - 0001
```
diff --git a/src/include/nodes/subscripting.h b/src/include/nodes/subscripting.h
index 234e8ad8012..5d576af346f 100644
--- a/src/include/nodes/subscripting.h
+++ b/src/include/nodes/subscripting.h
@@ -71,6 +71,11 @@ struct SubscriptExecSteps;
  * does not care to support slicing, it can just throw an error if isSlice.)
  * See array_subscript_transform() for sample code.
  *
+ * The transform method receives a pointer to a list of raw indirections.
+ * This allows the method to parse a sublist of the indirections (typically
+ * the prefix) and modify the original list in place, enabling the caller to
+ * either process the remaining indirections differently or raise an error.
+ *
  * The transform method is also responsible for identifying the result type
```

This is nit comment about the wording. “This allows the method to parse a sublist..." sounds like more from the patch perspective. It’s more suitable for git commit comments, but code comment, I feel it’s better to be more general, for example:

* The transform method receives a pointer to a list of raw indirections.
 * It can parse a sublist (typically the prefix) of these indirections and
 * modify the original list in place, allowing the caller to either handle
 * the remaining indirections differently or raise an error.

3 - 0005
```
diff --git a/src/backend/executor/execExpr.c b/src/backend/executor/execExpr.c
index f1569879b52..eac31b097e4 100644
--- a/src/backend/executor/execExpr.c
+++ b/src/backend/executor/execExpr.c
@@ -3320,50 +3320,54 @@ ExecInitSubscriptingRef(ExprEvalStep *scratch, SubscriptingRef *sbsref,
 								   state->steps_len - 1);
 	}
 
+			/* When slicing, individual subscript bounds can be omitted */
+			if (!e)
+			{
+				sbsrefstate->upperprovided[i] = false;
+				sbsrefstate->upperindexnull[i] = true;
+			}
+			else {
+				sbsrefstate->upperprovided[i] = true;
```

This is also a nit comment about code format. “{" should be put to the next line of “else” according to other existing code.

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/