Re: SQL:2023 JSON simplified accessor support - Mailing list pgsql-hackers

From Alexandra Wang
Subject Re: SQL:2023 JSON simplified accessor support
Date
Msg-id CAK98qZ1FK1ZhH_uyqg02_n7Q5gnPAooC8zUdFAx3_XQiBdXo6Q@mail.gmail.com
Whole thread Raw
In response to Re: SQL:2023 JSON simplified accessor support  (Chao Li <li.evan.chao@gmail.com>)
List pgsql-hackers
Hi Chao,

On Tue, Sep 9, 2025 at 8:52 PM Chao Li <li.evan.chao@gmail.com> wrote:

<v16-0007-Implement-jsonb-wildcard-member-accessor.patch><v16-0001-Allow-transformation-of-only-a-sublist-of-subscr.patch><v16-0004-Extract-coerce_jsonpath_subscript.patch><v16-0006-Implement-Jsonb-subscripting-with-slicing.patch><v16-0005-Implement-read-only-dot-notation-for-jsonb.patch><v16-0003-Export-jsonPathFromParseResult.patch><v16-0002-Allow-Generic-Type-Subscripting-to-Accept-Dot-No.patch>

A few more comment for v16.

1 - 0002
```
--- a/contrib/hstore/hstore_subs.c
+++ b/contrib/hstore/hstore_subs.c

- if (isSlice || list_length(*indirection) != 1)
+ if (ai->is_slice || list_length(*indirection) != 1)
```

We should put list_length() check before ai->is_slace. Because when indirection length is greater than 1, it take a single check; other it may need to check the both conditions.

2 - 0002 - also in hstore_subs.c 
```
   *indirection = NIL;
```

We should do “list_delete_first_n(indirection, 1)”.

3 - 0003
```
+Datum
+jsonPathFromParseResult(JsonPathParseResult *jsonpath, int estimated_len,
+ struct Node *escontext)
+{
```

`jsonpath` is not changed in this function, so it should be defined as a const: “const JsonPathParseResult *jsonpath”. 

4 - 0004
```
+ Oid targets[2] = {INT4OID, TEXTOID};
+
+ /*
+ * Jsonb can handle multiple subscript types, but cases when a
+ * subscript could be coerced to multiple target types must be
+ * avoided, similar to overloaded functions. It could be possibly
+ * extend with jsonpath in the future.
+ */
+ for (int i = 0; i < 2; i++)
```

This is a nit optimization. We can do:

Oid targets[] = {INT4OID, TEXTOID};
For (int I = 0; I < sizeof(targets) / sizeof(targets[0]); I ++) 

This way lets the compiler to decide length of “targets”. So that avoids hard code “2” in “for”. If we need to add more elements to “targets”, we can just add it without updating “for”.

Thanks for the feedback! Here’s v17. I’ve addressed 1 - 3 exactly as
you suggested. For 4, I made the targets array const and used the
lengthof macro to determine the array length.

Best,
Alex
Attachment

pgsql-hackers by date:

Previous
From: Daniel Gustafsson
Date:
Subject: Re: someone else to do the list of acknowledgments
Next
From: Peter Geoghegan
Date:
Subject: Re: Adding skip scan (including MDAM style range skip scan) to nbtree