Re: SQL:2023 JSON simplified accessor support

Hi Chao,

On Tue, Sep 9, 2025 at 8:52 PM Chao Li <li(dot)evan(dot)chao(at)gmail(dot)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