public inbox for [email protected]  

help / color / mirror / Atom feed

From: Peter Eisentraut <[email protected]>
To: Alexandra Wang <[email protected]>
To: jian he <[email protected]>
To: Andrew Dunstan <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Cc: [email protected]
Subject: Re: SQL:2023 JSON simplified accessor support
Date: Thu, 14 Nov 2024 13:31:35 +0100
Message-ID: <[email protected]> (raw)
In-Reply-To: <CAK98qZ3Hv6ndh2vaCyGUSdyE7AXTvCLUSYKTdHp2kaPDnp6S4w@mail.gmail.com>
References: <CAK98qZ1JNNAx4QneJG+eX7iLesOhd6A68FNQVvvHP6Up_THf3A@mail.gmail.com>
	<[email protected]>
	<CAK98qZ2grHAug2qVAH808drYLbSkdVM4KaM9wHxngdFgdFy1zg@mail.gmail.com>
	<CAK98qZ13+TXB5fxHakxSJL7MYHARXBvdpzN6tymSrNLMZiin0Q@mail.gmail.com>
	<CACJufxGKTN=bc73-Zhezz+7Gfy_sH8PyoVpd2Jf9wHgJ6ymmqw@mail.gmail.com>
	<CAK98qZ3Hv6ndh2vaCyGUSdyE7AXTvCLUSYKTdHp2kaPDnp6S4w@mail.gmail.com>

On 07.11.24 22:57, Alexandra Wang wrote:
> The v5 patch includes the following updates:
> 
> - Fixed the aforementioned issue and added more tests covering composite
> types with domains, nested domains, and arrays of domains over
> JSON/JSONB.
> 
> - Refactored the logic for parsing JSON/JSONB object fields by moving it
> from ParseFuncOrColumn() to transformIndirection() for improved
> readability. The ParseFuncOrColumn() function is already handling both
> single-argument function calls and composite types, and it has other
> callers besides transformIndirection().

This patch implements array subscripting support for the json type, but 
it does it in a non-standard way, using 
ParseJsonSimplifiedAccessorArrayElement().  This would be better done by 
providing a typsubscript function for the json type.  This is what jsonb 
already has, which is why your patch doesn't need to provide the array 
support for jsonb.  I suggest you implement the typsubscript support for 
the json type (make it a separate patch but you can keep it in this 
thread IMO) and remove the custom code from this patch.

A few comments on the tests:  The tests look good to me.  Good coverage 
of weirdly nested types.  Results look correct.

+drop table if exists test_json_dot;

This can be omitted, since we know that the table doesn't exist yet.

This code could be written in the more conventional insert ... values 
syntax:

+insert into test_json_dot select 1, '{"a": 1, "b": 42}'::json;
+insert into test_json_dot select 1, '{"a": 2, "b": {"c": 42}}'::json;
+insert into test_json_dot select 1, '{"a": 3, "b": {"c": "42"}, 
"d":[11, 12]}'::json;
+insert into test_json_dot select 1, '{"a": 3, "b": {"c": "42"}, 
"d":[{"x": [11, 12]}, {"y": [21, 22]}]}'::json;

Then the ::json casts can also go away.

Also, using a different value for "id" for each row would be more
useful, so that the subsequent tests could then be written like

     select id, (test_jsonb_dot.test_jsonb).b from test_jsonb_dot;

so we can see which result corresponds to which input row.  Also make
id the primary key in this table.

Also, let's keep the json and the jsonb variants aligned.  There are
some small differences, like the test_json_dot table having 4 rows but
the test_jsonb_dot having 3 rows.  And the array and wildcard tests in
the opposite order.  Not a big deal, but keeping these the same helps
eyeballing the test files.

Maybe add a comment somewhere in this file that you are running the
json_query equivalents to cross-check the semantics of the dot syntax.

Some documentation should be written.  This looks like this right place 
to start:

https://www.postgresql.org/docs/devel/sql-expressions.html#FIELD-SELECTION

and them maybe some cross-linking between there and the sections on JSON 
types and operators.

view thread (8+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: Re: SQL:2023 JSON simplified accessor support
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox