Horton
Menu

Re: SQL/JSON: functions

Nikita Glukhov <n.gluhov@postgrespro.ru>

View thread
From: Nikita Glukhov <n.gluhov@postgrespro.ru>
To: Pavel Stehule <pavel.stehule@gmail.com>, PostgreSQL-development <pgsql-hackers@postgresql.org>
Cc: Erik Rijkers <er@xs4all.nl>, Alvaro Herrera <alvherre@2ndquadrant.com>, Thomas Munro <thomas.munro@gmail.com>, Andrew Alsup <bluesbreaker@gmail.com>, Andres Freund <andres@anarazel.de>, Dmitry Dolgov <9erthalion6@gmail.com>, Oleg Bartunov <obartunov@postgrespro.ru>, Alexander Korotkov <a.korotkov@postgrespro.ru>, Michael Paquier <michael@paquier.xyz>, Simon Riggs <simon@2ndquadrant.com>, Andrew Dunstan <andrew.dunstan@2ndquadrant.com>
Date: 2020-03-19T22:57:21Z
Lists: pgsql-hackers

Attachments

Attached 45th version of the patches.

Nodes JsonFormat, JsonReturning, JsonPassing, JsonBehavior were fixed.

On 17.03.2020 21:35, Pavel Stehule wrote:
>
> út 17. 3. 2020 v 1:55 odesílatel Nikita Glukhov 
> <n.gluhov@postgrespro.ru <mailto:n.gluhov@postgrespro.ru>> napsal:
>
>     Attached 44th version of the patches.
>
>
>     On 12.03.2020 16:41, Pavel Stehule wrote:
>>
>>     On 12.03.2020 00:09 Nikita Glukhov wrote:
>>
>>         Attached 43rd version of the patches.
>>
>>         The previous patch #4 ("Add function formats") was removed.
>>         Instead, introduced new executor node JsonCtorExpr which is used to wrap
>>         SQL/JSON constructor function calls (FuncExpr, Aggref, WindowFunc).
>>
>>         Also, JsonIsPredicate node began to be used as executor node.
>>         This helped to remove unnecessary json[b]_is_valid() user functions.
>>
>>
>>     It looks very well - all tests passed, code looks well.
>>
>>     Now, when there are special nodes, then the introduction of
>>     COERCE_INTERNAL_CAST is not necessary, and it is only my one and
>>     last objection again this patch's set.
>>
>     I have removed patch #3 with COERCE_INTERNAL_CAST too.
>
>     Coercions in JsonValueExpr in JsonCtorExpr, which were previously hidden with
>     COERCE_INTERNAL_CAST and which were outputted as RETURNING or FORMAT JSON
>     clauses, now moved into separate expressions.
>
> I am looking on the code, and although the code is correct it doesn't 
> look well (consistently with other node types).
>
> I think so JsonFormat and JsonReturning should be node types, not just 
> structs. If these types will be nodes, then you can simplify 
> _readJsonExpr and all node operations on this node.

JsonFormat and JsonReturning was transformed into nodes, and not included
directly into other nodes now.


>     User functions json[b]_build_object_ext() and json[b]_build_array_ext() also
>     can be easily removed.   But it seems harder to remove new aggregate functions
>     json[b]_objectagg() and json[b]_agg_strict(), because they can't be called
>     directly from JsonCtorExpr node.
>
>
> I don't see reasons for another reduction now. Can be great if you can 
> finalize work what you plan for pg13.
>

> +<->READ_ENUM_FIELD(on_error.btype, JsonBehaviorType);
> +<->READ_NODE_FIELD(on_error.default_expr);
> +<->READ_ENUM_FIELD(on_empty.btype, JsonBehaviorType);
> +<->READ_NODE_FIELD(on_empty.default_expr);
>
> JsonBehavior is node type. Then why you don't write just
>
> READ_NODE_FIELD(on_error);
> READ_NODE_FIELD(on_empty)
>
> ??

JsonBehavior now used in JsonExpr as a pointer to node.


> And maybe the code can be better if you don't use JsonPassing struct 
> (or use it only inside gram.y) and pass just List *names, List *values.
>
> Nodes should to contains another nodes or scalar types. Using structs 
> (that are not nodes)  inside doesn't look consistently.
>
JsonPassing was replaced with two Lists.


> I found some not finished code in 0003 patch
> +
> +json_name_and_value:
> +/* TODO
> +<-><--><-->KEY c_expr VALUE_P json_value_expr %prec POSTFIXOP
> +<-><--><--><-->{ $$ = makeJsonKeyValue($2, $4); }
> +<-><--><-->|
> +*/
>
This is unsupported variant of standard syntax, because it seems to lead
to unresolvable conflicts.  The only supports syntax is:

JSON_OBJECT(key : value)
JSON_OBJECT(key VALUE value)

>
>     The support for json type in jsonpath also seems to be immature, so I will try
>     to remove it in the next iteration.
>
> What do you think? This patch is little bit off topic, so if you don't 
> need it, then can be removed. Is there some dependency for "jsontable" ?

There is a dependency in SQL/JSON query functions executor, because executor
uses new structure JsonItem instead of plain JsonbValue.  I will try to
preserve refactoring with JsonItem introduction, but remove json support.
If it will be still unacceptable, I will try to completely remove patch #1.

-- 
Nikita Glukhov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. JSON_TABLE: Add support for NESTED paths and columns

  2. Add basic JSON_TABLE() functionality

  3. Add SQL/JSON query functions

  4. Add soft error handling to some expression nodes

  5. Adjust populate_record_field() to handle errors softly

  6. Refactor code used by jsonpath executor to fetch variables

  7. Add more SQL/JSON constructor functions

  8. SQL/JSON: support the IS JSON predicate

  9. SQL/JSON: add standard JSON constructor functions

  10. Improve readability of SQL/JSON documentation.

  11. Documentation for SQL/JSON features

  12. RETURNING clause for JSON() and JSON_SCALAR()

  13. SQL JSON functions

  14. SQL/JSON query functions

  15. IS JSON predicate

  16. SQL/JSON constructors

  17. Common SQL/JSON clauses

  18. Doc: remove obsolete entries in table of BRIN strategy numbers.

  19. Doc: standardize markup a bit more.

  20. Make pg_regress.c unset PGDATABASE during make installcheck.

  21. Fix handling of GENERATED columns in CREATE TABLE LIKE INCLUDING DEFAULTS.