Horton
Menu

Thread

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.

  1. SQL/JSON: functions

    Nikita Glukhov <n.gluhov@postgrespro.ru> — 2018-01-10T22:59:28Z

    Attached patches implementing all SQL/JSON functions excluding JSON_TABLE:

JSON_OBJECT()
JSON_OBJECTAGG()
JSON_ARRAY()
JSON_ARRAYAGG()

JSON_EXISTS()
JSON_VALUE()
JSON_QUERY()

IS JSON predicate


This patchset depends on 8th version of jsonpath patchset that was 
posted in 
https://www.postgresql.org/message-id/48f13b75-0be7-c356-ff26-1db743add56e%40postgrespro.ru 


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

  2. Re: SQL/JSON: functions

    Nikita Glukhov <n.gluhov@postgrespro.ru> — 2018-02-13T23:08:14Z

    Attached 9th version of SQL/JSON patches rebased onto the latest master.

Displayed column name for SQL/JSON functions was fixed.

Documentation drafts written by Oleg Bartunov:
https://github.com/obartunov/sqljsondoc

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

  3. Re: SQL/JSON: functions

    Nikita Glukhov <n.gluhov@postgrespro.ru> — 2018-02-26T15:52:48Z

    Attached 10th version of SQL/JSON patches rebased onto the latest master.

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

  4. Re: SQL/JSON: functions

    Nikita Glukhov <n.gluhov@postgrespro.ru> — 2018-03-05T23:51:08Z

    Attached 11th version of SQL/JSON patches rebased onto the latest master.

Fixed uninitialized FORMAT JSON location in gram.y and column names for 
JSON constructors.

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

  5. Re: SQL/JSON: functions

    Nikita Glukhov <n.gluhov@postgrespro.ru> — 2018-03-07T14:34:24Z

    Attached 12th version of SQL/JSON patches rebased onto the latest master.

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

  6. Re: SQL/JSON: functions

    Simon Riggs <simon@2ndquadrant.com> — 2018-03-13T11:04:03Z

    On 7 March 2018 at 14:34, Nikita Glukhov <n.gluhov@postgrespro.ru> wrote:

> Attached 12th version of SQL/JSON patches rebased onto the latest master.

Please write some docs or notes to go with this.

If you drop a big pile of code with no explanation it will just be ignored.

I think many people want SQL/JSON, but the purpose of a patch
submission is to allow a committer to review and commit without
needing to edit anything. It shouldn't be like assembling flat pack
furniture while wearing a blindfold.

-- 
Simon Riggs                http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

  7. Re: SQL/JSON: functions

    Oleg Bartunov <obartunov@gmail.com> — 2018-03-13T13:08:01Z

    On Tue, Mar 13, 2018 at 2:04 PM, Simon Riggs <simon@2ndquadrant.com> wrote:
> On 7 March 2018 at 14:34, Nikita Glukhov <n.gluhov@postgrespro.ru> wrote:
>
>> Attached 12th version of SQL/JSON patches rebased onto the latest master.
>
> Please write some docs or notes to go with this.
>
> If you drop a big pile of code with no explanation it will just be ignored.
>
> I think many people want SQL/JSON, but the purpose of a patch
> submission is to allow a committer to review and commit without
> needing to edit anything. It shouldn't be like assembling flat pack
> furniture while wearing a blindfold.

The docs are here
https://github.com/obartunov/sqljsondoc/blob/master/README.jsonpath.md

It's not easy to write docs for SQL/JSON in xml, so I decided to write in more
friendly way. We'll have time to convert it to postgres format.


>
> --
> Simon Riggs                http://www.2ndQuadrant.com/
> PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

  8. Re: SQL/JSON: functions

    Michael Paquier <michael@paquier.xyz> — 2018-03-13T22:54:35Z

    On Tue, Mar 13, 2018 at 04:08:01PM +0300, Oleg Bartunov wrote:
> The docs are here
> https://github.com/obartunov/sqljsondoc/blob/master/README.jsonpath.md
> 
> It's not easy to write docs for SQL/JSON in xml, so I decided to write in more
> friendly way. We'll have time to convert it to postgres format.

If you aim at getting a feature committed first without its
documentation, and getting the docs written after the feature freeze
using a dedicated open item or such, this is much acceptable in my
opinion and the CF is running short in time.
--
Michael

  9. Re: SQL/JSON: functions

    Andres Freund <andres@anarazel.de> — 2018-03-13T23:10:58Z

    On 2018-03-14 07:54:35 +0900, Michael Paquier wrote:
> On Tue, Mar 13, 2018 at 04:08:01PM +0300, Oleg Bartunov wrote:
> > The docs are here
> > https://github.com/obartunov/sqljsondoc/blob/master/README.jsonpath.md
> > 
> > It's not easy to write docs for SQL/JSON in xml, so I decided to write in more
> > friendly way. We'll have time to convert it to postgres format.
> 
> If you aim at getting a feature committed first without its
> documentation, and getting the docs written after the feature freeze
> using a dedicated open item or such, this is much acceptable in my
> opinion and the CF is running short in time.

Given that this patch still uses PG_TRY/CATCH around as wide paths of
code as a whole ExecEvalExpr() invocation, basically has gotten no
review, I don't see this going anywhere for v11.

Greetings,

Andres Freund

  10. Re: SQL/JSON: functions

    Oleg Bartunov <obartunov@gmail.com> — 2018-03-14T00:08:17Z

    On 14 Mar 2018 01:54, "Michael Paquier" <michael@paquier.xyz> wrote:

On Tue, Mar 13, 2018 at 04:08:01PM +0300, Oleg Bartunov wrote:
> The docs are here
> https://github.com/obartunov/sqljsondoc/blob/master/README.jsonpath.md
>
> It's not easy to write docs for SQL/JSON in xml, so I decided to write in
more
> friendly way. We'll have time to convert it to postgres format.

If you aim at getting a feature committed first without its
documentation, and getting the docs written after the feature freeze
using a dedicated open item or such,


Exactly. SQL/JSON is rather complex thing and "converting" the standard to
the user level understanding is a separate challenge and I'd like to
continue to work on it. It's mostly written, we need to understand how to
organize it.

this

is much acceptable in my
opinion and the CF is running short in time.
--
Michael

  11. Re: SQL/JSON: functions

    Alexander Korotkov <a.korotkov@postgrespro.ru> — 2018-03-14T14:11:01Z

    On Wed, Mar 14, 2018 at 2:10 AM, Andres Freund <andres@anarazel.de> wrote:

> On 2018-03-14 07:54:35 +0900, Michael Paquier wrote:
> > On Tue, Mar 13, 2018 at 04:08:01PM +0300, Oleg Bartunov wrote:
> > > The docs are here
> > > https://github.com/obartunov/sqljsondoc/blob/master/README.jsonpath.md
> > >
> > > It's not easy to write docs for SQL/JSON in xml, so I decided to write
> in more
> > > friendly way. We'll have time to convert it to postgres format.
> >
> > If you aim at getting a feature committed first without its
> > documentation, and getting the docs written after the feature freeze
> > using a dedicated open item or such, this is much acceptable in my
> > opinion and the CF is running short in time.
>
> Given that this patch still uses PG_TRY/CATCH around as wide paths of
> code as a whole ExecEvalExpr() invocation,


I agree that we should either use PG_TRY/CATCH over some small and safe
codepaths or surround PG_TRY/CATCH with subtransactions.  PG_TRY/CATCH over
ExecEvalExpr() looks really unacceptable.

basically has gotten no
> review, I don't see this going anywhere for v11.
>

I wouldn't be co categorical at this point.  Patchset is there for about
year.
Some parts of code received more of review while some parts receives less.
We can surround all dangerous PG_TRY/CATCH pairs with subtransactions,
tolerate performance penalty and leave further optimizations for future
releases.
In worst case, we can remove codepaths which use PG_TRY/CATCH and
leave only ERROR ON ERROR behavior of SQL/JSON.

------
Alexander Korotkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company

  12. Re: SQL/JSON: functions

    Pavel Stehule <pavel.stehule@gmail.com> — 2018-03-14T19:33:50Z

    2018-03-14 15:11 GMT+01:00 Alexander Korotkov <a.korotkov@postgrespro.ru>:

> On Wed, Mar 14, 2018 at 2:10 AM, Andres Freund <andres@anarazel.de> wrote:
>
>> On 2018-03-14 07:54:35 +0900, Michael Paquier wrote:
>> > On Tue, Mar 13, 2018 at 04:08:01PM +0300, Oleg Bartunov wrote:
>> > > The docs are here
>> > > https://github.com/obartunov/sqljsondoc/blob/master/README.j
>> sonpath.md
>> > >
>> > > It's not easy to write docs for SQL/JSON in xml, so I decided to
>> write in more
>> > > friendly way. We'll have time to convert it to postgres format.
>> >
>> > If you aim at getting a feature committed first without its
>> > documentation, and getting the docs written after the feature freeze
>> > using a dedicated open item or such, this is much acceptable in my
>> > opinion and the CF is running short in time.
>>
>> Given that this patch still uses PG_TRY/CATCH around as wide paths of
>> code as a whole ExecEvalExpr() invocation,
>
>
> I agree that we should either use PG_TRY/CATCH over some small and safe
> codepaths or surround PG_TRY/CATCH with subtransactions.  PG_TRY/CATCH
> over
> ExecEvalExpr() looks really unacceptable.
>
> basically has gotten no
>> review, I don't see this going anywhere for v11.
>>
>
> I wouldn't be co categorical at this point.  Patchset is there for about
> year.
> Some parts of code received more of review while some parts receives less.
> We can surround all dangerous PG_TRY/CATCH pairs with subtransactions,
> tolerate performance penalty and leave further optimizations for future
> releases.
> In worst case, we can remove codepaths which use PG_TRY/CATCH and
> leave only ERROR ON ERROR behavior of SQL/JSON.
>

I am thinking so using subtransactions on few places are acceptable.
PLpgSQL uses it years, and it is working good enough.

Regards

Pavel

>
>

> ------
> Alexander Korotkov
> Postgres Professional: http://www.postgrespro.com
> The Russian Postgres Company
>

  13. Re: SQL/JSON: functions

    Oleg Bartunov <obartunov@postgrespro.ru> — 2018-03-15T08:08:02Z

    On 14 Mar 2018 17:11, "Alexander Korotkov" <a.korotkov@postgrespro.ru>
wrote:

On Wed, Mar 14, 2018 at 2:10 AM, Andres Freund <andres@anarazel.de> wrote:

> On 2018-03-14 07:54:35 +0900, Michael Paquier wrote:
> > On Tue, Mar 13, 2018 at 04:08:01PM +0300, Oleg Bartunov wrote:
> > > The docs are here
> > > https://github.com/obartunov/sqljsondoc/blob/master/README.jsonpath.md
> > >
> > > It's not easy to write docs for SQL/JSON in xml, so I decided to write
> in more
> > > friendly way. We'll have time to convert it to postgres format.
> >
> > If you aim at getting a feature committed first without its
> > documentation, and getting the docs written after the feature freeze
> > using a dedicated open item or such, this is much acceptable in my
> > opinion and the CF is running short in time.
>
> Given that this patch still uses PG_TRY/CATCH around as wide paths of
> code as a whole ExecEvalExpr() invocation,


I agree that we should either use PG_TRY/CATCH over some small and safe
codepaths or surround PG_TRY/CATCH with subtransactions.  PG_TRY/CATCH over
ExecEvalExpr() looks really unacceptable.

basically has gotten no
> review, I don't see this going anywhere for v11.
>

I wouldn't be co categorical at this point.  Patchset is there for about
year.
Some parts of code received more of review while some parts receives less.
We can surround all dangerous PG_TRY/CATCH pairs with subtransactions,
tolerate performance penalty and leave further optimizations for future
releases.


Agree it's not difficult.

In worst case, we can remove codepaths which use PG_TRY/CATCH and
leave only ERROR ON ERROR behavior of SQL/JSON.


No-no, json user will be really upset on this. Our goal is to be the first
relational database with strong standard compliance.


------
Alexander Korotkov

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

  14. Re: SQL/JSON: functions

    Nikita Glukhov <n.gluhov@postgrespro.ru> — 2018-03-15T17:04:07Z

    Attached 13th version of the patches:

* Subtransactions in PG_TRY/CATCH in ExecEvalJsonExpr() were made unconditional,
   regardless of the volatility of expressions.

* PG_TRY/CATCH in ExecEvalExprPassingCaseValue() was removed along with the
   entire function.

On 15.03.2018 11:08, Oleg Bartunov wrote:

> On 14 Mar 2018 17:11, "Alexander Korotkov" <a.korotkov@postgrespro.ru 
> <mailto:a.korotkov@postgrespro.ru>> wrote:
>
>     On Wed, Mar 14, 2018 at 2:10 AM, Andres Freund <andres@anarazel.de
>     <mailto:andres@anarazel.de>> wrote:
>
>         On 2018-03-14 07:54:35 +0900, Michael Paquier wrote:
>         > On Tue, Mar 13, 2018 at 04:08:01PM +0300, Oleg Bartunov wrote:
>         > > The docs are here
>         > >
>         https://github.com/obartunov/sqljsondoc/blob/master/README.jsonpath.md
>         <https://github.com/obartunov/sqljsondoc/blob/master/README.jsonpath.md>
>         > >
>         > > It's not easy to write docs for SQL/JSON in xml, so I
>         decided to write in more
>         > > friendly way. We'll have time to convert it to postgres
>         format.
>         >
>         > If you aim at getting a feature committed first without its
>         > documentation, and getting the docs written after the
>         feature freeze
>         > using a dedicated open item or such, this is much acceptable
>         in my
>         > opinion and the CF is running short in time.
>
>         Given that this patch still uses PG_TRY/CATCH around as wide
>         paths of
>         code as a whole ExecEvalExpr() invocation,
>
>     I agree that we should either use PG_TRY/CATCH over some small and
>     safe
>     codepaths or surround PG_TRY/CATCH with subtransactions.
>     PG_TRY/CATCH over
>     ExecEvalExpr() looks really unacceptable.
>
>         basically has gotten no
>         review, I don't see this going anywhere for v11.
>
>
>     I wouldn't be co categorical at this point. Patchset is there for
>     about year.
>     Some parts of code received more of review while some parts
>     receives less.
>     We can surround all dangerous PG_TRY/CATCH pairs with subtransactions,
>     tolerate performance penalty and leave further optimizations for
>     future releases.
>
>
> Agree it's not difficult.
>
>     In worst case, we can remove codepaths which use PG_TRY/CATCH and
>     leave only ERROR ON ERROR behavior of SQL/JSON.
>
>
> No-no, json user will be really upset on this. Our goal is to be the 
> first relational database with strong standard compliance.

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

  15. Re: SQL/JSON: functions

    Nikita Glukhov <n.gluhov@postgrespro.ru> — 2018-06-28T00:18:03Z

    On 15.03.2018 20:04, Nikita Glukhov wrote:
> Attached 13th version of the patches:
>
> * Subtransactions in PG_TRY/CATCH in ExecEvalJsonExpr() were made unconditional,
>   regardless of the volatility of expressions.
>
> * PG_TRY/CATCH in ExecEvalExprPassingCaseValue() was removed along with the
>   entire function.


Attached 15th version of the patches:
  * disabled parallel execution of SQL/JSON query functions when internal
    subtransactions are used (if ERROR ON ERROR is not specified)
  * added experimental optimization of internal subtransactions (see below)


The new patch #14 is an experimental attempt to reduce overhead of
subtransaction start/commit which can result in 2x-slowdown in the simplest
cases.  By the idea of Alexander Korotkov, subtransaction is not really
committed if it has not touched the database and its XID has not been assigned
(DB modification is not expected in type casts functions) and then can be reused
when the next subtransaction is started.  So, all rows in JsonExpr can be
executed in the single cached subtransaction.  This optimization really helps
to reduce overhead from 100% to 5-10%:

-- without subtransactions
=# EXPLAIN ANALYZE
    SELECT JSON_VALUE('true'::jsonb, '$' RETURNING boolean ERROR ON ERROR)
    FROM generate_series(1, 10000000) i;
  ...
  Execution Time: 2785.410 ms

-- cached subtransactions
=# EXPLAIN ANALYZE
    SELECT JSON_VALUE('true'::jsonb, '$' RETURNING boolean)
    FROM generate_series(1, 10000000) i;
  ...
  Execution Time: 2939.363 ms

-- ordinary subtransactions
=# EXPLAIN ANALYZE
    SELECT JSON_VALUE('true'::jsonb, '$' RETURNING boolean)
    FROM generate_series(1, 10000000) i;
  ...
  Execution Time: 5417.268 ms


But, unfortunately, I don't believe that this patch is completely correct,
mainly because the behavior of subtransaction callbacks (and their expectations
about subtransaction's lifecycle too) seems unpredictable to me.



Even with this optimization, internal subtransactions still have one major
drawback -- they disallow parallel query execution, because background
workers do not support subtransactions now.  Example:

=# CREATE TABLE test_parallel_json_value AS
    SELECT i::text::jsonb AS js FROM generate_series(1, 5000000) i;
CREATE TABLE

=# EXPLAIN ANALYZE
    SELECT sum(JSON_VALUE(js, '$' RETURNING numeric ERROR ON ERROR))
    FROM test_parallel_json_value;
                                                                QUERY PLAN
-----------------------------------------------------------------------------------------------------------------------------------------
  Finalize Aggregate  (cost=79723.15..79723.16 rows=1 width=32) (actual time=455.062..455.062 rows=1 loops=1)
    ->  Gather  (cost=79722.93..79723.14 rows=2 width=32) (actual time=455.052..455.055 rows=3 loops=1)
          Workers Planned: 2
          Workers Launched: 2
          ->  Partial Aggregate  (cost=78722.93..78722.94 rows=1 width=32) (actual time=446.000..446.000 rows=1 loops=3)
                ->  Parallel Seq Scan on t  (cost=0.00..52681.30 rows=2083330 width=18) (actual time=0.023..104.779 rows=1666667 loops=3)
  Planning Time: 0.044 ms
  Execution Time: 456.460 ms
(8 rows)

=# EXPLAIN ANALYZE
    SELECT sum(JSON_VALUE(js, '$' RETURNING numeric))
    FROM test_parallel_json_value;
                                                      QUERY PLAN
--------------------------------------------------------------------------------------------------------------------
  Aggregate  (cost=144347.82..144347.83 rows=1 width=32) (actual time=1381.938..1381.938 rows=1 loops=1)
    ->  Seq Scan on t  (cost=0.00..81847.92 rows=4999992 width=18) (actual time=0.076..309.676 rows=5000000 loops=1)
  Planning Time: 0.082 ms
  Execution Time: 1384.133 ms
(4 rows)


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

  16. Re: SQL/JSON: functions

    Pavel Stehule <pavel.stehule@gmail.com> — 2018-06-28T04:25:12Z

    2018-06-28 2:18 GMT+02:00 Nikita Glukhov <n.gluhov@postgrespro.ru>:

> On 15.03.2018 20:04, Nikita Glukhov wrote:
>
>> Attached 13th version of the patches:
>>
>> * Subtransactions in PG_TRY/CATCH in ExecEvalJsonExpr() were made
>> unconditional,
>>   regardless of the volatility of expressions.
>>
>> * PG_TRY/CATCH in ExecEvalExprPassingCaseValue() was removed along with
>> the
>>   entire function.
>>
>
>
> Attached 15th version of the patches:
>  * disabled parallel execution of SQL/JSON query functions when internal
>    subtransactions are used (if ERROR ON ERROR is not specified)
>  * added experimental optimization of internal subtransactions (see below)
>
>
> The new patch #14 is an experimental attempt to reduce overhead of
> subtransaction start/commit which can result in 2x-slowdown in the simplest
> cases.  By the idea of Alexander Korotkov, subtransaction is not really
> committed if it has not touched the database and its XID has not been
> assigned
> (DB modification is not expected in type casts functions) and then can be
> reused
> when the next subtransaction is started.  So, all rows in JsonExpr can be
> executed in the single cached subtransaction.  This optimization really
> helps
> to reduce overhead from 100% to 5-10%:
>

I read a technical report for SQL/JSON. If I understand it well, then ON
ERROR clause is primary related to structural errors, not to all errors.

So your implementation is maybe too tolerant, what has this issue. There
was not any example, so this clause should to handle cast errors or any
other errors than JSON structural.

The playing with other implementation of subtransactions doesn't look like
safe way, more if it is not necessary

The other possible error are casts errors. We can introduce new exception
safe input functions. These functions can be interesting for fault tolerant
COPY for example.

Regards

Pavel



> -- without subtransactions
> =# EXPLAIN ANALYZE
>    SELECT JSON_VALUE('true'::jsonb, '$' RETURNING boolean ERROR ON ERROR)
>    FROM generate_series(1, 10000000) i;
>  ...
>  Execution Time: 2785.410 ms
>
> -- cached subtransactions
> =# EXPLAIN ANALYZE
>    SELECT JSON_VALUE('true'::jsonb, '$' RETURNING boolean)
>    FROM generate_series(1, 10000000) i;
>  ...
>  Execution Time: 2939.363 ms
>
> -- ordinary subtransactions
> =# EXPLAIN ANALYZE
>    SELECT JSON_VALUE('true'::jsonb, '$' RETURNING boolean)
>    FROM generate_series(1, 10000000) i;
>  ...
>  Execution Time: 5417.268 ms
>
>
> But, unfortunately, I don't believe that this patch is completely correct,
> mainly because the behavior of subtransaction callbacks (and their
> expectations
> about subtransaction's lifecycle too) seems unpredictable to me.
>
>
>
> Even with this optimization, internal subtransactions still have one major
> drawback -- they disallow parallel query execution, because background
> workers do not support subtransactions now.  Example:
>
> =# CREATE TABLE test_parallel_json_value AS
>    SELECT i::text::jsonb AS js FROM generate_series(1, 5000000) i;
> CREATE TABLE
>
> =# EXPLAIN ANALYZE
>    SELECT sum(JSON_VALUE(js, '$' RETURNING numeric ERROR ON ERROR))
>    FROM test_parallel_json_value;
>                                                                QUERY PLAN
> ------------------------------------------------------------
> ------------------------------------------------------------
> -----------------
>  Finalize Aggregate  (cost=79723.15..79723.16 rows=1 width=32) (actual
> time=455.062..455.062 rows=1 loops=1)
>    ->  Gather  (cost=79722.93..79723.14 rows=2 width=32) (actual
> time=455.052..455.055 rows=3 loops=1)
>          Workers Planned: 2
>          Workers Launched: 2
>          ->  Partial Aggregate  (cost=78722.93..78722.94 rows=1 width=32)
> (actual time=446.000..446.000 rows=1 loops=3)
>                ->  Parallel Seq Scan on t  (cost=0.00..52681.30
> rows=2083330 width=18) (actual time=0.023..104.779 rows=1666667 loops=3)
>  Planning Time: 0.044 ms
>  Execution Time: 456.460 ms
> (8 rows)
>
> =# EXPLAIN ANALYZE
>    SELECT sum(JSON_VALUE(js, '$' RETURNING numeric))
>    FROM test_parallel_json_value;
>                                                      QUERY PLAN
> ------------------------------------------------------------
> --------------------------------------------------------
>  Aggregate  (cost=144347.82..144347.83 rows=1 width=32) (actual
> time=1381.938..1381.938 rows=1 loops=1)
>    ->  Seq Scan on t  (cost=0.00..81847.92 rows=4999992 width=18) (actual
> time=0.076..309.676 rows=5000000 loops=1)
>  Planning Time: 0.082 ms
>  Execution Time: 1384.133 ms
> (4 rows)
>
>
>
> --
> Nikita Glukhov
> Postgres Professional: http://www.postgrespro.com
> The Russian Postgres Company
>
>

  17. Re: SQL/JSON: functions

    Nikita Glukhov <n.gluhov@postgrespro.ru> — 2018-07-03T12:30:27Z

    Attached 16th version of the patches:
  * changed type of new SQL keyword STRING
    (STRING is used as a function parameter name in Pl/Tcl tests)
  * removed implicit coercion via I/O from JSON_VALUE (see below)

On 28.06.2018 07:25, Pavel Stehule wrote:
> 2018-06-28 2:18 GMT+02:00 Nikita Glukhov <n.gluhov@postgrespro.ru 
> <mailto:n.gluhov@postgrespro.ru>>:
>
>     On 15.03.2018 20:04, Nikita Glukhov wrote:
>
>         Attached 13th version of the patches:
>
>         * Subtransactions in PG_TRY/CATCH in ExecEvalJsonExpr() were
>         made unconditional,
>           regardless of the volatility of expressions.
>
>         * PG_TRY/CATCH in ExecEvalExprPassingCaseValue() was removed
>         along with the
>           entire function.
>
>
>
>     Attached 15th version of the patches:
>      * disabled parallel execution of SQL/JSON query functions when
>     internal
>        subtransactions are used (if ERROR ON ERROR is not specified)
>      * added experimental optimization of internal subtransactions
>     (see below)
>
>
>     The new patch #14 is an experimental attempt to reduce overhead of
>     subtransaction start/commit which can result in 2x-slowdown in the
>     simplest
>     cases.  By the idea of Alexander Korotkov, subtransaction is not
>     really
>     committed if it has not touched the database and its XID has not
>     been assigned
>     (DB modification is not expected in type casts functions) and then
>     can be reused
>     when the next subtransaction is started.  So, all rows in JsonExpr
>     can be
>     executed in the single cached subtransaction.  This optimization
>     really helps
>     to reduce overhead from 100% to 5-10%:
>
>
> I read a technical report for SQL/JSON. If I understand it well, then 
> ON ERROR clause is primary related to structural errors, not to all 
> errors.
>
> So your implementation is maybe too tolerant, what has this issue. 
> There was not any example, so this clause should to handle cast errors 
> or any other errors than JSON structural.
>
> The playing with other implementation of subtransactions doesn't look 
> like safe way, more if it is not necessary
>
> The other possible error are casts errors. We can introduce new 
> exception safe input functions. These functions can be interesting for 
> fault tolerant COPY for example.
>
SQL/JSON standard requires handling of cast errors too.

9.40 Casting an SQL/JSON sequence to an SQL type (pages 724-725):

4) If TEMPST is successful completion, then:
   b) If the length of SEQ is 1 (one), then let I be the SQL/JSON item in SEQ.
     Case:
       ...
     iii) Otherwise, let IDT be the data type of I.
       Case:
       1) If IDT cannot be cast to target type DT according to the Syntax Rules
          of Subclause 6.13, "<cast specification>", then let TEMPST be data
          exception — SQL/JSON item cannot be cast to target type.
       2) Otherwise, let X be an SQL variable whose value is I. Let V be the
          value of CAST (X AS DT).  If an exception condition is raised by this
          <cast specification>, then let TEMPST be that exception condition.
   ...
5) Case:
   a) If TEMPST is successful completion, then let OUTST be successful
      completion.
   b) If ONERROR is ERROR, then let OUTST be TEMPST.
   c) If ONERROR is NULL, then let V be the SQL null value and let OUTST be
      successful completion.
   d) If ONERROR immediately contains DEFAULT, then let VE be the
      <value expression> immediately contained in ONERROR. Let V be the value of
      CAST (VE AS DT)
     Case:
     i) If an exception condition is raised by this <cast specification>, then
        let OUTST be that exception condition.
     ii) Otherwise, let OUTST be successful completion.


In 4.b.iii.1 said that there should be an error if the desired cast does not exist.
In the previous versions of the patches there was implicit coercion via I/O here
instead of error, so I decided to fix it the last version (fix is combined with a
minor refactoring of ExecEvalJsonExpr()).

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

  18. Re: SQL/JSON: functions

    Pavel Stehule <pavel.stehule@gmail.com> — 2018-07-03T12:56:52Z

    2018-07-03 14:30 GMT+02:00 Nikita Glukhov <n.gluhov@postgrespro.ru>:

> Attached 16th version of the patches:
>  * changed type of new SQL keyword STRING
>    (STRING is used as a function parameter name in Pl/Tcl tests)
>  * removed implicit coercion via I/O from JSON_VALUE (see below)
>
>
> On 28.06.2018 07:25, Pavel Stehule wrote:
>
> 2018-06-28 2:18 GMT+02:00 Nikita Glukhov <n.gluhov@postgrespro.ru>:
>
>> On 15.03.2018 20:04, Nikita Glukhov wrote:
>>
>>> Attached 13th version of the patches:
>>>
>>> * Subtransactions in PG_TRY/CATCH in ExecEvalJsonExpr() were made
>>> unconditional,
>>>   regardless of the volatility of expressions.
>>>
>>> * PG_TRY/CATCH in ExecEvalExprPassingCaseValue() was removed along with
>>> the
>>>   entire function.
>>>
>>
>>
>> Attached 15th version of the patches:
>>  * disabled parallel execution of SQL/JSON query functions when internal
>>    subtransactions are used (if ERROR ON ERROR is not specified)
>>  * added experimental optimization of internal subtransactions (see below)
>>
>>
>> The new patch #14 is an experimental attempt to reduce overhead of
>> subtransaction start/commit which can result in 2x-slowdown in the
>> simplest
>> cases.  By the idea of Alexander Korotkov, subtransaction is not really
>> committed if it has not touched the database and its XID has not been
>> assigned
>> (DB modification is not expected in type casts functions) and then can be
>> reused
>> when the next subtransaction is started.  So, all rows in JsonExpr can be
>> executed in the single cached subtransaction.  This optimization really
>> helps
>> to reduce overhead from 100% to 5-10%:
>>
>
> I read a technical report for SQL/JSON. If I understand it well, then ON
> ERROR clause is primary related to structural errors, not to all errors.
>
> So your implementation is maybe too tolerant, what has this issue. There
> was not any example, so this clause should to handle cast errors or any
> other errors than JSON structural.
>
> The playing with other implementation of subtransactions doesn't look like
> safe way, more if it is not necessary
>
> The other possible error are casts errors. We can introduce new exception
> safe input functions. These functions can be interesting for fault tolerant
> COPY for example.
>
> SQL/JSON standard requires handling of cast errors too.
>
> I didn't speak something else. But cast (and in this case it is from JSON
to some else) can be exception safe.

Regards

Pavel


>
> 9.40 Casting an SQL/JSON sequence to an SQL type (pages 724-725):
>
> 4) If TEMPST is successful completion, then:
>   b) If the length of SEQ is 1 (one), then let I be the SQL/JSON item in SEQ.
>     Case:
>       ...
>     iii) Otherwise, let IDT be the data type of I.
>       Case:
>       1) If IDT cannot be cast to target type DT according to the Syntax Rules
>          of Subclause 6.13, "<cast specification>", then let TEMPST be data
>          exception — SQL/JSON item cannot be cast to target type.
>       2) Otherwise, let X be an SQL variable whose value is I. Let V be the
>          value of CAST (X AS DT).  If an exception condition is raised by this
>          <cast specification>, then let TEMPST be that exception condition.
>   ...
> 5) Case:
>   a) If TEMPST is successful completion, then let OUTST be successful
>      completion.
>   b) If ONERROR is ERROR, then let OUTST be TEMPST.
>   c) If ONERROR is NULL, then let V be the SQL null value and let OUTST be
>      successful completion.
>   d) If ONERROR immediately contains DEFAULT, then let VE be the
>      <value expression> immediately contained in ONERROR. Let V be the value of
>      CAST (VE AS DT)
>     Case:
>     i) If an exception condition is raised by this <cast specification>, then
>        let OUTST be that exception condition.
>     ii) Otherwise, let OUTST be successful completion.
>
>
> In 4.b.iii.1 said that there should be an error if the desired cast does not exist.
> In the previous versions of the patches there was implicit coercion via I/O here
> instead of error, so I decided to fix it the last version (fix is combined with a
> minor refactoring of ExecEvalJsonExpr()).
>
> --
> Nikita Glukhov
> Postgres Professional: http://www.postgrespro.com
> The Russian Postgres Company
>
>

  19. Re: SQL/JSON: functions

    Dmitry Dolgov <9erthalion6@gmail.com> — 2018-11-26T12:57:01Z

    > On Tue, Jul 3, 2018 at 2:57 PM Pavel Stehule <pavel.stehule@gmail.com> wrote:
>
> 2018-07-03 14:30 GMT+02:00 Nikita Glukhov <n.gluhov@postgrespro.ru>:
>>
>> Attached 16th version of the patches:
>>  * changed type of new SQL keyword STRING
>>    (STRING is used as a function parameter name in Pl/Tcl tests)
>>  * removed implicit coercion via I/O from JSON_VALUE (see below)

Unfortunately, the current version of patch 0010-add-invisible-coercion-form
doesn't not apply anymore without conflicts, could you please rebase it?

  20. Re: SQL/JSON: functions

    Nikita Glukhov <n.gluhov@postgrespro.ru> — 2018-11-26T16:13:42Z

    On 26.11.2018 15:57, Dmitry Dolgov wrote:

>> On Tue, Jul 3, 2018 at 2:57 PM Pavel Stehule <pavel.stehule@gmail.com> wrote:
>>
>> 2018-07-03 14:30 GMT+02:00 Nikita Glukhov <n.gluhov@postgrespro.ru>:
>>> Attached 16th version of the patches:
>>>   * changed type of new SQL keyword STRING
>>>     (STRING is used as a function parameter name in Pl/Tcl tests)
>>>   * removed implicit coercion via I/O from JSON_VALUE (see below)
> Unfortunately, the current version of patch 0010-add-invisible-coercion-form
> doesn't not apply anymore without conflicts, could you please rebase it?

Attached 20th version of the patches rebased onto the current master.


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

  21. Re: SQL/JSON: functions

    Dmitry Dolgov <9erthalion6@gmail.com> — 2018-12-01T12:36:18Z

    > On Mon, Nov 26, 2018 at 5:11 PM Nikita Glukhov <n.gluhov@postgrespro.ru> wrote:
>
> On 26.11.2018 15:57, Dmitry Dolgov wrote:
>
> >> On Tue, Jul 3, 2018 at 2:57 PM Pavel Stehule <pavel.stehule@gmail.com> wrote:
> >>
> >> 2018-07-03 14:30 GMT+02:00 Nikita Glukhov <n.gluhov@postgrespro.ru>:
> >>> Attached 16th version of the patches:
> >>>   * changed type of new SQL keyword STRING
> >>>     (STRING is used as a function parameter name in Pl/Tcl tests)
> >>>   * removed implicit coercion via I/O from JSON_VALUE (see below)
> > Unfortunately, the current version of patch 0010-add-invisible-coercion-form
> > doesn't not apply anymore without conflicts, could you please rebase it?
>
> Attached 20th version of the patches rebased onto the current master.

Thank you,

Unfortunately, looks like we're in the loop "Rebase. Wait. Repeat", since the
patch again has some conflicts (this time I hope just minor conflicts). Which
is probably understandable taking into account the size of it. Maybe we need to
think about how to proceed with this, do you have any ideas?

  22. Re: SQL/JSON: functions

    Nikita Glukhov <n.gluhov@postgrespro.ru> — 2018-12-04T23:01:19Z

    On 01.12.2018 15:36, Dmitry Dolgov wrote:
>> On Mon, Nov 26, 2018 at 5:11 PM Nikita Glukhov <n.gluhov@postgrespro.ru> wrote:
>>
>> On 26.11.2018 15:57, Dmitry Dolgov wrote:
>>
>>>> On Tue, Jul 3, 2018 at 2:57 PM Pavel Stehule <pavel.stehule@gmail.com> wrote:
>>>>
>>>> 2018-07-03 14:30 GMT+02:00 Nikita Glukhov <n.gluhov@postgrespro.ru>:
>>>>> Attached 16th version of the patches:
>>>>>    * changed type of new SQL keyword STRING
>>>>>      (STRING is used as a function parameter name in Pl/Tcl tests)
>>>>>    * removed implicit coercion via I/O from JSON_VALUE (see below)
>>> Unfortunately, the current version of patch 0010-add-invisible-coercion-form
>>> doesn't not apply anymore without conflicts, could you please rebase it?
>> Attached 20th version of the patches rebased onto the current master.
> Thank you,
>
> Unfortunately, looks like we're in the loop "Rebase. Wait. Repeat", since the
> patch again has some conflicts (this time I hope just minor conflicts). Which
> is probably understandable taking into account the size of it. Maybe we need to
> think about how to proceed with this, do you have any ideas?

Attached 21st version of the patches.

I decided to include here patch 0000 with complete jsonpath implementation (it
is a squash of all 6 jsonpath-v21 patches). I hope this will simplify reviewing
and testing in cfbot.cputube.org.


If it will conflict again, then you can see all SQL/JSON v21 patches
successfully applied in our GitHub repository on the following branches:
https://github.com/postgrespro/sqljson/tree/sqljson_v21  (one commit per patch)
https://github.com/postgrespro/sqljson/tree/sqljson   (original commit history)

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

  23. Re: SQL/JSON: functions

    Andres Freund <andres@anarazel.de> — 2019-02-16T05:12:39Z

    Hi,

On 2018-12-05 02:01:19 +0300, Nikita Glukhov wrote:

> +	if (!PG_ARGISNULL(1) &&
> +		strncmp("any", VARDATA(type), VARSIZE_ANY_EXHDR(type)))
> +	{
> +		JsonLexContext *lex;
> +		JsonTokenType tok;
> +
> +		lex = makeJsonLexContext(json, false);
> +
> +		/* Lex exactly one token from the input and check its type. */
> +		PG_TRY();
> +		{
> +			json_lex(lex);
> +		}
> +		PG_CATCH();
> +		{
> +			if (ERRCODE_TO_CATEGORY(geterrcode()) == ERRCODE_DATA_EXCEPTION)
> +			{
> +				FlushErrorState();
> +				MemoryContextSwitchTo(mcxt);
> +				PG_RETURN_BOOL(false);	/* invalid json */
> +			}
> +			PG_RE_THROW();
> +		}
> +		PG_END_TRY();


It baffles me that a year after I raised this as a serious issue, in
this thread, this patch still contains code like this.


Greetings,

Andres Freund

  24. Re: SQL/JSON: functions

    Nikita Glukhov <n.gluhov@postgrespro.ru> — 2019-03-01T01:14:09Z

    Attached 34th version of the patches.

On 16.02.2019 8:12, Andres Freund wrote:

> On 2018-12-05 02:01:19 +0300, Nikita Glukhov wrote:
>> +		JsonLexContext *lex;
>> +		JsonTokenType tok;
>> +
>> +		lex = makeJsonLexContext(json, false);
>> +
>> +		/* Lex exactly one token from the input and check its type. */
>> +		PG_TRY();
>> +		{
>> +			json_lex(lex);
>> +		}
>> +		PG_CATCH();
>> +		{
>> +			if (ERRCODE_TO_CATEGORY(geterrcode()) == ERRCODE_DATA_EXCEPTION)
>> +			{
>> +				FlushErrorState();
>> +				MemoryContextSwitchTo(mcxt);
>> +				PG_RETURN_BOOL(false);	/* invalid json */
>> +			}
>> +			PG_RE_THROW();
>> +		}
>> +		PG_END_TRY();
>
> It baffles me that a year after I raised this as a serious issue, in
> this thread, this patch still contains code like this.

PG_TRY/PG_CATCH was removed here: 'throwErrors' flag was added to JsonLexContext
instead.


Also usage of subtransactions is SQL/JSON functions (JsonExpr node) was
optimized: they can be not only omitted in ERROR ON ERROR case but also when
the resulting conversion from the SQL/JSON item type to the target SQL type is
no-op.


Below are the results of simple performance test
(operator #>> uses optimization which I recently posted in the separate patch):

                         query                           | time, ms
-------------------------------------------------------------------
  JSON_VALUE(js, '$.x.y.z' RETURNING text) = '123'       | 1923,360 (subtrans!)
  JSON_VALUE(js, '$.x.y.z' RETURNING text
                           ERROR ON ERROR) = '123'       |  970,604
  JSON_VALUE(js, '$.x.y.z' RETURNING numeric) = '123'    |  792,412
  JSON_VALUE(js, '$.x.y.z' RETURNING numeric
                           ERROR ON ERROR) = '123'       |  786,647

  (js->'x'->'y'->'z') = '123'                            | 1104,470
  (js->'x'->'y'->'z')::numeric = '123'                   |  940,037
  (js->'x'->'y'->>'z') = '123'                           |  688,484

  (js #>  '{x,y,z}') = '123'                             | 1127,661
  (js #>  '{x,y,z}')::numeric = '123'                    |  971,931
  (js #>> '{x,y,z}') = '123'                             |  718,173


Table with jsonb rows like  '{"x": {"y": {"z": 123}}}':
CREATE TABLE t AS
SELECT JSON_OBJECT('x' : JSON_OBJECT('y' : JSON_OBJECT('z' : i))) js
FROM generate_series(1, 3000000) i;

Example query:
SELECT * FROM t WHERE JSON_VALUE(js, '$.x.y.z' RETURNING numeric) = '123';

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

  25. Re: SQL/JSON: functions

    Nikita Glukhov <n.gluhov@postgrespro.ru> — 2019-03-05T22:35:29Z

    Attached 36th version of the patches rebased onto jsonpath v36.

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

  26. Re: Re: SQL/JSON: functions

    Andy Alsup <bluesbreaker@gmail.com> — 2019-05-14T00:53:51Z

    On 3/5/19 5:35 PM, Nikita Glukhov wrote:
> Attached 36th version of the patches rebased onto jsonpath v36.
While testing this patch a found a few issues:

[1] I was not able to apply the patch to the current HEAD. However, it 
applies cleanly to commit: e988878f85 (NOTE: I did not investigate which 
commit between e988878f85 and HEAD caused problems).

[2] JsonPath array slicing does not work. I'm not aware of a 
comprehensive list of JsonPath features/syntax that is targeted for 
support; however, I did try various forms of array slicing, which don't 
currently work.

Here are a few examples:

The input document is the same in each example.

{
   "a1": 123,
   "b1": "xxx",
   "c1": {
     "a2": 456,
     "b2": "yyy",
     "c2": [
       {"a3": 777, "b3": "7z"},
       {"a3": 888, "b3": "8z"}
     ]
   }
}

array wildcard selector [*] works: $.c1.c2[*].b3

# select json_path_query('{"a1": 123, "b1": "xxx", "c1": {"a2": 456, 
"b2": "yyy", "c2": [{"a3": 777, "b3": "7z"},{"a3": 888, "b3": 
"8z"}]}}'::json, '$.c1.c2[*].b3'::jsonpath);
  json_path_query
-----------------
  "7z"
  "8z"
(2 rows)

array index selector [0] works: $.c1.c2[0].b3

jsonpatch=# select json_path_query('{"a1": 123, "b1": "xxx", "c1": 
{"a2": 456, "b2": "yyy", "c2": [{"a3": 777, "b3": "7z"},{"a3": 888, 
"b3": "8z"}]}}'::json, '$.c1.c2[0].b3'::jsonpath);
  json_path_query
-----------------
  "7z"
(1 row)

array slicing [0:], [:1], and [0:1] do not work:$.c1.c2[0:].b3

# select json_path_query('{"a1": 123, "b1": "xxx", "c1": {"a2": 456, 
"b2": "yyy", "c2": [{"a3": 777, "b3": "7z"},{"a3": 888, "b3": 
"8z"}]}}'::json, '$.c1.c2[0:].b3'::jsonpath);
2019-05-13 20:47:48.740 EDT [21856] ERROR:  bad jsonpath representation 
at character 147
2019-05-13 20:47:48.740 EDT [21856] DETAIL:  syntax error, unexpected 
':', expecting ',' or ']' at or near ":"
2019-05-13 20:47:48.740 EDT [21856] STATEMENT:  select 
json_path_query('{"a1": 123, "b1": "xxx", "c1": {"a2": 456, "b2": "yyy", 
"c2": [{"a3": 777, "b3": "7z"},{"a3": 888, "b3": "8z"}]}}'::json, 
'$.c1.c2[0:].b3'::jsonpath);
ERROR:  bad jsonpath representation
LINE 1: ...7, "b3": "7z"},{"a3": 888, "b3": "8z"}]}}'::json, '$.c1.c2[0...
                                                              ^
DETAIL:  syntax error, unexpected ':', expecting ',' or ']' at or near ":"

  27. Re: Re: SQL/JSON: functions

    Alexander Korotkov <a.korotkov@postgrespro.ru> — 2019-05-17T00:56:44Z

    Hi!

On Tue, May 14, 2019 at 3:54 AM Andrew Alsup <bluesbreaker@gmail.com> wrote:
> array slicing [0:], [:1], and [0:1] do not work:$.c1.c2[0:].b3
>
> # select json_path_query('{"a1": 123, "b1": "xxx", "c1": {"a2": 456,
> "b2": "yyy", "c2": [{"a3": 777, "b3": "7z"},{"a3": 888, "b3":
> "8z"}]}}'::json, '$.c1.c2[0:].b3'::jsonpath);
> 2019-05-13 20:47:48.740 EDT [21856] ERROR:  bad jsonpath representation
> at character 147
> 2019-05-13 20:47:48.740 EDT [21856] DETAIL:  syntax error, unexpected
> ':', expecting ',' or ']' at or near ":"
> 2019-05-13 20:47:48.740 EDT [21856] STATEMENT:  select
> json_path_query('{"a1": 123, "b1": "xxx", "c1": {"a2": 456, "b2": "yyy",
> "c2": [{"a3": 777, "b3": "7z"},{"a3": 888, "b3": "8z"}]}}'::json,
> '$.c1.c2[0:].b3'::jsonpath);
> ERROR:  bad jsonpath representation
> LINE 1: ...7, "b3": "7z"},{"a3": 888, "b3": "8z"}]}}'::json, '$.c1.c2[0...
>                                                               ^
> DETAIL:  syntax error, unexpected ':', expecting ',' or ']' at or near ":"

There is no color syntax from array slicing in jsonpath.  Instead of
"[0:]" one should write "[0 to last]".  See documentation
https://www.postgresql.org/docs/devel/datatype-json.html#TYPE-JSONPATH-ACCESSORS

------
Alexander Korotkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company

  28. Re: Re: SQL/JSON: functions

    Thomas Munro <thomas.munro@gmail.com> — 2019-07-15T02:35:39Z

    On Tue, May 14, 2019 at 12:54 PM Andrew Alsup <bluesbreaker@gmail.com> wrote:
> On 3/5/19 5:35 PM, Nikita Glukhov wrote:
> > Attached 36th version of the patches rebased onto jsonpath v36.
> While testing this patch a found a few issues:
>
> [1] I was not able to apply the patch to the current HEAD. However, it
> applies cleanly to commit: e988878f85 (NOTE: I did not investigate which
> commit between e988878f85 and HEAD caused problems).

Thanks for that note, which should help other reviewers/testers
looking a this patch set in CF1.  I hope we can eventually get a
rebased patch set, though.

-- 
Thomas Munro
https://enterprisedb.com

  29. Re: SQL/JSON: functions

    Nikita Glukhov <n.gluhov@postgrespro.ru> — 2019-07-16T13:59:58Z

    On 15.07.2019 5:35, Thomas Munro wrote:

> On Tue, May 14, 2019 at 12:54 PM Andrew Alsup <bluesbreaker@gmail.com> wrote:
>> On 3/5/19 5:35 PM, Nikita Glukhov wrote:
>>> Attached 36th version of the patches rebased onto jsonpath v36.
>> While testing this patch a found a few issues:
>>
>> [1] I was not able to apply the patch to the current HEAD. However, it
>> applies cleanly to commit: e988878f85 (NOTE: I did not investigate which
>> commit between e988878f85 and HEAD caused problems).
> Thanks for that note, which should help other reviewers/testers
> looking a this patch set in CF1.  I hope we can eventually get a
> rebased patch set, though.
>
Attached 37th version of the patches rebased onto current master.

Preliminary patch #1 contains:
  - implementation of jsonpath .datetime() method (see [1])
  - jsonpath support for json type (should be posted later in a separate 
thread)


[1] 
https://www.postgresql.org/message-id/CAPpHfduLcTtOx5dqs6ehDVy2cbLDci9JTkKdwFf1DzSy5dMjBA%40mail.gmail.com

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

  30. Re: SQL/JSON: functions

    Alvaro Herrera <alvherre@2ndquadrant.com> — 2019-09-03T22:27:11Z

    On 2019-Jul-16, Nikita Glukhov wrote:

> Attached 37th version of the patches rebased onto current master.
> 
> Preliminary patch #1 contains:
>  - implementation of jsonpath .datetime() method (see [1])
>  - jsonpath support for json type (should be posted later in a separate
> thread)

0001 doesn't apply anymore.  Can you please rebase?

(Only now I realize that this SQL/JSON stuff has its own commitfest
entry, separate from the JSON_TABLE one.)

Thanks,

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

  31. Re: SQL/JSON: functions

    Nikita Glukhov <n.gluhov@postgrespro.ru> — 2019-09-16T21:28:30Z

    On 04.09.2019 1:27, Alvaro Herrera wrote:

> On 2019-Jul-16, Nikita Glukhov wrote:
>
>> Attached 37th version of the patches rebased onto current master.
>>
>> Preliminary patch #1 contains:
>>   - implementation of jsonpath .datetime() method (see [1])
>>   - jsonpath support for json type (should be posted later in a separate
>> thread)
> 0001 doesn't apply anymore.  Can you please rebase?
>
> (Only now I realize that this SQL/JSON stuff has its own commitfest
> entry, separate from the JSON_TABLE one.)
>
> Thanks,

Attached 38th version of the patches rebased onto current master.

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

  32. Re: SQL/JSON: functions

    Nikita Glukhov <n.gluhov@postgrespro.ru> — 2019-09-17T00:07:55Z

    On 17.09.2019 0:28, Nikita Glukhov wrote:

> On 04.09.2019 1:27, Alvaro Herrera wrote:
>
>> On 2019-Jul-16, Nikita Glukhov wrote:
>>
>>> Attached 37th version of the patches rebased onto current master.
>>>
>>> Preliminary patch #1 contains:
>>>   - implementation of jsonpath .datetime() method (see [1])
>>>   - jsonpath support for json type (should be posted later in a 
>>> separate
>>> thread)
>> 0001 doesn't apply anymore.  Can you please rebase?
>>
>> (Only now I realize that this SQL/JSON stuff has its own commitfest
>> entry, separate from the JSON_TABLE one.)
>>
>> Thanks,
>
> Attached 38th version of the patches rebased onto current master.
>
Sorry, attached new edition of 38th version with minor corrections
described in "SQL/JSON: JSON_TABLE" thread.


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

  33. Re: SQL/JSON: functions

    Alvaro Herrera <alvherre@2ndquadrant.com> — 2019-09-25T20:55:15Z

    This has recently been broken -- please rebase.  (JSON_TABLE likewise).

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

  34. Re: SQL/JSON: functions

    Nikita Glukhov <n.gluhov@postgrespro.ru> — 2019-09-28T01:42:16Z

    On 25.09.2019 23:55, Alvaro Herrera wrote:

> This has recently been broken -- please rebase.  (JSON_TABLE likewise).

Attached 39th version of the patches rebased onto current master.

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

  35. Re: Re: SQL/JSON: functions

    Andy Alsup <bluesbreaker@gmail.com> — 2019-10-21T16:00:36Z

    On 9/27/19 9:42 PM, Nikita Glukhov wrote:
> Attached 39th version of the patches rebased onto current master.
>
> -- 
> Nikita Glukhov
> Postgres Professional: http://www.postgrespro.com
> The Russian Postgres Company
I am unable to cleanly apply this patch.
I've tried applying to the current master (4f4061b2dd)
I've also tried apply to commit b81a9c2fc5 back through 709d003fbd, with 
no success.

Can you please tell me to which commit I can apply the patch? Is the 
following command correct for applying the patch files?

`patch -p1 < ~/Downloads/0001-Jsonpath-support-for-json-v39.patch`

Thanks for your help,
Andrew Alsup

  36. Re: SQL/JSON: functions

    Nikita Glukhov <n.gluhov@postgrespro.ru> — 2019-10-21T16:44:29Z

    On 21.10.2019 19:00, Andrew Alsup wrote:
> On 9/27/19 9:42 PM, Nikita Glukhov wrote:
>> Attached 39th version of the patches rebased onto current master.
> I am unable to cleanly apply this patch.
> I've tried applying to the current master (4f4061b2dd)
> I've also tried apply to commit b81a9c2fc5 back through 709d003fbd, 
> with no success.
>
> Can you please tell me to which commit I can apply the patch? Is the 
> following command correct for applying the patch files?
>
> `patch -p1 < ~/Downloads/0001-Jsonpath-support-for-json-v39.patch`
>
> Thanks for your help,
> Andrew Alsup
>
v39 patch is based on 5ee96b3e2221d154ffcb719bd2dee1179c53f821

Use the following git command to apply patches:

git am ~/Downloads/0001-Jsonpath-support-for-json-v39.patch

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

  37. Re: Re: SQL/JSON: functions

    Andy Alsup <bluesbreaker@gmail.com> — 2019-10-22T00:50:02Z

    On 10/21/19 12:44 PM, Nikita Glukhov wrote:
>
> v39 patch is based on 5ee96b3e2221d154ffcb719bd2dee1179c53f821
>
> Use the following git command to apply patches:
>
> git am ~/Downloads/0001-Jsonpath-support-for-json-v39.patch
>

Thank you. The patch applied fine, with no errors.

Is this the type of testing that would be helpful? I plan to construct a 
number of separate queries to test more nuanced edge cases. This test 
simply compares the output of 4 separate sub-queries that should provide 
the same results.

SELECT
   CASE WHEN jop = jp_expr AND jop = jval AND jop = jval_path
     THEN 'pass'
     ELSE 'fail'
   END
FROM
   (
     -- jsonb operator
     SELECT count(*)
     FROM testjsonb
     WHERE j->>'abstract' LIKE 'A%'
   ) as jop,
   (
     -- jsonpath expression
     SELECT count(*)
     FROM testjsonb
     WHERE j @? '$.abstract ? (@ starts with "A")'
   ) as jp_expr,
   (
     -- json_value()
     SELECT count(*)
     FROM testjsonb
     WHERE JSON_VAlUE(j, 'lax $.abstract') LIKE 'A%'
   ) as jval,
   (
     -- json_value(jsonpath)
     SELECT count(*)
     FROM testjsonb
     WHERE JSON_VALUE(j, 'lax $.abstract ? (@ starts with "A")') IS NOT NULL
   ) as jval_path;


If I'm completely off base for how testing is normally conducted, please 
let me know.

Thanks,
Andrew Alsup

  38. Re: SQL/JSON: functions

    Nikita Glukhov <n.gluhov@postgrespro.ru> — 2019-11-14T16:42:28Z

    Attached 40th version of the patches.


I have added some documentation which has been simply copied from [1].

Also, I have split patches more correctly, and extracted patch #2 with common
SQL/JSON clauses, that are used by both constructors and query functions.
Patches #3 and #4 are necessary only for constructors (patch #5).
Patches #5 and #6 are almost independent on patch #1, which is needed only for
query functions.

So, patches #5, #6, #7 are independent now, but the code changes in them are
still conflicting.  I can rebase #6 and #7 on top of #2, if it is necessary
for separate review/commit.


[1] https://www.postgresql.org/message-id/732208d3-56c3-25a4-8f08-3be1d54ad51b%40postgrespro.ru

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

  39. Re: SQL/JSON: functions

    Pavel Stehule <pavel.stehule@gmail.com> — 2020-01-17T06:54:29Z

    Hi

čt 14. 11. 2019 v 17:46 odesílatel Nikita Glukhov <n.gluhov@postgrespro.ru>
napsal:

> Attached 40th version of the patches.
>
>
> I have added some documentation which has been simply copied from [1].
>
> Also, I have split patches more correctly, and extracted patch #2 with common
> SQL/JSON clauses, that are used by both constructors and query functions.
> Patches #3 and #4 are necessary only for constructors (patch #5).
> Patches #5 and #6 are almost independent on patch #1, which is needed only for
> query functions.
>
> So, patches #5, #6, #7 are independent now, but the code changes in them are
> still conflicting.  I can rebase #6 and #7 on top of #2, if it is necessary
> for separate review/commit.
>
>
> [1] https://www.postgresql.org/message-id/732208d3-56c3-25a4-8f08-3be1d54ad51b%40postgrespro.ru
>
> I tested cumulative patch - sent in json_table patch.

I almost satisfied by quality of this patch. There is very good conformance
with standard and with Oracle. Unfortunately MySQL in this part of JSON
support is not compatible.

I found one issue, when I tested some examples from Oracle.

SELECT JSON_VALUE('{a:100}', '$.a' RETURNING int) AS value;

then the result was null.

But it is wrong, because it should to raise a exception, because this json
is broken on Postgres (Postgres requires quoted attribute names)

json_query has same problem

postgres=# SELECT JSON_QUERY('{a:100, b:200, c:300}', '$') AS value;
┌───────┐
│ value │
╞═══════╡
│ ∅     │
└───────┘
(1 row)

It should to check if input is correct json

All others was good.

I'll mark this patch as waiting for author

Regards

Pavel



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

  40. Re: SQL/JSON: functions

    Nikita Glukhov <n.gluhov@postgrespro.ru> — 2020-01-18T17:41:02Z

    Attached 41th version of the patches.

Changes since previous version:
  * Enabled DEFAULT clause for ON ERROR/ON EMPTY behaviors in JSON_QUERY()
  * Added RETURNING clause to JSON_EXISTS() ("side effect" of implementation
    EXISTS PATH columns in JSON_TABLE)
  * ARRAY in EMPTY ARRAY ON ERROR clause is optional now for better Oracle
    compatibility


On 17.01.2020 9:54, Pavel Stehule wrote:

> On 14. 11. 2019 v 17:46 Nikita Glukhov <n.gluhov@postgrespro.ru 
> <mailto:n.gluhov@postgrespro.ru>> wrote:
>
>     Attached 40th version of the patches.
>
>     I have added some documentation which has been simply copied from [1].
>
>     Also, I have split patches more correctly, and extracted patch #2 with common
>     SQL/JSON clauses, that are used by both constructors and query functions.
>     Patches #3 and #4 are necessary only for constructors (patch #5).
>     Patches #5 and #6 are almost independent on patch #1, which is needed only for
>     query functions.
>
>     So, patches #5, #6, #7 are independent now, but the code changes in them are
>     still conflicting.  I can rebase #6 and #7 on top of #2, if it is necessary
>     for separate review/commit.
>
>
>     [1]https://www.postgresql.org/message-id/732208d3-56c3-25a4-8f08-3be1d54ad51b%40postgrespro.ru
>
> I tested cumulative patch - sent in json_table patch.
>
> I almost satisfied by quality of this patch. There is very good 
> conformance with standard and with Oracle. Unfortunately MySQL in this 
> part of JSON support is not compatible.
>
> I found one issue, when I tested some examples from Oracle.
>
> SELECT JSON_VALUE('{a:100}', '$.a' RETURNING int) AS value;
>
> then the result was null.
>
> But it is wrong, because it should to raise a exception, because this 
> json is broken on Postgres (Postgres requires quoted attribute names)
>
> json_query has same problem
>
> postgres=# SELECT JSON_QUERY('{a:100, b:200, c:300}', '$') AS value;
> ┌───────┐
> │ value │
> ╞═══════╡
> │ ∅     │
> └───────┘
> (1 row)
>
> It should to check if input is correct json

By the standard, it is implementation-defined whether JSON parsing errors
should be caught by ON ERROR clause.

SQL/JSON query functions use "JSON API common syntax" which is a combination
of JSON context item and JSON path.  It passes context item to JSON path
engine with ALREADY PARSED flag set to False.  ALREADY PARSED flag can enable
special parsing rules.

Corresponding quotes from the standard:

10.14 <JSON API common syntax>
   <JSON API common syntax> (
      Parameter: "JSON API COMMON SYNTAX"
   ) Returns: "STATUS" and "SQL/JSON SEQUENCE"

General Rules:
...
3) General Rules of Subclause 9.39, "SQL/JSON path language: syntax and
    semantics", are applied with P as PATH SPECIFICATION, C as CONTEXT ITEM,
    False as ALREADY PARSED, and PC as PASSING CLAUSE; let ST be the STATUS
    and let SEQ be the SQL/JSON SEQUENCE returned from the application of
    those General Rules.

9.39 SQL/JSON path language: syntax and semantics

"SQL/JSON path language: syntax and semantics" [General Rules] (
   Parameter: "PATH SPECIFICATION",
   Parameter: "CONTEXT ITEM",
   Parameter: "ALREADY PARSED",
   Parameter: "PASSING CLAUSE"
) Returns: "STATUS" and "SQL/JSON SEQUENCE"

General Rules:
...

4) If ALREADY PARSED is False, then it is implementation-defined whether the
    following rules are applied:
   a) The General Rules of Subclause 9.36, "Parsing JSON text", are applied with
      JT as JSON TEXT, an implementation-defined <JSON key uniqueness constraint>
      as UNIQUENESS CONSTRAINT, and FO as FORMAT OPTION; let ST be the STATUS and
      let CISJI be the SQL/JSON ITEM returned from the application of those
      General Rules.
   b) If ST is not successful completion, then ST is returned as the STATUS of
      this application of these General Rules, and no further General Rules of
      this Subclause are applied.


I decided to apply this rules, so the parsing errors are caught now by ON ERROR
(NULL ON ERROR is by default).

postgres=# SELECT JSON_VALUE('error', '$' ERROR ON ERROR);
ERROR:  invalid input syntax for type json
DETAIL:  Token "error" is invalid.
CONTEXT:  JSON data, line 1: error

I'm not sure if it would be better to add an implicit cast to json type that
will be executed before so that parsing errors can no longer be caught.
But implicit casting can simplify a bit execution of SQL/JSON query functions.


I have checked error handling in JSON parsing in Oracle 18c/19c, and it behaves
like our current implementation.  But Oracle seems to do JSON parsing on demand:

Oracle19c> SELECT JSON_VALUE('{a:1 error, b:2}', '$.a' ERROR ON ERROR) FROM dual;
1

Oracle19c> SELECT JSON_VALUE('{a:1 error, b:2}', '$.b' ERROR ON ERROR) FROM dual;
ORA-40441: JSON syntax error

Oracle19c> SELECT JSON_VALUE('{a:1 error, b:2}', '$.b') FROM dual;
NULL

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

  41. Re: SQL/JSON: functions

    Pavel Stehule <pavel.stehule@gmail.com> — 2020-01-18T18:21:00Z

    so 18. 1. 2020 v 18:46 odesílatel Nikita Glukhov <n.gluhov@postgrespro.ru>
napsal:

> Attached 41th version of the patches.
>
> Changes since previous version:
>  * Enabled DEFAULT clause for ON ERROR/ON EMPTY behaviors in JSON_QUERY()
>  * Added RETURNING clause to JSON_EXISTS() ("side effect" of implementation
>    EXISTS PATH columns in JSON_TABLE)
>  * ARRAY in EMPTY ARRAY ON ERROR clause is optional now for better Oracle
>    compatibility
>
>
> On 17.01.2020 9:54, Pavel Stehule wrote:
>
> On 14. 11. 2019 v 17:46 Nikita Glukhov <n.gluhov@postgrespro.ru> wrote:
>
>> Attached 40th version of the patches.
>>
>> I have added some documentation which has been simply copied from [1].
>>
>> Also, I have split patches more correctly, and extracted patch #2 with common
>> SQL/JSON clauses, that are used by both constructors and query functions.
>> Patches #3 and #4 are necessary only for constructors (patch #5).
>> Patches #5 and #6 are almost independent on patch #1, which is needed only for
>> query functions.
>>
>> So, patches #5, #6, #7 are independent now, but the code changes in them are
>> still conflicting.  I can rebase #6 and #7 on top of #2, if it is necessary
>> for separate review/commit.
>>
>>
>> [1] https://www.postgresql.org/message-id/732208d3-56c3-25a4-8f08-3be1d54ad51b%40postgrespro.ru
>>
>> I tested cumulative patch - sent in json_table patch.
>
> I almost satisfied by quality of this patch. There is very good
> conformance with standard and with Oracle. Unfortunately MySQL in this part
> of JSON support is not compatible.
>
> I found one issue, when I tested some examples from Oracle.
>
> SELECT JSON_VALUE('{a:100}', '$.a' RETURNING int) AS value;
>
> then the result was null.
>
> But it is wrong, because it should to raise a exception, because this json
> is broken on Postgres (Postgres requires quoted attribute names)
>
> json_query has same problem
>
> postgres=# SELECT JSON_QUERY('{a:100, b:200, c:300}', '$') AS value;
> ┌───────┐
> │ value │
> ╞═══════╡
> │ ∅     │
> └───────┘
> (1 row)
>
> It should to check if input is correct json
>
> By the standard, it is implementation-defined whether JSON parsing errors
> should be caught by ON ERROR clause.
>
> SQL/JSON query functions use "JSON API common syntax" which is a combination
> of JSON context item and JSON path.  It passes context item to JSON path
> engine with ALREADY PARSED flag set to False.  ALREADY PARSED flag can enable
> special parsing rules.
>
> Corresponding quotes from the standard:
>
> 10.14 <JSON API common syntax>
>   <JSON API common syntax> (
>      Parameter: "JSON API COMMON SYNTAX"
>   ) Returns: "STATUS" and "SQL/JSON SEQUENCE"
>
> General Rules:
> ...
> 3) General Rules of Subclause 9.39, "SQL/JSON path language: syntax and
>    semantics", are applied with P as PATH SPECIFICATION, C as CONTEXT ITEM,
>    False as ALREADY PARSED, and PC as PASSING CLAUSE; let ST be the STATUS
>    and let SEQ be the SQL/JSON SEQUENCE returned from the application of
>    those General Rules.
>
> 9.39 SQL/JSON path language: syntax and semantics
>
> "SQL/JSON path language: syntax and semantics" [General Rules] (
>   Parameter: "PATH SPECIFICATION",
>   Parameter: "CONTEXT ITEM",
>   Parameter: "ALREADY PARSED",
>   Parameter: "PASSING CLAUSE"
> ) Returns: "STATUS" and "SQL/JSON SEQUENCE"
>
> General Rules:
> ...
>
> 4) If ALREADY PARSED is False, then it is implementation-defined whether the
>    following rules are applied:
>   a) The General Rules of Subclause 9.36, "Parsing JSON text", are applied with
>      JT as JSON TEXT, an implementation-defined <JSON key uniqueness constraint>
>      as UNIQUENESS CONSTRAINT, and FO as FORMAT OPTION; let ST be the STATUS and
>      let CISJI be the SQL/JSON ITEM returned from the application of those
>      General Rules.
>   b) If ST is not successful completion, then ST is returned as the STATUS of
>      this application of these General Rules, and no further General Rules of
>      this Subclause are applied.
>
>
> I decided to apply this rules, so the parsing errors are caught now by ON ERROR
> (NULL ON ERROR is by default).
>
> postgres=# SELECT JSON_VALUE('error', '$' ERROR ON ERROR);
> ERROR:  invalid input syntax for type json
> DETAIL:  Token "error" is invalid.
> CONTEXT:  JSON data, line 1: error
>
> I'm not sure if it would be better to add an implicit cast to json type that
> will be executed before so that parsing errors can no longer be caught.
> But implicit casting can simplify a bit execution of SQL/JSON query functions.
>
>
> I have checked error handling in JSON parsing in Oracle 18c/19c, and it behaves
> like our current implementation.  But Oracle seems to do JSON parsing on demand:
>
> Oracle19c> SELECT JSON_VALUE('{a:1 error, b:2}', '$.a' ERROR ON ERROR) FROM dual;
> 1
>
> Oracle19c> SELECT JSON_VALUE('{a:1 error, b:2}', '$.b' ERROR ON ERROR) FROM dual;
> ORA-40441: JSON syntax error
>
> Oracle19c> SELECT JSON_VALUE('{a:1 error, b:2}', '$.b') FROM dual;
> NULL
>
>
Everywhere I don't like default masking error. I think so can be very
confusing to get NULL (by default) instead a error of broken format.

I vote for check of input is correct JSON, and if it, then start
processing. Else to raise a error.

More - our JSON Parser is different than Oracle's JSON parser. And if
somebody will run Oracle's JSONs, then he get some result on Oracle. But on
Postgres, same JSON can be invalid, and he get NULL.

The raising some errors looks like only one safe variant.

Regards

Pavel


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

  42. Re: SQL/JSON: functions

    Nikita Glukhov <n.gluhov@postgrespro.ru> — 2020-03-02T22:33:57Z

    Attached 42th version of the patches.

On 18.01.2020 21:21, Pavel Stehule wrote:
> On 18. 1. 2020 v 18:46 Nikita Glukhov <n.gluhov@postgrespro.ru 
> <mailto:n.gluhov@postgrespro.ru>> wrote:
>
>     Attached 41th version of the patches.
>
>     Changes since previous version:
>       * Enabled DEFAULT clause for ON ERROR/ON EMPTY behaviors in JSON_QUERY()
>       * Added RETURNING clause to JSON_EXISTS() ("side effect" of implementation
>         EXISTS PATH columns in JSON_TABLE)
>       * ARRAY in EMPTY ARRAY ON ERROR clause is optional now for better Oracle
>         compatibility
>
>
>     On 17.01.2020 9:54, Pavel Stehule wrote:
>
>>     I tested cumulative patch - sent in json_table patch.
>>
>>     I almost satisfied by quality of this patch. There is very good
>>     conformance with standard and with Oracle. Unfortunately MySQL in
>>     this part of JSON support is not compatible.
>>
>>     I found one issue, when I tested some examples from Oracle.
>>
>>     SELECT JSON_VALUE('{a:100}', '$.a' RETURNING int) AS value;
>>
>>     then the result was null.
>>
>>     But it is wrong, because it should to raise a exception, because
>>     this json is broken on Postgres (Postgres requires quoted
>>     attribute names)
>>
>>     json_query has same problem
>>
>>     postgres=# SELECT JSON_QUERY('{a:100, b:200, c:300}', '$') AS value;
>>     ┌───────┐
>>     │ value │
>>     ╞═══════╡
>>     │ ∅     │
>>     └───────┘
>>     (1 row)
>>
>>     It should to check if input is correct json
>
>     By the standard, it is implementation-defined whether JSON parsing errors
>     should be caught by ON ERROR clause.
>
>     SQL/JSON query functions use "JSON API common syntax" which is a combination
>     of JSON context item and JSON path.  It passes context item to JSON path
>     engine with ALREADY PARSED flag set to False.  ALREADY PARSED flag can enable
>     special parsing rules.
>
>     Corresponding quotes from the standard:
>
>     10.14 <JSON API common syntax>
>        <JSON API common syntax> (
>           Parameter: "JSON API COMMON SYNTAX"
>        ) Returns: "STATUS" and "SQL/JSON SEQUENCE"
>
>     General Rules:
>     ...
>     3) General Rules of Subclause 9.39, "SQL/JSON path language: syntax and
>         semantics", are applied with P as PATH SPECIFICATION, C as CONTEXT ITEM,
>         False as ALREADY PARSED, and PC as PASSING CLAUSE; let ST be the STATUS
>         and let SEQ be the SQL/JSON SEQUENCE returned from the application of
>         those General Rules.
>
>     9.39 SQL/JSON path language: syntax and semantics
>
>     "SQL/JSON path language: syntax and semantics" [General Rules] (
>        Parameter: "PATH SPECIFICATION",
>        Parameter: "CONTEXT ITEM",
>        Parameter: "ALREADY PARSED",
>        Parameter: "PASSING CLAUSE"
>     ) Returns: "STATUS" and "SQL/JSON SEQUENCE"
>
>     General Rules:
>     ...
>
>     4) If ALREADY PARSED is False, then it is implementation-defined whether the
>         following rules are applied:
>        a) The General Rules of Subclause 9.36, "Parsing JSON text", are applied with
>           JT as JSON TEXT, an implementation-defined <JSON key uniqueness constraint>
>           as UNIQUENESS CONSTRAINT, and FO as FORMAT OPTION; let ST be the STATUS and
>           let CISJI be the SQL/JSON ITEM returned from the application of those
>           General Rules.
>        b) If ST is not successful completion, then ST is returned as the STATUS of
>           this application of these General Rules, and no further General Rules of
>           this Subclause are applied.
>
>
>     I decided to apply this rules, so the parsing errors are caught now by ON ERROR
>     (NULL ON ERROR is by default).
>
>     postgres=# SELECT JSON_VALUE('error', '$' ERROR ON ERROR);
>     ERROR:  invalid input syntax for type json
>     DETAIL:  Token "error" is invalid.
>     CONTEXT:  JSON data, line 1: error
>
>     I'm not sure if it would be better to add an implicit cast to json type that
>     will be executed before so that parsing errors can no longer be caught.
>     But implicit casting can simplify a bit execution of SQL/JSON query functions.
>
>
>     I have checked error handling in JSON parsing in Oracle 18c/19c, and it behaves
>     like our current implementation.  But Oracle seems to do JSON parsing on demand:
>
>     Oracle19c> SELECT JSON_VALUE('{a:1 error, b:2}', '$.a' ERROR ON ERROR) FROM dual;
>     1
>
>     Oracle19c> SELECT JSON_VALUE('{a:1 error, b:2}', '$.b' ERROR ON ERROR) FROM dual;
>     ORA-40441: JSON syntax error
>
>     Oracle19c> SELECT JSON_VALUE('{a:1 error, b:2}', '$.b') FROM dual;
>     NULL
>
>
> Everywhere I don't like default masking error. I think so can be very 
> confusing to get NULL
> (by default) instead a error of broken format.
>
> I vote for check of input is correct JSON, and if it, then start 
> processing. Else to raise a error.
>
> More - our JSON Parser is different than Oracle's JSON parser. And if 
> somebody will run Oracle's JSONs,
> then he get some result on Oracle. But on Postgres, same JSON can be 
> invalid, and he get NULL.
>
> The raising some errors looks like only one safe variant.

I have removed handling of parsing errors in SQL/JSON functions and JSON_TABLE.
Now, FORMAT JSON expressions (implicit or explicit) are simply transformed into
ordinary casts to json type, and these casts are executed before execution of
SQL/JSON functions.  Previously, separate expression was created for such casts,
and it was executed in the separate subtransaction in ExecEvalJsonExpr().  So,
this change also simplifies the code a bit.

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

  43. Re: SQL/JSON: functions

    Erik Rijkers <er@xs4all.nl> — 2020-03-02T23:12:46Z

    On 2020-03-02 23:33, Nikita Glukhov wrote:
> Attached 42th version of the patches.

> v1-0001-Add-jsonpath-pg-modifier-for-enabling-extensions.patch
> v1-0002-Add-raw-jbvArray-and-jbvObject-support-to-jsonpat.patch
> v1-0003-Add-jsonpath-sequence-constructors.patch
> v1-0004-Add-jsonpath-array-constructors.patch
> v1-0005-Add-jsonpath-object-constructors.patch
> v1-0006-Add-jsonpath-object-subscripting.patch

I can't get these to apply against master.


$ patch --dry-run -b -l -F 15 -p 1 < 
0001-Jsonpath-support-for-json-v42.patch
can't find file to patch at input line 38

(tried  -p o and -p 1 -- am I doing it wrong?)


Maybe it needs something else applied first?


Erik Rijkers

  44. Re: SQL/JSON: functions

    Nikita Glukhov <n.gluhov@postgrespro.ru> — 2020-03-02T23:24:17Z

    On 03.03.2020 2:12, Erik Rijkers wrote:

> On 2020-03-02 23:33, Nikita Glukhov wrote:
>> Attached 42th version of the patches.
>
>> v1-0001-Add-jsonpath-pg-modifier-for-enabling-extensions.patch
>> v1-0002-Add-raw-jbvArray-and-jbvObject-support-to-jsonpat.patch
>> v1-0003-Add-jsonpath-sequence-constructors.patch
>> v1-0004-Add-jsonpath-array-constructors.patch
>> v1-0005-Add-jsonpath-object-constructors.patch
>> v1-0006-Add-jsonpath-object-subscripting.patch
>
> I can't get these to apply against master.
>
>
> $ patch --dry-run -b -l -F 15 -p 1 < 
> 0001-Jsonpath-support-for-json-v42.patch
> can't find file to patch at input line 38
>
> (tried  -p o and -p 1 -- am I doing it wrong?)
>
>
> Maybe it needs something else applied first?
>
>
> Erik Rijkers

Sorry, the wrong patch set was attached.

Patches can be applied the following command (which also creates a commit):
$ git am 0001-Jsonpath-support-for-json-v42.patch

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

  45. Re: SQL/JSON: functions

    Erik Rijkers <er@xs4all.nl> — 2020-03-03T01:48:24Z

    On 2020-03-03 00:24, Nikita Glukhov wrote:
> On 03.03.2020 2:12, Erik Rijkers wrote:
> 
>> On 2020-03-02 23:33, Nikita Glukhov wrote:
>>> Attached 42th version of the patches.


20200302/0001-Jsonpath-support-for-json-v42.patch  +
20200302/0002-Add-common-SQL_JSON-clauses-v42.patch+
20200302/0003-Add-invisible-coercion-form-v42.patch+
20200302/0004-Add-function-formats-v42.patch       +
20200302/0005-SQLJSON-constructors-v42.patch       +
20200302/0006-IS-JSON-predicate-v42.patch          +
20200302/0007-SQLJSON-query-functions-v42.patch    +

Thanks -- those applied fine.


Compiling, I get this error from the pg_stat_statements module (in 
contrib):

-- [2020.03.03 02:22:20 json_functions/0] make contrib
pg_stat_statements.c: In function ‘JumbleExpr’:
pg_stat_statements.c:2933:29: error: ‘JsonExpr’ {aka ‘struct JsonExpr’} 
has no member named ‘raw_expr’
  2933 |     JumbleExpr(jstate, jexpr->raw_expr);
       |                             ^~
make[1]: *** [pg_stat_statements.o] Error 1
make: *** [all-pg_stat_statements-recurse] Error 2
-- contrib make returned 2 - abort
../../src/Makefile.global:919: recipe for target 'pg_stat_statements.o' 
failed
Makefile:93: recipe for target 'all-pg_stat_statements-recurse' failed
pg_stat_statements.c: In function ‘JumbleExpr’:
pg_stat_statements.c:2933:29: error: ‘JsonExpr’ {aka ‘struct JsonExpr’} 
has no member named ‘raw_expr’
  2933 |     JumbleExpr(jstate, jexpr->raw_expr);
       |                             ^~
make[1]: *** [pg_stat_statements.o] Error 1
make: *** [install-pg_stat_statements-recurse] Error 2
-- contrib make install returned 2 - abort


(so I've edited out pg_stat_statements from the contrib/Makefile and 
compiled a working server for further testing.)


Thanks,

Erik Rijkers

  46. Re: SQL/JSON: functions

    Pavel Stehule <pavel.stehule@gmail.com> — 2020-03-06T08:16:59Z

    út 3. 3. 2020 v 0:24 odesílatel Nikita Glukhov <n.gluhov@postgrespro.ru>
napsal:

> On 03.03.2020 2:12, Erik Rijkers wrote:
>
> > On 2020-03-02 23:33, Nikita Glukhov wrote:
> >> Attached 42th version of the patches.
> >
> >> v1-0001-Add-jsonpath-pg-modifier-for-enabling-extensions.patch
> >> v1-0002-Add-raw-jbvArray-and-jbvObject-support-to-jsonpat.patch
> >> v1-0003-Add-jsonpath-sequence-constructors.patch
> >> v1-0004-Add-jsonpath-array-constructors.patch
> >> v1-0005-Add-jsonpath-object-constructors.patch
> >> v1-0006-Add-jsonpath-object-subscripting.patch
> >
> > I can't get these to apply against master.
> >
> >
> > $ patch --dry-run -b -l -F 15 -p 1 <
> > 0001-Jsonpath-support-for-json-v42.patch
> > can't find file to patch at input line 38
> >
> > (tried  -p o and -p 1 -- am I doing it wrong?)
> >
> >
> > Maybe it needs something else applied first?
> >
> >
> > Erik Rijkers
>
> Sorry, the wrong patch set was attached.
>
> Patches can be applied the following command (which also creates a commit):
> $ git am 0001-Jsonpath-support-for-json-v42.patch
>

make check fails

but probably it is only forgotten actualization

Regards

Pavel


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

  47. Re: SQL/JSON: functions

    Nikita Glukhov <n.gluhov@postgrespro.ru> — 2020-03-11T23:09:44Z

    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.

On 06.03.2020 11:16, Pavel Stehule wrote:

> make check fails
> but probably it is only forgotten actualization

Fixed.


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

  48. Re: SQL/JSON: functions

    Pavel Stehule <pavel.stehule@gmail.com> — 2020-03-12T13:41:17Z

    čt 12. 3. 2020 v 0:09 odesílatel Nikita Glukhov <n.gluhov@postgrespro.ru>
napsal:

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

Regards

Pavel


> On 06.03.2020 11:16, Pavel Stehule wrote:
>
> make check fails
>
> but probably it is only forgotten actualization
>
> Fixed.
>
>
> --
> Nikita Glukhov
> Postgres Professional: http://www.postgrespro.com
> The Russian Postgres Company
>

  49. Re: SQL/JSON: functions

    Nikita Glukhov <n.gluhov@postgrespro.ru> — 2020-03-17T00:55:06Z

    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.


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.


The support for json type in jsonpath also seems to be immature, so I will try
to remove it in the next iteration.

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

  50. Re: SQL/JSON: functions

    Pavel Stehule <pavel.stehule@gmail.com> — 2020-03-17T18:35:00Z

    út 17. 3. 2020 v 1:55 odesílatel Nikita Glukhov <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.


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

??

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.



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); }
+<-><--><-->|
+*/



> 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" ?

Regards

Pavel





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

  51. Re: SQL/JSON: functions

    Nikita Glukhov <n.gluhov@postgrespro.ru> — 2020-03-19T22:57:21Z

    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

  52. Re: SQL/JSON: functions

    Pavel Stehule <pavel.stehule@gmail.com> — 2020-03-20T19:34:12Z

    čt 19. 3. 2020 v 23:57 odesílatel Nikita Glukhov <n.gluhov@postgrespro.ru>
napsal:

> 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>
> 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)
>
> ok. So please change comment from ToDo to this explanation. Maybe note in
doc (unimplemented features can be good idea)

Some these unresolvable conflicts are solved with extra parenthesis in
Postgres.


>
> 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.
>
>
I have much better feeling from version 45 (now it looks like Postgres C
:)). Still there are some small issues.

1. commented code

+json_encoding:
+<-><--><-->name<--><--><--><--><--><--><--><--><-->{ $$ =
makeJsonEncoding($1); }
+<->/*
+<-><--><-->| UTF8<><--><--><--><--><--><--><--><-->{ $$ = JS_ENC_UTF8; }
+<-><--><-->| UTF16><--><--><--><--><--><--><--><-->{ $$ = JS_ENC_UTF16; }
+<-><--><-->| UTF32 <--><--><--><--><--><--><--><-->{ $$ = JS_ENC_UTF32; }
+<->*/
+<-><-->;

2. sometimes useless empty rows

                              silent boolean DEFAULT false)
+RETURNS text
+LANGUAGE INTERNAL
+STRICT IMMUTABLE PARALLEL SAFE
+AS 'jsonb_path_query_first_text';
+
+
+


+<-><-->if (!coerced)
+<-><-->{
+
+<-><--><-->/* If coercion failed, use to_json()/to_jsonb() functions. */


All tests passed
build without any problem

looking for next update

Regards

Pavel


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

  53. Re: SQL/JSON: functions

    Nikita Glukhov <n.gluhov@postgrespro.ru> — 2020-03-21T10:07:35Z

    Attached 46th version of the patches.


On 20.03.2020 22:34, Pavel Stehule wrote:
>
> čt 19. 3. 2020 v 23:57 odesílatel Nikita Glukhov 
> <n.gluhov@postgrespro.ru <mailto:n.gluhov@postgrespro.ru>> napsal:
>
>     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.
>>
I have removed json[b]_build_object_ext() and json[b]_build_array_ext().

But json[b]_objectagg() and json[b]_agg_strict() are still present.
It seems that removing them requires majors refactoring of the execution
of Aggref and WindowFunc nodes.

>>     +<->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)
>
> ok. So please change comment from ToDo to this explanation. Maybe note 
> in doc (unimplemented features can be good idea)
>
> Some these unresolvable conflicts are solved with extra parenthesis in 
> Postgres.
>
>>
>>         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.
>
I have completely removed patch #1. It turned out to be not so difficult.


>
> I have much better feeling from version 45 (now it looks like Postgres 
> C :)). Still there are some small issues.
>
> 1. commented code
>
> +json_encoding:
> +<-><--><-->name<--><--><--><--><--><--><--><--><-->{ $$ = 
> makeJsonEncoding($1); }
> +<->/*
> +<-><--><-->| UTF8<><--><--><--><--><--><--><--><-->{ $$ = JS_ENC_UTF8; }
> +<-><--><-->| UTF16><--><--><--><--><--><--><--><-->{ $$ = JS_ENC_UTF16; }
> +<-><--><-->| UTF32 <--><--><--><--><--><--><--><-->{ $$ = JS_ENC_UTF32; }
> +<->*/
> +<-><-->;
>
Fixed.

> 2. sometimes useless empty rows
>
>                               silent boolean DEFAULT false)
> +RETURNS text
> +LANGUAGE INTERNAL
> +STRICT IMMUTABLE PARALLEL SAFE
> +AS 'jsonb_path_query_first_text';
> +
> +
> +
>
>
> +<-><-->if (!coerced)
> +<-><-->{
> +
> +<-><--><-->/* If coercion failed, use to_json()/to_jsonb() functions. */

Fixed.


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

  54. Re: SQL/JSON: functions

    Pavel Stehule <pavel.stehule@gmail.com> — 2020-03-21T19:38:31Z

    so 21. 3. 2020 v 11:07 odesílatel Nikita Glukhov <n.gluhov@postgrespro.ru>
napsal:

> Attached 46th version of the patches.
>
>
> On 20.03.2020 22:34, Pavel Stehule wrote:
>
>
> čt 19. 3. 2020 v 23:57 odesílatel Nikita Glukhov <n.gluhov@postgrespro.ru>
> napsal:
>
>> 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>
>> 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.
>>
>> I have removed json[b]_build_object_ext() and json[b]_build_array_ext().
>
> But json[b]_objectagg() and json[b]_agg_strict() are still present.
> It seems that removing them requires majors refactoring of the execution
> of Aggref and WindowFunc nodes.
>
>
> +<->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)
>>
>> ok. So please change comment from ToDo to this explanation. Maybe note in
> doc (unimplemented features can be good idea)
>
> Some these unresolvable conflicts are solved with extra parenthesis in
> Postgres.
>
>
>>
>> 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.
>>
>> I have completely removed patch #1. It turned out to be not so difficult.
>
>
>
> I have much better feeling from version 45 (now it looks like Postgres C
> :)). Still there are some small issues.
>
> 1. commented code
>
> +json_encoding:
> +<-><--><-->name<--><--><--><--><--><--><--><--><-->{ $$ =
> makeJsonEncoding($1); }
> +<->/*
> +<-><--><-->| UTF8<><--><--><--><--><--><--><--><-->{ $$ = JS_ENC_UTF8; }
> +<-><--><-->| UTF16><--><--><--><--><--><--><--><-->{ $$ = JS_ENC_UTF16; }
> +<-><--><-->| UTF32 <--><--><--><--><--><--><--><-->{ $$ = JS_ENC_UTF32; }
> +<->*/
> +<-><-->;
>
> Fixed.
>
> 2. sometimes useless empty rows
>
>                               silent boolean DEFAULT false)
> +RETURNS text
> +LANGUAGE INTERNAL
> +STRICT IMMUTABLE PARALLEL SAFE
> +AS 'jsonb_path_query_first_text';
> +
> +
> +
>
>
> +<-><-->if (!coerced)
> +<-><-->{
> +
> +<-><--><-->/* If coercion failed, use to_json()/to_jsonb() functions. */
>
> Fixed.
>
I like this version. I checked code and I don't see any issue. It looks
very well.

The build is without any problems, all tests passed.

I'll mark this patch as ready for commiters.

Thank your good work.

Regards

Pavel


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

  55. Re: SQL/JSON: functions

    Nikita Glukhov <n.gluhov@postgrespro.ru> — 2020-03-23T17:28:52Z

    Attached 47th version of the patches.

On 21.03.2020 22:38, Pavel Stehule wrote:
>
> On 21. 3. 2020 v 11:07 Nikita Glukhov <n.gluhov@postgrespro.ru 
> <mailto:n.gluhov@postgrespro.ru>> wrote:
>
>     Attached 46th version of the patches.
>
>     On 20.03.2020 22:34, Pavel Stehule wrote:
>>
>>     On 19.03.2020 23:57 Nikita Glukhov <n.gluhov@postgrespro.ru
>>     <mailto:n.gluhov@postgrespro.ru>> wrote:
>>
>>         Attached 45th version of the patches.
>>
>>         Nodes JsonFormat, JsonReturning, JsonPassing, JsonBehavior were fixed.
>>
>>         On 17.03.2020 21:35, Pavel Stehule wrote:
>>>
>>>             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.
>>>
>     I have removed json[b]_build_object_ext() and json[b]_build_array_ext().
>
>     But json[b]_objectagg() and json[b]_agg_strict() are still present.
>     It seems that removing them requires majors refactoring of the execution
>     of Aggref and WindowFunc nodes.
>
I have replaced aggregate function

json[b]_objectagg(key any, val any, absent_on_null boolean, unique_keys boolean)

with three separate functions:

json[b]_object_agg_strict(any, any)
json[b]_object_agg_unique(any, any)
json[b]_object_agg_unique_strict(any, any)


This should be more correct than single aggregate with additional parameters.


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

  56. Re: SQL/JSON: functions

    Alexander Korotkov <a.korotkov@postgrespro.ru> — 2020-04-05T16:50:17Z

    On Mon, Mar 23, 2020 at 8:28 PM Nikita Glukhov <n.gluhov@postgrespro.ru> wrote:
> Attached 47th version of the patches.
>
> On 21.03.2020 22:38, Pavel Stehule wrote:
>
>
> On 21. 3. 2020 v 11:07 Nikita Glukhov <n.gluhov@postgrespro.ru> wrote:
>>
>> Attached 46th version of the patches.
>>
>> On 20.03.2020 22:34, Pavel Stehule wrote:
>>
>>
>> On 19.03.2020 23:57 Nikita Glukhov <n.gluhov@postgrespro.ru> wrote:
>>>
>>> Attached 45th version of the patches.
>>>
>>> Nodes JsonFormat, JsonReturning, JsonPassing, JsonBehavior were fixed.
>>>
>>> On 17.03.2020 21:35, Pavel Stehule wrote:
>>>>
>>>> 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.
>>>
>> I have removed json[b]_build_object_ext() and json[b]_build_array_ext().
>>
>> But json[b]_objectagg() and json[b]_agg_strict() are still present.
>> It seems that removing them requires majors refactoring of the execution
>> of Aggref and WindowFunc nodes.
>
> I have replaced aggregate function
>
> json[b]_objectagg(key any, val any, absent_on_null boolean, unique_keys boolean)
>
> with three separate functions:
>
> json[b]_object_agg_strict(any, any)
> json[b]_object_agg_unique(any, any)
> json[b]_object_agg_unique_strict(any, any)
>
>
> This should be more correct than single aggregate with additional parameters.

I've following notes about this patchset.

1) Uniqueness checks using JsonbUniqueCheckContext and
JsonUniqueCheckContext have quadratic complexity over number of keys.
That doesn't look good especially for jsonb, which anyway sorts object
keys before object serialization.
2) We have two uniqueness checks for json type, which use
JsonbUniqueCheckContext and JsonUniqueState.  JsonUniqueState uses
stack of hashes, while JsonbUniqueCheckContext have just plain array
of keys.  I think we can make JsonUniqueState use single hash, where
object identifies would be part of hash key.  And we should replace
JsonbUniqueCheckContext with JsonUniqueState.  That would eliminate
extra entities and provide reasonable complexity for cases, which now
use JsonbUniqueCheckContext.
3) New SQL/JSON clauses don't use timezone and considered as immutable
assuming all the children are immutable.  Immutability is good, but
ignoring timezone in all the cases is plain wrong.  The first thing we
can do is to use timezone and make SQL/JSON clauses stable.  But that
limits their usage in functional and partial indexes.  I see couple of
things we can do next (one of them or probably both).
3.1) Provide user a way so specify that we should ignore timezone in
particular case (IGNORE TIMEZONE clause or something like that).  Then
SQL/JSON clause will be considered as immutable.
3.2) Automatically detect whether jsonpath might use timezone.  If
jsonpath doesn't use .datetime() method, it doesn't need timezone for
sure.  Also, from the datetime format specifiers we can get that we
don't compare non-timezoned values to timezoned values.  So, if we
detect this jsonpath never uses timezone, we can consider SQL/JSON
clause as immutable.

------
Alexander Korotkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company

  57. Re: SQL/JSON: functions

    Justin Pryzby <pryzby@telsasoft.com> — 2020-07-05T17:29:56Z

    On Mon, Mar 23, 2020 at 08:28:52PM +0300, Nikita Glukhov wrote:
> Attached 47th version of the patches.

The patch checker/cfbot says this doesn't apply ; could you send a rebasified
version ?

-- 
Justin

  58. Re: SQL/JSON: functions

    Andrew Dunstan <andrew.dunstan@2ndquadrant.com> — 2020-07-14T17:00:02Z

    On 7/5/20 1:29 PM, Justin Pryzby wrote:
> On Mon, Mar 23, 2020 at 08:28:52PM +0300, Nikita Glukhov wrote:
>> Attached 47th version of the patches.
> The patch checker/cfbot says this doesn't apply ; could you send a rebasified
> version ?
>

To keep things moving, I've rebased these patches. However, 1) the docs
patches use <replaceble class="parameter"> in many cases where they
should now just use <parameter> and b) patch 4 fails when run under
force_parallel=regress.


cheers


andrew



-- 
Andrew Dunstan                https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

  59. Re: SQL/JSON: functions

    Andrew Dunstan <andrew.dunstan@2ndquadrant.com> — 2020-07-14T21:09:30Z

    On 7/14/20 1:00 PM, Andrew Dunstan wrote:
> On 7/5/20 1:29 PM, Justin Pryzby wrote:
>> On Mon, Mar 23, 2020 at 08:28:52PM +0300, Nikita Glukhov wrote:
>>> Attached 47th version of the patches.
>> The patch checker/cfbot says this doesn't apply ; could you send a rebasified
>> version ?
>>
> To keep things moving, I've rebased these patches. However, 1) the docs
> patches use <replaceble class="parameter"> in many cases where they
> should now just use <parameter> and b) patch 4 fails when run under
> force_parallel=regress.
>
>


Turns out these patches also need to get the message on the new way of
writing entries in func.sgml - I'll publish some updates on that in the
next day or so so that "make doc" will succeed.


cheers


andrew

-- 
Andrew Dunstan                https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

  60. Re: SQL/JSON: functions

    Nikita Glukhov <n.gluhov@postgrespro.ru> — 2020-07-15T01:47:21Z

    Attached 49th version of the patches with two new patches #5 and #6.

On 15.07.2020 00:09, Andrew Dunstan wrote:
> On 7/14/20 1:00 PM, Andrew Dunstan wrote:
>> On 7/5/20 1:29 PM, Justin Pryzby wrote:
>>> On Mon, Mar 23, 2020 at 08:28:52PM +0300, Nikita Glukhov wrote:
>>>> Attached 47th version of the patches.
>>> The patch checker/cfbot says this doesn't apply ; could you send a rebasified
>>> version ?
>>>
>> To keep things moving, I've rebased these patches. However, 1) the docs
>> patches use <replaceble class="parameter"> in many cases where they
>> should now just use <parameter>

I haven't touched <replaceable class="parameter"> yet, because I'm not sure
if <replaceable> or <parameter> is correct here at all.

> Turns out these patches also need to get the message on the new way of
> writing entries in func.sgml - I'll publish some updates on that in the
> next day or so so that "make doc" will succeed.

I can do it by myself, but I just need to understand what to fix and how.

>> and b) patch 4 fails when run under force_parallel=regress.

Fixed parallel-safety check for RETURNING clause of JSON_EXISTS().


On 05.04.2020 19:50, Alexander Korotkov wrote:
> 1) Uniqueness checks using JsonbUniqueCheckContext and
> JsonUniqueCheckContext have quadratic complexity over number of keys.
> That doesn't look good especially for jsonb, which anyway sorts object
> keys before object serialization.
> 2) We have two uniqueness checks for json type, which use
> JsonbUniqueCheckContext and JsonUniqueState.  JsonUniqueState uses
> stack of hashes, while JsonbUniqueCheckContext have just plain array
> of keys.  I think we can make JsonUniqueState use single hash, where
> object identifies would be part of hash key.  And we should replace
> JsonbUniqueCheckContext with JsonUniqueState.  That would eliminate
> extra entities and provide reasonable complexity for cases, which now
> use JsonbUniqueCheckContext.

Unique checks were refactored as Alexander proposed.

> 3) New SQL/JSON clauses don't use timezone and considered as immutable
> assuming all the children are immutable.  Immutability is good, but
> ignoring timezone in all the cases is plain wrong.  The first thing we
> can do is to use timezone and make SQL/JSON clauses stable.  But that
> limits their usage in functional and partial indexes.  I see couple of
> things we can do next (one of them or probably both).
> 3.1) Provide user a way so specify that we should ignore timezone in
> particular case (IGNORE TIMEZONE clause or something like that).  Then
> SQL/JSON clause will be considered as immutable.
> 3.2) Automatically detect whether jsonpath might use timezone.  If
> jsonpath doesn't use .datetime() method, it doesn't need timezone for
> sure.  Also, from the datetime format specifiers we can get that we
> don't compare non-timezoned values to timezoned values.  So, if we
> detect this jsonpath never uses timezone, we can consider SQL/JSON
> clause as immutable.

Implemented second variant with automatic detection.

I also tried to add explicit IGNORE TIMEZONE / IMMUTABLE clauses, but all of
them lead to shift/reduce conflicts that seem not easy to resolve.



Patch #5 implements functions for new JSON type that is expected to appear in
the upcoming SQL/JSON standard:

  - JSON() is for constructing JSON typed values from JSON text.
    It is almost equivalent to text::json[b] cast, except that it has additional
    ability to specify WITH UNIQUE KEYS constraint.

  - JSON_SCALAR() is for constructing JSON typed values from SQL scalars.
    It is equivalent to to_json[b]().

  - JSON_SERIALIZE() is for serializing JSON typed values to character strings.
    It is almost equivalent to json[b]::character_type cast, but it also
    supports output to bytea.

Upcoming Oracle 20c will have JSON datatype and these functions [1], so we
decided also to implement them for compatibility, despite that they do not make
sense for real PG users.



Patch #6 allows the user to use PG jsonb type as an effective implementation of
SQL JSON type.  By explicitly setting GUC sql_json = jsonb, JSON will be mapped
to jsonb, and JSON TEXT (may be named named differently) will be mapped to json.

This seems to be a hack, but we failed to propose something more simpler.

Example of usage GUC sql_json:

=# CREATE TABLE test1 (js json, jb jsonb, jt json text);
CREATE TABLE

=# \d test1
               Table "public.test1"
  Column | Type  | Collation | Nullable | Default
--------+-------+-----------+----------+---------
  js     | json  |           |          |
  jb     | jsonb |           |          |
  jt     | json  |           |          |

=# SET sql_json = jsonb;
SET

=# \d test1
                 Table "public.test1"
  Column |   Type    | Collation | Nullable | Default
--------+-----------+-----------+----------+---------
  js     | json text |           |          |
  jb     | json      |           |          |
  jt     | json text |           |          |

=# CREATE TABLE test2 (js json, jb jsonb, jt json text);
CREATE TABLE

=# \d test2
                 Table "public.test2"
  Column |   Type    | Collation | Nullable | Default
--------+-----------+-----------+----------+---------
  js     | json      |           |          |
  jb     | json      |           |          |
  jt     | json text |           |          |

=# SET sql_json = json;
SET
=# \d test2
               Table "public.test2"
  Column | Type  | Collation | Nullable | Default
--------+-------+-----------+----------+---------
  js     | jsonb |           |          |
  jb     | jsonb |           |          |
  jt     | json  |           |          |



[1] https://docs.oracle.com/en/database/oracle/oracle-database/20/adjsn/json-in-oracle-database.html#GUID-CBEDC779-39A3-43C9-AF38-861AE3FC0AEC

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

  61. Re: SQL/JSON: functions

    Andrew Dunstan <andrew.dunstan@2ndquadrant.com> — 2020-07-15T11:50:03Z

    On 7/14/20 9:47 PM, Nikita Glukhov wrote:
> Attached 49th version of the patches with two new patches #5 and #6.
>
> On 15.07.2020 00:09, Andrew Dunstan wrote:
>> On 7/14/20 1:00 PM, Andrew Dunstan wrote:
>>> On 7/5/20 1:29 PM, Justin Pryzby wrote:
>>>> On Mon, Mar 23, 2020 at 08:28:52PM +0300, Nikita Glukhov wrote:
>>>>> Attached 47th version of the patches.
>>>> The patch checker/cfbot says this doesn't apply ; could you send a rebasified
>>>> version ?
>>>>
>>> To keep things moving, I've rebased these patches. However, 1) the docs
>>> patches use <replaceble class="parameter"> in many cases where they
>>> should now just use <parameter> 
> I haven't touched <replaceable class="parameter"> yet, because I'm not sure
> if <replaceable> or <parameter> is correct here at all.



Here's the relevant commit message that explains the policy:


commit 47046763c3
Author: Tom Lane <tgl@sss.pgh.pa.us>
Date:   Mon May 4 13:48:30 2020 -0400

    Doc: standardize markup a bit more.
   
    We had a mishmash of <replaceable>, <replaceable class="parameter">,
    and <parameter> markup for operator/function arguments.  Use <parameter>
    consistently for things that are in fact names of parameters (including
    OUT parameters), reserving <replaceable> for things that aren't.  The
    latter class includes some made-up-by-the-docs type class names, like
    "numeric_type", as well as placeholders for arguments that don't have
    well-defined types.  Possibly we could do better with those categories
    as well, but for the moment I'm content not to have parameter names
    marked up in different ways in different places.



>
>> Turns out these patches also need to get the message on the new way of
>> writing entries in func.sgml - I'll publish some updates on that in the
>> next day or so so that "make doc" will succeed.
> I can do it by myself, but I just need to understand what to fix and how.



Here's an example that shows how it was transformed for json_agg:


-     <row>
-      <entry>
-       <indexterm>
-        <primary>json_agg</primary>
-       </indexterm>
-       <function>json_agg(<replaceable
class="parameter">expression</replaceable>)</function>
-      </entry>
-      <entry>
-       <type>any</type>
-      </entry>
-      <entry>
-       <type>json</type>
-      </entry>
-      <entry>No</entry>
-      <entry>aggregates values, including nulls, as a JSON array</entry>
-     </row>
+      <row>
+       <entry role="func_table_entry"><para role="func_signature">
+        <indexterm>
+         <primary>json_agg</primary>
+        </indexterm>
+        <function>json_agg</function> ( <type>anyelement</type> )
+        <returnvalue>json</returnvalue>
+       </para>
+       <para role="func_signature">
+        <indexterm>
+         <primary>jsonb_agg</primary>
+        </indexterm>
+        <function>jsonb_agg</function> ( <type>anyelement</type> )
+        <returnvalue>jsonb</returnvalue>
+       </para>
+       <para>
+        Collects all the input values, including nulls, into a JSON array.
+        Values are converted to JSON as per <function>to_json</function>
+        or <function>to_jsonb</function>.
+       </para></entry>
+       <entry>No</entry>
+      </row>
 



>>> and b) patch 4 fails when run under force_parallel=regress.
> Fixed parallel-safety check for RETURNING clause of JSON_EXISTS().
>
> On 05.04.2020 19:50, Alexander Korotkov wrote:
> > 1) Uniqueness checks using JsonbUniqueCheckContext and
> > JsonUniqueCheckContext have quadratic complexity over number of keys.
> > That doesn't look good especially for jsonb, which anyway sorts object
> > keys before object serialization.
> > 2) We have two uniqueness checks for json type, which use
> > JsonbUniqueCheckContext and JsonUniqueState.  JsonUniqueState uses
> > stack of hashes, while JsonbUniqueCheckContext have just plain array
> > of keys.  I think we can make JsonUniqueState use single hash, where
> > object identifies would be part of hash key.  And we should replace
> > JsonbUniqueCheckContext with JsonUniqueState.  That would eliminate
> > extra entities and provide reasonable complexity for cases, which now
> > use JsonbUniqueCheckContext.
>
> Unique checks were refactored as Alexander proposed.
>
> > 3) New SQL/JSON clauses don't use timezone and considered as immutable
> > assuming all the children are immutable.  Immutability is good, but
> > ignoring timezone in all the cases is plain wrong.  The first thing we
> > can do is to use timezone and make SQL/JSON clauses stable.  But that
> > limits their usage in functional and partial indexes.  I see couple of
> > things we can do next (one of them or probably both).
> > 3.1) Provide user a way so specify that we should ignore timezone in
> > particular case (IGNORE TIMEZONE clause or something like that).  Then
> > SQL/JSON clause will be considered as immutable.
> > 3.2) Automatically detect whether jsonpath might use timezone.  If
> > jsonpath doesn't use .datetime() method, it doesn't need timezone for
> > sure.  Also, from the datetime format specifiers we can get that we
> > don't compare non-timezoned values to timezoned values.  So, if we
> > detect this jsonpath never uses timezone, we can consider SQL/JSON
> > clause as immutable.
>
> Implemented second variant with automatic detection.
>
> I also tried to add explicit IGNORE TIMEZONE / IMMUTABLE clauses, but all of
> them lead to shift/reduce conflicts that seem not easy to resolve.




Not sure if this helps:


https://wiki.postgresql.org/wiki/Debugging_the_PostgreSQL_grammar_(Bison)



>
>
>
> Patch #5 implements functions for new JSON type that is expected to appear in
> the upcoming SQL/JSON standard:
>
>  - JSON() is for constructing JSON typed values from JSON text.
>    It is almost equivalent to text::json[b] cast, except that it has additional
>    ability to specify WITH UNIQUE KEYS constraint.
>
>  - JSON_SCALAR() is for constructing JSON typed values from SQL scalars.
>    It is equivalent to to_json[b]().
>
>  - JSON_SERIALIZE() is for serializing JSON typed values to character strings.
>    It is almost equivalent to json[b]::character_type cast, but it also
>    supports output to bytea.
>
> Upcoming Oracle 20c will have JSON datatype and these functions [1], so we
> decided also to implement them for compatibility, despite that they do not make
> sense for real PG users.
>


Are these functions in the standard, or are they Oracle extensions? If
the latter maybe they belong in an options extension.



>
> Patch #6 allows the user to use PG jsonb type as an effective implementation of
> SQL JSON type.  By explicitly setting GUC sql_json = jsonb, JSON will be mapped
> to jsonb, and JSON TEXT (may be named named differently) will be mapped to json.
>
> This seems to be a hack, but we failed to propose something more simpler.



What is going to be the effect of that on things like index expressions?
This strikes me at first glance as something of a potential footgun, but
maybe I'm being overcautious.


cheers


andrew




-- 
Andrew Dunstan                https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

  62. Re: SQL/JSON: functions

    Nikita Glukhov <n.gluhov@postgrespro.ru> — 2020-07-17T20:26:04Z

    Attached 50th version of the patches. Only the documentation was changed
since the previous version.

On 15.07.2020 14:50, Andrew Dunstan wrote:

> On 7/14/20 9:47 PM, Nikita Glukhov wrote:
>
> On 15.07.2020 00:09, Andrew Dunstan wrote:
>>> On 7/14/20 1:00 PM, Andrew Dunstan wrote:
>>>> To keep things moving, I've rebased these patches. However, 1) the docs
>>>> patches use <replaceble class="parameter"> in many cases where they
>>>> should now just use <parameter>
>> I haven't touched <replaceable class="parameter"> yet, because I'm not sure
>> if <replaceable> or <parameter> is correct here at all.
> Here's the relevant commit message that explains the policy:
>
> commit 47046763c3
> Author: Tom Lane <tgl@sss.pgh.pa.us>
> Date:   Mon May 4 13:48:30 2020 -0400
>
>      Doc: standardize markup a bit more.
>     
>      We had a mishmash of <replaceable>, <replaceable class="parameter">,
>      and <parameter> markup for operator/function arguments.  Use <parameter>
>      consistently for things that are in fact names of parameters (including
>      OUT parameters), reserving <replaceable> for things that aren't.  The
>      latter class includes some made-up-by-the-docs type class names, like
>      "numeric_type", as well as placeholders for arguments that don't have
>      well-defined types.  Possibly we could do better with those categories
>      as well, but for the moment I'm content not to have parameter names
>      marked up in different ways in different places.
>>> Turns out these patches also need to get the message on the new way of
>>> writing entries in func.sgml - I'll publish some updates on that in the
>>> next day or so so that "make doc" will succeed.
>> I can do it by myself, but I just need to understand what to fix and how.

Ok, I replaced some <replaceable> with <parameter> in SQL/JSON constructors.
Also I replaced all '[]' with <optional>.


>> Patch #5 implements functions for new JSON type that is expected to appear in
>> the upcoming SQL/JSON standard:
>>
>>   - JSON() is for constructing JSON typed values from JSON text.
>>     It is almost equivalent to text::json[b] cast, except that it has additional
>>     ability to specify WITH UNIQUE KEYS constraint.
>>
>>   - JSON_SCALAR() is for constructing JSON typed values from SQL scalars.
>>     It is equivalent to to_json[b]().
>>
>>   - JSON_SERIALIZE() is for serializing JSON typed values to character strings.
>>     It is almost equivalent to json[b]::character_type cast, but it also
>>     supports output to bytea.
>>
>> Upcoming Oracle 20c will have JSON datatype and these functions [1], so we
>> decided also to implement them for compatibility, despite that they do not make
>> sense for real PG users.
> Are these functions in the standard, or are they Oracle extensions? If
> the latter maybe they belong in an options extension.

The new SQL type JSON and these functions are in to the new standard SQL/JSON 2.0.
It is at the proposal stage now, but Oracle 20c has already implemented it.

The document is not publicly available, so Oleg should have sent it to you in a
private message.

>> Patch #6 allows the user to use PG jsonb type as an effective implementation of
>> SQL JSON type.  By explicitly setting GUC sql_json = jsonb, JSON will be mapped
>> to jsonb, and JSON TEXT (may be named named differently) will be mapped to json.
>>
>> This seems to be a hack, but we failed to propose something more simpler.
> What is going to be the effect of that on things like index expressions?
> This strikes me at first glance as something of a potential footgun, but
> maybe I'm being overcautious.

This allows users of 'sql_json=jsonb' to create GIN indexes on SQL type JSON
(but such creation indexes are still not SQL standard conforming). Maybe I do
not correctly understand the question or the consequences of type name
rewriting.

The type names are rewritten on the input at the initial parsing stage and on
the output in format_type_be(), like it is done for "timestamp with timezone" =>
timestamptz, integer => int4.  Internal representation of query expressions
remains the same.  Affected only representation of JSON types to/from user.

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

  63. Re: SQL/JSON: functions

    Andrew Dunstan <andrew.dunstan@2ndquadrant.com> — 2020-07-18T13:24:11Z

    On 7/17/20 4:26 PM, Nikita Glukhov wrote:
>
>>> Patch #5 implements functions for new JSON type that is expected to appear in
>>> the upcoming SQL/JSON standard:
>>>
>>>  - JSON() is for constructing JSON typed values from JSON text.
>>>    It is almost equivalent to text::json[b] cast, except that it has additional
>>>    ability to specify WITH UNIQUE KEYS constraint.
>>>
>>>  - JSON_SCALAR() is for constructing JSON typed values from SQL scalars.
>>>    It is equivalent to to_json[b]().
>>>
>>>  - JSON_SERIALIZE() is for serializing JSON typed values to character strings.
>>>    It is almost equivalent to json[b]::character_type cast, but it also
>>>    supports output to bytea.
>>>
>>> Upcoming Oracle 20c will have JSON datatype and these functions [1], so we
>>> decided also to implement them for compatibility, despite that they do not make
>>> sense for real PG users.
>> Are these functions in the standard, or are they Oracle extensions? If
>> the latter maybe they belong in an options extension.
> The new SQL type JSON and these functions are in to the new standard SQL/JSON 2.0. 
> It is at the proposal stage now, but Oracle 20c has already implemented it.
>
> The document is not publicly available, so Oleg should have sent it to you in a 
> private message.
>
>>> Patch #6 allows the user to use PG jsonb type as an effective implementation of
>>> SQL JSON type.  By explicitly setting GUC sql_json = jsonb, JSON will be mapped
>>> to jsonb, and JSON TEXT (may be named named differently) will be mapped to json.
>>>
>>> This seems to be a hack, but we failed to propose something more simpler.
>> What is going to be the effect of that on things like index expressions?
>> This strikes me at first glance as something of a potential footgun, but
>> maybe I'm being overcautious.
> This allows users of 'sql_json=jsonb' to create GIN indexes on SQL type JSON
> (but such creation indexes are still not SQL standard conforming). Maybe I do 
> not correctly understand the question or the consequences of type name 
> rewriting.
>
> The type names are rewritten on the input at the initial parsing stage and on 
> the output in format_type_be(), like it is done for "timestamp with timezone" =>
> timestamptz, integer => int4.  Internal representation of query expressions 
> remains the same.  Affected only representation of JSON types to/from user.
>


I think patches 5 and 6 need to be submitted to the next commitfest,
This is far too much scope creep to be snuck in under the current CF item.


I'll look at patches 1-4.


cheers


andrew


-- 
Andrew Dunstan                https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

  64. Re: SQL/JSON: functions

    Michael Paquier <michael@paquier.xyz> — 2020-09-17T05:41:00Z

    On Sat, Jul 18, 2020 at 09:24:11AM -0400, Andrew Dunstan wrote:
> I think patches 5 and 6 need to be submitted to the next commitfest,
> This is far too much scope creep to be snuck in under the current CF item.
> 
> I'll look at patches 1-4.

Even with that, the patch set has been waiting on author for the last
six weeks, so I am marking it as RwF for now.  Please feel free to
resubmit.
--
Michael

  65. Re: SQL/JSON: functions

    Simon Riggs <simon@2ndquadrant.com> — 2020-12-15T17:00:45Z

    On Fri, 17 Jul 2020 at 21:26, Nikita Glukhov <n.gluhov@postgrespro.ru> wrote:
>
> Attached 50th version of the patches. Only the documentation was changed
> since the previous version.

I can imagine the effort required to get to v50, so I salute your efforts.

The document for SQL Standard has now been published as CD
9075-2-Foundation (Thanks Peter).

That gives us a clearer picture of what is being voted on and should
allow Nikita to complete his work.

I suggest we move forwards on this now, but if anyone objects to
including this in PG14 in favour of waiting for the vote, please say
so clearly so we can skip to PG15.

-- 
Simon Riggs                http://www.EnterpriseDB.com/

  66. Re: SQL/JSON: functions

    Pavel Stehule <pavel.stehule@gmail.com> — 2020-12-15T17:36:36Z

    út 15. 12. 2020 v 18:00 odesílatel Simon Riggs <simon@2ndquadrant.com>
napsal:

> On Fri, 17 Jul 2020 at 21:26, Nikita Glukhov <n.gluhov@postgrespro.ru>
> wrote:
> >
> > Attached 50th version of the patches. Only the documentation was changed
> > since the previous version.
>
> I can imagine the effort required to get to v50, so I salute your efforts.
>
> The document for SQL Standard has now been published as CD
> 9075-2-Foundation (Thanks Peter).
>
> That gives us a clearer picture of what is being voted on and should
> allow Nikita to complete his work.
>
> I suggest we move forwards on this now, but if anyone objects to
> including this in PG14 in favour of waiting for the vote, please say
> so clearly so we can skip to PG15.
>

Maybe this set of patches can be reorganized and divided. Some parts like
json generating functions are almost trivial and without controversions
with clean benefits for users.

The most complexity is related to json_table function. Nikita did a very
good job and implemented this function in maximal conformance with ANSI SQL
with a maximal set of features. On second hand it is hard to do review
because this patch is really complex, and a lot of functionality was not
implemented elsewhere (so isn't possible to compare results). I think it
should be possible to reduce complexity and divide acceptance of json_table
to some steps like basic functionality (on MySQL level), enhanced
functionality (on Oracle level), and full functionality (the Postgres will
be first).  This functionality is interesting and maximal conformity with
SQL/JSON is great so I am for merging it. But if it will be divided into
some chronological steps, then there can be higher probability of merging
to upstream in a good time.

Regards

Pavel


> --
> Simon Riggs                http://www.EnterpriseDB.com/
>

  67. Re: SQL/JSON: functions

    Oleg Bartunov <obartunov@postgrespro.ru> — 2020-12-15T18:52:20Z

    On Tue, Dec 15, 2020 at 8:01 PM Simon Riggs <simon@2ndquadrant.com> wrote:
>
> On Fri, 17 Jul 2020 at 21:26, Nikita Glukhov <n.gluhov@postgrespro.ru> wrote:
> >
> > Attached 50th version of the patches. Only the documentation was changed
> > since the previous version.
>
> I can imagine the effort required to get to v50, so I salute your efforts.
>
> The document for SQL Standard has now been published as CD
> 9075-2-Foundation (Thanks Peter).

SQL-2016 is already 4 years old and this what we are using for our development.
What is CD 9075-2-Foundation ?


>
> That gives us a clearer picture of what is being voted on and should
> allow Nikita to complete his work.
>
> I suggest we move forwards on this now, but if anyone objects to
> including this in PG14 in favour of waiting for the vote, please say
> so clearly so we can skip to PG15.

There is upcoming JSON v2. SQL Standard, which  specifies JSON data type,
that mean we have to protect our users, or SQL-compliant applications will be
slow and developers will be disappointed.  I discussed all these in my talk
at Postgres Build
http://www.sai.msu.su/~megera/postgres/talks/json-build-2020.pdf
Andrew Dunstan has volunteered to work with us on completing SQL/JSON standard,
but ...

I think we need SQL/JSON functions in PG14 and  SQL/JSON compatibility mode for
json/jsonb.


>
> --
> Simon Riggs                http://www.EnterpriseDB.com/



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

  68. Re: SQL/JSON: functions

    Oleg Bartunov <obartunov@postgrespro.ru> — 2020-12-15T18:55:43Z

    On Tue, Dec 15, 2020 at 8:37 PM Pavel Stehule <pavel.stehule@gmail.com> wrote:
>
>
>
> út 15. 12. 2020 v 18:00 odesílatel Simon Riggs <simon@2ndquadrant.com> napsal:
>>
>> On Fri, 17 Jul 2020 at 21:26, Nikita Glukhov <n.gluhov@postgrespro.ru> wrote:
>> >
>> > Attached 50th version of the patches. Only the documentation was changed
>> > since the previous version.
>>
>> I can imagine the effort required to get to v50, so I salute your efforts.
>>
>> The document for SQL Standard has now been published as CD
>> 9075-2-Foundation (Thanks Peter).
>>
>> That gives us a clearer picture of what is being voted on and should
>> allow Nikita to complete his work.
>>
>> I suggest we move forwards on this now, but if anyone objects to
>> including this in PG14 in favour of waiting for the vote, please say
>> so clearly so we can skip to PG15.
>
>
> Maybe this set of patches can be reorganized and divided. Some parts like json generating functions are almost trivial and without controversions with clean benefits for users.

I agree with this, most interesting is JSON_TABLE.
>
> The most complexity is related to json_table function. Nikita did a very good job and implemented this function in maximal conformance with ANSI SQL with a maximal set of features. On second hand it is hard to do review because this patch is really complex, and a lot of functionality was not implemented elsewhere (so isn't possible to compare results). I think it should be possible to reduce complexity and divide acceptance of json_table to some steps like basic functionality (on MySQL level), enhanced functionality (on Oracle level), and full functionality (the Postgres will be first).  This functionality is interesting and maximal conformity with SQL/JSON is great so I am for merging it. But if it will be divided into some chronological steps, then there can be higher probability of merging to upstream in a good time.

I think it's shame to look up on MySQL and Oracle, since we have much
better and complete implementation of the Standard.




>
> Regards
>
> Pavel
>
>>
>> --
>> Simon Riggs                http://www.EnterpriseDB.com/



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

  69. Re: SQL/JSON: functions

    Pavel Stehule <pavel.stehule@gmail.com> — 2020-12-15T19:50:33Z

    út 15. 12. 2020 v 19:56 odesílatel Oleg Bartunov <obartunov@postgrespro.ru>
napsal:

> On Tue, Dec 15, 2020 at 8:37 PM Pavel Stehule <pavel.stehule@gmail.com>
> wrote:
> >
> >
> >
> > út 15. 12. 2020 v 18:00 odesílatel Simon Riggs <simon@2ndquadrant.com>
> napsal:
> >>
> >> On Fri, 17 Jul 2020 at 21:26, Nikita Glukhov <n.gluhov@postgrespro.ru>
> wrote:
> >> >
> >> > Attached 50th version of the patches. Only the documentation was
> changed
> >> > since the previous version.
> >>
> >> I can imagine the effort required to get to v50, so I salute your
> efforts.
> >>
> >> The document for SQL Standard has now been published as CD
> >> 9075-2-Foundation (Thanks Peter).
> >>
> >> That gives us a clearer picture of what is being voted on and should
> >> allow Nikita to complete his work.
> >>
> >> I suggest we move forwards on this now, but if anyone objects to
> >> including this in PG14 in favour of waiting for the vote, please say
> >> so clearly so we can skip to PG15.
> >
> >
> > Maybe this set of patches can be reorganized and divided. Some parts
> like json generating functions are almost trivial and without
> controversions with clean benefits for users.
>
> I agree with this, most interesting is JSON_TABLE.
>

It is very interesting, but it is very complex too. There is not any
similarly complex function in ANSI SQL. This function defines its own
language.

>
> > The most complexity is related to json_table function. Nikita did a very
> good job and implemented this function in maximal conformance with ANSI SQL
> with a maximal set of features. On second hand it is hard to do review
> because this patch is really complex, and a lot of functionality was not
> implemented elsewhere (so isn't possible to compare results). I think it
> should be possible to reduce complexity and divide acceptance of json_table
> to some steps like basic functionality (on MySQL level), enhanced
> functionality (on Oracle level), and full functionality (the Postgres will
> be first).  This functionality is interesting and maximal conformity with
> SQL/JSON is great so I am for merging it. But if it will be divided into
> some chronological steps, then there can be higher probability of merging
> to upstream in a good time.
>
> I think it's shame to look up on MySQL and Oracle, since we have much
> better and complete implementation of the Standard.
>

Maybe I used bad words. I would not reduce json_table patch to MySQL or
Oracle level. I proposed merging this patch in a few steps when any step
can be functional.

Standard divides JSON supports to some levels too.

There is about 50% code (and features) when review is not a problem,
because this functionality is very clean and natural. Another 50% can be
possibly problematic because Nikita implementation is first in the world
and the description in standard is complex and hard to read, and hard to
test. Because these patches are not divided we have to do lot of repeated
work in every cycle. I proposed to do work more in style step by step than
in big bang style.

I think there is a lot of code that can be commitable immediately (maybe
half). This can be done quickly without any controversies. This reduces
complexity for 50% so we can concentrate better on the rest of patches.
The final target is full support of standard and full merge of Nikita's
patches. Nikita did hard and good work and it is nonsense to throw away any
part of his work - and it is a pity so the merging process is too long
already . But I understand it is pretty hard to commit to this patch in
complexity like this patch has.




>
>
>
> >
> > Regards
> >
> > Pavel
> >
> >>
> >> --
> >> Simon Riggs                http://www.EnterpriseDB.com/
>
>
>
> --
> Postgres Professional: http://www.postgrespro.com
> The Russian Postgres Company
>

  70. Re: SQL/JSON: functions

    Nikita Glukhov <n.gluhov@postgrespro.ru> — 2020-12-25T20:26:30Z

    On 17.09.2020 08:41, Michael Paquier wrote:

> On Sat, Jul 18, 2020 at 09:24:11AM -0400, Andrew Dunstan wrote:
>> I think patches 5 and 6 need to be submitted to the next commitfest,
>> This is far too much scope creep to be snuck in under the current CF item.
>>
>> I'll look at patches 1-4.
> Even with that, the patch set has been waiting on author for the last
> six weeks, so I am marking it as RwF for now.  Please feel free to
> resubmit.

Attached 51st version of the patches rebased onto current master.


There were some shift/reduce conflicts in SQL grammar that have appeared
after "expr AS keyword" refactoring in 06a7c3154f.  I'm not sure if I resolved
them correctly.  JSON TEXT pseudotype, introduced in #0006, caused a lot of
grammar conflicts, so it was replaced with simple explicit pg_catalog.json.

Also new CoercionForm COERCE_SQL_SYNTAX was introduced, and this reminds custom
function formats that I have used in earlier version of the patches for
deparsing of SQL/JSON constructor expressions that were based on raw json[b]
function calls.  These custom function formats were replaced in v43 with
dedicated executor nodes for SQL/JSON constructors.  So, I'm not sure is it
worth to try to replace back nodes with new COERCE_SQL_SYNTAX.

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

  71. Re: SQL/JSON: functions

    Zhihong Yu <zyu@yugabyte.com> — 2020-12-26T01:19:35Z

    For 0001-Common-SQL-JSON-clauses-v51.patch :

+       /*  | implementation_defined_JSON_representation_option (BSON, AVRO
etc) */

I don't find implementation_defined_JSON_representation_option in the
patchset. Maybe rephrase the above as a comment
without implementation_defined_JSON_representation_option ?

For getJsonEncodingConst(), should the method error out for the default
case of switch (encoding) ?

0002-SQL-JSON-constructors-v51.patch :

+                   Assert(!OidIsValid(collation)); /* result is always an
json[b] type */

an json -> a json

+           /* XXX TEXTOID is default by standard */
+           returning->typid = JSONOID;

Comment doesn't seem to match the assignment.

For json_object_agg_transfn :

+       if (out->len > 2)
+           appendStringInfoString(out, ", ");

Why length needs to be at least 3 (instead of 2) ?

Cheers

On Fri, Dec 25, 2020 at 12:26 PM Nikita Glukhov <n.gluhov@postgrespro.ru>
wrote:

> On 17.09.2020 08:41, Michael Paquier wrote:
>
> On Sat, Jul 18, 2020 at 09:24:11AM -0400, Andrew Dunstan wrote:
>
> I think patches 5 and 6 need to be submitted to the next commitfest,
> This is far too much scope creep to be snuck in under the current CF item.
>
> I'll look at patches 1-4.
>
> Even with that, the patch set has been waiting on author for the last
> six weeks, so I am marking it as RwF for now.  Please feel free to
> resubmit.
>
> Attached 51st version of the patches rebased onto current master.
>
>
> There were some shift/reduce conflicts in SQL grammar that have appeared
> after "expr AS keyword" refactoring in 06a7c3154f.  I'm not sure if I resolved
> them correctly.  JSON TEXT pseudotype, introduced in #0006, caused a lot of
> grammar conflicts, so it was replaced with simple explicit pg_catalog.json.
>
> Also new CoercionForm COERCE_SQL_SYNTAX was introduced, and this reminds custom
> function formats that I have used in earlier version of the patches for
> deparsing of SQL/JSON constructor expressions that were based on raw json[b]
> function calls.  These custom function formats were replaced in v43 with
> dedicated executor nodes for SQL/JSON constructors.  So, I'm not sure is it
> worth to try to replace back nodes with new COERCE_SQL_SYNTAX.
>
> --
> Nikita Glukhov
> Postgres Professional: http://www.postgrespro.com
> The Russian Postgres Company
>
>

  72. Re: SQL/JSON: functions

    Zhihong Yu <zyu@yugabyte.com> — 2020-12-26T19:12:02Z

    Hi,
For ExecEvalJsonExprSubtrans(), if you check !subtrans first,

+       /* No need to use subtransactions. */
+       return func(op, econtext, res, resnull, p, error);

The return statement would allow omitting the else keyword and left-indent
the code in the current if block.

For ExecEvalJsonExpr()

+           *resnull = !DatumGetPointer(res);
+           if (error && *error)
+               return (Datum) 0;

Suppose *resnull is false and *error is true, 0 would be returned
with *resnull as false. Should the *resnull be consistent with the actual
return value ?

For ExecEvalJson() :

+       Assert(*op->resnull);
+       *op->resnull = true;

I am not sure of the purpose for the assignment since *op->resnull should
be true by the assertion.

For raw_expression_tree_walker :

+               if (walker(jfe->on_empty, context))
+                   return true;

Should the if condition include jfe->on_empty prior to walking ?

nit: for contain_mutable_functions_walker, if !IsA(jexpr->path_spec, Const)
is checked first (and return), the current if block can be left indented.

For JsonPathDatatypeStatus,

+   jpdsDateTime,               /* unknown datetime type */

Should the enum be named jpdsUnknownDateTime so that its meaning is clear
to people reading the code ?

For get_json_behavior(), I wonder if mapping from behavior->btype to the
string form would shorten the body of switch statement.
e.g.
char* map[] = {
  " NULL",
  " ERROR",
  " EMPTY",
...
};

Cheers

On Fri, Dec 25, 2020 at 5:19 PM Zhihong Yu <zyu@yugabyte.com> wrote:

> For 0001-Common-SQL-JSON-clauses-v51.patch :
>
> +       /*  | implementation_defined_JSON_representation_option (BSON,
> AVRO etc) */
>
> I don't find implementation_defined_JSON_representation_option in the
> patchset. Maybe rephrase the above as a comment
> without implementation_defined_JSON_representation_option ?
>
> For getJsonEncodingConst(), should the method error out for the default
> case of switch (encoding) ?
>
> 0002-SQL-JSON-constructors-v51.patch :
>
> +                   Assert(!OidIsValid(collation)); /* result is always an
> json[b] type */
>
> an json -> a json
>
> +           /* XXX TEXTOID is default by standard */
> +           returning->typid = JSONOID;
>
> Comment doesn't seem to match the assignment.
>
> For json_object_agg_transfn :
>
> +       if (out->len > 2)
> +           appendStringInfoString(out, ", ");
>
> Why length needs to be at least 3 (instead of 2) ?
>
> Cheers
>
> On Fri, Dec 25, 2020 at 12:26 PM Nikita Glukhov <n.gluhov@postgrespro.ru>
> wrote:
>
>> On 17.09.2020 08:41, Michael Paquier wrote:
>>
>> On Sat, Jul 18, 2020 at 09:24:11AM -0400, Andrew Dunstan wrote:
>>
>> I think patches 5 and 6 need to be submitted to the next commitfest,
>> This is far too much scope creep to be snuck in under the current CF item.
>>
>> I'll look at patches 1-4.
>>
>> Even with that, the patch set has been waiting on author for the last
>> six weeks, so I am marking it as RwF for now.  Please feel free to
>> resubmit.
>>
>> Attached 51st version of the patches rebased onto current master.
>>
>>
>> There were some shift/reduce conflicts in SQL grammar that have appeared
>> after "expr AS keyword" refactoring in 06a7c3154f.  I'm not sure if I resolved
>> them correctly.  JSON TEXT pseudotype, introduced in #0006, caused a lot of
>> grammar conflicts, so it was replaced with simple explicit pg_catalog.json.
>>
>> Also new CoercionForm COERCE_SQL_SYNTAX was introduced, and this reminds custom
>> function formats that I have used in earlier version of the patches for
>> deparsing of SQL/JSON constructor expressions that were based on raw json[b]
>> function calls.  These custom function formats were replaced in v43 with
>> dedicated executor nodes for SQL/JSON constructors.  So, I'm not sure is it
>> worth to try to replace back nodes with new COERCE_SQL_SYNTAX.
>>
>> --
>> Nikita Glukhov
>> Postgres Professional: http://www.postgrespro.com
>> The Russian Postgres Company
>>
>>

  73. Re: SQL/JSON: functions

    Nikita Glukhov <n.gluhov@postgrespro.ru> — 2021-01-20T02:49:13Z

    Hi, Zhihong.  Thank your for your review.  Attached 52nd version of the
patches rebased onto master and fixed as you suggested.


On 26.12.2020 04:19, Zhihong Yu wrote:
> For 0001-Common-SQL-JSON-clauses-v51.patch :
>
> +       /*  | implementation_defined_JSON_representation_option (BSON, 
> AVRO etc) */
>
> I don't find implementation_defined_JSON_representation_option in the 
> patchset. Maybe rephrase the above as a comment 
> without implementation_defined_JSON_representation_option ?
>
Fixed.


> For getJsonEncodingConst(), should the method error out for the 
> default case of switch (encoding) ?

Added default case with elog(ERROR).


>
> 0002-SQL-JSON-constructors-v51.patch :
>
> +                   Assert(!OidIsValid(collation)); /* result is 
> always an json[b] type */
> an json -> a json

Fixed.


>
> +           /* XXX TEXTOID is default by standard */
> +           returning->typid = JSONOID;
>
> Comment doesn't seem to match the assignment.

Comment corrected.

> For json_object_agg_transfn :
>
> +       if (out->len > 2)
> +           appendStringInfoString(out, ", ");
>
> Why length needs to be at least 3 (instead of 2) ?

2 is a length of starting string "{ ".  len > 2 means that we have already
outputted some fields and we need to output comma delimiter.



On 26.12.2020 22:12, Zhihong Yu wrote:
> Hi,
> For ExecEvalJsonExprSubtrans(), if you check !subtrans first,
>
> +       /* No need to use subtransactions. */
> +       return func(op, econtext, res, resnull, p, error);
>
> The return statement would allow omitting the else keyword and 
> left-indent the code in the current if block.

"If" statement was refactored as you suggest.


> For ExecEvalJsonExpr()
>
> +           *resnull = !DatumGetPointer(res);
> +           if (error && *error)
> +               return (Datum) 0;
>
> Suppose *resnull is false and *error is true, 0 would be returned 
> with *resnull as false. Should the *resnull be consistent with the 
> actual return value ?

Now *resnull is set to false in case of error.


> For ExecEvalJson() :
>
> +       Assert(*op->resnull);
> +       *op->resnull = true;
>
> I am not sure of the purpose for the assignment since *op->resnull 
> should be true by the assertion.

Assignment was removed.

> For raw_expression_tree_walker :
>
> +               if (walker(jfe->on_empty, context))
> +                   return true;
>
> Should the if condition include jfe->on_empty prior to walking ?
>
Yes, jfe->on_empty like jfe->on_error can be NULL, and NULL check here is
a walker's responsibility.  But in expression_tree_walker() there is a check
for jfe->on_empty, because only on_empty (not jfe->on_error) can be NULL, and
we are calling walker() on jfe->on_empty->default_expr, not on jfe->on_empty.


>
> nit: for contain_mutable_functions_walker, if !IsA(jexpr->path_spec, 
> Const) is checked first (and return), the current if block can be left 
> indented.

Code was refactored as you suggested.


> For JsonPathDatatypeStatus,
>
> +   jpdsDateTime,               /* unknown datetime type */
>
> Should the enum be named jpdsUnknownDateTime so that its meaning is 
> clear to people reading the code ?

jpdsDateTime was renamed to jpdsUnknownDateTime.


>
> For get_json_behavior(), I wonder if mapping from behavior->btype to 
> the string form would shorten the body of switch statement.
> e.g.
> char* map[] = {
>   " NULL",
>   " ERROR",
>   " EMPTY",
> ...
> };
>
"Switch" statement was replaced with array lookup.


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

  74. Re: SQL/JSON: functions

    Erik Rijkers <er@xs4all.nl> — 2021-01-23T10:37:41Z

    On 2021-01-20 03:49, Nikita Glukhov wrote:

> [0001-Add-common-SQL-JSON-clauses-v52.patch.gz]
> [0002-SQL-JSON-constructors-v52.patch.gz]
> [0003-IS-JSON-predicate-v52.patch.gz]
> [0004-SQL-JSON-query-functions-v52.patch.gz]
> [0005-SQL-JSON-functions-for-json-type-v52.patch.gz]
> [0006-GUC-sql_json-v52.patch.gz]

Hi,

I read through the file func.sgml (only that file) and put the 
errors/peculiarities in the attached diff.  (Small stuff; typos really)


Your patch includes a CREATE TABLE my_films + INSERT, to run the 
examples against.  I think this is a great idea and we should do it more 
often.

But, the table has a text-column to contain the subsequently inserted 
json values. The insert runs fine but it turns out that some later 
examples queries only run against a jsonb column.  So I propose to 
change:
   CREATE TABLE my_films (js text);
to:
   CREATE TABLE my_films (js jsonb);

This change is not yet included in the attached file.  An alternative 
would be to cast the text-column in the example queries as js::jsonb


I also noticed that some errors were different in the sgml file than 'in 
the event':


    SELECT JSON_QUERY(js, '$.favorites[*].kind' ERROR ON ERROR) FROM 
my_films_jsonb;
    (table 'my_films_jsonb' is the same as your 'my_films', but with js 
as a jsonb column)

manual says: "ERROR: more than one SQL/JSON item"
  in reality: "ERROR: JSON path expression in JSON_QUERY should return 
singleton item without wrapper"
         and:   "HINT: use WITH WRAPPER clause to wrap SQL/JSON item 
sequence into array"


Thanks,

Erik Rijkers

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

  75. Re: SQL/JSON: functions

    Ibrar Ahmed <ibrar.ahmad@gmail.com> — 2021-03-08T18:55:32Z

    On Sat, Jan 23, 2021 at 3:37 PM Erik Rijkers <er@xs4all.nl> wrote:

> On 2021-01-20 03:49, Nikita Glukhov wrote:
>
> > [0001-Add-common-SQL-JSON-clauses-v52.patch.gz]
> > [0002-SQL-JSON-constructors-v52.patch.gz]
> > [0003-IS-JSON-predicate-v52.patch.gz]
> > [0004-SQL-JSON-query-functions-v52.patch.gz]
> > [0005-SQL-JSON-functions-for-json-type-v52.patch.gz]
> > [0006-GUC-sql_json-v52.patch.gz]
>
> Hi,
>
> I read through the file func.sgml (only that file) and put the
> errors/peculiarities in the attached diff.  (Small stuff; typos really)
>
>
> Your patch includes a CREATE TABLE my_films + INSERT, to run the
> examples against.  I think this is a great idea and we should do it more
> often.
>
> But, the table has a text-column to contain the subsequently inserted
> json values. The insert runs fine but it turns out that some later
> examples queries only run against a jsonb column.  So I propose to
> change:
>    CREATE TABLE my_films (js text);
> to:
>    CREATE TABLE my_films (js jsonb);
>
> This change is not yet included in the attached file.  An alternative
> would be to cast the text-column in the example queries as js::jsonb
>
>
> I also noticed that some errors were different in the sgml file than 'in
> the event':
>
>
>     SELECT JSON_QUERY(js, '$.favorites[*].kind' ERROR ON ERROR) FROM
> my_films_jsonb;
>     (table 'my_films_jsonb' is the same as your 'my_films', but with js
> as a jsonb column)
>
> manual says: "ERROR: more than one SQL/JSON item"
>   in reality: "ERROR: JSON path expression in JSON_QUERY should return
> singleton item without wrapper"
>          and:   "HINT: use WITH WRAPPER clause to wrap SQL/JSON item
> sequence into array"
>
>
> Thanks,
>
> Erik Rijkers
>
> >
> > --
> > Nikita Glukhov
> > Postgres Professional: http://www.postgrespro.com
> > The Russian Postgres Company
>

The patch (func.sgml.20210123.diff) does not apply successfully.

http://cfbot.cputube.org/patch_32_2901.log

----

=== Applying patches on top of PostgreSQL commit ID
0ce4cd04da558178b0186057b721c50a00b7a945 ===
=== applying patch ./func.sgml.20210123.diff
patching file doc/src/sgml/func.sgml
Hunk #1 FAILED at 16968.
Hunk #2 FAILED at 17034.
...

Hunk #19 FAILED at 18743.
19 out of 19 hunks FAILED -- saving rejects to file doc/src/sgml/func.sgml.rej

----

Can we get a rebase?

I am marking the patch "Waiting on Author".

-- 
Ibrar Ahmed

  76. Re: SQL/JSON: functions

    Andrew Dunstan <andrew@dunslane.net> — 2021-03-26T20:22:44Z

    On 3/8/21 1:55 PM, Ibrar Ahmed wrote:
>
>
> On Sat, Jan 23, 2021 at 3:37 PM Erik Rijkers <er@xs4all.nl
> <mailto:er@xs4all.nl>> wrote:
>
>     On 2021-01-20 03:49, Nikita Glukhov wrote:
>
>     > [0001-Add-common-SQL-JSON-clauses-v52.patch.gz]
>     > [0002-SQL-JSON-constructors-v52.patch.gz]
>     > [0003-IS-JSON-predicate-v52.patch.gz]
>     > [0004-SQL-JSON-query-functions-v52.patch.gz]
>     > [0005-SQL-JSON-functions-for-json-type-v52.patch.gz]
>     > [0006-GUC-sql_json-v52.patch.gz]
>
>     Hi,
>
>     I read through the file func.sgml (only that file) and put the
>     errors/peculiarities in the attached diff.  (Small stuff; typos
>     really)
>
>
>     Your patch includes a CREATE TABLE my_films + INSERT, to run the
>     examples against.  I think this is a great idea and we should do
>     it more
>     often.
>
>     But, the table has a text-column to contain the subsequently inserted
>     json values. The insert runs fine but it turns out that some later
>     examples queries only run against a jsonb column.  So I propose to
>     change:
>        CREATE TABLE my_films (js text);
>     to:
>        CREATE TABLE my_films (js jsonb);
>
>     This change is not yet included in the attached file.  An alternative
>     would be to cast the text-column in the example queries as js::jsonb
>
>
>     I also noticed that some errors were different in the sgml file
>     than 'in
>     the event':
>
>
>         SELECT JSON_QUERY(js, '$.favorites[*].kind' ERROR ON ERROR) FROM
>     my_films_jsonb;
>         (table 'my_films_jsonb' is the same as your 'my_films', but
>     with js
>     as a jsonb column)
>
>     manual says: "ERROR: more than one SQL/JSON item"
>       in reality: "ERROR: JSON path expression in JSON_QUERY should
>     return
>     singleton item without wrapper"
>              and:   "HINT: use WITH WRAPPER clause to wrap SQL/JSON item
>     sequence into array"
>
>
>     Thanks,
>
>     Erik Rijkers
>
>     >
>     > --
>     > Nikita Glukhov
>     > Postgres Professional: http://www.postgrespro.com
>     <http://www.postgrespro.com>
>     > The Russian Postgres Company
>
>
> The patch (func.sgml.20210123.diff) does not apply successfully.
>
> http://cfbot.cputube.org/patch_32_2901.log
> <http://cfbot.cputube.org/patch_32_2901.log>
>
> ----
> === Applying patches on top of PostgreSQL commit ID 0ce4cd04da558178b0186057b721c50a00b7a945 ===
> === applying patch ./func.sgml.20210123.diff
> patching file doc/src/sgml/func.sgml
> Hunk #1 FAILED at 16968.
> Hunk #2 FAILED at 17034.
> ...
> Hunk #19 FAILED at 18743.
> 19 out of 19 hunks FAILED -- saving rejects to file doc/src/sgml/func.sgml.rej
> ----
>
> Can we get a rebase? 
>
> I am marking the patch "Waiting on Author".




I've rebased this, and applied some of Erik's changes.


I'll set it back to 'Needs Review' if the cfbot is happy.


cheers


andrew


--
Andrew Dunstan
EDB: https://www.enterprisedb.com

  77. Re: SQL/JSON: functions

    Andrew Dunstan <andrew@dunslane.net> — 2021-03-26T20:49:26Z

    On 3/26/21 4:22 PM, Andrew Dunstan wrote:
> On 3/8/21 1:55 PM, Ibrar Ahmed wrote:
>>
>> On Sat, Jan 23, 2021 at 3:37 PM Erik Rijkers <er@xs4all.nl
>> <mailto:er@xs4all.nl>> wrote:
>>
>>     On 2021-01-20 03:49, Nikita Glukhov wrote:
>>
>>     > [0001-Add-common-SQL-JSON-clauses-v52.patch.gz]
>>     > [0002-SQL-JSON-constructors-v52.patch.gz]
>>     > [0003-IS-JSON-predicate-v52.patch.gz]
>>     > [0004-SQL-JSON-query-functions-v52.patch.gz]
>>     > [0005-SQL-JSON-functions-for-json-type-v52.patch.gz]
>>     > [0006-GUC-sql_json-v52.patch.gz]
>>
>>     Hi,
>>
>>     I read through the file func.sgml (only that file) and put the
>>     errors/peculiarities in the attached diff.  (Small stuff; typos
>>     really)
>>
>>
>>     Your patch includes a CREATE TABLE my_films + INSERT, to run the
>>     examples against.  I think this is a great idea and we should do
>>     it more
>>     often.
>>
>>     But, the table has a text-column to contain the subsequently inserted
>>     json values. The insert runs fine but it turns out that some later
>>     examples queries only run against a jsonb column.  So I propose to
>>     change:
>>        CREATE TABLE my_films (js text);
>>     to:
>>        CREATE TABLE my_films (js jsonb);
>>
>>     This change is not yet included in the attached file.  An alternative
>>     would be to cast the text-column in the example queries as js::jsonb
>>
>>
>>     I also noticed that some errors were different in the sgml file
>>     than 'in
>>     the event':
>>
>>
>>         SELECT JSON_QUERY(js, '$.favorites[*].kind' ERROR ON ERROR) FROM
>>     my_films_jsonb;
>>         (table 'my_films_jsonb' is the same as your 'my_films', but
>>     with js
>>     as a jsonb column)
>>
>>     manual says: "ERROR: more than one SQL/JSON item"
>>       in reality: "ERROR: JSON path expression in JSON_QUERY should
>>     return
>>     singleton item without wrapper"
>>              and:   "HINT: use WITH WRAPPER clause to wrap SQL/JSON item
>>     sequence into array"
>>
>>
>>     Thanks,
>>
>>     Erik Rijkers
>>
>>     >
>>     > --
>>     > Nikita Glukhov
>>     > Postgres Professional: http://www.postgrespro.com
>>     <http://www.postgrespro.com>
>>     > The Russian Postgres Company
>>
>>
>> The patch (func.sgml.20210123.diff) does not apply successfully.
>>
>> http://cfbot.cputube.org/patch_32_2901.log
>> <http://cfbot.cputube.org/patch_32_2901.log>
>>
>> ----
>> === Applying patches on top of PostgreSQL commit ID 0ce4cd04da558178b0186057b721c50a00b7a945 ===
>> === applying patch ./func.sgml.20210123.diff
>> patching file doc/src/sgml/func.sgml
>> Hunk #1 FAILED at 16968.
>> Hunk #2 FAILED at 17034.
>> ...
>> Hunk #19 FAILED at 18743.
>> 19 out of 19 hunks FAILED -- saving rejects to file doc/src/sgml/func.sgml.rej
>> ----
>>
>> Can we get a rebase? 
>>
>> I am marking the patch "Waiting on Author".
>
>
>
> I've rebased this, and applied some of Erik's changes.
>
>
> I'll set it back to 'Needs Review' if the cfbot is happy.


It's not. There are errors  when building with llvm. I'll investigate.


cheers


andrew


--
Andrew Dunstan
EDB: https://www.enterprisedb.com

  78. Re: SQL/JSON: functions

    Andrew Dunstan <andrew@dunslane.net> — 2021-03-26T22:30:17Z

    On 3/26/21 4:49 PM, Andrew Dunstan wrote:
> On 3/26/21 4:22 PM, Andrew Dunstan wrote:
>> On 3/8/21 1:55 PM, Ibrar Ahmed wrote:
>>> On Sat, Jan 23, 2021 at 3:37 PM Erik Rijkers <er@xs4all.nl
>>> <mailto:er@xs4all.nl>> wrote:
>>>
>>>     On 2021-01-20 03:49, Nikita Glukhov wrote:
>>>
>>>     > [0001-Add-common-SQL-JSON-clauses-v52.patch.gz]
>>>     > [0002-SQL-JSON-constructors-v52.patch.gz]
>>>     > [0003-IS-JSON-predicate-v52.patch.gz]
>>>     > [0004-SQL-JSON-query-functions-v52.patch.gz]
>>>     > [0005-SQL-JSON-functions-for-json-type-v52.patch.gz]
>>>     > [0006-GUC-sql_json-v52.patch.gz]
>>>
>>>     Hi,
>>>
>>>     I read through the file func.sgml (only that file) and put the
>>>     errors/peculiarities in the attached diff.  (Small stuff; typos
>>>     really)
>>>
>>>
>>>     Your patch includes a CREATE TABLE my_films + INSERT, to run the
>>>     examples against.  I think this is a great idea and we should do
>>>     it more
>>>     often.
>>>
>>>     But, the table has a text-column to contain the subsequently inserted
>>>     json values. The insert runs fine but it turns out that some later
>>>     examples queries only run against a jsonb column.  So I propose to
>>>     change:
>>>        CREATE TABLE my_films (js text);
>>>     to:
>>>        CREATE TABLE my_films (js jsonb);
>>>
>>>     This change is not yet included in the attached file.  An alternative
>>>     would be to cast the text-column in the example queries as js::jsonb
>>>
>>>
>>>     I also noticed that some errors were different in the sgml file
>>>     than 'in
>>>     the event':
>>>
>>>
>>>         SELECT JSON_QUERY(js, '$.favorites[*].kind' ERROR ON ERROR) FROM
>>>     my_films_jsonb;
>>>         (table 'my_films_jsonb' is the same as your 'my_films', but
>>>     with js
>>>     as a jsonb column)
>>>
>>>     manual says: "ERROR: more than one SQL/JSON item"
>>>       in reality: "ERROR: JSON path expression in JSON_QUERY should
>>>     return
>>>     singleton item without wrapper"
>>>              and:   "HINT: use WITH WRAPPER clause to wrap SQL/JSON item
>>>     sequence into array"
>>>
>>>
>>>     Thanks,
>>>
>>>     Erik Rijkers
>>>
>>>     >
>>>     > --
>>>     > Nikita Glukhov
>>>     > Postgres Professional: http://www.postgrespro.com
>>>     <http://www.postgrespro.com>
>>>     > The Russian Postgres Company
>>>
>>>
>>> The patch (func.sgml.20210123.diff) does not apply successfully.
>>>
>>> http://cfbot.cputube.org/patch_32_2901.log
>>> <http://cfbot.cputube.org/patch_32_2901.log>
>>>
>>> ----
>>> === Applying patches on top of PostgreSQL commit ID 0ce4cd04da558178b0186057b721c50a00b7a945 ===
>>> === applying patch ./func.sgml.20210123.diff
>>> patching file doc/src/sgml/func.sgml
>>> Hunk #1 FAILED at 16968.
>>> Hunk #2 FAILED at 17034.
>>> ...
>>> Hunk #19 FAILED at 18743.
>>> 19 out of 19 hunks FAILED -- saving rejects to file doc/src/sgml/func.sgml.rej
>>> ----
>>>
>>> Can we get a rebase? 
>>>
>>> I am marking the patch "Waiting on Author".
>>
>>
>> I've rebased this, and applied some of Erik's changes.
>>
>>
>> I'll set it back to 'Needs Review' if the cfbot is happy.
>
> It's not. There are errors  when building with llvm. I'll investigate.



Specifically, patch 4 (SQL-JSON-query-functions) fails with this when
built with LLVM:


cache gcc -Wall -Wmissing-prototypes -Wpointer-arith
-Wdeclaration-after-statement -Werror=vla -Wendif-labels
-Wmissing-format-attribute -Wimplicit-fallthrough=3 -Wcast-function-type
-Wformat-security -fno-strict-aliasing -fwrapv
-fexcess-precision=standard -Wno-format-truncation
-Wno-stringop-truncation -g -O2  -fPIC -D__STDC_LIMIT_MACROS
-D__STDC_FORMAT_MACROS -D__STDC_CONSTANT_MACROS -D_GNU_SOURCE
-I/usr/include  -I../../../../src/include
-I/home/andrew/pgl/pg_head/src/include  -D_GNU_SOURCE   -c -o
llvmjit_expr.o /home/andrew/pgl/pg_head/src/backend/jit/llvm/llvmjit_expr.c
In file included from /home/andrew/pgl/pg_head/src/include/postgres.h:46,
                 from
/home/andrew/pgl/pg_head/src/backend/jit/llvm/llvmjit_expr.c:16:
/home/andrew/pgl/pg_head/src/backend/jit/llvm/llvmjit_expr.c: In
function ‘llvm_compile_expr’:
/home/andrew/pgl/pg_head/src/backend/jit/llvm/llvmjit_expr.c:2348:30:
warning: initialization of ‘struct LLVMOpaqueValue *’ from incompatible
pointer type ‘ExprEvalStep *’ {aka ‘struct ExprEvalStep *’}
[-Wincompatible-pointer-types]
 2348 |         v_state, v_econtext, op);
      |                              ^~
/home/andrew/pgl/pg_head/src/include/c.h:734:34: note: in definition of
macro ‘lengthof’
  734 | #define lengthof(array) (sizeof (array) / sizeof ((array)[0]))
      |                                  ^~~~~
/home/andrew/pgl/pg_head/src/backend/jit/llvm/llvmjit_expr.c:2347:5:
note: in expansion of macro ‘build_EvalXFunc’
 2347 |     build_EvalXFunc(b, mod, "ExecEvalJson",
      |     ^~~~~~~~~~~~~~~
/home/andrew/pgl/pg_head/src/backend/jit/llvm/llvmjit_expr.c:2348:30:
note: (near initialization for ‘(anonymous)[0]’)
 2348 |         v_state, v_econtext, op);
      |                              ^~
/home/andrew/pgl/pg_head/src/include/c.h:734:34: note: in definition of
macro ‘lengthof’
  734 | #define lengthof(array) (sizeof (array) / sizeof ((array)[0]))
      |                                  ^~~~~
/home/andrew/pgl/pg_head/src/backend/jit/llvm/llvmjit_expr.c:2347:5:
note: in expansion of macro ‘build_EvalXFunc’
 2347 |     build_EvalXFunc(b, mod, "ExecEvalJson",
      |     ^~~~~~~~~~~~~~~
/home/andrew/pgl/pg_head/src/backend/jit/llvm/llvmjit_expr.c:2348:30:
warning: initialization of ‘struct LLVMOpaqueValue *’ from incompatible
pointer type ‘ExprEvalStep *’ {aka ‘struct ExprEvalStep *’}
[-Wincompatible-pointer-types]
 2348 |         v_state, v_econtext, op);
      |                              ^~
/home/andrew/pgl/pg_head/src/include/c.h:734:52: note: in definition of
macro ‘lengthof’
  734 | #define lengthof(array) (sizeof (array) / sizeof ((array)[0]))
      |                                                    ^~~~~
/home/andrew/pgl/pg_head/src/backend/jit/llvm/llvmjit_expr.c:2347:5:
note: in expansion of macro ‘build_EvalXFunc’
 2347 |     build_EvalXFunc(b, mod, "ExecEvalJson",
      |     ^~~~~~~~~~~~~~~
/home/andrew/pgl/pg_head/src/backend/jit/llvm/llvmjit_expr.c:2348:30:
note: (near initialization for ‘(anonymous)[0]’)
 2348 |         v_state, v_econtext, op);
      |                              ^~
/home/andrew/pgl/pg_head/src/include/c.h:734:52: note: in definition of
macro ‘lengthof’
  734 | #define lengthof(array) (sizeof (array) / sizeof ((array)[0]))
      |                                                    ^~~~~
/home/andrew/pgl/pg_head/src/backend/jit/llvm/llvmjit_expr.c:2347:5:
note: in expansion of macro ‘build_EvalXFunc’
 2347 |     build_EvalXFunc(b, mod, "ExecEvalJson",
      |     ^~~~~~~~~~~~~~~
/home/andrew/pgl/pg_head/src/backend/jit/llvm/llvmjit_expr.c:2348:30:
warning: initialization of ‘struct LLVMOpaqueValue *’ from incompatible
pointer type ‘ExprEvalStep *’ {aka ‘struct ExprEvalStep *’}
[-Wincompatible-pointer-types]
 2348 |         v_state, v_econtext, op);
      |                              ^~
/home/andrew/pgl/pg_head/src/backend/jit/llvm/llvmjit_expr.c:71:27:
note: in definition of macro ‘build_EvalXFunc’
   71 |         ((LLVMValueRef[]){__VA_ARGS__}))
      |                           ^~~~~~~~~~~
/home/andrew/pgl/pg_head/src/backend/jit/llvm/llvmjit_expr.c:2348:30:
note: (near initialization for ‘(anonymous)[0]’)
 2348 |         v_state, v_econtext, op);
      |                              ^~
/home/andrew/pgl/pg_head/src/backend/jit/llvm/llvmjit_expr.c:71:27:
note: in definition of macro ‘build_EvalXFunc’
   71 |         ((LLVMValueRef[]){__VA_ARGS__}))
      |                           ^~~~~~~~~~~
/home/andrew/pgl/pg_head/src/backend/jit/llvm/llvmjit_expr.c:2348:18:
warning: passing argument 5 of ‘build_EvalXFuncInt’ from incompatible
pointer type [-Wincompatible-pointer-types]
 2348 |         v_state, v_econtext, op);
      |                  ^~~~~~~~~~
      |                  |
      |                  LLVMValueRef {aka struct LLVMOpaqueValue *}
/home/andrew/pgl/pg_head/src/backend/jit/llvm/llvmjit_expr.c:69:48:
note: in definition of macro ‘build_EvalXFunc’
   69 |  build_EvalXFuncInt(b, mod, funcname, v_state, op, \
      |                                                ^~
/home/andrew/pgl/pg_head/src/backend/jit/llvm/llvmjit_expr.c:63:27:
note: expected ‘ExprEvalStep *’ {aka ‘struct ExprEvalStep *’} but
argument is of type ‘LLVMValueRef’ {aka ‘struct LLVMOpaqueValue *’}
   63 |             ExprEvalStep *op,
      |             ~~~~~~~~~~~~~~^~
/home/andrew/pgl/pg_head/src/backend/jit/llvm/llvmjit_expr.c:2349:29:
error: ‘i’ undeclared (first use in this function)
 2349 |     LLVMBuildBr(b, opblocks[i + 1]);
      |                             ^
/home/andrew/pgl/pg_head/src/backend/jit/llvm/llvmjit_expr.c:2349:29:
note: each undeclared identifier is reported only once for each function
it appears in
/home/andrew/pgl/pg_head/src/backend/jit/llvm/llvmjit_expr.c:248:3:
warning: enumeration value ‘EEOP_JSON_CONSTRUCTOR’ not handled in switch
[-Wswitch]
  248 |   switch (opcode)
      |   ^~~~~~
/home/andrew/pgl/pg_head/src/backend/jit/llvm/llvmjit_expr.c:248:3:
warning: enumeration value ‘EEOP_IS_JSON’ not handled in switch [-Wswitch]
make[2]: *** [<builtin>: llvmjit_expr.o] Error 1
make[2]: Leaving directory
'/home/andrew/pgl/pg_head/bfroot/HEAD/pgsql.build/src/backend/jit/llvm'
make[1]: *** [Makefile:42: all-backend/jit/llvm-recurse] Error 2
make[1]: Leaving directory
'/home/andrew/pgl/pg_head/bfroot/HEAD/pgsql.build/src'


There is also a bug that results in a warning in gram.y, but fixing it
doesn't affect this issue. Nikita, please look into this ASAP.


cheers


andrew


--
Andrew Dunstan
EDB: https://www.enterprisedb.com

  79. Re: SQL/JSON: functions

    Nikita Glukhov <n.gluhov@postgrespro.ru> — 2021-03-27T01:14:12Z

    Attached 54th version of the patches rebased onto current master.

On 27.03.2021 01:30, Andrew Dunstan wrote:
> Specifically, patch 4 (SQL-JSON-query-functions) fails with this when
> built with LLVM:
>
>
> There is also a bug that results in a warning in gram.y, but fixing it
> doesn't affect this issue. Nikita, please look into this ASAP.

LLVM issues and gram.y are fixed.


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

  80. Re: SQL/JSON: functions

    Andrew Dunstan <andrew@dunslane.net> — 2021-04-28T21:55:54Z

    On Fri, Mar 26, 2021 at 9:14 PM Nikita Glukhov <n.gluhov@postgrespro.ru>
wrote:

> Attached 54th version of the patches rebased onto current master.
>
>
> On 27.03.2021 01:30, Andrew Dunstan wrote:
>
> Specifically, patch 4 (SQL-JSON-query-functions) fails with this when
> built with LLVM:
>
>
> There is also a bug that results in a warning in gram.y, but fixing it
> doesn't affect this issue. Nikita, please look into this ASAP.
>
> LLVM issues and gram.y are fixed.
>
>
>
>
It's apparently bitrotted again. See <
http://cfbot.cputube.org/patch_33_2901.log>

cheers

andrew

  81. Re: SQL/JSON: functions

    Andrew Dunstan <andrew@dunslane.net> — 2021-05-08T18:21:22Z

    On 4/28/21 5:55 PM, Andrew Dunstan wrote:
>
>
> On Fri, Mar 26, 2021 at 9:14 PM Nikita Glukhov
> <n.gluhov@postgrespro.ru <mailto:n.gluhov@postgrespro.ru>> wrote:
>
>     Attached 54th version of the patches rebased onto current master.
>
>     On 27.03.2021 01:30, Andrew Dunstan wrote:
>>     Specifically, patch 4 (SQL-JSON-query-functions) fails with this when
>>     built with LLVM:
>>
>>
>>     There is also a bug that results in a warning in gram.y, but fixing it
>>     doesn't affect this issue. Nikita, please look into this ASAP.
>
>     LLVM issues and gram.y are fixed.
>
>
>
>
> It's apparently bitrotted again. See
> <http://cfbot.cputube.org/patch_33_2901.log
> <http://cfbot.cputube.org/patch_33_2901.log>>
>
>


This set should remove the bitrot.


cheers


andrew





--
Andrew Dunstan
EDB: https://www.enterprisedb.com

  82. Re: SQL/JSON: functions

    Zhihong Yu <zyu@yugabyte.com> — 2021-05-08T23:54:02Z

    On Sat, May 8, 2021 at 11:21 AM Andrew Dunstan <andrew@dunslane.net> wrote:

>
> On 4/28/21 5:55 PM, Andrew Dunstan wrote:
> >
> >
> > On Fri, Mar 26, 2021 at 9:14 PM Nikita Glukhov
> > <n.gluhov@postgrespro.ru <mailto:n.gluhov@postgrespro.ru>> wrote:
> >
> >     Attached 54th version of the patches rebased onto current master.
> >
> >     On 27.03.2021 01:30, Andrew Dunstan wrote:
> >>     Specifically, patch 4 (SQL-JSON-query-functions) fails with this
> when
> >>     built with LLVM:
> >>
> >>
> >>     There is also a bug that results in a warning in gram.y, but fixing
> it
> >>     doesn't affect this issue. Nikita, please look into this ASAP.
> >
> >     LLVM issues and gram.y are fixed.
> >
> >
> >
> >
> > It's apparently bitrotted again. See
> > <http://cfbot.cputube.org/patch_33_2901.log
> > <http://cfbot.cputube.org/patch_33_2901.log>>
> >
> >
>
>
> This set should remove the bitrot.
>
>
> cheers
>
>
> andrew
>
>
>
>
>
> --
> Andrew Dunstan
> EDB: https://www.enterprisedb.com
>
> Hi,
For 0005-SQL-JSON-functions-for-json-type-v55.patch:

+      Alternatively, you can construct <acronym>JSON</acronym> values
simply
+      using <productname>PostgreSQL</productname>-specific casts to
+      <type>json</type> and <type>jsonb</type> types.

I think the 'and' in the 3rd line should be 'or'.

+     <title>Examples</title>
+     <para>
+      Construct a JSON the provided strings:

A word seems to be missing between JSON and the.

Cheers

  83. Re: SQL/JSON: functions

    Andrew Dunstan <andrew.dunstan@2ndquadrant.com> — 2021-05-18T19:22:33Z

    On 5/8/21 2:21 PM, Andrew Dunstan wrote:
> On 4/28/21 5:55 PM, Andrew Dunstan wrote:
>>
>> On Fri, Mar 26, 2021 at 9:14 PM Nikita Glukhov
>> <n.gluhov@postgrespro.ru <mailto:n.gluhov@postgrespro.ru>> wrote:
>>
>>     Attached 54th version of the patches rebased onto current master.
>>
>>     On 27.03.2021 01:30, Andrew Dunstan wrote:
>>>     Specifically, patch 4 (SQL-JSON-query-functions) fails with this when
>>>     built with LLVM:
>>>
>>>
>>>     There is also a bug that results in a warning in gram.y, but fixing it
>>>     doesn't affect this issue. Nikita, please look into this ASAP.
>>     LLVM issues and gram.y are fixed.
>>
>>
>>
>>
>> It's apparently bitrotted again. See
>> <http://cfbot.cputube.org/patch_33_2901.log
>> <http://cfbot.cputube.org/patch_33_2901.log>>
>>
>>
>
> This set should remove the bitrot.
>
>



Rebased for removal of serial schedule


cheers


andrew


--
Andrew Dunstan
EDB: https://www.enterprisedb.com

  84. Re: SQL/JSON: functions

    Andrew Dunstan <andrew@dunslane.net> — 2021-09-02T18:50:00Z

    On 5/18/21 3:22 PM, Andrew Dunstan wrote:
> On 5/8/21 2:21 PM, Andrew Dunstan wrote:
>> On 4/28/21 5:55 PM, Andrew Dunstan wrote:
>>> On Fri, Mar 26, 2021 at 9:14 PM Nikita Glukhov
>>> <n.gluhov@postgrespro.ru <mailto:n.gluhov@postgrespro.ru>> wrote:
>>>
>>>     Attached 54th version of the patches rebased onto current master.
>>>
>>>     On 27.03.2021 01:30, Andrew Dunstan wrote:
>>>>     Specifically, patch 4 (SQL-JSON-query-functions) fails with this when
>>>>     built with LLVM:
>>>>
>>>>
>>>>     There is also a bug that results in a warning in gram.y, but fixing it
>>>>     doesn't affect this issue. Nikita, please look into this ASAP.
>>>     LLVM issues and gram.y are fixed.
>>>
>>>
>>>
>>>
>>> It's apparently bitrotted again. See
>>> <http://cfbot.cputube.org/patch_33_2901.log
>>> <http://cfbot.cputube.org/patch_33_2901.log>>
>>>
>>>
>> This set should remove the bitrot.
>>
>>
>
>
> Rebased for removal of serial schedule
>
>

rebased on master and incorporating fixes from Erik Rijkers


cheers


andrew

--
Andrew Dunstan
EDB: https://www.enterprisedb.com

  85. Re: SQL/JSON: functions

    Andrew Dunstan <andrew.dunstan@2ndquadrant.com> — 2021-09-14T12:55:59Z

    On 9/2/21 2:50 PM, Andrew Dunstan wrote:
> On 5/18/21 3:22 PM, Andrew Dunstan wrote:
>> On 5/8/21 2:21 PM, Andrew Dunstan wrote:
>>> On 4/28/21 5:55 PM, Andrew Dunstan wrote:
>>>> On Fri, Mar 26, 2021 at 9:14 PM Nikita Glukhov
>>>> <n.gluhov@postgrespro.ru <mailto:n.gluhov@postgrespro.ru>> wrote:
>>>>
>>>>     Attached 54th version of the patches rebased onto current master.
>>>>
>>>>     On 27.03.2021 01:30, Andrew Dunstan wrote:
>>>>>     Specifically, patch 4 (SQL-JSON-query-functions) fails with this when
>>>>>     built with LLVM:
>>>>>
>>>>>
>>>>>     There is also a bug that results in a warning in gram.y, but fixing it
>>>>>     doesn't affect this issue. Nikita, please look into this ASAP.
>>>>     LLVM issues and gram.y are fixed.
>>>>
>>>>
>>>>
>>>>
>>>> It's apparently bitrotted again. See
>>>> <http://cfbot.cputube.org/patch_33_2901.log
>>>> <http://cfbot.cputube.org/patch_33_2901.log>>
>>>>
>>>>
>>> This set should remove the bitrot.
>>>
>>>
>>
>> Rebased for removal of serial schedule
>>
>>
> rebased on master and incorporating fixes from Erik Rijkers
>
>

rebased to remove bitrot from the removal of the  Value node type.


cheers


andrew


-- 

Andrew Dunstan
EDB: https://www.enterprisedb.com

  86. Re: SQL/JSON: functions

    Andrew Dunstan <andrew@dunslane.net> — 2021-09-16T14:52:24Z

    On 9/14/21 8:55 AM, Andrew Dunstan wrote:
> On 9/2/21 2:50 PM, Andrew Dunstan wrote:
>> On 5/18/21 3:22 PM, Andrew Dunstan wrote:
>>> On 5/8/21 2:21 PM, Andrew Dunstan wrote:
>>>> On 4/28/21 5:55 PM, Andrew Dunstan wrote:
>>>>> On Fri, Mar 26, 2021 at 9:14 PM Nikita Glukhov
>>>>> <n.gluhov@postgrespro.ru <mailto:n.gluhov@postgrespro.ru>> wrote:
>>>>>
>>>>>     Attached 54th version of the patches rebased onto current master.
>>>>>
>>>>>     On 27.03.2021 01:30, Andrew Dunstan wrote:
>>>>>>     Specifically, patch 4 (SQL-JSON-query-functions) fails with this when
>>>>>>     built with LLVM:
>>>>>>
>>>>>>
>>>>>>     There is also a bug that results in a warning in gram.y, but fixing it
>>>>>>     doesn't affect this issue. Nikita, please look into this ASAP.
>>>>>     LLVM issues and gram.y are fixed.
>>>>>
>>>>>
>>>>>
>>>>>
>>>>> It's apparently bitrotted again. See
>>>>> <http://cfbot.cputube.org/patch_33_2901.log
>>>>> <http://cfbot.cputube.org/patch_33_2901.log>>
>>>>>
>>>>>
>>>> This set should remove the bitrot.
>>>>
>>>>
>>> Rebased for removal of serial schedule
>>>
>>>
>> rebased on master and incorporating fixes from Erik Rijkers
>>
>>
> rebased to remove bitrot from the removal of the  Value node type.
>
>

Rebased, and fixed a bug (which I had faithfully replicated above) in
the APP_JUMB code, as reported by Erik Rijkers.


cheers


andrew


--
Andrew Dunstan
EDB: https://www.enterprisedb.com

  87. Re: SQL/JSON: functions

    Himanshu Upadhyaya <upadhyaya.himanshu@gmail.com> — 2021-12-01T11:13:04Z

    Hi Andrew,

The latest version (v59) is not applying on head.
Could you please help to rebase?

Thanks,
Himanshu

On Thu, Sep 16, 2021 at 8:23 PM Andrew Dunstan <andrew@dunslane.net> wrote:

>
> On 9/14/21 8:55 AM, Andrew Dunstan wrote:
> > On 9/2/21 2:50 PM, Andrew Dunstan wrote:
> >> On 5/18/21 3:22 PM, Andrew Dunstan wrote:
> >>> On 5/8/21 2:21 PM, Andrew Dunstan wrote:
> >>>> On 4/28/21 5:55 PM, Andrew Dunstan wrote:
> >>>>> On Fri, Mar 26, 2021 at 9:14 PM Nikita Glukhov
> >>>>> <n.gluhov@postgrespro.ru <mailto:n.gluhov@postgrespro.ru>> wrote:
> >>>>>
> >>>>>     Attached 54th version of the patches rebased onto current master.
> >>>>>
> >>>>>     On 27.03.2021 01:30, Andrew Dunstan wrote:
> >>>>>>     Specifically, patch 4 (SQL-JSON-query-functions) fails with
> this when
> >>>>>>     built with LLVM:
> >>>>>>
> >>>>>>
> >>>>>>     There is also a bug that results in a warning in gram.y, but
> fixing it
> >>>>>>     doesn't affect this issue. Nikita, please look into this ASAP.
> >>>>>     LLVM issues and gram.y are fixed.
> >>>>>
> >>>>>
> >>>>>
> >>>>>
> >>>>> It's apparently bitrotted again. See
> >>>>> <http://cfbot.cputube.org/patch_33_2901.log
> >>>>> <http://cfbot.cputube.org/patch_33_2901.log>>
> >>>>>
> >>>>>
> >>>> This set should remove the bitrot.
> >>>>
> >>>>
> >>> Rebased for removal of serial schedule
> >>>
> >>>
> >> rebased on master and incorporating fixes from Erik Rijkers
> >>
> >>
> > rebased to remove bitrot from the removal of the  Value node type.
> >
> >
>
> Rebased, and fixed a bug (which I had faithfully replicated above) in
> the APP_JUMB code, as reported by Erik Rijkers.
>
>
> cheers
>
>
> andrew
>
>
> --
> Andrew Dunstan
> EDB: https://www.enterprisedb.com
>
>

  88. Re: SQL/JSON: functions

    Andrew Dunstan <andrew@dunslane.net> — 2021-12-01T14:26:38Z

    On 12/1/21 06:13, Himanshu Upadhyaya wrote:
> Hi Andrew,
>
> The latest version (v59) is not applying on head.
> Could you please help to rebase?
>
>


(Please don't top-post on PostgreSQL lists)


The patches apply for me and for the cfbot:
<http://cfbot.cputube.org/patch_35_2901.log>. I'm not sure what's not
working for you. I apply them using "patch -p 1 < $patchfile"


cheers


andrew


--
Andrew Dunstan
EDB: https://www.enterprisedb.com

  89. Re: SQL/JSON: functions

    Himanshu Upadhyaya <upadhyaya.himanshu@gmail.com> — 2021-12-02T06:07:47Z

    On Wed, Dec 1, 2021 at 7:56 PM Andrew Dunstan <andrew@dunslane.net> wrote:

>
> On 12/1/21 06:13, Himanshu Upadhyaya wrote:
> > Hi Andrew,
> >
> > The latest version (v59) is not applying on head.
> > Could you please help to rebase?
> >
> >
>
> (Please don't top-post on PostgreSQL lists)
>
> Sure, I will take care of that in the future.

The patches apply for me and for the cfbot:
> <http://cfbot.cputube.org/patch_35_2901.log>. I'm not sure what's not
> working for you. I apply them using "patch -p 1 < $patchfile"
>
> Mistakenly I was using git apply, sorry about that. It's working fine with
"patch -p 1 < $patchfile".

-- 
Regards,
Himanshu Upadhyaya
EnterpriseDB: http://www.enterprisedb.com

  90. Re: SQL/JSON: functions

    Himanshu Upadhyaya <upadhyaya.himanshu@gmail.com> — 2021-12-09T14:04:37Z

    On Thu, Sep 16, 2021 at 8:23 PM Andrew Dunstan <andrew@dunslane.net> wrote:

>
> On 9/14/21 8:55 AM, Andrew Dunstan wrote:
>

 I have tried with few of the test cases of constructor function, wanted to
check on the below scenarios:

1)
Why we don't support KEY(however is optional as per SQL standard) keyword?
SELECT JSON_OBJECT(KEY 'a' VALUE '123');
ERROR:  type "key" does not exist
LINE 1: SELECT JSON_OBJECT(KEY 'a' VALUE '123');

ORACLE is supporting the above syntax.

I can see TODO as below
+json_name_and_value:
+/* TODO This is not supported due to conflicts
+                       KEY c_expr VALUE_P json_value_expr %prec POSTFIXOP
+                               { $$ = makeJsonKeyValue($2, $4); }
+                       |
+*/

but still not very clear what kind of conflict we are mentioning here, also
any plan of finding a solution to that conflict?

2)
I am not sure if below is required as per SQL standard, ORACLE is allowing
to construct JSON_OBJECT bases on the records in the table as below, but
postgres parser is not allowing:
create table test (id varchar(10), value int);
insert into test values ('a',1);
insert into test values ('b',2);
insert into test values ('c',3);
select json_object(*) from test; --postgres does not support
postgres=# select json_object(*) from test;
ERROR:  syntax error at or near "*"
LINE 1: select json_object(*) from test;

3)
Is not that result of the two below queries should match because both are
trying to retrieve the information from the JSON object.

postgres=# SELECT JSON_OBJECT('track' VALUE '{
    "segments": [
      {
        "location":   [ 47.763, 13.4034 ],
        "start time": "2018-10-14 10:05:14",
        "HR": 73
      },
      {
        "location":   [ 47.706, 13.2635 ],
        "start time": "2018-10-14 101:39:21",
        "HR": 135
      }
    ]
  }
}')->'track'->'segments';
 ?column?
----------

(1 row)

postgres=# select '{
  "track": {
    "segments": [
      {
        "location":   [ 47.763, 13.4034 ],
        "start time": "2018-10-14 10:05:14",
        "HR": 73
      },
      {
        "location":   [ 47.706, 13.2635 ],
        "start time": "2018-10-14 10:39:21",
        "HR": 135
      }
    ]
  }
}'::jsonb->'track'->'segments';

 ?column?
-------------------------------------------------------------------------------------------------------------------------------------------------------------------
 [{"HR": 73, "location": [47.763, 13.4034], "start time": "2018-10-14
10:05:14"}, {"HR": 135, "location": [47.706, 13.2635], "start time":
"2018-10-14 10:39:21"}]
(1 row)

4)
Are we intentionally allowing numeric keys in JSON_OBJECT but somehow these
are not allowed in ORACLE?
‘postgres[151876]=#’select JSON_OBJECT( 3+1:2, 2+2:1);
    json_object
--------------------
 {"4" : 2, "4" : 1}
(1 row)

In ORACLE we are getting error("ORA-00932: inconsistent datatypes: expected
CHAR got NUMBER") which seems to be more reasonable.
"ORA-00932: inconsistent datatypes: expected CHAR got NUMBER"

Postgres is also dis-allowing below then why allow numeric keys in
JSON_OBJECT?
‘postgres[151876]=#’select '{
  "track": {
    "segments": [
      {
        "location":   [ 47.763, 13.4034 ],
        "start time": "2018-10-14 10:05:14",
        "HR": 73
      },
      {
        "location":   [ 47.706, 13.2635 ],
        "start time": "2018-10-14 10:39:21",
        3: 135
      }
    ]
  }
}'::jsonb;
ERROR:  22P02: invalid input syntax for type json
LINE 1: select '{
               ^
DETAIL:  Expected string, but found "3".
CONTEXT:  JSON data, line 12:         3...
LOCATION:  json_ereport_error, jsonfuncs.c:621

Also, JSON_OBJECTAGG is failing if we have any numeric key, however, the
message is not very appropriate.
SELECT JSON_OBJECTAGG(k: v ABSENT ON NULL) AS apt
FROM (VALUES ('no', 5), ('area', 50), ('rooms', 2), ('foo', NULL), (5,5))
kv(k, v);
ERROR:  22P02: invalid input syntax for type integer: "no"
LINE 2: FROM (VALUES ('no', 5), ('area', 50), ('rooms', 2), ('foo', ...
                      ^
LOCATION:  pg_strtoint32, numutils.c:320


Few comments For 0002-SQL-JSON-constructors-v59.patch:
1)
+       if (IsA(node, JsonConstructorExpr))
+       {
+               JsonConstructorExpr *ctor = (JsonConstructorExpr *) node;
+               ListCell   *lc;
+               bool            is_jsonb =
+                       ctor->returning->format->format == JS_FORMAT_JSONB;
+
+               /* Check argument_type => json[b] conversions */
+               foreach(lc, ctor->args)
+               {
+                       Oid                     typid =
exprType(lfirst(lc));
+
+                       if (is_jsonb ?
+                               !to_jsonb_is_immutable(typid) :
+                               !to_json_is_immutable(typid))
+                               return true;
+               }
+
+               /* Check all subnodes */
+       }
can have ctor as const pointer?

2)
+typedef struct JsonFormat
+{
+       NodeTag         type;
+       JsonFormatType format;          /* format type */
+       JsonEncoding encoding;          /* JSON encoding */
+       int                     location;               /* token location,
or -1 if unknown */
+} JsonFormat;

I think it will be good if we can have a JsonformatType(defined in patch
0001-Common-SQL-JSON-clauses-v59.patch) member named as
format_type or formatType instead of format?
There are places in the patch where we access it as "if (format->format ==
JS_FORMAT_DEFAULT)". "format->format" looks little difficult to understand.
"format->format_type == JS_FORMAT_DEFAULT" will be easy to follow.

3)
+               if (have_jsonb)
+               {
+                       returning->typid = JSONBOID;
+                       returning->format->format = JS_FORMAT_JSONB;
+               }
+               else if (have_json)
+               {
+                       returning->typid = JSONOID;
+                       returning->format->format = JS_FORMAT_JSON;
+               }
+               else
+               {
+                       /* XXX TEXT is default by the standard, but we
return JSON */
+                       returning->typid = JSONOID;
+                       returning->format->format = JS_FORMAT_JSON;
+               }

why we need a separate "else if (have_json)" statement in the below code,
"else" is also doing the same thing?

4)
-test: json jsonb json_encoding jsonpath jsonpath_encoding jsonb_jsonpath
+test: json jsonb json_encoding jsonpath jsonpath_encoding jsonb_jsonpath
sqljson

can we rename sqljson sql test file to json_constructor?

-- 
Regards,
Himanshu Upadhyaya
EnterpriseDB: http://www.enterprisedb.com

  91. Re: SQL/JSON: functions

    Peter Eisentraut <peter.eisentraut@enterprisedb.com> — 2021-12-09T15:08:50Z

    On 09.12.21 15:04, Himanshu Upadhyaya wrote:
> 1)
> Why we don't support KEY(however is optional as per SQL standard) keyword?
> SELECT JSON_OBJECT(KEY 'a' VALUE '123');
> ERROR:  type "key" does not exist
> LINE 1: SELECT JSON_OBJECT(KEY 'a' VALUE '123');
> 
> ORACLE is supporting the above syntax.
> 
> I can see TODO as below
> +json_name_and_value:
> +/* TODO This is not supported due to conflicts
> +                       KEY c_expr VALUE_P json_value_expr %prec POSTFIXOP
> +                               { $$ = makeJsonKeyValue($2, $4); }
> +                       |
> +*/
> 
> but still not very clear what kind of conflict we are mentioning here, 
> also any plan of finding a solution to that conflict?

The conflict is this:

Consider in subclause 6.33, “<JSON value constructor>”:

<JSON name and value> ::= [ KEY ] <JSON name> VALUE <JSON input expression>
| ...

Because KEY is a <non-reserved word>, this creates an ambiguity. For 
example:

key(x) VALUE foo

could be

KEY x VALUE foo

with KEY being the key word and “x” (a <column reference>) as “<JSON 
name>”, or

KEY key(x) VALUE foo

with “key(x)” (a <routine invocation>) as “<JSON name>”.

In existing implementations, KEY is resolved as a keyword.  So if you 
can figure out a way to implement that, go ahead, but I imagine it might 
be tricky.

  92. Re: SQL/JSON: functions

    Andrew Dunstan <andrew@dunslane.net> — 2021-12-15T21:36:26Z

    On 12/9/21 09:04, Himanshu Upadhyaya wrote:
>
>
>
> 4)
> Are we intentionally allowing numeric keys in JSON_OBJECT but somehow
> these are not allowed in ORACLE?
> ‘postgres[151876]=#’select JSON_OBJECT( 3+1:2, 2+2:1);
>     json_object
> --------------------
>  {"4" : 2, "4" : 1}
> (1 row)
>
> In ORACLE we are getting error("ORA-00932: inconsistent datatypes:
> expected CHAR got NUMBER") which seems to be more reasonable.
> "ORA-00932: inconsistent datatypes: expected CHAR got NUMBER"
>
> Postgres is also dis-allowing below then why allow numeric keys in
> JSON_OBJECT?
> ‘postgres[151876]=#’select '{
>   "track": {
>     "segments": [
>       {
>         "location":   [ 47.763, 13.4034 ],
>         "start time": "2018-10-14 10:05:14",
>         "HR": 73
>       },
>       {
>         "location":   [ 47.706, 13.2635 ],
>         "start time": "2018-10-14 10:39:21",
>         3: 135
>       }
>     ]
>   }
> }'::jsonb;
> ERROR:  22P02: invalid input syntax for type json
> LINE 1: select '{
>                ^
> DETAIL:  Expected string, but found "3".
> CONTEXT:  JSON data, line 12:         3...
> LOCATION:  json_ereport_error, jsonfuncs.c:621
>
> Also, JSON_OBJECTAGG is failing if we have any numeric key, however,
> the message is not very appropriate.
> SELECT JSON_OBJECTAGG(k: v ABSENT ON NULL) AS apt
> FROM (VALUES ('no', 5), ('area', 50), ('rooms', 2), ('foo', NULL),
> (5,5)) kv(k, v);
> ERROR:  22P02: invalid input syntax for type integer: "no"
> LINE 2: FROM (VALUES ('no', 5), ('area', 50), ('rooms', 2), ('foo', ...
>                       ^
> LOCATION:  pg_strtoint32, numutils.c:320
>
>
>

The literal above is simply not legal json, so the json parser is going
to reject it outright. However it is quite reasonable for JSON
constructors to convert non-string key values to strings. Otherwise we'd
be rejecting not just numbers but for example dates as key values. c.f.
json_build_object(), the documentation for which says "Key arguments are
coerced to text."


cheers


andrew

--
Andrew Dunstan
EDB: https://www.enterprisedb.com

  93. Re: SQL/JSON: functions

    Andrew Dunstan <andrew@dunslane.net> — 2021-12-15T21:48:09Z

    On 12/9/21 09:04, Himanshu Upadhyaya wrote:
>
>
>
> 2)
> I am not sure if below is required as per SQL standard, ORACLE is
> allowing to construct JSON_OBJECT bases on the records in the table as
> below, but postgres parser is not allowing:
> create table test (id varchar(10), value int);
> insert into test values ('a',1);
> insert into test values ('b',2);
> insert into test values ('c',3);
> select json_object(*) from test; --postgres does not support
> postgres=# select json_object(*) from test;
> ERROR:  syntax error at or near "*"
> LINE 1: select json_object(*) from test;
>
>

You can spell that a bit differently today, e.g.

    select to_json(r) from test r;

I don't know either if it's in the spec, but building in support for *
in this context seems likely to be fairly complex and have very little
added utility.


cheers


andrew

--
Andrew Dunstan
EDB: https://www.enterprisedb.com

  94. Re: SQL/JSON: functions

    Himanshu Upadhyaya <upadhyaya.himanshu@gmail.com> — 2022-01-04T09:18:33Z

    On Thu, Dec 16, 2021 at 3:06 AM Andrew Dunstan <andrew@dunslane.net> wrote:
>
>
> On 12/9/21 09:04, Himanshu Upadhyaya wrote:
> >
> >
> >
> > 4)
> > Are we intentionally allowing numeric keys in JSON_OBJECT but somehow
> > these are not allowed in ORACLE?
> > ‘postgres[151876]=#’select JSON_OBJECT( 3+1:2, 2+2:1);
> >     json_object
> > --------------------
> >  {"4" : 2, "4" : 1}
> > (1 row)
> >
> > In ORACLE we are getting error("ORA-00932: inconsistent datatypes:
> > expected CHAR got NUMBER") which seems to be more reasonable.
> > "ORA-00932: inconsistent datatypes: expected CHAR got NUMBER"
> >
> > Postgres is also dis-allowing below then why allow numeric keys in
> > JSON_OBJECT?
> > ‘postgres[151876]=#’select '{
> >   "track": {
> >     "segments": [
> >       {
> >         "location":   [ 47.763, 13.4034 ],
> >         "start time": "2018-10-14 10:05:14",
> >         "HR": 73
> >       },
> >       {
> >         "location":   [ 47.706, 13.2635 ],
> >         "start time": "2018-10-14 10:39:21",
> >         3: 135
> >       }
> >     ]
> >   }
> > }'::jsonb;
> > ERROR:  22P02: invalid input syntax for type json
> > LINE 1: select '{
> >                ^
> > DETAIL:  Expected string, but found "3".
> > CONTEXT:  JSON data, line 12:         3...
> > LOCATION:  json_ereport_error, jsonfuncs.c:621
> >
> > Also, JSON_OBJECTAGG is failing if we have any numeric key, however,
> > the message is not very appropriate.
> > SELECT JSON_OBJECTAGG(k: v ABSENT ON NULL) AS apt
> > FROM (VALUES ('no', 5), ('area', 50), ('rooms', 2), ('foo', NULL),
> > (5,5)) kv(k, v);
> > ERROR:  22P02: invalid input syntax for type integer: "no"
> > LINE 2: FROM (VALUES ('no', 5), ('area', 50), ('rooms', 2), ('foo', ...
> >                       ^
> > LOCATION:  pg_strtoint32, numutils.c:320
> >
> >
> >
>
> The literal above is simply not legal json, so the json parser is going
> to reject it outright. However it is quite reasonable for JSON
> constructors to convert non-string key values to strings. Otherwise we'd
> be rejecting not just numbers but for example dates as key values. c.f.
> json_build_object(), the documentation for which says "Key arguments are
> coerced to text."
>
Yes Agree on this, but just thinking if we can differentiate dates and
numeric keys to have consistent behaviour and simply reject if we have
numeric keys(to match it with the behaviour of JSON parser) because
JSON with numeric keys is actually not a valid JSON.

SELECT JSON_OBJECTAGG(k: v ABSENT ON NULL) AS apt
FROM (VALUES ('no', 5), ('area', 50), ('rooms', 2), ('foo', NULL),
(5,5)) kv(k, v);
ERROR:  22P02: invalid input syntax for type integer: "no"
LINE 2: FROM (VALUES ('no', 5), ('area', 50), ('rooms', 2), ('foo', ...
                      ^
LOCATION:  pg_strtoint32, numutils.c:320

Above call to JSON_OBJECTAGG is failing because we have the numeric
key, is not that it also needs to follow the same context  of
converting key argument to text? or both(JSON_OBJECTAGG  and
JSON_OBJECT) should not allow numeric keys in the JSON object and
allow date (if that is the only use case)?

Thoughts?
-- 
Regards,
Himanshu Upadhyaya
EnterpriseDB: http://www.enterprisedb.com

  95. Re: SQL/JSON: functions

    Pavel Stehule <pavel.stehule@gmail.com> — 2022-01-04T09:55:06Z

    út 4. 1. 2022 v 10:19 odesílatel Himanshu Upadhyaya <
upadhyaya.himanshu@gmail.com> napsal:

> On Thu, Dec 16, 2021 at 3:06 AM Andrew Dunstan <andrew@dunslane.net>
> wrote:
> >
> >
> > On 12/9/21 09:04, Himanshu Upadhyaya wrote:
> > >
> > >
> > >
> > > 4)
> > > Are we intentionally allowing numeric keys in JSON_OBJECT but somehow
> > > these are not allowed in ORACLE?
> > > ‘postgres[151876]=#’select JSON_OBJECT( 3+1:2, 2+2:1);
> > >     json_object
> > > --------------------
> > >  {"4" : 2, "4" : 1}
> > > (1 row)
> > >
> > > In ORACLE we are getting error("ORA-00932: inconsistent datatypes:
> > > expected CHAR got NUMBER") which seems to be more reasonable.
> > > "ORA-00932: inconsistent datatypes: expected CHAR got NUMBER"
> > >
> > > Postgres is also dis-allowing below then why allow numeric keys in
> > > JSON_OBJECT?
> > > ‘postgres[151876]=#’select '{
> > >   "track": {
> > >     "segments": [
> > >       {
> > >         "location":   [ 47.763, 13.4034 ],
> > >         "start time": "2018-10-14 10:05:14",
> > >         "HR": 73
> > >       },
> > >       {
> > >         "location":   [ 47.706, 13.2635 ],
> > >         "start time": "2018-10-14 10:39:21",
> > >         3: 135
> > >       }
> > >     ]
> > >   }
> > > }'::jsonb;
> > > ERROR:  22P02: invalid input syntax for type json
> > > LINE 1: select '{
> > >                ^
> > > DETAIL:  Expected string, but found "3".
> > > CONTEXT:  JSON data, line 12:         3...
> > > LOCATION:  json_ereport_error, jsonfuncs.c:621
> > >
> > > Also, JSON_OBJECTAGG is failing if we have any numeric key, however,
> > > the message is not very appropriate.
> > > SELECT JSON_OBJECTAGG(k: v ABSENT ON NULL) AS apt
> > > FROM (VALUES ('no', 5), ('area', 50), ('rooms', 2), ('foo', NULL),
> > > (5,5)) kv(k, v);
> > > ERROR:  22P02: invalid input syntax for type integer: "no"
> > > LINE 2: FROM (VALUES ('no', 5), ('area', 50), ('rooms', 2), ('foo', ...
> > >                       ^
> > > LOCATION:  pg_strtoint32, numutils.c:320
> > >
> > >
> > >
> >
> > The literal above is simply not legal json, so the json parser is going
> > to reject it outright. However it is quite reasonable for JSON
> > constructors to convert non-string key values to strings. Otherwise we'd
> > be rejecting not just numbers but for example dates as key values. c.f.
> > json_build_object(), the documentation for which says "Key arguments are
> > coerced to text."
> >
> Yes Agree on this, but just thinking if we can differentiate dates and
> numeric keys to have consistent behaviour and simply reject if we have
> numeric keys(to match it with the behaviour of JSON parser) because
> JSON with numeric keys is actually not a valid JSON.
>
> +1

Pavel



> SELECT JSON_OBJECTAGG(k: v ABSENT ON NULL) AS apt
> FROM (VALUES ('no', 5), ('area', 50), ('rooms', 2), ('foo', NULL),
> (5,5)) kv(k, v);
> ERROR:  22P02: invalid input syntax for type integer: "no"
> LINE 2: FROM (VALUES ('no', 5), ('area', 50), ('rooms', 2), ('foo', ...
>                       ^
> LOCATION:  pg_strtoint32, numutils.c:320
>
> Above call to JSON_OBJECTAGG is failing because we have the numeric
> key, is not that it also needs to follow the same context  of
> converting key argument to text? or both(JSON_OBJECTAGG  and
> JSON_OBJECT) should not allow numeric keys in the JSON object and
> allow date (if that is the only use case)?
>
> Thoughts?
> --
> Regards,
> Himanshu Upadhyaya
> EnterpriseDB: http://www.enterprisedb.com
>
>
>

  96. Re: SQL/JSON: functions

    Andrew Dunstan <andrew@dunslane.net> — 2022-01-04T13:53:13Z

    On 1/4/22 04:18, Himanshu Upadhyaya wrote:
> On Thu, Dec 16, 2021 at 3:06 AM Andrew Dunstan <andrew@dunslane.net> wrote:
>>
>>
>> SELECT JSON_OBJECTAGG(k: v ABSENT ON NULL) AS apt
>> FROM (VALUES ('no', 5), ('area', 50), ('rooms', 2), ('foo', NULL),
>> (5,5)) kv(k, v);
>> ERROR:  22P02: invalid input syntax for type integer: "no"
>> LINE 2: FROM (VALUES ('no', 5), ('area', 50), ('rooms', 2), ('foo', ...
>>                       ^
>> LOCATION:  pg_strtoint32, numutils.c:320
>>
>> Above call to JSON_OBJECTAGG is failing because we have the numeric
>> key, is not that it also needs to follow the same context  of
>> converting key argument to text? or both(JSON_OBJECTAGG  and
>> JSON_OBJECT) should not allow numeric keys in the JSON object and
>> allow date (if that is the only use case)?
>>


this error has nothing at all to do with the json code. You simply have
an invalid VALUES expression:


postgres=# select * FROM (VALUES ('no', 5), ('area', 50), ('rooms', 2),
('foo', NULL), (5,5)) kv(k, v);
ERROR:  invalid input syntax for type integer: "no"
LINE 1: select * FROM (VALUES ('no', 5), ('area', 50), ('rooms', 2),...

cheers


andrew

-- 

Andrew Dunstan
EDB: https://www.enterprisedb.com

  97. Re: SQL/JSON: functions

    Andrew Dunstan <andrew@dunslane.net> — 2022-01-04T14:01:31Z

    On 9/16/21 10:52, Andrew Dunstan wrote:
> On 9/14/21 8:55 AM, Andrew Dunstan wrote:
>> On 9/2/21 2:50 PM, Andrew Dunstan wrote:
>>> On 5/18/21 3:22 PM, Andrew Dunstan wrote:
>>>> On 5/8/21 2:21 PM, Andrew Dunstan wrote:
>>>>> On 4/28/21 5:55 PM, Andrew Dunstan wrote:
>>>>>> On Fri, Mar 26, 2021 at 9:14 PM Nikita Glukhov
>>>>>> <n.gluhov@postgrespro.ru <mailto:n.gluhov@postgrespro.ru>> wrote:
>>>>>>
>>>>>>     Attached 54th version of the patches rebased onto current master.
>>>>>>
>>>>>>     On 27.03.2021 01:30, Andrew Dunstan wrote:
>>>>>>>     Specifically, patch 4 (SQL-JSON-query-functions) fails with this when
>>>>>>>     built with LLVM:
>>>>>>>
>>>>>>>
>>>>>>>     There is also a bug that results in a warning in gram.y, but fixing it
>>>>>>>     doesn't affect this issue. Nikita, please look into this ASAP.
>>>>>>     LLVM issues and gram.y are fixed.
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>> It's apparently bitrotted again. See
>>>>>> <http://cfbot.cputube.org/patch_33_2901.log
>>>>>> <http://cfbot.cputube.org/patch_33_2901.log>>
>>>>>>
>>>>>>
>>>>> This set should remove the bitrot.
>>>>>
>>>>>
>>>> Rebased for removal of serial schedule
>>>>
>>>>
>>> rebased on master and incorporating fixes from Erik Rijkers
>>>
>>>
>> rebased to remove bitrot from the removal of the  Value node type.
>>
>>
> Rebased, and fixed a bug (which I had faithfully replicated above) in
> the APP_JUMB code, as reported by Erik Rijkers.
>
>

rebased once more


cheers


andrew


--
Andrew Dunstan
EDB: https://www.enterprisedb.com

  98. Re: SQL/JSON: functions

    Himanshu Upadhyaya <upadhyaya.himanshu@gmail.com> — 2022-01-05T05:51:44Z

    On Thu, Dec 9, 2021 at 7:34 PM Himanshu Upadhyaya
<upadhyaya.himanshu@gmail.com> wrote:
> 3)
> Is not that result of the two below queries should match because both are trying to retrieve the information from the JSON object.
>
> postgres=# SELECT JSON_OBJECT('track' VALUE '{
>     "segments": [
>       {
>         "location":   [ 47.763, 13.4034 ],
>         "start time": "2018-10-14 10:05:14",
>         "HR": 73
>       },
>       {
>         "location":   [ 47.706, 13.2635 ],
>         "start time": "2018-10-14 101:39:21",
>         "HR": 135
>       }
>     ]
>   }
> }')->'track'->'segments';
>  ?column?
> ----------
>
> (1 row)
>
> postgres=# select '{
>   "track": {
>     "segments": [
>       {
>         "location":   [ 47.763, 13.4034 ],
>         "start time": "2018-10-14 10:05:14",
>         "HR": 73
>       },
>       {
>         "location":   [ 47.706, 13.2635 ],
>         "start time": "2018-10-14 10:39:21",
>         "HR": 135
>       }
>     ]
>   }
> }'::jsonb->'track'->'segments';
>                                                                              ?column?
> -------------------------------------------------------------------------------------------------------------------------------------------------------------------
>  [{"HR": 73, "location": [47.763, 13.4034], "start time": "2018-10-14 10:05:14"}, {"HR": 135, "location": [47.706, 13.2635], "start time": "2018-10-14 10:39:21"}]
> (1 row)
>
just wanted to check your opinion on the above, is this an expected behaviour?

> Few comments For 0002-SQL-JSON-constructors-v59.patch:
Also, any thoughts on this?

-- 
Regards,
Himanshu Upadhyaya
EnterpriseDB: http://www.enterprisedb.com

  99. Re: SQL/JSON: functions

    Andrew Dunstan <andrew@dunslane.net> — 2022-01-05T17:03:15Z

    On 1/5/22 00:51, Himanshu Upadhyaya wrote:
> On Thu, Dec 9, 2021 at 7:34 PM Himanshu Upadhyaya
> <upadhyaya.himanshu@gmail.com> wrote:
>> 3)
>> Is not that result of the two below queries should match because both are trying to retrieve the information from the JSON object.
>>
>> postgres=# SELECT JSON_OBJECT('track' VALUE '{
>>     "segments": [
>>       {
>>         "location":   [ 47.763, 13.4034 ],
>>         "start time": "2018-10-14 10:05:14",
>>         "HR": 73
>>       },
>>       {
>>         "location":   [ 47.706, 13.2635 ],
>>         "start time": "2018-10-14 101:39:21",
>>         "HR": 135
>>       }
>>     ]
>>   }
>> }')->'track'->'segments';
>>  ?column?
>> ----------
>>
>> (1 row)
>>
>> postgres=# select '{
>>   "track": {
>>     "segments": [
>>       {
>>         "location":   [ 47.763, 13.4034 ],
>>         "start time": "2018-10-14 10:05:14",
>>         "HR": 73
>>       },
>>       {
>>         "location":   [ 47.706, 13.2635 ],
>>         "start time": "2018-10-14 10:39:21",
>>         "HR": 135
>>       }
>>     ]
>>   }
>> }'::jsonb->'track'->'segments';
>>                                                                              ?column?
>> -------------------------------------------------------------------------------------------------------------------------------------------------------------------
>>  [{"HR": 73, "location": [47.763, 13.4034], "start time": "2018-10-14 10:05:14"}, {"HR": 135, "location": [47.706, 13.2635], "start time": "2018-10-14 10:39:21"}]
>> (1 row)
>>
> just wanted to check your opinion on the above, is this an expected behaviour?


Your VALUE clause is actually not legal JSON - it has one too many
braces at the end. The reason postgres didn't complain about it is that
JSON_OBJECT is treating it as a string. If you correct the JSON and cast
it as jsonb you get the desired result:

andrew=# SELECT JSON_OBJECT('track' VALUE '{
    "segments": [
      {
        "location":   [ 47.763, 13.4034 ],
        "start time": "2018-10-14 10:05:14",
        "HR": 73
      },
      {
        "location":   [ 47.706, 13.2635 ],
        "start time": "2018-10-14 101:39:21",
        "HR": 135
      }
    ]
  }'::jsonb)->'track'->'segments';
                                                                              ?column?                                                                              
--------------------------------------------------------------------------------------------------------------------------------------------------------------------
 [{"HR": 73, "location": [47.763, 13.4034], "start time": "2018-10-14 10:05:14"}, {"HR": 135, "location": [47.706, 13.2635], "start time": "2018-10-14 101:39:21"}]
(1 row)


>> Few comments For 0002-SQL-JSON-constructors-v59.patch:
> Also, any thoughts on this?



I will look at that separately.


cheers


andrew


--
Andrew Dunstan
EDB: https://www.enterprisedb.com

  100. Re: SQL/JSON: functions

    Himanshu Upadhyaya <upadhyaya.himanshu@gmail.com> — 2022-01-06T11:24:32Z

    On Tue, Jan 4, 2022 at 7:32 PM Andrew Dunstan <andrew@dunslane.net> wrote:

I have one general question on the below scenario.
CREATE TABLE T (Id INTEGER PRIMARY KEY,Jcol CHARACTER VARYING ( 5000
)CHECK ( Jcol IS JSON ) );
insert into T values (1,323);
 ORACLE is giving an error(check constraint...violated ORA-06512) for
the above insert but Postgres is allowing it, however is not related
to this patch but just thinking if this is expected.

‘postgres[22198]=#’SELECT * FROM T WHERE Jcol IS JSON;
 id | jcol
----+------
  1 | 323
How come number 323 is the valid json?

Few comments/doubts on 0003-IS-JSON-predicate-v59.patch and
0004-SQL-JSON-query-functions-v59.patch patch:
1) I am not able to find a case where "IS JSON" and "IS JSON VALUE"
gives a different result, is they intended to give the same result(and
two are replaceably used) when applied on any input.

2) Not sure why we return true for the below query?
+-- extension: boolean expressions
+SELECT JSON_EXISTS(jsonb '1', '$ > 2');
+ json_exists
+-------------
+ t
+(1 row)

3)
+-- Strict mode with ERROR on ERROR clause
+SELECT JSON_EXISTS(jsonb '{"a": [1,2,3]}', 'strict $.a[5]' ERROR ON ERROR);
+ERROR: Invalid SQL/JSON subscript

The above example in documentation is not actually matching when I am
trying to run with the patch as below.
‘postgres[28411]=#’SELECT JSON_EXISTS(jsonb '{"a": [1,2,3]}', 'strict
$.a[5]' ERROR ON ERROR);
ERROR:  22033: jsonpath array subscript is out of bounds
LOCATION:  executeItemOptUnwrapTarget, jsonpath_exec.c:769

+SELECT JSON_VALUE('"123.45"', '$' RETURNING float);
+ json_value
+------------
+     123.45
+(1 row)

Above is also not matching:
‘postgres[28411]=#’SELECT JSON_VALUE('"123.45"', '$' RETURNING float);
ERROR:  0A000: JSON_VALUE() is not yet implemented for json type
LINE 1: SELECT JSON_VALUE('"123.45"', '$' RETURNING float);

There is more such example that does not actually produce the same
result when we try to run after applying this patch, seems like we
just need to update the documentation with regards to our new patch.
+SELECT JSON_VALUE(jsonb '[1,2]', 'strict $[*]' ERROR ON ERROR);
+ERROR: more than one SQL/JSON item

‘postgres[28411]=#’SELECT JSON_VALUE(jsonb '[1,2]', 'strict $[*]'
ERROR ON ERROR);
ERROR:  22034: JSON path expression in JSON_VALUE should return
singleton scalar item

4)
index f46786231e..c1951c1caf 100644
--- a/src/backend/optimizer/util/clauses.c
+++ b/src/backend/optimizer/util/clauses.c
@@ -28,6 +28,7 @@
 #include "catalog/pg_type.h"
 #include "executor/executor.h"
 #include "executor/functions.h"
+#include "executor/execExpr.h"
 #include "funcapi.h"
 #include "miscadmin.h"
 #include "nodes/makefuncs.h"

can we adjust the include file in the alphabetic order please?

5)
+SELECT
+       JSON_QUERY(js, '$'),
+       JSON_QUERY(js, '$' WITHOUT WRAPPER),
+       JSON_QUERY(js, '$' WITH CONDITIONAL WRAPPER),
+       JSON_QUERY(js, '$' WITH UNCONDITIONAL ARRAY WRAPPER),
+       JSON_QUERY(js, '$' WITH ARRAY WRAPPER)
+FROM
+       (VALUES
+               (jsonb 'null'),
+               ('12.3'),
+               ('true'),
+               ('"aaa"'),
+               ('[1, null, "2"]'),
+               ('{"a": 1, "b": [2]}')
+       ) foo(js);
+     json_query     |     json_query     |     json_query     |
json_query      |      json_query
+--------------------+--------------------+--------------------+----------------------+----------------------
+ null               | null               | [null]             |
[null]               | [null]
+ 12.3               | 12.3               | [12.3]             |
[12.3]               | [12.3]
+ true               | true               | [true]             |
[true]               | [true]
+ "aaa"              | "aaa"              | ["aaa"]            |
["aaa"]              | ["aaa"]
+ [1, null, "2"]     | [1, null, "2"]     | [1, null, "2"]     | [[1,
null, "2"]]     | [[1, null, "2"]]
+ {"a": 1, "b": [2]} | {"a": 1, "b": [2]} | {"a": 1, "b": [2]} |
[{"a": 1, "b": [2]}] | [{"a": 1, "b": [2]}]
+(6 rows)

Just a suggestion if we can have column aliases for better
understanding like we are doing for other test cases in the same
patch?

-- 
Regards,
Himanshu Upadhyaya
EnterpriseDB: http://www.enterprisedb.com

  101. Re: SQL/JSON: functions

    Andrew Dunstan <andrew@dunslane.net> — 2022-01-06T16:35:52Z

    On 1/6/22 06:24, Himanshu Upadhyaya wrote:
> I have one general question on the below scenario.
> CREATE TABLE T (Id INTEGER PRIMARY KEY,Jcol CHARACTER VARYING ( 5000
> )CHECK ( Jcol IS JSON ) );
> insert into T values (1,323);
>  ORACLE is giving an error(check constraint...violated ORA-06512) for
> the above insert but Postgres is allowing it, however is not related
> to this patch but just thinking if this is expected.
>
> ‘postgres[22198]=#’SELECT * FROM T WHERE Jcol IS JSON;
>  id | jcol
> ----+------
>   1 | 323
> How come number 323 is the valid json?


If you look at the JSON grammar at <https://www.json.org/json-en.html>
or
<https://www.ecma-international.org/wp-content/uploads/ECMA-404_2nd_edition_december_2017.pdf>
it's clear that a bare number is valid json. Our parser implements that
grammar pretty faithfully, in fact rather more faithfully than many
implementations (e.g. we allow huge number strings). So as far as I'm
concerned, we are right and Oracle is wrong. It would hardly be the
first time such a thing has happened.

Oracle is not the definer of the JSON standard. ECMA is.


cheers


andrew


--
Andrew Dunstan
EDB: https://www.enterprisedb.com

  102. Re: SQL/JSON: functions

    Andrew Dunstan <andrew@dunslane.net> — 2022-01-12T20:49:41Z

    On 12/9/21 09:04, Himanshu Upadhyaya wrote:
>
>
>
> Few comments For 0002-SQL-JSON-constructors-v59.patch:
> 1)
> +       if (IsA(node, JsonConstructorExpr))
> +       {
> +               JsonConstructorExpr *ctor = (JsonConstructorExpr *) node;
> +               ListCell   *lc;
> +               bool            is_jsonb =
> +                       ctor->returning->format->format ==
> JS_FORMAT_JSONB;
> +
> +               /* Check argument_type => json[b] conversions */
> +               foreach(lc, ctor->args)
> +               {
> +                       Oid                     typid =
> exprType(lfirst(lc));
> +
> +                       if (is_jsonb ?
> +                               !to_jsonb_is_immutable(typid) :
> +                               !to_json_is_immutable(typid))
> +                               return true;
> +               }
> +
> +               /* Check all subnodes */
> +       }
> can have ctor as const pointer?


I guess we could, although I'm not sure it's an important advance.


>
> 2)
> +typedef struct JsonFormat
> +{
> +       NodeTag         type;
> +       JsonFormatType format;          /* format type */
> +       JsonEncoding encoding;          /* JSON encoding */
> +       int                     location;               /* token
> location, or -1 if unknown */
> +} JsonFormat;
>
> I think it will be good if we can have a JsonformatType(defined in
> patch 0001-Common-SQL-JSON-clauses-v59.patch) member named as
> format_type or formatType instead of format?
> There are places in the patch where we access it as "if
> (format->format == JS_FORMAT_DEFAULT)". "format->format" looks little
> difficult to understand.
> "format->format_type == JS_FORMAT_DEFAULT" will be easy to follow.


That's a fair criticism.



>
> 3)
> +               if (have_jsonb)
> +               {
> +                       returning->typid = JSONBOID;
> +                       returning->format->format = JS_FORMAT_JSONB;
> +               }
> +               else if (have_json)
> +               {
> +                       returning->typid = JSONOID;
> +                       returning->format->format = JS_FORMAT_JSON;
> +               }
> +               else
> +               {
> +                       /* XXX TEXT is default by the standard, but we
> return JSON */
> +                       returning->typid = JSONOID;
> +                       returning->format->format = JS_FORMAT_JSON;
> +               }
>
> why we need a separate "else if (have_json)" statement in the below
> code, "else" is also doing the same thing?



I imagine it's more or less a placeholder in case we want to do
something else in the default case. But I agree it's confusing.


>
> 4)
> -test: json jsonb json_encoding jsonpath jsonpath_encoding jsonb_jsonpath
> +test: json jsonb json_encoding jsonpath jsonpath_encoding
> jsonb_jsonpath sqljson
>
> can we rename sqljson sql test file to json_constructor?


Not really - the later patches in the series add to it, so it ends up
being more than just constructor tests.


Thanks for reviewing,


cheers


andrew

--
Andrew Dunstan
EDB: https://www.enterprisedb.com

  103. Re: SQL/JSON: functions

    Julien Rouhaud <rjuju123@gmail.com> — 2022-01-18T06:33:14Z

    Hi,

The last version conflicts with recent c4cc2850f4d1 (Rename value node fields).
Can you send a rebased version?

  104. Re: SQL/JSON: functions

    Andrew Dunstan <andrew@dunslane.net> — 2022-01-18T21:22:36Z

    On 1/18/22 01:33, Julien Rouhaud wrote:
> Hi,
>
> The last version conflicts with recent c4cc2850f4d1 (Rename value node fields).
> Can you send a rebased version?


Attached. I didn't make the minor changes discussed with Himanshu
upthread. They aren't terribly urgent, but I haven't forgotten them.


cheers


andrew

--
Andrew Dunstan
EDB: https://www.enterprisedb.com

  105. Re: SQL/JSON: functions

    Andrew Dunstan <andrew@dunslane.net> — 2022-02-01T19:11:32Z

    On 1/12/22 15:49, Andrew Dunstan wrote:
> On 12/9/21 09:04, Himanshu Upadhyaya wrote:
>>
>>
>> Few comments For 0002-SQL-JSON-constructors-v59.patch:
>> 1)
>> +       if (IsA(node, JsonConstructorExpr))
>> +       {
>> +               JsonConstructorExpr *ctor = (JsonConstructorExpr *) node;
>> +               ListCell   *lc;
>> +               bool            is_jsonb =
>> +                       ctor->returning->format->format ==
>> JS_FORMAT_JSONB;
>> +
>> +               /* Check argument_type => json[b] conversions */
>> +               foreach(lc, ctor->args)
>> +               {
>> +                       Oid                     typid =
>> exprType(lfirst(lc));
>> +
>> +                       if (is_jsonb ?
>> +                               !to_jsonb_is_immutable(typid) :
>> +                               !to_json_is_immutable(typid))
>> +                               return true;
>> +               }
>> +
>> +               /* Check all subnodes */
>> +       }
>> can have ctor as const pointer?
>
> I guess we could, although I'm not sure it's an important advance.
>
>
>> 2)
>> +typedef struct JsonFormat
>> +{
>> +       NodeTag         type;
>> +       JsonFormatType format;          /* format type */
>> +       JsonEncoding encoding;          /* JSON encoding */
>> +       int                     location;               /* token
>> location, or -1 if unknown */
>> +} JsonFormat;
>>
>> I think it will be good if we can have a JsonformatType(defined in
>> patch 0001-Common-SQL-JSON-clauses-v59.patch) member named as
>> format_type or formatType instead of format?
>> There are places in the patch where we access it as "if
>> (format->format == JS_FORMAT_DEFAULT)". "format->format" looks little
>> difficult to understand.
>> "format->format_type == JS_FORMAT_DEFAULT" will be easy to follow.
>
> That's a fair criticism.
>
>
>
>> 3)
>> +               if (have_jsonb)
>> +               {
>> +                       returning->typid = JSONBOID;
>> +                       returning->format->format = JS_FORMAT_JSONB;
>> +               }
>> +               else if (have_json)
>> +               {
>> +                       returning->typid = JSONOID;
>> +                       returning->format->format = JS_FORMAT_JSON;
>> +               }
>> +               else
>> +               {
>> +                       /* XXX TEXT is default by the standard, but we
>> return JSON */
>> +                       returning->typid = JSONOID;
>> +                       returning->format->format = JS_FORMAT_JSON;
>> +               }
>>
>> why we need a separate "else if (have_json)" statement in the below
>> code, "else" is also doing the same thing?
>
>
> I imagine it's more or less a placeholder in case we want to do
> something else in the default case. But I agree it's confusing.
>
>
>> 4)
>> -test: json jsonb json_encoding jsonpath jsonpath_encoding jsonb_jsonpath
>> +test: json jsonb json_encoding jsonpath jsonpath_encoding
>> jsonb_jsonpath sqljson
>>
>> can we rename sqljson sql test file to json_constructor?
>
> Not really - the later patches in the series add to it, so it ends up
> being more than just constructor tests.
>
>
> Thanks for reviewing,
>
>


Here's a patch set with all of these except the last fixed.

There are a couple of things that bother me:

1. This involves a sizeable addition to the grammar, with something over
20 new keywords, including "string" as a  TYPE_FUNC_NAME_KEYWORD. I
guess we can't control what the SQL Standards Committee does, so if we
want to implement this we just need to suck it up. But it's annoying
that they are so profligate with grammar symbols.

2. The new GUC "sql_json" is a bit of a worry. I understand what it's
trying to do, but I'm trying to convince myself it's not going to be a
fruitful source of error reports, especially if people switch it in the
middle of a session. Maybe it should be an initdb option instead of a GUC?


cheers


andrew

--
Andrew Dunstan
EDB: https://www.enterprisedb.com

  106. Re: SQL/JSON: functions

    Andrew Dunstan <andrew@dunslane.net> — 2022-03-01T21:41:38Z

    On 2/1/22 14:11,I wrote:
>
> 2. The new GUC "sql_json" is a bit of a worry. I understand what it's
> trying to do, but I'm trying to convince myself it's not going to be a
> fruitful source of error reports, especially if people switch it in the
> middle of a session. Maybe it should be an initdb option instead of a GUC?
>
>


So far my efforts have not borne fruit. Here's why:


andrew=# set sql_json = jsonb;
SET
andrew=# create table abc (x text, y json);
CREATE TABLE
andrew=# \d abc
               Table "public.abc"
 Column | Type | Collation | Nullable | Default
--------+------+-----------+----------+---------
 x      | text |           |          |
 y      | json |           |          |

andrew=# insert into abc values ('a','{"q":1}');
INSERT 0 1
andrew=# select json_each(y) from abc;
ERROR:  function json_each(json) does not exist
LINE 1: select json_each(y) from abc;
               ^
HINT:  No function matches the given name and argument types. You might
need to add explicit type casts.
andrew=# select jsonb_each(y) from abc;
 jsonb_each
------------
 (q,1)
(1 row)


The description tells them the column is json, but the json_* functions
don't work on the column and you need to use the jsonb functions. That
seems to me a recipe for major confusion. It might be better if we set
it at initdb time so it couldn't be changed, but even so it could be
horribly confusing.

This is certainly severable from the rest of these patches. I'm not sure
how severable it is from the SQL/JSON Table patches.


cheers


andrew

--
Andrew Dunstan
EDB: https://www.enterprisedb.com

  107. Re: SQL/JSON: functions

    Andrew Dunstan <andrew@dunslane.net> — 2022-03-02T15:19:43Z

    On 3/1/22 16:41, Andrew Dunstan wrote:
> On 2/1/22 14:11,I wrote:
>> 2. The new GUC "sql_json" is a bit of a worry. I understand what it's
>> trying to do, but I'm trying to convince myself it's not going to be a
>> fruitful source of error reports, especially if people switch it in the
>> middle of a session. Maybe it should be an initdb option instead of a GUC?
>>
>>
>
> So far my efforts have not borne fruit. Here's why:
>
>
> andrew=# set sql_json = jsonb;
> SET
> andrew=# create table abc (x text, y json);
> CREATE TABLE
> andrew=# \d abc
>                Table "public.abc"
>  Column | Type | Collation | Nullable | Default
> --------+------+-----------+----------+---------
>  x      | text |           |          |
>  y      | json |           |          |
>
> andrew=# insert into abc values ('a','{"q":1}');
> INSERT 0 1
> andrew=# select json_each(y) from abc;
> ERROR:  function json_each(json) does not exist
> LINE 1: select json_each(y) from abc;
>                ^
> HINT:  No function matches the given name and argument types. You might
> need to add explicit type casts.
> andrew=# select jsonb_each(y) from abc;
>  jsonb_each
> ------------
>  (q,1)
> (1 row)
>
>
> The description tells them the column is json, but the json_* functions
> don't work on the column and you need to use the jsonb functions. That
> seems to me a recipe for major confusion. It might be better if we set
> it at initdb time so it couldn't be changed, but even so it could be
> horribly confusing.
>
> This is certainly severable from the rest of these patches. I'm not sure
> how severable it is from the SQL/JSON Table patches.
>
>

I have confirmed that this is not required at all for the JSON_TABLE
patch set.


I'll submit new patch sets omitting it shortly. The GUC patch can be
considered separately, probably as release 16 material, but I think as
is it's at best quite incomplete.


cheers


andrew

--
Andrew Dunstan
EDB: https://www.enterprisedb.com

  108. Re: SQL/JSON: functions

    Andrew Dunstan <andrew@dunslane.net> — 2022-03-04T16:28:47Z

    On 3/2/22 10:19, Andrew Dunstan wrote:
> On 3/1/22 16:41, Andrew Dunstan wrote:
>> On 2/1/22 14:11,I wrote:
>>> 2. The new GUC "sql_json" is a bit of a worry. I understand what it's
>>> trying to do, but I'm trying to convince myself it's not going to be a
>>> fruitful source of error reports, especially if people switch it in the
>>> middle of a session. Maybe it should be an initdb option instead of a GUC?
>>>
>>>
>> So far my efforts have not borne fruit. Here's why:
>>
>>
>> andrew=# set sql_json = jsonb;
>> SET
>> andrew=# create table abc (x text, y json);
>> CREATE TABLE
>> andrew=# \d abc
>>                Table "public.abc"
>>  Column | Type | Collation | Nullable | Default
>> --------+------+-----------+----------+---------
>>  x      | text |           |          |
>>  y      | json |           |          |
>>
>> andrew=# insert into abc values ('a','{"q":1}');
>> INSERT 0 1
>> andrew=# select json_each(y) from abc;
>> ERROR:  function json_each(json) does not exist
>> LINE 1: select json_each(y) from abc;
>>                ^
>> HINT:  No function matches the given name and argument types. You might
>> need to add explicit type casts.
>> andrew=# select jsonb_each(y) from abc;
>>  jsonb_each
>> ------------
>>  (q,1)
>> (1 row)
>>
>>
>> The description tells them the column is json, but the json_* functions
>> don't work on the column and you need to use the jsonb functions. That
>> seems to me a recipe for major confusion. It might be better if we set
>> it at initdb time so it couldn't be changed, but even so it could be
>> horribly confusing.
>>
>> This is certainly severable from the rest of these patches. I'm not sure
>> how severable it is from the SQL/JSON Table patches.
>>
>>
> I have confirmed that this is not required at all for the JSON_TABLE
> patch set.
>
>
> I'll submit new patch sets omitting it shortly. The GUC patch can be
> considered separately, probably as release 16 material, but I think as
> is it's at best quite incomplete.


here's a new set of patches, omitting the GUC patch and with the
beginnings of some message cleanup - there's more work to do there.


cheers


andrew


--
Andrew Dunstan
EDB: https://www.enterprisedb.com

  109. Re: SQL/JSON: functions

    Andrew Dunstan <andrew@dunslane.net> — 2022-03-05T14:39:05Z

    On 3/4/22 11:28, Andrew Dunstan wrote:
> On 3/2/22 10:19, Andrew Dunstan wrote:
>> On 3/1/22 16:41, Andrew Dunstan wrote:
>>> On 2/1/22 14:11,I wrote:
>>>> 2. The new GUC "sql_json" is a bit of a worry. I understand what it's
>>>> trying to do, but I'm trying to convince myself it's not going to be a
>>>> fruitful source of error reports, especially if people switch it in the
>>>> middle of a session. Maybe it should be an initdb option instead of a GUC?
>>>>
>>>>
>>> So far my efforts have not borne fruit. Here's why:
>>>
>>>
>>> andrew=# set sql_json = jsonb;
>>> SET
>>> andrew=# create table abc (x text, y json);
>>> CREATE TABLE
>>> andrew=# \d abc
>>>                Table "public.abc"
>>>  Column | Type | Collation | Nullable | Default
>>> --------+------+-----------+----------+---------
>>>  x      | text |           |          |
>>>  y      | json |           |          |
>>>
>>> andrew=# insert into abc values ('a','{"q":1}');
>>> INSERT 0 1
>>> andrew=# select json_each(y) from abc;
>>> ERROR:  function json_each(json) does not exist
>>> LINE 1: select json_each(y) from abc;
>>>                ^
>>> HINT:  No function matches the given name and argument types. You might
>>> need to add explicit type casts.
>>> andrew=# select jsonb_each(y) from abc;
>>>  jsonb_each
>>> ------------
>>>  (q,1)
>>> (1 row)
>>>
>>>
>>> The description tells them the column is json, but the json_* functions
>>> don't work on the column and you need to use the jsonb functions. That
>>> seems to me a recipe for major confusion. It might be better if we set
>>> it at initdb time so it couldn't be changed, but even so it could be
>>> horribly confusing.
>>>
>>> This is certainly severable from the rest of these patches. I'm not sure
>>> how severable it is from the SQL/JSON Table patches.
>>>
>>>
>> I have confirmed that this is not required at all for the JSON_TABLE
>> patch set.
>>
>>
>> I'll submit new patch sets omitting it shortly. The GUC patch can be
>> considered separately, probably as release 16 material, but I think as
>> is it's at best quite incomplete.
>
> here's a new set of patches, omitting the GUC patch and with the
> beginnings of some message cleanup - there's more work to do there.
>
>


This patchset restores the RETURNING clause for JSON() and JSON_SCALAR()
but without the GUC


cheers


andrew

--
Andrew Dunstan
EDB: https://www.enterprisedb.com

  110. Re: SQL/JSON: functions

    Andrew Dunstan <andrew@dunslane.net> — 2022-03-22T22:16:34Z

    On 3/5/22 09:39, Andrew Dunstan wrote:
>
> here's a new set of patches, omitting the GUC patch and with the
> beginnings of some message cleanup - there's more work to do there.
>
>
>
> This patchset restores the RETURNING clause for JSON() and JSON_SCALAR()
> but without the GUC
>
>


I have committed the first of these. That will break the cfbot for this
and the json_table patches. The remainder should be committed in the
following days.


cheers


andew


--
Andrew Dunstan
EDB: https://www.enterprisedb.com

  111. Re: SQL/JSON: functions

    Tom Lane <tgl@sss.pgh.pa.us> — 2022-03-22T22:20:09Z

    Andrew Dunstan <andrew@dunslane.net> writes:
> I have committed the first of these. That will break the cfbot for this
> and the json_table patches. The remainder should be committed in the
> following days.

That patch is 0-for-three on my buildfarm animals.

			regards, tom lane

  112. Re: SQL/JSON: functions

    Tom Lane <tgl@sss.pgh.pa.us> — 2022-03-22T22:31:39Z

    I wrote:
> That patch is 0-for-three on my buildfarm animals.

... the reason being that they use -Werror, and this patch spews
a bunch of warnings.  This is not acceptable, especially not in
the middle of a CF when other people are trying to get work done.
Please revert.

			regards, tom lane

  113. Re: SQL/JSON: functions

    Andres Freund <andres@anarazel.de> — 2022-03-22T23:33:33Z

    Hi,

On 2022-03-22 18:31:39 -0400, Tom Lane wrote:
> I wrote:
> > That patch is 0-for-three on my buildfarm animals.
> 
> ... the reason being that they use -Werror, and this patch spews
> a bunch of warnings.  This is not acceptable, especially not in
> the middle of a CF when other people are trying to get work done.
> Please revert.

There's also https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=jabiru&dt=2022-03-22%2022%3A25%3A26
that started failing with
../../preproc/ecpg --regression -I./../../include -I. -o test1.c test1.pgc
test1.pgc:12: ERROR: syntax error at or near "int"
with this commit.

And the bison warnings I complained about on -committers:
https://www.postgresql.org/message-id/20220322223319.so4ajcki7wwaujin%40alap3.anarazel.de

Greetings,

Andres Freund

  114. Re: SQL/JSON: functions

    Tom Lane <tgl@sss.pgh.pa.us> — 2022-03-22T23:48:55Z

    Andres Freund <andres@anarazel.de> writes:
> There's also
> https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=jabiru&dt=2022-03-22%2022%3A25%3A26
> that started failing with
> ../../preproc/ecpg --regression -I./../../include -I. -o test1.c test1.pgc
> test1.pgc:12: ERROR: syntax error at or near "int"
> with this commit.

Yeah, I was just scratching my head about that.  It's not clear how
that could be related; but jabiru doesn't seem to have a history of
random failures, so maybe it's real.

			regards, tom lane

  115. Re: SQL/JSON: functions

    Andrew Dunstan <andrew@dunslane.net> — 2022-03-22T23:56:56Z

    On 3/22/22 18:31, Tom Lane wrote:
> I wrote:
>> That patch is 0-for-three on my buildfarm animals.
> ... the reason being that they use -Werror, and this patch spews
> a bunch of warnings.  This is not acceptable, especially not in
> the middle of a CF when other people are trying to get work done.
> Please revert.
>
> 			



reverted.


cheers


andrew

--
Andrew Dunstan
EDB: https://www.enterprisedb.com

  116. Re: SQL/JSON: functions

    Tom Lane <tgl@sss.pgh.pa.us> — 2022-03-23T01:07:30Z

    I wrote:
> Andres Freund <andres@anarazel.de> writes:
>> There's also
>> https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=jabiru&dt=2022-03-22%2022%3A25%3A26
>> that started failing with
>> ../../preproc/ecpg --regression -I./../../include -I. -o test1.c test1.pgc
>> test1.pgc:12: ERROR: syntax error at or near "int"
>> with this commit.

> Yeah, I was just scratching my head about that.

I have a possibly-far-fetched theory: the ecpg grammar builder has
certainly never been validated against a backend grammar that
contains unused rules or unused nonterminals.  Maybe it generates
a subtly incorrect .y file in such cases, and on this particular
platform that results in bad code generated by bison, and ensuing
bogus syntax errors.

The lack of other failures weakens this theory.  Notably, I failed
to duplicate the problem on florican's host, which has the same
nominal bison version 3.7.6.  But it wouldn't surprise me a bit
to find that OpenBSD is carrying some private patches for their
build, so maybe that matters?

In any case, I think it's a bit pointless to chase these issues
with respect to this intermediate state of the JSON patch.
Let's merge in the next step, get to a state that does not
generate build warnings, and see what happens then.

			regards, tom lane

  117. Re: SQL/JSON: functions

    Justin Pryzby <pryzby@telsasoft.com> — 2022-03-23T12:24:23Z

    At least 0002-SQL-JSON-constructors-v64.patch has an issue with nodes,
per COPY_PARSE_PLAN_TREES.

+ERROR:  unrecognized node type: 157

  118. Re: SQL/JSON: functions

    Andrew Dunstan <andrew@dunslane.net> — 2022-03-23T19:49:17Z

    On 3/23/22 08:24, Justin Pryzby wrote:
> At least 0002-SQL-JSON-constructors-v64.patch has an issue with nodes,
> per COPY_PARSE_PLAN_TREES.
>
> +ERROR:  unrecognized node type: 157



I just tried to reproduce this and was unable to.  Ubuntu 20.04, gcc 9.4.0.


the build was done with CPPFLAGS=-DCOPY_PARSE_PLAN_TREES. To be on the
safe side I took ccache out of the equation.


Can you give me any more details?


cheers


andrew


--
Andrew Dunstan
EDB: https://www.enterprisedb.com

  119. Re: SQL/JSON: functions

    Andrew Dunstan <andrew@dunslane.net> — 2022-03-23T20:23:58Z

    On 3/23/22 15:49, Andrew Dunstan wrote:
> On 3/23/22 08:24, Justin Pryzby wrote:
>> At least 0002-SQL-JSON-constructors-v64.patch has an issue with nodes,
>> per COPY_PARSE_PLAN_TREES.
>>
>> +ERROR:  unrecognized node type: 157
>
>
> I just tried to reproduce this and was unable to.  Ubuntu 20.04, gcc 9.4.0.
>
>
> the build was done with CPPFLAGS=-DCOPY_PARSE_PLAN_TREES. To be on the
> safe side I took ccache out of the equation.
>
>
> Can you give me any more details?
>
>

I have reproduced similar errors from patch 4 in the series.


cheers


andrew


--
Andrew Dunstan
EDB: https://www.enterprisedb.com

  120. Re: SQL/JSON: functions

    Justin Pryzby <pryzby@telsasoft.com> — 2022-03-23T20:56:39Z

    On Wed, Mar 23, 2022 at 03:49:17PM -0400, Andrew Dunstan wrote:
> 
> On 3/23/22 08:24, Justin Pryzby wrote:
> > At least 0002-SQL-JSON-constructors-v64.patch has an issue with nodes,
> > per COPY_PARSE_PLAN_TREES.
> >
> > +ERROR:  unrecognized node type: 157
> 
> I just tried to reproduce this and was unable to.  Ubuntu 20.04, gcc 9.4.0.
> 
> the build was done with CPPFLAGS=-DCOPY_PARSE_PLAN_TREES. To be on the
> safe side I took ccache out of the equation.
> 
> Can you give me any more details?

Sorry, the issue was actually with 

#define RAW_EXPRESSION_COVERAGE_TEST

make check is enough to see it:

@@ -6,181 +6,78 @@
 (1 row)
 
 SELECT JSON_OBJECT(RETURNING json);
- json_object 
--------------
- {}
-(1 row)
-
+ERROR:  unrecognized node type: 157
[...]

  121. Re: SQL/JSON: functions

    Andrew Dunstan <andrew@dunslane.net> — 2022-03-24T22:51:30Z

    On 3/23/22 16:56, Justin Pryzby wrote:
> On Wed, Mar 23, 2022 at 03:49:17PM -0400, Andrew Dunstan wrote:
>> On 3/23/22 08:24, Justin Pryzby wrote:
>>> At least 0002-SQL-JSON-constructors-v64.patch has an issue with nodes,
>>> per COPY_PARSE_PLAN_TREES.
>>>
>>> +ERROR:  unrecognized node type: 157
>> I just tried to reproduce this and was unable to.  Ubuntu 20.04, gcc 9.4.0.
>>
>> the build was done with CPPFLAGS=-DCOPY_PARSE_PLAN_TREES. To be on the
>> safe side I took ccache out of the equation.
>>
>> Can you give me any more details?
> Sorry, the issue was actually with 
>
> #define RAW_EXPRESSION_COVERAGE_TEST
>
> make check is enough to see it:
>
> @@ -6,181 +6,78 @@
>  (1 row)
>  
>  SELECT JSON_OBJECT(RETURNING json);
> - json_object 
> --------------
> - {}
> -(1 row)
> -
> +ERROR:  unrecognized node type: 157
> [...]


Here's a new patch set that fixes this and several similar issues.
Thanks to my colleague  Masahiko Sawada for providing fixes for some of
these at very short notice.


I wonder if we should add these compile flags to the cfbot's setup?


cheers


andrew


--
Andrew Dunstan
EDB: https://www.enterprisedb.com

  122. Re: SQL/JSON: functions

    Andres Freund <andres@anarazel.de> — 2022-03-25T00:09:33Z

    Hi,

On 2022-03-24 18:51:30 -0400, Andrew Dunstan wrote:
> I wonder if we should add these compile flags to the cfbot's setup?

Yes, I think we should. There's a bit of discussion of that in and below
https://postgr.es/m/20220213051937.GO31460%40telsasoft.com - that veered a bit
of course, so I haven't done anything about it yet.  Perhaps one build
COPY_PARSE_PLAN_TREES and RAW_EXPRESSION_COVERAGE_TEST another
WRITE_READ_PARSE_PLAN_TREES?  We should add the slower to the macos build,
that's plenty fast and I'm intending to slow the linux test by using ubsan,
which works better on linux.

Greetings,

Andres Freund

  123. Re: SQL/JSON: functions

    Tom Lane <tgl@sss.pgh.pa.us> — 2022-03-27T23:14:25Z

    I wrote:
>> Andres Freund <andres@anarazel.de> writes:
>>> There's also
>>> https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=jabiru&dt=2022-03-22%2022%3A25%3A26
>>> that started failing with
>>> ../../preproc/ecpg --regression -I./../../include -I. -o test1.c test1.pgc
>>> test1.pgc:12: ERROR: syntax error at or near "int"
>>> with this commit.

>> Yeah, I was just scratching my head about that.

This problem came back as soon as we de-reverted that patch :-(.
So much for my guess about unused rules.

What's worse, I'm unable to replicate the failure on an OpenBSD 7.0
system here.  So there's something odd about jabiru's build
environment; but what?

			regards, tom lane

  124. Re: SQL/JSON: functions

    Andrew Dunstan <andrew@dunslane.net> — 2022-03-28T00:21:05Z

    
> On Mar 27, 2022, at 7:14 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> 
> I wrote:
>>> Andres Freund <andres@anarazel.de> writes:
>>>> There's also
>>>> https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=jabiru&dt=2022-03-22%2022%3A25%3A26
>>>> that started failing with
>>>> ../../preproc/ecpg --regression -I./../../include -I. -o test1.c test1.pgc
>>>> test1.pgc:12: ERROR: syntax error at or near "int"
>>>> with this commit.
> 
>>> Yeah, I was just scratching my head about that.
> 
> This problem came back as soon as we de-reverted that patch :-(.
> So much for my guess about unused rules.
> 
> What's worse, I'm unable to replicate the failure on an OpenBSD 7.0
> system here.  So there's something odd about jabiru's build
> environment; but what?
> 

Very odd. How did it pick up just this commit and not the following one which was pushed at the same time?

Cheers

Andrew

  125. Re: SQL/JSON: functions

    Andres Freund <andres@anarazel.de> — 2022-03-28T00:50:11Z

    Hi,

On 2022-03-27 20:21:05 -0400, Andrew Dunstan wrote:
> Very odd. How did it pick up just this commit and not the following one which was pushed at the same time?

The first recent failing run was with
f4fb45d15c Sun Mar 27 21:03:34 2022 UTC  SQL/JSON constructors
f79b803dcc Sun Mar 27 21:03:33 2022 UTC  Common SQL/JSON clauses
b64c3bd62e Sun Mar 27 20:26:40 2022 UTC  Remove more unused module imports from TAP tests

And then a second failure with:
cc7401d5ca Sun Mar 27 22:32:40 2022 UTC  Fix up compiler warnings/errors from f4fb45d15.

Which is what I'd expect?

Greetings,

Andres Freund

  126. Re: SQL/JSON: functions

    Andrew Dunstan <andrew@dunslane.net> — 2022-03-28T01:32:02Z

    On 3/27/22 20:50, Andres Freund wrote:
> Hi,
>
> On 2022-03-27 20:21:05 -0400, Andrew Dunstan wrote:
>> Very odd. How did it pick up just this commit and not the following one which was pushed at the same time?
> The first recent failing run was with
> f4fb45d15c Sun Mar 27 21:03:34 2022 UTC  SQL/JSON constructors
> f79b803dcc Sun Mar 27 21:03:33 2022 UTC  Common SQL/JSON clauses
> b64c3bd62e Sun Mar 27 20:26:40 2022 UTC  Remove more unused module imports from TAP tests
>
> And then a second failure with:
> cc7401d5ca Sun Mar 27 22:32:40 2022 UTC  Fix up compiler warnings/errors from f4fb45d15.
>
> Which is what I'd expect?
>


Yes, sorry, The url was for the previous commit, not today's. My mistake.


I'll look into it tomorrow, too tired tonight to be very productive.


cheers


andrew


--
Andrew Dunstan
EDB: https://www.enterprisedb.com

  127. Re: SQL/JSON: functions

    Andrew Dunstan <andrew@dunslane.net> — 2022-03-28T11:04:05Z

    On 3/27/22 19:14, Tom Lane wrote:
> I wrote:
>>> Andres Freund <andres@anarazel.de> writes:
>>>> There's also
>>>> https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=jabiru&dt=2022-03-22%2022%3A25%3A26
>>>> that started failing with
>>>> ../../preproc/ecpg --regression -I./../../include -I. -o test1.c test1.pgc
>>>> test1.pgc:12: ERROR: syntax error at or near "int"
>>>> with this commit.
>>> Yeah, I was just scratching my head about that.
> This problem came back as soon as we de-reverted that patch :-(.
> So much for my guess about unused rules.
>
> What's worse, I'm unable to replicate the failure on an OpenBSD 7.0
> system here.  So there's something odd about jabiru's build
> environment; but what?
>
> 			



It's hard to see how this could be caused by the OS environment. Maybe a
flaky bison/flex? I'm going to be pretty reluctant to revert this based
on this error.


cheers


andrew


--
Andrew Dunstan
EDB: https://www.enterprisedb.com

  128. Re: SQL/JSON: functions

    Tom Lane <tgl@sss.pgh.pa.us> — 2022-03-28T13:35:31Z

    Andrew Dunstan <andrew@dunslane.net> writes:
> On 3/27/22 19:14, Tom Lane wrote:
>> What's worse, I'm unable to replicate the failure on an OpenBSD 7.0
>> system here.  So there's something odd about jabiru's build
>> environment; but what?

> It's hard to see how this could be caused by the OS environment. Maybe a
> flaky bison/flex? I'm going to be pretty reluctant to revert this based
> on this error.

No, I wouldn't recommend reverting.  Perhaps if we dig down and find
something reproducible here, we could fix it --- but right now,
given my failure to reproduce, I think there's just something broken
on jabiru.

			regards, tom lane

  129. Re: SQL/JSON: functions

    Andrew Dunstan <andrew@dunslane.net> — 2022-03-28T14:30:32Z

    On 3/28/22 09:35, Tom Lane wrote:
> Andrew Dunstan <andrew@dunslane.net> writes:
>> On 3/27/22 19:14, Tom Lane wrote:
>>> What's worse, I'm unable to replicate the failure on an OpenBSD 7.0
>>> system here.  So there's something odd about jabiru's build
>>> environment; but what?
>> It's hard to see how this could be caused by the OS environment. Maybe a
>> flaky bison/flex? I'm going to be pretty reluctant to revert this based
>> on this error.
> No, I wouldn't recommend reverting.  Perhaps if we dig down and find
> something reproducible here, we could fix it --- but right now,
> given my failure to reproduce, I think there's just something broken
> on jabiru.
>
> 			



Yeah. I have just duplicated your non-replication on a fresh instance.


Nikola Ivanov, can you give us any assistance or give us access to the
machine?


cheers


andrew


--
Andrew Dunstan
EDB: https://www.enterprisedb.com

  130. Re: SQL/JSON: functions

    Nikola Ivanov <kolioffx@gmail.com> — 2022-03-28T15:05:19Z

    Hi Andrew,

Let me know check what can I do with the access. I will get back to you in
an hour.

Regards

On Mon, Mar 28, 2022, 17:30 Andrew Dunstan <andrew@dunslane.net> wrote:

>
> On 3/28/22 09:35, Tom Lane wrote:
> > Andrew Dunstan <andrew@dunslane.net> writes:
> >> On 3/27/22 19:14, Tom Lane wrote:
> >>> What's worse, I'm unable to replicate the failure on an OpenBSD 7.0
> >>> system here.  So there's something odd about jabiru's build
> >>> environment; but what?
> >> It's hard to see how this could be caused by the OS environment. Maybe a
> >> flaky bison/flex? I'm going to be pretty reluctant to revert this based
> >> on this error.
> > No, I wouldn't recommend reverting.  Perhaps if we dig down and find
> > something reproducible here, we could fix it --- but right now,
> > given my failure to reproduce, I think there's just something broken
> > on jabiru.
> >
> >
>
>
>
> Yeah. I have just duplicated your non-replication on a fresh instance.
>
>
> Nikola Ivanov, can you give us any assistance or give us access to the
> machine?
>
>
> cheers
>
>
> andrew
>
>
> --
> Andrew Dunstan
> EDB: https://www.enterprisedb.com
>
>

  131. Re: SQL/JSON: functions

    Andrew Dunstan <andrew@dunslane.net> — 2022-03-28T15:25:09Z

    On 3/28/22 11:05, Nikola Ivanov wrote:
> Hi Andrew,
>
> Let me know check what can I do with the access. I will get back to
> you in an hour.


Thanks for you help and prompt response.

In the first instance we'd like to know what might be different about
jabiru from the openbsd7/clang11 instances Tom and I have just
successfully tested on. In the last resort we might need to run ecpg
under a debugger on jabiru to see why it's failing there and not
elsewhere. To set up for that run the buildfarm script with --keepall.


cheers


andrew


--
Andrew Dunstan
EDB: https://www.enterprisedb.com

  132. Re: SQL/JSON: functions

    Andres Freund <andres@anarazel.de> — 2022-03-28T15:39:09Z

    Hi,

On 2022-03-28 18:05:19 +0300, Nikola Ivanov wrote:
> Let me know check what can I do with the access. I will get back to you in
> an hour.

Perhaps you can temporarily enable keep_error_builds, and send in
src/interfaces/ecpg/preproc/c_kwlist_d.h
src/interfaces/ecpg/preproc/pgc.c
src/interfaces/ecpg/preproc/preproc.h
src/interfaces/ecpg/preproc/ecpg_kwlist_d.h
src/interfaces/ecpg/preproc/preproc.c
from the failed build directory? It seems something there have to differ.

Regards,

Andres

  133. Re: SQL/JSON: functions

    Nikola Ivanov <kolioffx@gmail.com> — 2022-03-28T15:50:07Z

    ok, I have enabled it. Will send it after the next build.


Regards

On Mon, 28 Mar 2022 at 18:39, Andres Freund <andres@anarazel.de> wrote:

> Hi,
>
> On 2022-03-28 18:05:19 +0300, Nikola Ivanov wrote:
> > Let me know check what can I do with the access. I will get back to you
> in
> > an hour.
>
> Perhaps you can temporarily enable keep_error_builds, and send in
> src/interfaces/ecpg/preproc/c_kwlist_d.h
> src/interfaces/ecpg/preproc/pgc.c
> src/interfaces/ecpg/preproc/preproc.h
> src/interfaces/ecpg/preproc/ecpg_kwlist_d.h
> src/interfaces/ecpg/preproc/preproc.c
> from the failed build directory? It seems something there have to differ.
>
> Regards,
>
> Andres
>

  134. Re: SQL/JSON: functions

    Nikola Ivanov <kolioffx@gmail.com> — 2022-03-28T18:31:35Z

    Hi Andreas,

Archive with the files is attached.

On Mon, 28 Mar 2022 at 18:50, Nikola Ivanov <kolioffx@gmail.com> wrote:

> ok, I have enabled it. Will send it after the next build.
>
>
> Regards
>
> On Mon, 28 Mar 2022 at 18:39, Andres Freund <andres@anarazel.de> wrote:
>
>> Hi,
>>
>> On 2022-03-28 18:05:19 +0300, Nikola Ivanov wrote:
>> > Let me know check what can I do with the access. I will get back to you
>> in
>> > an hour.
>>
>> Perhaps you can temporarily enable keep_error_builds, and send in
>> src/interfaces/ecpg/preproc/c_kwlist_d.h
>> src/interfaces/ecpg/preproc/pgc.c
>> src/interfaces/ecpg/preproc/preproc.h
>> src/interfaces/ecpg/preproc/ecpg_kwlist_d.h
>> src/interfaces/ecpg/preproc/preproc.c
>> from the failed build directory? It seems something there have to differ.
>>
>> Regards,
>>
>> Andres
>>
>

  135. Re: SQL/JSON: functions

    Andrew Dunstan <andrew@dunslane.net> — 2022-03-28T18:57:20Z

    On 3/28/22 14:31, Nikola Ivanov wrote:
> Hi Andreas,
>
> Archive with the files is attached.



That didn't help, there are no differences that matter (just #line
directives as I did a vpath build). :-(


cheers


andrew


--
Andrew Dunstan
EDB: https://www.enterprisedb.com

  136. Re: SQL/JSON: functions

    Andres Freund <andres@anarazel.de> — 2022-03-28T19:27:25Z

    Hi,

On 2022-03-28 14:57:20 -0400, Andrew Dunstan wrote:
> That didn't help, there are no differences that matter (just #line
> directives as I did a vpath build). :-(

Yea. I didn't see any differences when comparing to a non-vpath build that
runs tests successfully. Pretty weird.

Nikola, unless remote access turns out to be possible for one of us, could you
perhaps try to build interactively and see whether it reproduces there?

Greetings,

Andres Freund

  137. Re: SQL/JSON: functions

    Tom Lane <tgl@sss.pgh.pa.us> — 2022-03-28T19:36:29Z

    Andres Freund <andres@anarazel.de> writes:
> On 2022-03-28 14:57:20 -0400, Andrew Dunstan wrote:
>> That didn't help, there are no differences that matter (just #line
>> directives as I did a vpath build). :-(

> Yea. I didn't see any differences when comparing to a non-vpath build that
> runs tests successfully. Pretty weird.

Unsurprisingly, these files match what I built, too.  So the problem
is downstream of the flex/bison runs.  Baffling :-(

			regards, tom lane

  138. Re: SQL/JSON: functions

    Tom Lane <tgl@sss.pgh.pa.us> — 2022-03-28T23:25:51Z

    ... even more baffling: jabiru went green after the IS JSON patch.

			regards, tom lane

  139. Re: SQL/JSON: functions

    Andrew Dunstan <andrew@dunslane.net> — 2022-03-29T01:03:53Z

    


> On Mar 28, 2022, at 7:25 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> 
> ... even more baffling: jabiru went green after the IS JSON patch.
> 
>           

Yeah, bizarre. Let’s see if I can upset that tomorrow with the next patch :-)

cheers

andrew

  140. Re: SQL/JSON: functions

    Andres Freund <andres@anarazel.de> — 2022-03-29T03:41:36Z

    On 2022-03-28 19:25:51 -0400, Tom Lane wrote:
> ... even more baffling: jabiru went green after the IS JSON patch.

Broken ccache contents somehow?

  141. Re: SQL/JSON: functions

    Nikola Ivanov <kolioffx@gmail.com> — 2022-03-29T04:09:05Z

    Yes, it seems it was the ccache - I had enabled the 'ccache_failure_remove'
flag last night and run a build, which failed, but the builds after that
went green.

Regards

On Tue, 29 Mar 2022 at 06:41, Andres Freund <andres@anarazel.de> wrote:

> On 2022-03-28 19:25:51 -0400, Tom Lane wrote:
> > ... even more baffling: jabiru went green after the IS JSON patch.
>
> Broken ccache contents somehow?
>

  142. Re: SQL/JSON: functions

    Greg Stark <stark@mit.edu> — 2022-03-31T19:54:19Z

    I see several commits referencing this entry. Can we mark it committed
or are there still chunks of commits to go?

  143. Re: SQL/JSON: functions

    Andrew Dunstan <andrew@dunslane.net> — 2022-03-31T20:25:58Z

    On 3/31/22 15:54, Greg Stark wrote:
> I see several commits referencing this entry. Can we mark it committed
> or are there still chunks of commits to go?



No code chunks left, only a documentation patch which should land
tomorrow or Saturday.


I am also planning on committing the JSON_TABLE patches before feature
freeze (April 7). They depend on this set.


cheers


andrew

--
Andrew Dunstan
EDB: https://www.enterprisedb.com

  144. Re: SQL/JSON: functions

    Justin Pryzby <pryzby@telsasoft.com> — 2022-04-08T12:02:02Z

    On Thu, Mar 31, 2022 at 04:25:58PM -0400, Andrew Dunstan wrote:
> No code chunks left, only a documentation patch which should land

Documentation review for a6baa4bad.

> Construct a JSON the provided strings:

a JSON what ?
*from* the provided strings ?

> Construct a JSON from the provided values various types:

should say "a JSON scalar" ?
*of* various types ?

> Construct a JSON object from the provided key/value pairs of various types:

For comparison, that one looks ok.

+      <function>JSON_EXISTS</function> function checks whether the provided
+   <function>JSON_VALUE</function> function extracts a value from the provided
+   <function>JSON_QUERY</function> function extracts an <acronym>SQL/JSON</acronym>
+      <function>JSON_TABLE</function> function queries <acronym>JSON</acronym> data
+     <function>JSON_TABLE</function> uses the
+      <function>JSON_SERIALIZE</function> function transforms a SQL/JSON value

I think all these should all begin with "THE >...< function ...", like the
others do.

+To use other types, you must create the <literal>CAST</literal> from <type>json</type> for this type.
=> create a cast from json to this type.

+Values can be null, but not keys.
I think it's clearer to say "..but keys cannot."

+          For any scalar other than a number or a Boolean the text

Boolean COMMA the text

+     The path name must be unique and cannot coincide with column names.
Maybe say "name must be unique and distinct from the column names."

+      ... If you specify a <command>GROUP BY</command>
+      or an <command>ORDER BY</command> clause, this function returns a separate JSON object
+      for each table row.

"for each table row" sounds inaccurate or imprecise.  The SELECT docs say this:
| GROUP BY will condense into a single row all selected rows that share the same values for the grouped expressions

BTW, the documentation references look a little like OIDs...
Does someone already have an SNMP-based doc browser ?
| For details, see Section 9.16.3.4.2.

  145. Re: SQL/JSON: functions

    Andrew Dunstan <andrew@dunslane.net> — 2022-04-08T12:15:08Z

    On 4/8/22 08:02, Justin Pryzby wrote:
> On Thu, Mar 31, 2022 at 04:25:58PM -0400, Andrew Dunstan wrote:
>> No code chunks left, only a documentation patch which should land
> Documentation review for a6baa4bad.
>
>> Construct a JSON the provided strings:
> a JSON what ?
> *from* the provided strings ?
>
>> Construct a JSON from the provided values various types:
> should say "a JSON scalar" ?
> *of* various types ?
>
>> Construct a JSON object from the provided key/value pairs of various types:
> For comparison, that one looks ok.
>
> +      <function>JSON_EXISTS</function> function checks whether the provided
> +   <function>JSON_VALUE</function> function extracts a value from the provided
> +   <function>JSON_QUERY</function> function extracts an <acronym>SQL/JSON</acronym>
> +      <function>JSON_TABLE</function> function queries <acronym>JSON</acronym> data
> +     <function>JSON_TABLE</function> uses the
> +      <function>JSON_SERIALIZE</function> function transforms a SQL/JSON value
>
> I think all these should all begin with "THE >...< function ...", like the
> others do.
>
> +To use other types, you must create the <literal>CAST</literal> from <type>json</type> for this type.
> => create a cast from json to this type.
>
> +Values can be null, but not keys.
> I think it's clearer to say "..but keys cannot."
>
> +          For any scalar other than a number or a Boolean the text
>
> Boolean COMMA the text
>
> +     The path name must be unique and cannot coincide with column names.
> Maybe say "name must be unique and distinct from the column names."
>
> +      ... If you specify a <command>GROUP BY</command>
> +      or an <command>ORDER BY</command> clause, this function returns a separate JSON object
> +      for each table row.
>
> "for each table row" sounds inaccurate or imprecise.  The SELECT docs say this:
> | GROUP BY will condense into a single row all selected rows that share the same values for the grouped expressions
>
> BTW, the documentation references look a little like OIDs...
> Does someone already have an SNMP-based doc browser ?
> | For details, see Section 9.16.3.4.2.



Many thanks, useful.

I already had a couple of these items on my list but I ran out of time
before tiredness overcame me last night.

I'm planning on removing some of that stuff that generates the last
complaint if I can do it without too much violence.


cheers


andrew

--
Andrew Dunstan
EDB: https://www.enterprisedb.com

  146. Re: SQL/JSON: functions

    Andrew Dunstan <andrew@dunslane.net> — 2022-04-11T15:54:11Z

    On 2022-04-08 Fr 08:15, Andrew Dunstan wrote:
> On 4/8/22 08:02, Justin Pryzby wrote:
>> On Thu, Mar 31, 2022 at 04:25:58PM -0400, Andrew Dunstan wrote:
>>> No code chunks left, only a documentation patch which should land
>> Documentation review for a6baa4bad.
>>
>>> Construct a JSON the provided strings:
>> a JSON what ?
>> *from* the provided strings ?
>>
>>> Construct a JSON from the provided values various types:
>> should say "a JSON scalar" ?
>> *of* various types ?
>>
>>> Construct a JSON object from the provided key/value pairs of various types:
>> For comparison, that one looks ok.
>>
>> +      <function>JSON_EXISTS</function> function checks whether the provided
>> +   <function>JSON_VALUE</function> function extracts a value from the provided
>> +   <function>JSON_QUERY</function> function extracts an <acronym>SQL/JSON</acronym>
>> +      <function>JSON_TABLE</function> function queries <acronym>JSON</acronym> data
>> +     <function>JSON_TABLE</function> uses the
>> +      <function>JSON_SERIALIZE</function> function transforms a SQL/JSON value
>>
>> I think all these should all begin with "THE >...< function ...", like the
>> others do.
>>
>> +To use other types, you must create the <literal>CAST</literal> from <type>json</type> for this type.
>> => create a cast from json to this type.
>>
>> +Values can be null, but not keys.
>> I think it's clearer to say "..but keys cannot."
>>
>> +          For any scalar other than a number or a Boolean the text
>>
>> Boolean COMMA the text
>>
>> +     The path name must be unique and cannot coincide with column names.
>> Maybe say "name must be unique and distinct from the column names."
>>
>> +      ... If you specify a <command>GROUP BY</command>
>> +      or an <command>ORDER BY</command> clause, this function returns a separate JSON object
>> +      for each table row.
>>
>> "for each table row" sounds inaccurate or imprecise.  The SELECT docs say this:
>> | GROUP BY will condense into a single row all selected rows that share the same values for the grouped expressions
>>
>> BTW, the documentation references look a little like OIDs...
>> Does someone already have an SNMP-based doc browser ?
>> | For details, see Section 9.16.3.4.2.
>
>
> Many thanks, useful.
>
> I already had a couple of these items on my list but I ran out of time
> before tiredness overcame me last night.
>
> I'm planning on removing some of that stuff that generates the last
> complaint if I can do it without too much violence.
>
>

I have attended to most of these. I just removed the GROUP BY sentence,
I don't think it added anything useful. We already refer users to the
aggregates page.


I will deal with the structural issues soon.


cheers


andrew

--
Andrew Dunstan
EDB: https://www.enterprisedb.com

  147. Re: SQL/JSON: functions

    Justin Pryzby <pryzby@telsasoft.com> — 2022-04-11T16:09:05Z

    On Mon, Apr 11, 2022 at 11:54:11AM -0400, Andrew Dunstan wrote:
> >> BTW, the documentation references look a little like OIDs...
> >> Does someone already have an SNMP-based doc browser ?
> >> | For details, see Section 9.16.3.4.2.
> >
> > I already had a couple of these items on my list but I ran out of time
> > before tiredness overcame me last night.
> >
> > I'm planning on removing some of that stuff that generates the last
> > complaint if I can do it without too much violence.
> 
> I will deal with the structural issues soon.

I didn't actually intend this as a complaint.  The ability for a reference to
be specific and granular is good.  So there may be reasons to change the
structure, but my comment is only about a +0.1.

  148. Re: SQL/JSON: functions

    Tom Lane <tgl@sss.pgh.pa.us> — 2022-04-11T16:25:50Z

    Justin Pryzby <pryzby@telsasoft.com> writes:
> On Mon, Apr 11, 2022 at 11:54:11AM -0400, Andrew Dunstan wrote:
>> I will deal with the structural issues soon.

> I didn't actually intend this as a complaint.  The ability for a reference to
> be specific and granular is good.  So there may be reasons to change the
> structure, but my comment is only about a +0.1.

I was going to complain about it on the grounds that it looks
approximately nothing like other sections of func.sgml.  The layout
of the rest of that file is hard-won presentation knowledge and
should not be lightly ignored.

			regards, tom lane

  149. Re: SQL/JSON: functions

    a.kozhemyakin <a.kozhemyakin@postgrespro.ru> — 2022-07-07T16:38:51Z

    Hello hackers,
On branch REL_15_STABLE the following query: SELECT 
JSON_SERIALIZE('{"a":1}' RETURNING jsonb);

produces SIGSEGV for me:
#0  getJsonbOffset (index=39920251, jc=0x563cc5601d5c) at 
jsonb_util.c:148
148                     offset += JBE_OFFLENFLD(jc->children[i]);
(gdb) bt
#0  getJsonbOffset (index=39920251, jc=0x563cc5601d5c) at 
jsonb_util.c:148
#1  JsonbIteratorNext (it=0x7ffdaff752c8, val=0x7ffdaff752d0, 
skipNested=false) at jsonb_util.c:933
#2  0x0000563cc3e6934b in JsonbToCStringWorker (out=0x563cc55fe9a0, 
in=<optimized out>, estimated_len=<optimized out>, indent=false) at 
jsonb.c:496
#3  0x0000563cc3f35c68 in FunctionCall1Coll (arg1=<optimized out>, 
collation=0, flinfo=0x563cc55fb8a0) at fmgr.c:1124
#4  OutputFunctionCall (flinfo=0x563cc55fb8a0, val=<optimized out>) at 
fmgr.c:1561
#5  0x0000563cc3a8ebc5 in printtup (slot=0x563cc55faed8, 
self=0x563cc5602758) at printtup.c:357
#6  0x0000563cc3c51917 in ExecutePlan (execute_once=<optimized out>, 
dest=0x563cc5602758, direction=<optimized out>, numberTuples=0, 
sendTuples=<optimized out>, operation=CMD_SELECT, 
use_parallel_mode=<optimized out>, planstate=0x563cc55fabb8,
     estate=0x563cc55fa980) at execMain.c:1667
#7  standard_ExecutorRun (queryDesc=0x563cc5553e60, direction=<optimized 
out>, count=0, execute_once=<optimized out>) at execMain.c:363
#8  0x0000563cc3df741f in PortalRunSelect (portal=0x563cc55a3c40, 
forward=<optimized out>, count=0, dest=<optimized out>) at pquery.c:924
#9  0x0000563cc3df8b81 in PortalRun (portal=portal@entry=0x563cc55a3c40, 
count=count@entry=9223372036854775807, isTopLevel=isTopLevel@entry=true, 
run_once=run_once@entry=true, dest=dest@entry=0x563cc5602758, 
altdest=altdest@entry=0x563cc5602758,
     qc=0x7ffdaff75650) at pquery.c:768
#10 0x0000563cc3df498d in exec_simple_query (query_string=0x563cc5531f00 
"SELECT JSON_SERIALIZE('{\"a\":1}' RETURNING jsonb);") at 
postgres.c:1250
#11 0x0000563cc3df651a in PostgresMain (dbname=<optimized out>, 
username=<optimized out>) at postgres.c:4558
#12 0x0000563cc3d5b5e7 in BackendRun (port=<optimized out>, 
port=<optimized out>) at postmaster.c:4504
#13 BackendStartup (port=<optimized out>) at postmaster.c:4232
#14 ServerLoop () at postmaster.c:1806
#15 0x0000563cc3d5c6da in PostmasterMain (argc=argc@entry=3, 
argv=argv@entry=0x563cc552c6d0) at postmaster.c:1478
#16 0x0000563cc3a7b550 in main (argc=3, argv=0x563cc552c6d0) at 
main.c:202

The first bad commit is 606948b058dc16bce494270eea577011a602810e

Andrew Dunstan писал 2022-04-01 03:25:
> On 3/31/22 15:54, Greg Stark wrote:
>> I see several commits referencing this entry. Can we mark it committed
>> or are there still chunks of commits to go?
> 
> 
> 
> No code chunks left, only a documentation patch which should land
> tomorrow or Saturday.
> 
> 
> I am also planning on committing the JSON_TABLE patches before feature
> freeze (April 7). They depend on this set.
> 
> 
> cheers
> 
> 
> andrew
> 
> --
> Andrew Dunstan
> EDB: https://www.enterprisedb.com

  150. Re: SQL/JSON: functions

    Andrew Dunstan <andrew@dunslane.net> — 2022-07-07T18:35:00Z

    On 2022-07-07 Th 12:38, a.kozhemyakin@postgrespro.ru wrote:
> Hello hackers,
> On branch REL_15_STABLE the following query: SELECT
> JSON_SERIALIZE('{"a":1}' RETURNING jsonb);
>
> produces SIGSEGV for me:
> #0  getJsonbOffset (index=39920251, jc=0x563cc5601d5c) at
> jsonb_util.c:148
> 148                     offset += JBE_OFFLENFLD(jc->children[i]);
> (gdb) bt
> #0  getJsonbOffset (index=39920251, jc=0x563cc5601d5c) at
> jsonb_util.c:148
> #1  JsonbIteratorNext (it=0x7ffdaff752c8, val=0x7ffdaff752d0,
> skipNested=false) at jsonb_util.c:933
> #2  0x0000563cc3e6934b in JsonbToCStringWorker (out=0x563cc55fe9a0,
> in=<optimized out>, estimated_len=<optimized out>, indent=false) at
> jsonb.c:496
> #3  0x0000563cc3f35c68 in FunctionCall1Coll (arg1=<optimized out>,
> collation=0, flinfo=0x563cc55fb8a0) at fmgr.c:1124
> #4  OutputFunctionCall (flinfo=0x563cc55fb8a0, val=<optimized out>) at
> fmgr.c:1561
> #5  0x0000563cc3a8ebc5 in printtup (slot=0x563cc55faed8,
> self=0x563cc5602758) at printtup.c:357
> #6  0x0000563cc3c51917 in ExecutePlan (execute_once=<optimized out>,
> dest=0x563cc5602758, direction=<optimized out>, numberTuples=0,
> sendTuples=<optimized out>, operation=CMD_SELECT,
> use_parallel_mode=<optimized out>, planstate=0x563cc55fabb8,
>     estate=0x563cc55fa980) at execMain.c:1667
> #7  standard_ExecutorRun (queryDesc=0x563cc5553e60,
> direction=<optimized out>, count=0, execute_once=<optimized out>) at
> execMain.c:363
> #8  0x0000563cc3df741f in PortalRunSelect (portal=0x563cc55a3c40,
> forward=<optimized out>, count=0, dest=<optimized out>) at pquery.c:924
> #9  0x0000563cc3df8b81 in PortalRun
> (portal=portal@entry=0x563cc55a3c40,
> count=count@entry=9223372036854775807,
> isTopLevel=isTopLevel@entry=true, run_once=run_once@entry=true,
> dest=dest@entry=0x563cc5602758, altdest=altdest@entry=0x563cc5602758,
>     qc=0x7ffdaff75650) at pquery.c:768
> #10 0x0000563cc3df498d in exec_simple_query
> (query_string=0x563cc5531f00 "SELECT JSON_SERIALIZE('{\"a\":1}'
> RETURNING jsonb);") at postgres.c:1250
> #11 0x0000563cc3df651a in PostgresMain (dbname=<optimized out>,
> username=<optimized out>) at postgres.c:4558
> #12 0x0000563cc3d5b5e7 in BackendRun (port=<optimized out>,
> port=<optimized out>) at postmaster.c:4504
> #13 BackendStartup (port=<optimized out>) at postmaster.c:4232
> #14 ServerLoop () at postmaster.c:1806
> #15 0x0000563cc3d5c6da in PostmasterMain (argc=argc@entry=3,
> argv=argv@entry=0x563cc552c6d0) at postmaster.c:1478
> #16 0x0000563cc3a7b550 in main (argc=3, argv=0x563cc552c6d0) at
> main.c:202
>
> The first bad commit is 606948b058dc16bce494270eea577011a602810e



Thanks for the report. Looking into it.


cheers


andrew


--
Andrew Dunstan
EDB: https://www.enterprisedb.com

  151. Re: SQL/JSON: functions

    Andrew Dunstan <andrew@dunslane.net> — 2022-07-07T21:49:02Z

    On 2022-07-07 Th 14:35, Andrew Dunstan wrote:
> On 2022-07-07 Th 12:38, a.kozhemyakin@postgrespro.ru wrote:
>> Hello hackers,
>> On branch REL_15_STABLE the following query: SELECT
>> JSON_SERIALIZE('{"a":1}' RETURNING jsonb);
>>
>> produces SIGSEGV for me:





You're not supposed to be able to do that - see the docs on
json_serialize. I've committed a patch that detects this and causes an
error instead of a segfault.


cheers


andrew


--
Andrew Dunstan
EDB: https://www.enterprisedb.com