Re: SQL/JSON revisited

From: Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>
To: Andrew Dunstan <andrew(at)dunslane(dot)net>
Cc: Amit Langote <amitlangote09(at)gmail(dot)com>, Peter Eisentraut <peter(dot)eisentraut(at)enterprisedb(dot)com>, Andres Freund <andres(at)anarazel(dot)de>, vignesh C <vignesh21(at)gmail(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>, e(dot)indrupskaya(at)postgrespro(dot)ru
Subject: Re: SQL/JSON revisited
Date: 2023-03-27 18:54:56
Message-ID: 20230327185456.wbb5gchworsu3hf7@alvherre.pgsql
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Docs amended as I threatened. Other than that, this has required more
changes than I thought it would. For one thing, I've continued to
adjust the grammar a little bit, hopefully without breaking anything (at
least, the tests continue to work). Even so, I was unable to get bison
to accept the 'KEY name VALUES blah' syntax; it might be a
fun/challenging project to change the productions that we use for JSON
names and values:

+json_name_and_value:
+/* Supporting this syntax seems to require major surgery
+ KEY c_expr VALUE_P json_value_expr
+ { $$ = makeJsonKeyValue($2, $4); }
+ |
+*/
+ c_expr VALUE_P json_value_expr
+ { $$ = makeJsonKeyValue($1, $3); }
+ |
+ a_expr ':' json_value_expr
+ { $$ = makeJsonKeyValue($1, $3); }
+ ;

If we uncomment the KEY bit there, a bunch of conflicts emerge. Also,
the fact that we have a_expr on the third one but c_expr on the second
bothers me on consistency grounds; but really we should have a separate
production for things that can be JSON field names.

(I also removed json_object_constructor_args_opt and json_object_args as
separate productions, because I didn't see that they got us anything.)

I also noticed that we had a "string of skipped keys" thing in json.c
that was supposed to be used to detect keys already used but with only
NULL values; however, that stuff had already been rewritten by Nikita on
July 2020 to use a hash table, so the string itself was being built but
uselessly so AFAICS. Removed that one.

I added a bunch of comments in several places, and postponed addition of
a couple of structs that are not needed for this part of the features.
Some of these will have to come back with the IS JSON support (0002 in
the original set).

Anyway, barring objections or further problems, I intend to get this one
pushed early tomorrow. For the curious, I've pushed it here
https://github.com/alvherre/postgres/tree/sqljson-constructors
and the tests are currently running here:
https://cirrus-ci.com/build/5468150918021120

--
Álvaro Herrera Breisgau, Deutschland — https://www.EnterpriseDB.com/
"La experiencia nos dice que el hombre peló millones de veces las patatas,
pero era forzoso admitir la posibilidad de que en un caso entre millones,
las patatas pelarían al hombre" (Ijon Tichy)

Attachment Content-Type Size
v12-0001-SQL-JSON-constructors.patch text/x-diff 190.0 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Jeff Davis 2023-03-27 19:04:20 Re: running logical replication as the subscription owner
Previous Message Peter Geoghegan 2023-03-27 18:34:16 Re: pgsql: amcheck: Fix verify_heapam for tuples where xmin or xmax is 0.