SQL/JSON: functions

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

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

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

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

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

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

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

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.

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

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

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

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

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

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

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&gt;

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

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

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

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

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

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?

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