Re: SQL:2023 JSON simplified accessor support

From: Peter Eisentraut <peter(at)eisentraut(dot)org>
To: Alexandra Wang <alexandra(dot)wang(dot)oss(at)gmail(dot)com>, jian he <jian(dot)universality(at)gmail(dot)com>, Andrew Dunstan <andrew(at)dunslane(dot)net>
Cc: PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>, david(at)justatheory(dot)com
Subject: Re: SQL:2023 JSON simplified accessor support
Date: 2024-11-14 12:31:35
Message-ID: d2d872c3-84f9-48f6-9963-f4537f02bc91@eisentraut.org
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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.

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Bertrand Drouvot 2024-11-14 12:33:20 Re: define pg_structiszero(addr, s, r)
Previous Message Jelte Fennema-Nio 2024-11-14 12:30:21 Re: [PATCH] New predefined role pg_manage_extensions