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.
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 |