Re: [HACKERS] SQL/JSON in PostgreSQL

Hi there,

Attached patch is an implementation of SQL/JSON data model from SQL-2016
standard (ISO/IEC 9075-2:2016(E)), which was published 2016-12-15 and is
available only for purchase from ISO web site (
https://www.iso.org/standard/63556.html) Unfortunately I didn't find any
public sources of the standard or any preview documents, but Oracle
implementation of json support in 12c release 2 is very close (
http://docs.oracle.com/database/122/ADJSN/json-in-oracle-database.htm)
also we used https://livesql.oracle.com/ to understand some details.

Postgres has already two json data types - json and jsonb and implementing
another json data type, which strictly conforms the standard, would be not
a good idea. Moreover, SQL standard doesn’t describe data type, but only
data model, which “comprises SQL/JSON items and SQL/JSON sequences. The
components of the SQL/JSON data model are:

1) An SQL/JSON item is defined recursively as any of the following:

a) An SQL/JSON scalar, defined as a non-null value of any of the following
predefined (SQL) types:

character string with character set Unicode, numeric, Boolean, or datetime.

b) An SQL/JSON null, defined as a value that is distinct from any value of
any SQL type.

NOTE 122 — An SQL/JSON null is distinct from the SQL null value.

c) An SQL/JSON array, defined as an ordered list of zero or more SQL/JSON
items, called the SQL/JSON

elements of the SQL/JSON array.

d) An SQL/JSON object, defined as an unordered collection of zero or more
SQL/JSON members….

“

Our jsonb corresponds to SQL/JSON with UNIQUE KEYS and implicit ordering of
keys and our main intention was to provide support of jsonb as a most
important and usable data type.

We created repository for reviewing (ask for write access) -
https://github.com/postgrespro/sqljson/tree/sqljson

Examples of usage can be found in src/test/regress/sql/sql_json.sql

The whole documentation about json support should be reorganized and added,
and we plan to do this before release. We need help of community here.

Our goal is to provide support of main features of SQL/JSON to release 10,
as we discussed at developers meeting in Brussels (Andrew Dunstan has
kindly agreed to review the patch).

We had not much time to develop the complete support, because of standard
availability), but hope all major features are here, namely, all nine
functions as described in the standard (but see implementation notes below):

“All manipulation (e.g., retrieval, creation, testing) of SQL/JSON items is
performed through a number of SQL/JSON functions. There are nine such
functions, categorized as SQL/JSON retrieval functions and SQL/JSON
construction functions. The SQL/JSON retrieval functions are characterized
by operating on JSON data and returning an SQL value (possibly a Boolean
value) or a JSON value. The SQL/JSON construction functions return JSON
data created from operations on SQL data or other JSON data.

The SQL/JSON retrieval functions are:

— <JSON value function>: extracts an SQL value of a predefined type from a
JSON text.

— <JSON query>: extracts a JSON text from a JSON text.

— <JSON table>: converts a JSON text to an SQL table.

— <JSON predicate>: tests whether a string value is or is not properly
formed JSON text.

— <JSON exists predicate>: tests whether an SQL/JSON path expression
returns any SQL/JSON items.

The SQL/JSON construction functions are:

— <JSON object constructor>: generates a string that is a serialization of
an SQL/JSON object.

— <JSON array constructor>: generates a string that is a serialization of
an SQL/JSON array.

— <JSON object aggregate constructor>: generates, from an aggregation of
SQL data, a string that is a serialization

of an SQL/JSON object.

— <JSON array aggregate constructor>: generates, from an aggregation of SQL
data, a string that is a serialization

of an SQL/JSON array.

A JSON-returning function is an SQL/JSON construction function or
JSON_QUERY.”

The standard describes SQL/JSON path language, which used by SQL/JSON query
operators to query JSON. It defines path language as string literal. We
implemented the path language as JSONPATH data type, since other
approaches are not friendly to planner and executor.

The functions and JSONPATH provide a new functionality for json support,
namely, ability to operate (in standard specified way) with json structure
at SQL-language level - the often requested feature by the users.

The patch is consists of about 15000 insertions (about 5000 lines are from
tests), passes all regression tests and doesn’t touches critical parts, so
we hope with community help to bring it to committable state.

Authors: Nikita Glukhov, Teodor Sigaev, Oleg Bartunov and Alexander Korotkov

Implementation notes:

1.

We didn’t implemented ‘datetime’ support, since it’s not clear from
standard.
2.

JSON_OBJECT/JSON_OBJECTAGG (KEY <key> VALUE <value>, ...) doesn’t
implemented, only (<key>:<value>, …) and (<key> VALUE <value>, …) are
supported, because of grammar conflicts with leading KEY keyword.
3.

FORMAT (JSON|JSONB)) in JSON_ARRAYAGG with subquery doesn’t supported,
because of grammar conflicts with non-reserved word FORMAT.
4.

JSONPATH implemented only for jsonb data type , so JSON_EXISTS(),
JSON_VALUE(), JSON_QUERY() and JSON_TABLE() doesn’t works if context item
is of json data type.
5.

Some methods and predicates for JSONPATH not yet implemented, for
example .type(), .size(), .keyvalue(), predicates like_regex, starts
with, etc. They are not key features and we plan to make them in next
release.
6.

JSONPATH doesn’t support expression for index array, like [2+3 to
$upperbound], only simple constants like [5, 7 to 12] are supported.
7.

JSONPATH extensions to standard: .** (wildcard path accessor), .key
(member accessor without leading @).
8.

FORMAT JSONB extension to standard for returning jsonb - standard
specifies possibility of returning custom type.
9.

JSON_EXISTS(), JSON_VALUE(), JSON_QUERY() are implemented using new
executor node JsonExpr.
10.

JSON_TABLE() is transformed into joined subselects with JSON_VALUE() and
JSON_QUERY() in target list.
11.

JSON_OBJECT(), JSON_ARRAY() constructors and IS JSON predicate are
transformed into raw function calls.
12.

Added explicit casts bytea=>jsonb and jsonb=>bytea (for jsonb=>bytea
output using RETURNING bytea FORMAT JSONB and corresponding bytea=>jsonb
input using <jsonb_bytea_expr> FORMAT JSONB).

Best regards,

Oleg

Hi

2017-02-28 20:08 GMT+01:00 Oleg Bartunov <obartunov(at)gmail(dot)com>:

> Hi there,
>
>
> Attached patch is an implementation of SQL/JSON data model from SQL-2016
> standard (ISO/IEC 9075-2:2016(E)), which was published 2016-12-15 and is
> available only for purchase from ISO web site (
> https://www.iso.org/standard/63556.html) Unfortunately I didn't find any
> public sources of the standard or any preview documents, but Oracle
> implementation of json support in 12c release 2 is very close (
> http://docs.oracle.com/database/122/ADJSN/json-in-oracle-database.htm)
> also we used https://livesql.oracle.com/ to understand some details.
>
> Postgres has already two json data types - json and jsonb and implementing
> another json data type, which strictly conforms the standard, would be not
> a good idea. Moreover, SQL standard doesn’t describe data type, but only
> data model, which “comprises SQL/JSON items and SQL/JSON sequences. The
> components of the SQL/JSON data model are:
>
> 1) An SQL/JSON item is defined recursively as any of the following:
>
> a) An SQL/JSON scalar, defined as a non-null value of any of the following
> predefined (SQL) types:
>
> character string with character set Unicode, numeric, Boolean, or datetime.
>
> b) An SQL/JSON null, defined as a value that is distinct from any value of
> any SQL type.
>
> NOTE 122 — An SQL/JSON null is distinct from the SQL null value.
>
> c) An SQL/JSON array, defined as an ordered list of zero or more SQL/JSON
> items, called the SQL/JSON
>
> elements of the SQL/JSON array.
>
> d) An SQL/JSON object, defined as an unordered collection of zero or more
> SQL/JSON members….
>
> “
>
> Our jsonb corresponds to SQL/JSON with UNIQUE KEYS and implicit ordering
> of keys and our main intention was to provide support of jsonb as a most
> important and usable data type.
>
> We created repository for reviewing (ask for write access) -
> https://github.com/postgrespro/sqljson/tree/sqljson
>
> Examples of usage can be found in src/test/regress/sql/sql_json.sql
>
> The whole documentation about json support should be reorganized and
> added, and we plan to do this before release. We need help of community
> here.
>
> Our goal is to provide support of main features of SQL/JSON to release 10,
> as we discussed at developers meeting in Brussels (Andrew Dunstan has
> kindly agreed to review the patch).
>
> We had not much time to develop the complete support, because of standard
> availability), but hope all major features are here, namely, all nine
> functions as described in the standard (but see implementation notes below):
>
> “All manipulation (e.g., retrieval, creation, testing) of SQL/JSON items
> is performed through a number of SQL/JSON functions. There are nine such
> functions, categorized as SQL/JSON retrieval functions and SQL/JSON
> construction functions. The SQL/JSON retrieval functions are characterized
> by operating on JSON data and returning an SQL value (possibly a Boolean
> value) or a JSON value. The SQL/JSON construction functions return JSON
> data created from operations on SQL data or other JSON data.
>
> The SQL/JSON retrieval functions are:
>
> — <JSON value function>: extracts an SQL value of a predefined type from a
> JSON text.
>
> — <JSON query>: extracts a JSON text from a JSON text.
>
> — <JSON table>: converts a JSON text to an SQL table.
>
> — <JSON predicate>: tests whether a string value is or is not properly
> formed JSON text.
>
> — <JSON exists predicate>: tests whether an SQL/JSON path expression
> returns any SQL/JSON items.
>
> The SQL/JSON construction functions are:
>
> — <JSON object constructor>: generates a string that is a serialization of
> an SQL/JSON object.
>
> — <JSON array constructor>: generates a string that is a serialization of
> an SQL/JSON array.
>
> — <JSON object aggregate constructor>: generates, from an aggregation of
> SQL data, a string that is a serialization
>
> of an SQL/JSON object.
>
> — <JSON array aggregate constructor>: generates, from an aggregation of
> SQL data, a string that is a serialization
>
> of an SQL/JSON array.
>
> A JSON-returning function is an SQL/JSON construction function or
> JSON_QUERY.”
>
> The standard describes SQL/JSON path language, which used by SQL/JSON
> query operators to query JSON. It defines path language as string literal.
> We implemented the path language as JSONPATH data type, since other
> approaches are not friendly to planner and executor.
>
> The functions and JSONPATH provide a new functionality for json support,
> namely, ability to operate (in standard specified way) with json structure
> at SQL-language level - the often requested feature by the users.
>
> The patch is consists of about 15000 insertions (about 5000 lines are from
> tests), passes all regression tests and doesn’t touches critical parts, so
> we hope with community help to bring it to committable state.
>
> Authors: Nikita Glukhov, Teodor Sigaev, Oleg Bartunov and Alexander
> Korotkov
>
> Implementation notes:
>
>
> 1.
>
> We didn’t implemented ‘datetime’ support, since it’s not clear from
> standard.
> 2.
>
> JSON_OBJECT/JSON_OBJECTAGG (KEY <key> VALUE <value>, ...) doesn’t
> implemented, only (<key>:<value>, …) and (<key> VALUE <value>, …) are
> supported, because of grammar conflicts with leading KEY keyword.
> 3.
>
> FORMAT (JSON|JSONB)) in JSON_ARRAYAGG with subquery doesn’t
> supported, because of grammar conflicts with non-reserved word FORMAT.
> 4.
>
> JSONPATH implemented only for jsonb data type , so JSON_EXISTS(),
> JSON_VALUE(), JSON_QUERY() and JSON_TABLE() doesn’t works if context item
> is of json data type.
> 5.
>
> Some methods and predicates for JSONPATH not yet implemented, for
> example .type(), .size(), .keyvalue(), predicates like_regex, starts
> with, etc. They are not key features and we plan to make them in next
> release.
> 6.
>
> JSONPATH doesn’t support expression for index array, like [2+3 to
> $upperbound], only simple constants like [5, 7 to 12] are supported.
> 7.
>
> JSONPATH extensions to standard: .** (wildcard path accessor), .key
> (member accessor without leading @).
> 8.
>
> FORMAT JSONB extension to standard for returning jsonb - standard
> specifies possibility of returning custom type.
> 9.
>
> JSON_EXISTS(), JSON_VALUE(), JSON_QUERY() are implemented using new
> executor node JsonExpr.
> 10.
>
> JSON_TABLE() is transformed into joined subselects with JSON_VALUE()
> and JSON_QUERY() in target list.
> 11.
>
> JSON_OBJECT(), JSON_ARRAY() constructors and IS JSON predicate are
> transformed into raw function calls.
> 12.
>
> Added explicit casts bytea=>jsonb and jsonb=>bytea (for jsonb=>bytea
> output using RETURNING bytea FORMAT JSONB and corresponding bytea=>jsonb
> input using <jsonb_bytea_expr> FORMAT JSONB).
>
>
>
Good work - it will be pretty big patch.

There is a intersection with implementation of XMLTABLE. I prepared a
executor infrastructure. So it can little bit reduce size of this patch.

Taking only Oracle as origin can be risk - in details Oracle doesn't
respects owns proposal to standard.

This is last commitfest for current release cycle - are you sure, so is
good idea to push all mentioned features?

Regards

Pavel

> Best regards,
>
> Oleg
>
>
> --
> Sent via pgsql-hackers mailing list (pgsql-hackers(at)postgresql(dot)org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers
>
>

On Tue, Feb 28, 2017 at 10:55 PM, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
wrote:

> Hi
>
>
> 2017-02-28 20:08 GMT+01:00 Oleg Bartunov <obartunov(at)gmail(dot)com>:
>
>> Hi there,
>>
>>
>> Attached patch is an implementation of SQL/JSON data model from SQL-2016
>> standard (ISO/IEC 9075-2:2016(E)), which was published 2016-12-15 and is
>> available only for purchase from ISO web site (
>> https://www.iso.org/standard/63556.html) Unfortunately I didn't find
>> any public sources of the standard or any preview documents, but Oracle
>> implementation of json support in 12c release 2 is very close (
>> http://docs.oracle.com/database/122/ADJSN/json-in-oracle-database.htm)
>> also we used https://livesql.oracle.com/ to understand some details.
>>
>> Postgres has already two json data types - json and jsonb and
>> implementing another json data type, which strictly conforms the standard,
>> would be not a good idea. Moreover, SQL standard doesn’t describe data
>> type, but only data model, which “comprises SQL/JSON items and SQL/JSON
>> sequences. The components of the SQL/JSON data model are:
>>
>> 1) An SQL/JSON item is defined recursively as any of the following:
>>
>> a) An SQL/JSON scalar, defined as a non-null value of any of the
>> following predefined (SQL) types:
>>
>> character string with character set Unicode, numeric, Boolean, or
>> datetime.
>>
>> b) An SQL/JSON null, defined as a value that is distinct from any value
>> of any SQL type.
>>
>> NOTE 122 — An SQL/JSON null is distinct from the SQL null value.
>>
>> c) An SQL/JSON array, defined as an ordered list of zero or more SQL/JSON
>> items, called the SQL/JSON
>>
>> elements of the SQL/JSON array.
>>
>> d) An SQL/JSON object, defined as an unordered collection of zero or more
>> SQL/JSON members….
>>
>> “
>>
>> Our jsonb corresponds to SQL/JSON with UNIQUE KEYS and implicit ordering
>> of keys and our main intention was to provide support of jsonb as a most
>> important and usable data type.
>>
>> We created repository for reviewing (ask for write access) -
>> https://github.com/postgrespro/sqljson/tree/sqljson
>>
>> Examples of usage can be found in src/test/regress/sql/sql_json.sql
>>
>> The whole documentation about json support should be reorganized and
>> added, and we plan to do this before release. We need help of community
>> here.
>>
>> Our goal is to provide support of main features of SQL/JSON to release
>> 10, as we discussed at developers meeting in Brussels (Andrew Dunstan has
>> kindly agreed to review the patch).
>>
>> We had not much time to develop the complete support, because of standard
>> availability), but hope all major features are here, namely, all nine
>> functions as described in the standard (but see implementation notes below):
>>
>> “All manipulation (e.g., retrieval, creation, testing) of SQL/JSON items
>> is performed through a number of SQL/JSON functions. There are nine such
>> functions, categorized as SQL/JSON retrieval functions and SQL/JSON
>> construction functions. The SQL/JSON retrieval functions are characterized
>> by operating on JSON data and returning an SQL value (possibly a Boolean
>> value) or a JSON value. The SQL/JSON construction functions return JSON
>> data created from operations on SQL data or other JSON data.
>>
>> The SQL/JSON retrieval functions are:
>>
>> — <JSON value function>: extracts an SQL value of a predefined type from
>> a JSON text.
>>
>> — <JSON query>: extracts a JSON text from a JSON text.
>>
>> — <JSON table>: converts a JSON text to an SQL table.
>>
>> — <JSON predicate>: tests whether a string value is or is not properly
>> formed JSON text.
>>
>> — <JSON exists predicate>: tests whether an SQL/JSON path expression
>> returns any SQL/JSON items.
>>
>> The SQL/JSON construction functions are:
>>
>> — <JSON object constructor>: generates a string that is a serialization
>> of an SQL/JSON object.
>>
>> — <JSON array constructor>: generates a string that is a serialization of
>> an SQL/JSON array.
>>
>> — <JSON object aggregate constructor>: generates, from an aggregation of
>> SQL data, a string that is a serialization
>>
>> of an SQL/JSON object.
>>
>> — <JSON array aggregate constructor>: generates, from an aggregation of
>> SQL data, a string that is a serialization
>>
>> of an SQL/JSON array.
>>
>> A JSON-returning function is an SQL/JSON construction function or
>> JSON_QUERY.”
>>
>> The standard describes SQL/JSON path language, which used by SQL/JSON
>> query operators to query JSON. It defines path language as string literal.
>> We implemented the path language as JSONPATH data type, since other
>> approaches are not friendly to planner and executor.
>>
>> The functions and JSONPATH provide a new functionality for json support,
>> namely, ability to operate (in standard specified way) with json structure
>> at SQL-language level - the often requested feature by the users.
>>
>> The patch is consists of about 15000 insertions (about 5000 lines are
>> from tests), passes all regression tests and doesn’t touches critical
>> parts, so we hope with community help to bring it to committable state.
>>
>> Authors: Nikita Glukhov, Teodor Sigaev, Oleg Bartunov and Alexander
>> Korotkov
>>
>> Implementation notes:
>>
>>
>> 1.
>>
>> We didn’t implemented ‘datetime’ support, since it’s not clear from
>> standard.
>> 2.
>>
>> JSON_OBJECT/JSON_OBJECTAGG (KEY <key> VALUE <value>, ...) doesn’t
>> implemented, only (<key>:<value>, …) and (<key> VALUE <value>, …) are
>> supported, because of grammar conflicts with leading KEY keyword.
>> 3.
>>
>> FORMAT (JSON|JSONB)) in JSON_ARRAYAGG with subquery doesn’t
>> supported, because of grammar conflicts with non-reserved word FORMAT.
>> 4.
>>
>> JSONPATH implemented only for jsonb data type , so JSON_EXISTS(),
>> JSON_VALUE(), JSON_QUERY() and JSON_TABLE() doesn’t works if context item
>> is of json data type.
>> 5.
>>
>> Some methods and predicates for JSONPATH not yet implemented, for
>> example .type(), .size(), .keyvalue(), predicates like_regex, starts
>> with, etc. They are not key features and we plan to make them in next
>> release.
>> 6.
>>
>> JSONPATH doesn’t support expression for index array, like [2+3 to
>> $upperbound], only simple constants like [5, 7 to 12] are supported.
>> 7.
>>
>> JSONPATH extensions to standard: .** (wildcard path accessor), .key
>> (member accessor without leading @).
>> 8.
>>
>> FORMAT JSONB extension to standard for returning jsonb - standard
>> specifies possibility of returning custom type.
>> 9.
>>
>> JSON_EXISTS(), JSON_VALUE(), JSON_QUERY() are implemented using new
>> executor node JsonExpr.
>> 10.
>>
>> JSON_TABLE() is transformed into joined subselects with JSON_VALUE()
>> and JSON_QUERY() in target list.
>> 11.
>>
>> JSON_OBJECT(), JSON_ARRAY() constructors and IS JSON predicate are
>> transformed into raw function calls.
>> 12.
>>
>> Added explicit casts bytea=>jsonb and jsonb=>bytea (for jsonb=>bytea
>> output using RETURNING bytea FORMAT JSONB and corresponding bytea=>jsonb
>> input using <jsonb_bytea_expr> FORMAT JSONB).
>>
>>
>>
> Good work - it will be pretty big patch.
>
> There is a intersection with implementation of XMLTABLE. I prepared a
> executor infrastructure. So it can little bit reduce size of this patch.
>

we considered your XMLTABLE patch, but it's itself pretty big and in
unknown state.

>
> Taking only Oracle as origin can be risk - in details Oracle doesn't
> respects owns proposal to standard.
>

we used an original standard document ! I suggest Oracle to those, who
don't have access to standard. Yes, there are some problem in Oracle's
implementation.

>
> This is last commitfest for current release cycle - are you sure, so is
> good idea to push all mentioned features?
>

This would be a great feature for Release 10 and I understand all risks.
Hopefully, community will help us. We have resources to continue our work
and will do as much as possible to satisfy community requirements. It's not
our fault, that standard was released so late :)

>
> Regards
>
> Pavel
>
>
>
>
>> Best regards,
>>
>> Oleg
>>
>>
>> --
>> Sent via pgsql-hackers mailing list (pgsql-hackers(at)postgresql(dot)org)
>> To make changes to your subscription:
>> http://www.postgresql.org/mailpref/pgsql-hackers
>>
>>
>

>
>
>>>
>> Good work - it will be pretty big patch.
>>
>> There is a intersection with implementation of XMLTABLE. I prepared a
>> executor infrastructure. So it can little bit reduce size of this patch.
>>
>
> we considered your XMLTABLE patch, but it's itself pretty big and in
> unknown state.
>

It is big, but it is hard to expect so JSON_TABLE can be shorter if you are
solve all commiters requests.

Last patch should be near to final state.

>
>
>>
>> Taking only Oracle as origin can be risk - in details Oracle doesn't
>> respects owns proposal to standard.
>>
>
> we used an original standard document ! I suggest Oracle to those, who
> don't have access to standard. Yes, there are some problem in Oracle's
> implementation.
>
>
>>
>> This is last commitfest for current release cycle - are you sure, so is
>> good idea to push all mentioned features?
>>
>
> This would be a great feature for Release 10 and I understand all risks.
> Hopefully, community will help us. We have resources to continue our work
> and will do as much as possible to satisfy community requirements. It's not
> our fault, that standard was released so late :)
>

It is not your fault. Ok, I am looking for patches.

Regards

Pavel

>
>
>
>>
>> Regards
>>
>> Pavel
>>
>>
>>
>>
>>> Best regards,
>>>
>>> Oleg
>>>
>>>
>>> --
>>> Sent via pgsql-hackers mailing list (pgsql-hackers(at)postgresql(dot)org)
>>> To make changes to your subscription:
>>> http://www.postgresql.org/mailpref/pgsql-hackers
>>>
>>>
>>
>

>
>
>
> 1.
>
> Added explicit casts bytea=>jsonb and jsonb=>bytea (for jsonb=>bytea
> output using RETURNING bytea FORMAT JSONB and corresponding bytea=>jsonb
> input using <jsonb_bytea_expr> FORMAT JSONB).
>
>
This point has sense in Oracle, where JSON is blob. But it is little bit
obscure in PostgreSQL context.

Regards

Pavel

> Best regards,
>
> Oleg
>
>
> --
> Sent via pgsql-hackers mailing list (pgsql-hackers(at)postgresql(dot)org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers
>
>

Hi Oleg,

On 2/28/17 2:55 PM, Pavel Stehule wrote:
> 2017-02-28 20:08 GMT+01:00 Oleg Bartunov <obartunov(at)gmail(dot)com
>
> Attached patch is an implementation of SQL/JSON data model from
> SQL-2016 standard (ISO/IEC 9075-2:2016(E)), which was published
> 2016-12-15 and is available only for purchase from ISO web site
> (https://www.iso.org/standard/63556.html
> <https://www.iso.org/standard/63556.html>). Unfortunately I didn't
> find any public sources of the standard or any preview documents,
> but Oracle implementation of json support in 12c release 2 is very
> close
> (http://docs.oracle.com/database/122/ADJSN/json-in-oracle-database.htm
> <http://docs.oracle.com/database/122/ADJSN/json-in-oracle-database.htm>),
> also we used https://livesql.oracle.com/ to understand some details.

<...>

> This is last commitfest for current release cycle - are you sure, so is
> good idea to push all mentioned features?

Implementing standards is always a goal of the PostgreSQL community, but
this is a very large patch arriving very late in the release cycle with
no prior discussion.

That the patch proposed follows a standard which will not be available
to the majority of reviewers is very worrisome, let alone the sheer
size. While much of the code is new, I see many changes to core data
structures that could very easily be destabilizing.

I propose we move this patch to the 2017-07 CF so further development
and review can be done without haste and as the standard becomes more
accessible.

Regards,
--
-David
david(at)pgmasters(dot)net

2017-03-03 21:49 GMT+01:00 David Steele <david(at)pgmasters(dot)net>:

> Hi Oleg,
>
> On 2/28/17 2:55 PM, Pavel Stehule wrote:
> > 2017-02-28 20:08 GMT+01:00 Oleg Bartunov <obartunov(at)gmail(dot)com
> >
> > Attached patch is an implementation of SQL/JSON data model from
> > SQL-2016 standard (ISO/IEC 9075-2:2016(E)), which was published
> > 2016-12-15 and is available only for purchase from ISO web site
> > (https://www.iso.org/standard/63556.html
> > <https://www.iso.org/standard/63556.html>). Unfortunately I didn't
> > find any public sources of the standard or any preview documents,
> > but Oracle implementation of json support in 12c release 2 is very
> > close
> > (http://docs.oracle.com/database/122/ADJSN/json-in-
> oracle-database.htm
> > <http://docs.oracle.com/database/122/ADJSN/json-in-
> oracle-database.htm>),
> > also we used https://livesql.oracle.com/ to understand some
> details.
>
> <...>
>
> > This is last commitfest for current release cycle - are you sure, so is
> > good idea to push all mentioned features?
>
> Implementing standards is always a goal of the PostgreSQL community, but
> this is a very large patch arriving very late in the release cycle with
> no prior discussion.
>
> That the patch proposed follows a standard which will not be available
> to the majority of reviewers is very worrisome, let alone the sheer
> size. While much of the code is new, I see many changes to core data
> structures that could very easily be destabilizing.
>
> I propose we move this patch to the 2017-07 CF so further development
> and review can be done without haste and as the standard becomes more
> accessible.
>

Although I would to see these features in Postgres early I have same
feeling. Is it a question if some features can be implemented easy and can
be merged early?

The implementation of some JSON generation functions can be easy and the
verification should not be hard. Different situation is in JSON querying
functions. Merging JSONPath in first commitfest is better.

Regards

Pavel

> Regards,
> --
> -David
> david(at)pgmasters(dot)net
>

On Fri, Mar 3, 2017 at 11:49 PM, David Steele <david(at)pgmasters(dot)net> wrote:

> Hi Oleg,
>
> On 2/28/17 2:55 PM, Pavel Stehule wrote:
> > 2017-02-28 20:08 GMT+01:00 Oleg Bartunov <obartunov(at)gmail(dot)com
> >
> > Attached patch is an implementation of SQL/JSON data model from
> > SQL-2016 standard (ISO/IEC 9075-2:2016(E)), which was published
> > 2016-12-15 and is available only for purchase from ISO web site
> > (https://www.iso.org/standard/63556.html
> > <https://www.iso.org/standard/63556.html>). Unfortunately I didn't
> > find any public sources of the standard or any preview documents,
> > but Oracle implementation of json support in 12c release 2 is very
> > close
> > (http://docs.oracle.com/database/122/ADJSN/json-in-
> oracle-database.htm
> > <http://docs.oracle.com/database/122/ADJSN/json-in-
> oracle-database.htm>),
> > also we used https://livesql.oracle.com/ to understand some
> details.
>
> <...>
>
> > This is last commitfest for current release cycle - are you sure, so is
> > good idea to push all mentioned features?
>
> Implementing standards is always a goal of the PostgreSQL community, but
> this is a very large patch arriving very late in the release cycle with
> no prior discussion.
>

We discussed this in Brussels, but I agree, the patch is rather big.

>
> That the patch proposed follows a standard which will not be available
> to the majority of reviewers is very worrisome, let alone the sheer
> size. While much of the code is new, I see many changes to core data
> structures that could very easily be destabilizing.
>

I don't know when the standard will be publicly available.

>
> I propose we move this patch to the 2017-07 CF so further development
> and review can be done without haste and as the standard becomes more
> accessible.
>

I wanted to have one more good feature in 10 and let postgres be on par
with other competitors. SQL/JSON adds many interesting features and users
will be dissapointed if we postpone it for next two years. Let's wait for
reviewers, probably they will find the patch is not very intrusive. We
have a plenty of time and we dedicate one full-time developer for this
project.

>
> Regards,
> --
> -David
> david(at)pgmasters(dot)net
>

Hi,

On 2017-03-07 12:21:59 +0300, Oleg Bartunov wrote:
> On 2017-03-03 15:49:38 -0500, David Steele wrote:
> > I propose we move this patch to the 2017-07 CF so further development
> > and review can be done without haste and as the standard becomes more
> > accessible.

+1

> I wanted to have one more good feature in 10 and let postgres be on par
> with other competitors. SQL/JSON adds many interesting features and users
> will be dissapointed if we postpone it for next two years. Let's wait for
> reviewers, probably they will find the patch is not very intrusive.

I think it's way too late to late for a patch of this size for 10. And I
don't think it's fair to a lot of other patches of significant size that
have been submitted way earlier, that also need reviewing resources, to
say that we can just see whether it'll get the required resources.

> We have a plenty of time and we dedicate one full-time developer for
> this project.

How about having that, and perhaps others, developer participate in
reviewing patches and getting to the bottom of the commitfest? Should
we end up being done early, we can look at this patch... There's not
been review activity corresponding to the amount of submissions from
pgpro...

- Andres

Hi,

about the datetime issue: as far as I know, JSON does not define a
serialization format for dates and timestamps.

On the other hand, YAML (as a superset of JSON) already supports a
language-independent date(time) serialization format
(http://yaml.org/type/timestamp.html)

I haven't had a glance into the SQL/JSON standard yet and a quick search
didn't reveal anything. However, reading your test case here
https://github.com/postgrespro/sqljson/blob/5a8a241/src/test/regress/sql/sql_json.sql#L411
it seems as if you intend to parse all strings in the form of
"YYYY-MM-DD" as dates. This is problematic in case a string happens to
look like this but is not intended to be a date.

Just for the sake of completeness: YAML solves this issue by omitting
the quotation marks around the date string (just as JSON integers have
no quotations marks around them).

Regards,
Sven

On 3/7/17 11:38 AM, Andres Freund wrote:

<...>

>> We have a plenty of time and we dedicate one full-time developer for
>> this project.
>
> How about having that, and perhaps others, developer participate in
> reviewing patches and getting to the bottom of the commitfest? Should
> we end up being done early, we can look at this patch... There's not
> been review activity corresponding to the amount of submissions from
> pgpro...

This patch has been moved to CF 2017-07.

--
-David
david(at)pgmasters(dot)net

On Wed, Mar 8, 2017 at 12:43 AM, Sven R. Kunze <srkunze(at)mail(dot)de> wrote:

> Hi,
>
> about the datetime issue: as far as I know, JSON does not define a
> serialization format for dates and timestamps.
>
> On the other hand, YAML (as a superset of JSON) already supports a
> language-independent date(time) serialization format (
> http://yaml.org/type/timestamp.html)
>
> I haven't had a glance into the SQL/JSON standard yet and a quick search
> didn't reveal anything. However, reading your test case here
> https://github.com/postgrespro/sqljson/blob/5a8a241/src/
> test/regress/sql/sql_json.sql#L411 it seems as if you intend to parse all
> strings in the form of "YYYY-MM-DD" as dates. This is problematic in case a
> string happens to look like this but is not intended to be a date.
>

SQL/JSON defines methods in jsonpath, in particularly,

| datetime <left paren> [ <JSON datetime template> ] <right paren>
| keyvalue <left paren> <right paren>

<JSON datetime template> ::=
<JSON path string literal>

datetime template is also specified in the standard (very rich)

<datetime template> ::=
{ <datetime template part> }...
<datetime template part> ::=
<datetime template field>
| <datetime template delimiter>
<datetime template field> ::=
<datetime template year>
| <datetime template rounded year>
| <datetime template month>
| <datetime template day of month>
| <datetime template day of year>
| <datetime template 12-hour>
| <datetime template 24-hour>
| <datetime template minute>
| <datetime template second of minute>
| <datetime template second of day>
| <datetime template fraction>
| <datetime template am/pm>
| <datetime template time zone hour>
| <datetime template time zone minute>
<datetime template delimiter> ::=
<minus sign>
| <period>
| <solidus>
| <comma>
| <apostrophe>
| <semicolon>
| <colon>
| <space>
<datetime template year> ::=
YYYY | YYY | YY | Y
<datetime template rounded year> ::=
RRRR | RR
<datetime template month> ::=
MM
<datetime template day of month> ::=
DD
<datetime template day of year> ::=
DDD
<datetime template 12-hour> ::=
HH | HH12
<datetime template 24-hour> ::=
HH24
<datetime template minute> ::=
MI
<datetime template second of minute> ::=
SS
<datetime template second of day> ::=
SSSSS
<datetime template fraction> ::=
FF1 | FF2 | FF3 | FF4 | FF5 | FF6 | FF7 | FF8 | FF9
<datetime template am/pm> ::=
A.M. | P.M.
<datetime template time zone hour> ::=
TZH
<datetime template time zone minute> ::=
TZM

> Just for the sake of completeness: YAML solves this issue by omitting the
> quotation marks around the date string (just as JSON integers have no
> quotations marks around them).
>

interesting idea, but need to dig the standard first.

>
> Regards,
> Sven
>

On Wed, Mar 8, 2017 at 7:05 AM, David Steele <david(at)pgmasters(dot)net> wrote:

> On 3/7/17 11:38 AM, Andres Freund wrote:
>
> <...>
>
> We have a plenty of time and we dedicate one full-time developer for
>>> this project.
>>>
>>
>> How about having that, and perhaps others, developer participate in
>> reviewing patches and getting to the bottom of the commitfest? Should
>> we end up being done early, we can look at this patch... There's not
>> been review activity corresponding to the amount of submissions from
>> pgpro...
>>
>
> This patch has been moved to CF 2017-07.
>

Yes, after committing XMLTABLE, we anyway need to extend its infrastructure
to support JSON_TABLE.

>
> --
> -David
> david(at)pgmasters(dot)net
>

Small point of order: YAML is not strictly a super-set of JSON.

Editorializing slightly, I have not seen much interest in the world for
YAML support though I'd be interested in evidence to the contrary.

On Tue, Mar 7, 2017 at 1:43 PM, Sven R. Kunze <srkunze(at)mail(dot)de> wrote:

> Hi,
>
> about the datetime issue: as far as I know, JSON does not define a
> serialization format for dates and timestamps.
>
> On the other hand, YAML (as a superset of JSON) already supports a
> language-independent date(time) serialization format (
> http://yaml.org/type/timestamp.html)
>
> I haven't had a glance into the SQL/JSON standard yet and a quick search
> didn't reveal anything. However, reading your test case here
> https://github.com/postgrespro/sqljson/blob/5a8a241/src/
> test/regress/sql/sql_json.sql#L411 it seems as if you intend to parse all
> strings in the form of "YYYY-MM-DD" as dates. This is problematic in case a
> string happens to look like this but is not intended to be a date.
>
> Just for the sake of completeness: YAML solves this issue by omitting the
> quotation marks around the date string (just as JSON integers have no
> quotations marks around them).
>
> Regards,
> Sven
>
>
>
> --
> Sent via pgsql-hackers mailing list (pgsql-hackers(at)postgresql(dot)org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers
>

--
Peter van Hardenberg
San Francisco, California
"Everything was beautiful, and nothing hurt."—Kurt Vonnegut

On Wed, Mar 8, 2017 at 11:48 AM, Peter van Hardenberg <pvh(at)pvh(dot)ca> wrote:

> Small point of order: YAML is not strictly a super-set of JSON.
>
> Editorializing slightly, I have not seen much interest in the world for
> YAML support though I'd be interested in evidence to the contrary.
>
>
The world of configuration management seems to for some reason run off
YAML, but that's the only places I've seen it recently (ansible, puppet
etc).

That said if we're introducing something new, it's usually better to copy
from another format than to invite your own.

--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/

On Tue, Mar 7, 2017 at 2:38 PM, Andres Freund <andres(at)anarazel(dot)de> wrote:
> On 2017-03-07 12:21:59 +0300, Oleg Bartunov wrote:
>> On 2017-03-03 15:49:38 -0500, David Steele wrote:
>> > I propose we move this patch to the 2017-07 CF so further development
>> > and review can be done without haste and as the standard becomes more
>> > accessible.
>
> +1

I agree that this should not go into v10. February 28th is not the
right time for a large, never-before-seen patch to show up with
expectations of getting committed for the current cycle.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On 08.03.2017 20:48, Peter van Hardenberg wrote:
> Small point of order: YAML is not strictly a super-set of JSON.

I haven't read the whole standard, but from what I can see the standard
considers JSON an official subset of itself:
http://www.yaml.org/spec/1.2/spec.html

Regards,
Sven

On Thu, Mar 9, 2017 at 12:48 PM, Sven R. Kunze <srkunze(at)mail(dot)de> wrote:
> On 08.03.2017 20:48, Peter van Hardenberg wrote:
>>
>> Small point of order: YAML is not strictly a super-set of JSON.
>
> I haven't read the whole standard, but from what I can see the standard
> considers JSON an official subset of itself:
> http://www.yaml.org/spec/1.2/spec.html

But there's apparent sophistry, like this, in that spec:

SON's RFC4627 requires that mappings keys merely “SHOULD” be unique,
while YAML insists they “MUST” be. Technically, YAML therefore
complies with the JSON spec, choosing to treat duplicates as an error.
In practice, since JSON is silent on the semantics of such duplicates,
the only portable JSON files are those with unique keys, which are
therefore valid YAML files.

I don't see how YAML can impose a stronger requirement than JSON and
yet claim to be a superset; a JSON document that doesn't meet that
requirement will be legal (if stupid) as JSON but illegal as YAML.

Also, even if the superset thing were true on a theoretical plane, I'm
not sure it would do us much good in practice. If we start using
YAML-specific constructs, we won't have valid JSON any more. If we
use only things that are legal in JSON, YAML's irrelevant.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On 08.03.2017 20:52, Magnus Hagander wrote:
> On Wed, Mar 8, 2017 at 11:48 AM, Peter van Hardenberg <pvh(at)pvh(dot)ca
> <mailto:pvh(at)pvh(dot)ca>> wrote:
>
> Small point of order: YAML is not strictly a super-set of JSON.
>
> Editorializing slightly, I have not seen much interest in the
> world for YAML support though I'd be interested in evidence to the
> contrary.
>
>
> The world of configuration management seems to for some reason run off
> YAML, but that's the only places I've seen it recently (ansible,
> puppet etc).

SaltStack uses YAML for their tools, too. I personally can empathize
with them (as a user of configuration management) about this as writing
JSON would be nightmare with all the quoting, commas, curly braces etc.
But that's my own preference maybe.

(Btw. does "run off" mean like or avoid? At least my dictionaries tend
to the latter.)

> That said if we're introducing something new, it's usually better to
> copy from another format than to invite your own.

From my day-to-day work I can tell, the date(time) type is the only
missing piece of JSON to make it perfect for business applications
(besides, maybe, a "currency" type).

Regards,
Sven

On 09.03.2017 18:58, Robert Haas wrote:
> Also, even if the superset thing were true on a theoretical plane, I'm
> not sure it would do us much good in practice. If we start using
> YAML-specific constructs, we won't have valid JSON any more. If we
> use only things that are legal in JSON, YAML's irrelevant.

That's true. I just wanted to share my view of the "date guessing" part
of pgpro's commits.
I don't have a good solution for it either, I can only tell that where I
work we do have same issues: either we guess by looking at the string
value or we know that "this particular key" must be a date.
Unsatisfied with either solution, we tend to use YAML for our APIs if
possible.

Regards,
Sven

Anecdotally, we just stored dates as strings and used a convention (key
ends in "_at", I believe) to interpret them. The lack of support for dates
in JSON is well-known, universally decried... and not a problem the
PostgreSQL community can fix.

On Thu, Mar 9, 2017 at 10:24 AM, Sven R. Kunze <srkunze(at)mail(dot)de> wrote:

> On 09.03.2017 18:58, Robert Haas wrote:
>
>> Also, even if the superset thing were true on a theoretical plane, I'm
>> not sure it would do us much good in practice. If we start using
>> YAML-specific constructs, we won't have valid JSON any more. If we
>> use only things that are legal in JSON, YAML's irrelevant.
>>
>
> That's true. I just wanted to share my view of the "date guessing" part of
> pgpro's commits.
> I don't have a good solution for it either, I can only tell that where I
> work we do have same issues: either we guess by looking at the string value
> or we know that "this particular key" must be a date.
> Unsatisfied with either solution, we tend to use YAML for our APIs if
> possible.
>
>
> Regards,
> Sven
>
>
> --
> Sent via pgsql-hackers mailing list (pgsql-hackers(at)postgresql(dot)org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers
>

--
Peter van Hardenberg
San Francisco, California
"Everything was beautiful, and nothing hurt."—Kurt Vonnegut

On 09.03.2017 19:50, Peter van Hardenberg wrote:
> Anecdotally, we just stored dates as strings and used a convention
> (key ends in "_at", I believe) to interpret them. The lack of support
> for dates in JSON is well-known, universally decried... and not a
> problem the PostgreSQL community can fix.

I completely agree here.

Regards,
Sven

On 09/03/17 19:50, Peter van Hardenberg wrote:
> Anecdotally, we just stored dates as strings and used a convention (key
> ends in "_at", I believe) to interpret them. The lack of support for
> dates in JSON is well-known, universally decried... and not a problem
> the PostgreSQL community can fix.
>

The original complain was about JSON_VALUE extracting date but I don't
understand why there is problem with that, the SQL/JSON defines that
behavior. The RETURNING clause there is more or less just shorthand for
casting with some advanced options.

--
Petr Jelinek http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

On 10.03.2017 05:07, Petr Jelinek wrote:
> The original complain was about JSON_VALUE extracting date but I don't
> understand why there is problem with that, the SQL/JSON defines that
> behavior. The RETURNING clause there is more or less just shorthand for
> casting with some advanced options.

Thanks for clarifying. I mistook it as if JSON_VALUE itself returns a
date value.

Sven

On Thu, Mar 9, 2017 at 1:12 PM, Sven R. Kunze <srkunze(at)mail(dot)de> wrote:

> On 08.03.2017 20:52, Magnus Hagander wrote:
>
> On Wed, Mar 8, 2017 at 11:48 AM, Peter van Hardenberg <pvh(at)pvh(dot)ca> wrote:
>
>> Small point of order: YAML is not strictly a super-set of JSON.
>>
>> Editorializing slightly, I have not seen much interest in the world for
>> YAML support though I'd be interested in evidence to the contrary.
>>
>>
> The world of configuration management seems to for some reason run off
> YAML, but that's the only places I've seen it recently (ansible, puppet
> etc).
>
>
> SaltStack uses YAML for their tools, too. I personally can empathize with
> them (as a user of configuration management) about this as writing JSON
> would be nightmare with all the quoting, commas, curly braces etc. But
> that's my own preference maybe.
>
> (Btw. does "run off" mean like or avoid? At least my dictionaries tend to
> the latter.)
>

In this case, it means like. "run off" as in "the car runs off fuel" or
something like that. Probably a bad choice of words.

--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/

On 03/09/2017 10:12 AM, Sven R. Kunze wrote:
> On 08.03.2017 20:52, Magnus Hagander wrote:
>> On Wed, Mar 8, 2017 at 11:48 AM, Peter van Hardenberg <pvh(at)pvh(dot)ca
>> <mailto:pvh(at)pvh(dot)ca>> wrote:
>>
>> Small point of order: YAML is not strictly a super-set of JSON.
>>
>> Editorializing slightly, I have not seen much interest in the
>> world for YAML support though I'd be interested in evidence to the
>> contrary.
>>
>>
>> The world of configuration management seems to for some reason run off
>> YAML, but that's the only places I've seen it recently (ansible,
>> puppet etc).
>
> SaltStack uses YAML for their tools, too. I personally can empathize
> with them (as a user of configuration management) about this as writing
> JSON would be nightmare with all the quoting, commas, curly braces etc.
> But that's my own preference maybe.
>
> (Btw. does "run off" mean like or avoid? At least my dictionaries tend
> to the latter.)

Yes, but automated tools can easily convert between JSON and
newline-delimited YAML and back.

--
Josh Berkus
Containers & Databases Oh My!

On Fri, Mar 10, 2017 at 7:07 AM, Petr Jelinek <petr(dot)jelinek(at)2ndquadrant(dot)com>
wrote:

> On 09/03/17 19:50, Peter van Hardenberg wrote:
> > Anecdotally, we just stored dates as strings and used a convention (key
> > ends in "_at", I believe) to interpret them. The lack of support for
> > dates in JSON is well-known, universally decried... and not a problem
> > the PostgreSQL community can fix.
> >
>
> The original complain was about JSON_VALUE extracting date but I don't
> understand why there is problem with that, the SQL/JSON defines that
> behavior. The RETURNING clause there is more or less just shorthand for
> casting with some advanced options.
>

There is no problem with serializing date and SQL/JSON describes it rather
well. There is no correct procedure to deserialize date from a correct json
string and the standards keeps silence about this and now we understand
that date[time] is actually virtual and the only use of them is in jsonpath
(filter) expressions.

>
> --
> Petr Jelinek http://www.2ndQuadrant.com/
> PostgreSQL Development, 24x7 Support, Training & Services
>

On Tue, Mar 07, 2017 at 10:43:16PM +0100, Sven R. Kunze wrote:
> about the datetime issue: as far as I know, JSON does not define a
> serialization format for dates and timestamps.

Use strings in ISO 8601 format, with or without fractional seconds, and
maybe with 5-digit years.

> On the other hand, YAML (as a superset of JSON) already supports a
> language-independent date(time) serialization format
> (http://yaml.org/type/timestamp.html)

But YAML isn't what this is about.

Nico
--

On Thu, Mar 09, 2017 at 12:58:55PM -0500, Robert Haas wrote:
> On Thu, Mar 9, 2017 at 12:48 PM, Sven R. Kunze <srkunze(at)mail(dot)de> wrote:
> > On 08.03.2017 20:48, Peter van Hardenberg wrote:
> >>
> >> Small point of order: YAML is not strictly a super-set of JSON.
> >
> > I haven't read the whole standard, but from what I can see the standard
> > considers JSON an official subset of itself:
> > http://www.yaml.org/spec/1.2/spec.html
>
> But there's apparent sophistry, like this, in that spec:

I agree with you. But beware, the IETF has had multiple threads with
thousands of posts in them about these sorts of issues. If you're not
careful you'll have such a thread on this list too. It would be very
sad not to only let a group that really cares have such threads instead.

:)

Nico
--

On Thu, Mar 09, 2017 at 07:12:07PM +0100, Sven R. Kunze wrote:
> From my day-to-day work I can tell, the date(time) type is the only missing
> piece of JSON to make it perfect for business applications (besides, maybe,
> a "currency" type).

And a binary type. And a chunked-string type (to avoid having to escape
strings). And an interval type. And...

2017-03-13 7:24 GMT+01:00 Nico Williams <nico(at)cryptonector(dot)com>:

> On Thu, Mar 09, 2017 at 07:12:07PM +0100, Sven R. Kunze wrote:
> > From my day-to-day work I can tell, the date(time) type is the only
> missing
> > piece of JSON to make it perfect for business applications (besides,
> maybe,
> > a "currency" type).
>
> And a binary type. And a chunked-string type (to avoid having to escape
> strings). And an interval type. And...
>

It is designed/born be simple - if you need some more complex, then you can
use XML with schema, ...

Pavel

On Mon, Mar 13, 2017 at 9:24 AM, Nico Williams <nico(at)cryptonector(dot)com>
wrote:

> On Thu, Mar 09, 2017 at 07:12:07PM +0100, Sven R. Kunze wrote:
> > From my day-to-day work I can tell, the date(time) type is the only
> missing
> > piece of JSON to make it perfect for business applications (besides,
> maybe,
> > a "currency" type).
>
> And a binary type. And a chunked-string type (to avoid having to escape
> strings). And an interval type. And...
>

Let's first have this basic implementation in postgres, then we'll add
extendability support not only for types, but also for operators.
Right now I see in our regression tests:

select _jsonpath_object(
'["10.03.2017 12:34 +1", "10.03.2017 12:35 +1", "10.03.2017 12:36 +1",
"10.03.2017 12:35 +2", "10.03.2017 12:35 -2"]',
'$[*].datetime("dd.mm.yyyy HH24:MI TZH") ? (@ < "10.03.2017 12:35
+1".datetime("dd.mm.yyyy HH24:MI TZH"))'
);
_jsonpath_object
--------------------------
"2017-03-10 14:34:00+03"
"2017-03-10 13:35:00+03"
(2 rows)

On 10.03.2017 20:28, Josh Berkus wrote:
> On 03/09/2017 10:12 AM, Sven R. Kunze wrote:
>>
>> SaltStack uses YAML for their tools, too. I personally can empathize
>> with them (as a user of configuration management) about this as writing
>> JSON would be nightmare with all the quoting, commas, curly braces etc.
>> But that's my own preference maybe.
> Yes, but automated tools can easily convert between JSON and
> newline-delimited YAML and back.

Sure. That wasn't point, though.

Sven

On 13.03.2017 07:24, Nico Williams wrote:
> On Thu, Mar 09, 2017 at 07:12:07PM +0100, Sven R. Kunze wrote:
>> From my day-to-day work I can tell, the date(time) type is the only missing
>> piece of JSON to make it perfect for business applications (besides, maybe,
>> a "currency" type).
> And a binary type. And a chunked-string type (to avoid having to escape
> strings). And an interval type. And...

YMMV but I tend to say that those aren't the usual types of a business
application where I come from.

Answering questions like "how many" (integer), "what" (text) and "when"
(date) is far more common than "give me that binary blob" at least in
the domain where I work. Never had the necessity for an interval type;
usually had a start and end value where the "interval" was derived from
those values.

On 3/7/17 11:05 PM, David Steele wrote:
> On 3/7/17 11:38 AM, Andres Freund wrote:
>
> <...>
>
>>> We have a plenty of time and we dedicate one full-time developer for
>>> this project.
>>
>> How about having that, and perhaps others, developer participate in
>> reviewing patches and getting to the bottom of the commitfest? Should
>> we end up being done early, we can look at this patch... There's not
>> been review activity corresponding to the amount of submissions from
>> pgpro...
>
> This patch has been moved to CF 2017-07.

I did not manage to move this patch when I said had. It is now moved.

Thank,
--
-David
david(at)pgmasters(dot)net

On 3/15/17 11:56, David Steele wrote:
>> This patch has been moved to CF 2017-07.
>
> I did not manage to move this patch when I said had. It is now moved.

Unsurprisingly, this patch needs a major rebase.

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

> On 15 Aug 2017, at 04:30, Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com> wrote:
>
> On 3/15/17 11:56, David Steele wrote:
>>> This patch has been moved to CF 2017-07.
>>
>> I did not manage to move this patch when I said had. It is now moved.
>
> Unsurprisingly, this patch needs a major rebase.

Can we expect a rebased version of this patch for this commitfest? Since it’s
a rather large feature it would be good to get it in as early as we can in the
process.

cheers ./daniel

On Fri, Sep 15, 2017 at 10:10 AM, Daniel Gustafsson <daniel(at)yesql(dot)se> wrote:
> Can we expect a rebased version of this patch for this commitfest? Since it’s
> a rather large feature it would be good to get it in as early as we can in the
> process.

Again, given that this needs a "major" rebase and hasn't been updated
in a month, and given that the CF is already half over, this should
just be bumped to the next CF. We're supposed to be trying to review
things that were ready to go by the start of the CF, not the end.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On Fri, Sep 15, 2017 at 7:31 PM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
> On Fri, Sep 15, 2017 at 10:10 AM, Daniel Gustafsson <daniel(at)yesql(dot)se> wrote:
>> Can we expect a rebased version of this patch for this commitfest? Since it’s
>> a rather large feature it would be good to get it in as early as we can in the
>> process.
>
> Again, given that this needs a "major" rebase and hasn't been updated
> in a month, and given that the CF is already half over, this should
> just be bumped to the next CF. We're supposed to be trying to review
> things that were ready to go by the start of the CF, not the end.

We are supporting v10 branch in our github repository
https://github.com/postgrespro/sqljson/tree/sqljson_v10

Since the first post we made a lot of changes, mostly because of
better understanding the standard and availability of technical report
(http://standards.iso.org/ittf/PubliclyAvailableStandards/c067367_ISO_IEC_TR_19075-6_2017.zip)
Most important are:

1.We abandoned FORMAT support, which could confuse our users, since we
have data types json[b].

2. We use XMLTABLE infrastructure, extended for JSON_TABLE support.

3. Reorganize commits, so we could split one big patch by several
smaller patches, which could be reviewed independently.

4. The biggest problem is documentation, we are working on it.

Nikita will submit patches soon.

>
> --
> Robert Haas
> EnterpriseDB: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company

On 15.09.2017 22:36, Oleg Bartunov wrote:

> On Fri, Sep 15, 2017 at 7:31 PM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
>> On Fri, Sep 15, 2017 at 10:10 AM, Daniel Gustafsson <daniel(at)yesql(dot)se> wrote:
>>> Can we expect a rebased version of this patch for this commitfest? Since it’s
>>> a rather large feature it would be good to get it in as early as we can in the
>>> process.
>> Again, given that this needs a "major" rebase and hasn't been updated
>> in a month, and given that the CF is already half over, this should
>> just be bumped to the next CF. We're supposed to be trying to review
>> things that were ready to go by the start of the CF, not the end.
> We are supporting v10 branch in our github repository
> https://github.com/postgrespro/sqljson/tree/sqljson_v10
>
> Since the first post we made a lot of changes, mostly because of
> better understanding the standard and availability of technical report
> (http://standards.iso.org/ittf/PubliclyAvailableStandards/c067367_ISO_IEC_TR_19075-6_2017.zip)
> Most important are:
>
> 1.We abandoned FORMAT support, which could confuse our users, since we
> have data types json[b].
>
> 2. We use XMLTABLE infrastructure, extended for JSON_TABLE support.
>
> 3. Reorganize commits, so we could split one big patch by several
> smaller patches, which could be reviewed independently.
>
> 4. The biggest problem is documentation, we are working on it.
>
> Nikita will submit patches soon.

Attached archive with 9 patches rebased onto latest master.

0001-jsonpath-v02.patch:
- jsonpath type
- jsonpath execution on jsonb type
- jsonpath operators for jsonb type
- GIN support for jsonpath operators

0002-jsonpath-json-v02.patch:
- jsonb-like iterators for json type
- jsonpath execution on json type
- jsonpath operators for json type

0003-jsonpath-extensions-v02.patch:
0004-jsonpath-extensions-tests-for-json-v02.patch:
- some useful standard extensions with tests

0005-sqljson-v02.patch:
- SQL/JSON constructors (JSON_OBJECT[AGG], JSON_ARRAY[AGG])
- SQL/JSON query functions (JSON_VALUE, JSON_QUERY, JSON_EXISTS)
- IS JSON predicate

0006-sqljson-json-v02.patch:
- SQL/JSON support for json type and tests

0007-json_table-v02.patch:
- JSON_TABLE using XMLTABLE infrastructure

0008-json_table-json-v02.patch:
- JSON_TABLE support for json type

0009-wip-extensions-v02.patch:
- FORMAT JSONB
- jsonb to/from bytea casts
- jsonpath operators
- some unfinished jsonpath extensions

Originally, JSON path was implemented only for jsonb type, and I decided to
add jsonb-like iterators for json type for json support implementation with
minimal changes in JSON path code. This solution (see jsonpath_json.c from
patch 0002) looks a little dubious to me, so I separated json support into
independent patches.

The last WIP patch 0009 is unfinished and contains a lot of FIXMEs. But
the ability to use arbitrary Postgres operators in JSON path with explicitly
specified types is rather interesting, and I think it should be shown now
to get a some kind of pre-review.

We are supporting v11 and v10 branches in our github repository:

https://github.com/postgrespro/sqljson/tree/sqljson
https://github.com/postgrespro/sqljson/tree/sqljson_wip
https://github.com/postgrespro/sqljson/tree/sqljson_v10
https://github.com/postgrespro/sqljson/tree/sqljson_v10_wip

Attached patches can be produced simply by combining groups of consecutive
commits from these branches.

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

On 16 Sep 2017 02:32, "Nikita Glukhov" <n(dot)gluhov(at)postgrespro(dot)ru> wrote:

On 15.09.2017 22:36, Oleg Bartunov wrote:

On Fri, Sep 15, 2017 at 7:31 PM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
>
>> On Fri, Sep 15, 2017 at 10:10 AM, Daniel Gustafsson <daniel(at)yesql(dot)se>
>> wrote:
>>
>>> Can we expect a rebased version of this patch for this commitfest?
>>> Since it’s
>>> a rather large feature it would be good to get it in as early as we can
>>> in the
>>> process.
>>>
>> Again, given that this needs a "major" rebase and hasn't been updated
>> in a month, and given that the CF is already half over, this should
>> just be bumped to the next CF. We're supposed to be trying to review
>> things that were ready to go by the start of the CF, not the end.
>>
> We are supporting v10 branch in our github repository
> https://github.com/postgrespro/sqljson/tree/sqljson_v10
>
> Since the first post we made a lot of changes, mostly because of
> better understanding the standard and availability of technical report
> (http://standards.iso.org/ittf/PubliclyAvailableStandards/c0
> 67367_ISO_IEC_TR_19075-6_2017.zip).
> Most important are:
>
> 1.We abandoned FORMAT support, which could confuse our users, since we
> have data types json[b].
>
> 2. We use XMLTABLE infrastructure, extended for JSON_TABLE support.
>
> 3. Reorganize commits, so we could split one big patch by several
> smaller patches, which could be reviewed independently.
>
> 4. The biggest problem is documentation, we are working on it.
>
> Nikita will submit patches soon.
>

Attached archive with 9 patches rebased onto latest master.

0001-jsonpath-v02.patch:
- jsonpath type
- jsonpath execution on jsonb type
- jsonpath operators for jsonb type
- GIN support for jsonpath operators

0002-jsonpath-json-v02.patch:
- jsonb-like iterators for json type
- jsonpath execution on json type
- jsonpath operators for json type

0003-jsonpath-extensions-v02.patch:
0004-jsonpath-extensions-tests-for-json-v02.patch:
- some useful standard extensions with tests
0005-sqljson-v02.patch:
- SQL/JSON constructors (JSON_OBJECT[AGG], JSON_ARRAY[AGG])
- SQL/JSON query functions (JSON_VALUE, JSON_QUERY, JSON_EXISTS)
- IS JSON predicate

0006-sqljson-json-v02.patch:
- SQL/JSON support for json type and tests

0007-json_table-v02.patch:
- JSON_TABLE using XMLTABLE infrastructure

0008-json_table-json-v02.patch:
- JSON_TABLE support for json type

0009-wip-extensions-v02.patch:
- FORMAT JSONB
- jsonb to/from bytea casts
- jsonpath operators
- some unfinished jsonpath extensions

Originally, JSON path was implemented only for jsonb type, and I decided to
add jsonb-like iterators for json type for json support implementation with
minimal changes in JSON path code. This solution (see jsonpath_json.c from
patch 0002) looks a little dubious to me, so I separated json support into
independent patches.

The last WIP patch 0009 is unfinished and contains a lot of FIXMEs. But
the ability to use arbitrary Postgres operators in JSON path with explicitly
specified types is rather interesting, and I think it should be shown now
to get a some kind of pre-review.

We are supporting v11 and v10 branches in our github repository:

https://github.com/postgrespro/sqljson/tree/sqljson
https://github.com/postgrespro/sqljson/tree/sqljson_wip
https://github.com/postgrespro/sqljson/tree/sqljson_v10
https://github.com/postgrespro/sqljson/tree/sqljson_v10_wip

We provide web interface to our build
http://sqlfiddle.postgrespro.ru/#!21/

Attached patches can be produced simply by combining groups of consecutive
commits from these branches.

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

On Sun, Sep 17, 2017 at 11:08 AM, Oleg Bartunov <obartunov(at)gmail(dot)com> wrote:

> On 16 Sep 2017 02:32, "Nikita Glukhov" <n(dot)gluhov(at)postgrespro(dot)ru> wrote:
>
> On 15.09.2017 22:36, Oleg Bartunov wrote:
>
> On Fri, Sep 15, 2017 at 7:31 PM, Robert Haas <robertmhaas(at)gmail(dot)com>
>> wrote:
>>
>>> On Fri, Sep 15, 2017 at 10:10 AM, Daniel Gustafsson <daniel(at)yesql(dot)se>
>>> wrote:
>>>
>>>> Can we expect a rebased version of this patch for this commitfest?
>>>> Since it’s
>>>> a rather large feature it would be good to get it in as early as we can
>>>> in the
>>>> process.
>>>>
>>> Again, given that this needs a "major" rebase and hasn't been updated
>>> in a month, and given that the CF is already half over, this should
>>> just be bumped to the next CF. We're supposed to be trying to review
>>> things that were ready to go by the start of the CF, not the end.
>>>
>> We are supporting v10 branch in our github repository
>> https://github.com/postgrespro/sqljson/tree/sqljson_v10
>>
>> Since the first post we made a lot of changes, mostly because of
>> better understanding the standard and availability of technical report
>> (http://standards.iso.org/ittf/PubliclyAvailableStandards/c0
>> 67367_ISO_IEC_TR_19075-6_2017.zip).
>> Most important are:
>>
>> 1.We abandoned FORMAT support, which could confuse our users, since we
>> have data types json[b].
>>
>> 2. We use XMLTABLE infrastructure, extended for JSON_TABLE support.
>>
>> 3. Reorganize commits, so we could split one big patch by several
>> smaller patches, which could be reviewed independently.
>>
>> 4. The biggest problem is documentation, we are working on it.
>>
>> Nikita will submit patches soon.
>>
>
> Attached archive with 9 patches rebased onto latest master.
>
> 0001-jsonpath-v02.patch:
> - jsonpath type
> - jsonpath execution on jsonb type
> - jsonpath operators for jsonb type
> - GIN support for jsonpath operators
>
> 0002-jsonpath-json-v02.patch:
> - jsonb-like iterators for json type
> - jsonpath execution on json type
> - jsonpath operators for json type
>
> 0003-jsonpath-extensions-v02.patch:
> 0004-jsonpath-extensions-tests-for-json-v02.patch:
> - some useful standard extensions with tests
> 0005-sqljson-v02.patch:
> - SQL/JSON constructors (JSON_OBJECT[AGG], JSON_ARRAY[AGG])
> - SQL/JSON query functions (JSON_VALUE, JSON_QUERY, JSON_EXISTS)
> - IS JSON predicate
>
> 0006-sqljson-json-v02.patch:
> - SQL/JSON support for json type and tests
>
> 0007-json_table-v02.patch:
> - JSON_TABLE using XMLTABLE infrastructure
>
> 0008-json_table-json-v02.patch:
> - JSON_TABLE support for json type
>
> 0009-wip-extensions-v02.patch:
> - FORMAT JSONB
> - jsonb to/from bytea casts
> - jsonpath operators
> - some unfinished jsonpath extensions
>
>
> Originally, JSON path was implemented only for jsonb type, and I decided to
> add jsonb-like iterators for json type for json support implementation with
> minimal changes in JSON path code. This solution (see jsonpath_json.c from
> patch 0002) looks a little dubious to me, so I separated json support into
> independent patches.
>
> The last WIP patch 0009 is unfinished and contains a lot of FIXMEs. But
> the ability to use arbitrary Postgres operators in JSON path with
> explicitly
> specified types is rather interesting, and I think it should be shown now
> to get a some kind of pre-review.
>
> We are supporting v11 and v10 branches in our github repository:
>
> https://github.com/postgrespro/sqljson/tree/sqljson
> https://github.com/postgrespro/sqljson/tree/sqljson_wip
> https://github.com/postgrespro/sqljson/tree/sqljson_v10
> https://github.com/postgrespro/sqljson/tree/sqljson_v10_wip
>
>
> We provide web interface to our build
> http://sqlfiddle.postgrespro.ru/#!21/
>

+1,
For experimenting with SQL/JSON select "PostgreSQL 10dev+SQL/JSON" in the
version select field on top toolbar.

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

Nikita Glukhov wrote:

> 0007-json_table-v02.patch:
> - JSON_TABLE using XMLTABLE infrastructure
>
> 0008-json_table-json-v02.patch:
> - JSON_TABLE support for json type

I'm confused ... why are these two patches and not a single one?

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

On 18.09.2017 00:38, Alvaro Herrera wrote:

> Nikita Glukhov wrote:
>
>> 0007-json_table-v02.patch:
>> - JSON_TABLE using XMLTABLE infrastructure
>>
>> 0008-json_table-json-v02.patch:
>> - JSON_TABLE support for json type
> I'm confused ... why are these two patches and not a single one?
>
As I sad before, json support in jsonpath looks a bit dubious to me.  So if
patch no. 2 will not be accepted, then patches no. 4, 6, 8 should also be
simply skipped. But, of course, patches 1 and 2, 3 and 4, 5 and 6, 7 and 8
can be combined.

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

Hi

2017-09-16 1:31 GMT+02:00 Nikita Glukhov <n(dot)gluhov(at)postgrespro(dot)ru>:

> On 15.09.2017 22:36, Oleg Bartunov wrote:
>
> On Fri, Sep 15, 2017 at 7:31 PM, Robert Haas <robertmhaas(at)gmail(dot)com>
>> wrote:
>>
>>> On Fri, Sep 15, 2017 at 10:10 AM, Daniel Gustafsson <daniel(at)yesql(dot)se>
>>> wrote:
>>>
>>>> Can we expect a rebased version of this patch for this commitfest?
>>>> Since it’s
>>>> a rather large feature it would be good to get it in as early as we can
>>>> in the
>>>> process.
>>>>
>>> Again, given that this needs a "major" rebase and hasn't been updated
>>> in a month, and given that the CF is already half over, this should
>>> just be bumped to the next CF. We're supposed to be trying to review
>>> things that were ready to go by the start of the CF, not the end.
>>>
>> We are supporting v10 branch in our github repository
>> https://github.com/postgrespro/sqljson/tree/sqljson_v10
>>
>> Since the first post we made a lot of changes, mostly because of
>> better understanding the standard and availability of technical report
>> (http://standards.iso.org/ittf/PubliclyAvailableStandards/c0
>> 67367_ISO_IEC_TR_19075-6_2017.zip).
>> Most important are:
>>
>> 1.We abandoned FORMAT support, which could confuse our users, since we
>> have data types json[b].
>>
>> 2. We use XMLTABLE infrastructure, extended for JSON_TABLE support.
>>
>> 3. Reorganize commits, so we could split one big patch by several
>> smaller patches, which could be reviewed independently.
>>
>> 4. The biggest problem is documentation, we are working on it.
>>
>> Nikita will submit patches soon.
>>
>
> Attached archive with 9 patches rebased onto latest master.
>
> 0001-jsonpath-v02.patch:
> - jsonpath type
> - jsonpath execution on jsonb type
> - jsonpath operators for jsonb type
> - GIN support for jsonpath operators
>
> 0002-jsonpath-json-v02.patch:
> - jsonb-like iterators for json type
> - jsonpath execution on json type
> - jsonpath operators for json type
>
> 0003-jsonpath-extensions-v02.patch:
> 0004-jsonpath-extensions-tests-for-json-v02.patch:
> - some useful standard extensions with tests
> 0005-sqljson-v02.patch:
> - SQL/JSON constructors (JSON_OBJECT[AGG], JSON_ARRAY[AGG])
> - SQL/JSON query functions (JSON_VALUE, JSON_QUERY, JSON_EXISTS)
> - IS JSON predicate
>
> 0006-sqljson-json-v02.patch:
> - SQL/JSON support for json type and tests
>
> 0007-json_table-v02.patch:
> - JSON_TABLE using XMLTABLE infrastructure
>
> 0008-json_table-json-v02.patch:
> - JSON_TABLE support for json type
>
> 0009-wip-extensions-v02.patch:
> - FORMAT JSONB
> - jsonb to/from bytea casts
> - jsonpath operators
> - some unfinished jsonpath extensions
>
>
> Originally, JSON path was implemented only for jsonb type, and I decided to
> add jsonb-like iterators for json type for json support implementation with
> minimal changes in JSON path code. This solution (see jsonpath_json.c from
> patch 0002) looks a little dubious to me, so I separated json support into
> independent patches.
>
> The last WIP patch 0009 is unfinished and contains a lot of FIXMEs. But
> the ability to use arbitrary Postgres operators in JSON path with
> explicitly
> specified types is rather interesting, and I think it should be shown now
> to get a some kind of pre-review.
>
> We are supporting v11 and v10 branches in our github repository:
>
> https://github.com/postgrespro/sqljson/tree/sqljson
> https://github.com/postgrespro/sqljson/tree/sqljson_wip
> https://github.com/postgrespro/sqljson/tree/sqljson_v10
> https://github.com/postgrespro/sqljson/tree/sqljson_v10_wip
>
> Attached patches can be produced simply by combining groups of consecutive
> commits from these branches.
>
>
I have some free time now. Is it last version?

Regards

Pavel

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

On 29.09.2017 12:59, Pavel Stehule wrote:

> Hi
>
> 2017-09-16 1:31 GMT+02:00 Nikita Glukhov <n(dot)gluhov(at)postgrespro(dot)ru
> <mailto:n(dot)gluhov(at)postgrespro(dot)ru>>:
>
> On 15.09.2017 22:36, Oleg Bartunov wrote:
>
> On Fri, Sep 15, 2017 at 7:31 PM, Robert Haas
> <robertmhaas(at)gmail(dot)com <mailto:robertmhaas(at)gmail(dot)com>> wrote:
>
> On Fri, Sep 15, 2017 at 10:10 AM, Daniel Gustafsson
> <daniel(at)yesql(dot)se <mailto:daniel(at)yesql(dot)se>> wrote:
>
> Can we expect a rebased version of this patch for this
> commitfest?  Since it’s
> a rather large feature it would be good to get it in
> as early as we can in the
> process.
>
> Again, given that this needs a "major" rebase and hasn't
> been updated
> in a month, and given that the CF is already half over,
> this should
> just be bumped to the next CF.  We're supposed to be
> trying to review
> things that were ready to go by the start of the CF, not
> the end.
>
> We are supporting v10 branch in our github repository
> https://github.com/postgrespro/sqljson/tree/sqljson_v10
> <https://github.com/postgrespro/sqljson/tree/sqljson_v10>
>
> Since the first post we made a lot of changes, mostly because of
> better understanding the standard and availability of
> technical report
> (http://standards.iso.org/ittf/PubliclyAvailableStandards/c067367_ISO_IEC_TR_19075-6_2017.zip
> <http://standards.iso.org/ittf/PubliclyAvailableStandards/c067367_ISO_IEC_TR_19075-6_2017.zip>).
> Most important are:
>
> 1.We abandoned FORMAT support, which could confuse our users,
> since we
> have data types json[b].
>
> 2. We use XMLTABLE infrastructure, extended for JSON_TABLE
> support.
>
> 3. Reorganize commits, so we could split one big patch by several
> smaller patches, which could be reviewed independently.
>
> 4. The biggest problem is documentation, we are working on it.
>
> Nikita will submit patches soon.
>
>
> Attached archive with 9 patches rebased onto latest master.
>
> 0001-jsonpath-v02.patch:
>  - jsonpath type
>  - jsonpath execution on jsonb type
>  - jsonpath operators for jsonb type
>  - GIN support for jsonpath operators
>
> 0002-jsonpath-json-v02.patch:
>  - jsonb-like iterators for json type
>  - jsonpath execution on json type
>  - jsonpath operators for json type
>
> 0003-jsonpath-extensions-v02.patch:
> 0004-jsonpath-extensions-tests-for-json-v02.patch:
>  - some useful standard extensions with tests
>  0005-sqljson-v02.patch:
>  - SQL/JSON constructors (JSON_OBJECT[AGG], JSON_ARRAY[AGG])
>  - SQL/JSON query functions (JSON_VALUE, JSON_QUERY, JSON_EXISTS)
>  - IS JSON predicate
>
> 0006-sqljson-json-v02.patch:
>  - SQL/JSON support for json type and tests
>
> 0007-json_table-v02.patch:
>  - JSON_TABLE using XMLTABLE infrastructure
>
> 0008-json_table-json-v02.patch:
>  - JSON_TABLE support for json type
>
> 0009-wip-extensions-v02.patch:
>  - FORMAT JSONB
>  - jsonb to/from bytea casts
>  - jsonpath operators
>  - some unfinished jsonpath extensions
>
>
> Originally, JSON path was implemented only for jsonb type, and I
> decided to
> add jsonb-like iterators for json type for json support
> implementation with
> minimal changes in JSON path code.  This solution (see
> jsonpath_json.c from
> patch 0002) looks a little dubious to me, so I separated json
> support into
> independent patches.
>
> The last WIP patch 0009 is unfinished and contains a lot of
> FIXMEs.  But
> the ability to use arbitrary Postgres operators in JSON path with
> explicitly
> specified  types is rather interesting, and I think it should be
> shown now
> to get a some kind of pre-review.
>
> We are supporting v11 and v10 branches in our github repository:
>
> https://github.com/postgrespro/sqljson/tree/sqljson
> <https://github.com/postgrespro/sqljson/tree/sqljson>
> https://github.com/postgrespro/sqljson/tree/sqljson_wip
> <https://github.com/postgrespro/sqljson/tree/sqljson_wip>
> https://github.com/postgrespro/sqljson/tree/sqljson_v10
> <https://github.com/postgrespro/sqljson/tree/sqljson_v10>
> https://github.com/postgrespro/sqljson/tree/sqljson_v10_wip
> <https://github.com/postgrespro/sqljson/tree/sqljson_v10_wip>
>
> Attached patches can be produced simply by combining groups of
> consecutive
> commits from these branches.
>
>
> I have some free time now. Is it last version?
>
> Regards
>
> Pavel
>
Yes, this is still the latest version. Now I am working only on
unfinished WIP
patch no. 9, but I think it should be reviewed the last.

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

2017-09-29 12:09 GMT+02:00 Nikita Glukhov <n(dot)gluhov(at)postgrespro(dot)ru>:

>
>
> I have some free time now. Is it last version?
>
> Regards
>
> Pavel
>
> Yes, this is still the latest version. Now I am working only on unfinished
> WIP
> patch no. 9, but I think it should be reviewed the last.
>
>

ok

Thank you

Pavel

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

2017-09-29 12:15 GMT+02:00 Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>:

>
>
> 2017-09-29 12:09 GMT+02:00 Nikita Glukhov <n(dot)gluhov(at)postgrespro(dot)ru>:
>
>>
>>
>> I have some free time now. Is it last version?
>>
>> Regards
>>
>> Pavel
>>
>> Yes, this is still the latest version. Now I am working only on
>> unfinished WIP
>> patch no. 9, but I think it should be reviewed the last.
>>
>>
>
> ok
>
> Thank you
>

I have few queries and notes

1. Why first patch holds Gin related functionality? Can be it separated?

2. Why Json path functions starts by "_" ? These functions are not removed
by other patches.

3. What is base for jsonpath-extensions? ANSI/SQL?

This patch is pretty big - so I propose to push JSONPath and SQL/JSON
related patches first, and then in next iteration to push JSON_TABLE patch.
Is it acceptable strategy? I am sure so JSON_TABLE is pretty important
function, but it is pretty complex too (significantly more complex than
XMLTABLE), so it can be practiacal to move this function to separate
project. I hope so all patches will be merged in release 11 time.

Regards

Pavel

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

On 29.09.2017 20:07, Pavel Stehule wrote:

> 2017-09-29 12:15 GMT+02:00 Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com
> <mailto:pavel(dot)stehule(at)gmail(dot)com>>:
>
>
> 2017-09-29 12:09 GMT+02:00 Nikita Glukhov <n(dot)gluhov(at)postgrespro(dot)ru
> <mailto:n(dot)gluhov(at)postgrespro(dot)ru>>:
>
>
>>
>> I have some free time now. Is it last version?
>>
>> Regards
>>
>> Pavel
>>
> Yes, this is still the latest version. Now I am working only
> on unfinished WIP
> patch no. 9, but I think it should be reviewed the last.
>
>
>
> ok
>
> Thank you
>
>
> I have few queries and notes
>
> 1. Why first patch holds Gin related functionality? Can be it separated?
Yes, it can be easily separated. Attached archive with separated GIN
patch no.2.

> 2. Why Json path functions starts by "_" ? These functions are not
> removed by other patches.
Originally, these functions were created only for testing purposes and
should
be treated as "internal". But with introduction of jsonpath operators
jsonpath
tests can be completely rewritten using this operators.

> 3. What is base for jsonpath-extensions? ANSI/SQL?
Our jsonpath extensions are not based on any standards, so they are quite
dangerous because they can conflict with the standard in the future.

> This patch is pretty big - so I propose to push JSONPath and SQL/JSON
> related patches first, and then in next iteration to push JSON_TABLE
> patch. Is it acceptable strategy?
I think it's acceptable. And this was the main reason for the separation
of patches.

> I am sure so JSON_TABLE is pretty important function, but it is pretty
> complex too (significantly more complex than XMLTABLE), so it can be
> practiacal to move this function to separate project. I hope so all
> patches will be merged in release 11 time.

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

2017-09-30 1:06 GMT+02:00 Nikita Glukhov <n(dot)gluhov(at)postgrespro(dot)ru>:

> On 29.09.2017 20:07, Pavel Stehule wrote:
>
> 2017-09-29 12:15 GMT+02:00 Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>:
>
>>
>> 2017-09-29 12:09 GMT+02:00 Nikita Glukhov <n(dot)gluhov(at)postgrespro(dot)ru>:
>>
>>>
>>>
>>> I have some free time now. Is it last version?
>>>
>>> Regards
>>>
>>> Pavel
>>>
>>> Yes, this is still the latest version. Now I am working only on
>>> unfinished WIP
>>> patch no. 9, but I think it should be reviewed the last.
>>>
>>>
>>
>> ok
>>
>> Thank you
>>
>
> I have few queries and notes
>
> 1. Why first patch holds Gin related functionality? Can be it separated?
>
> Yes, it can be easily separated. Attached archive with separated GIN patch
> no.2.
>
> 2. Why Json path functions starts by "_" ? These functions are not removed
> by other patches.
>
> Originally, these functions were created only for testing purposes and
> should
> be treated as "internal". But with introduction of jsonpath operators
> jsonpath
> tests can be completely rewritten using this operators.
>
> 3. What is base for jsonpath-extensions? ANSI/SQL?
>
> Our jsonpath extensions are not based on any standards, so they are quite
> dangerous because they can conflict with the standard in the future.
>
> This patch is pretty big - so I propose to push JSONPath and SQL/JSON
> related patches first, and then in next iteration to push JSON_TABLE patch.
> Is it acceptable strategy?
>
> I think it's acceptable. And this was the main reason for the separation
> of patches.
>

I prefer to move it to another commit fest item. It will simplify a
communication between us and possible committers - and we can better
concentrate to smaller set of code.

> I am sure so JSON_TABLE is pretty important function, but it is pretty
> complex too (significantly more complex than XMLTABLE), so it can be
> practiacal to move this function to separate project. I hope so all patches
> will be merged in release 11 time.
>
>
> --
> Nikita Glukhov
> Postgres Professional: http://www.postgrespro.com
> The Russian Postgres Company
>

2017-09-30 1:06 GMT+02:00 Nikita Glukhov <n(dot)gluhov(at)postgrespro(dot)ru>:

> On 29.09.2017 20:07, Pavel Stehule wrote:
>
> 2017-09-29 12:15 GMT+02:00 Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>:
>
>>
>> 2017-09-29 12:09 GMT+02:00 Nikita Glukhov <n(dot)gluhov(at)postgrespro(dot)ru>:
>>
>>>
>>>
>>> I have some free time now. Is it last version?
>>>
>>> Regards
>>>
>>> Pavel
>>>
>>> Yes, this is still the latest version. Now I am working only on
>>> unfinished WIP
>>> patch no. 9, but I think it should be reviewed the last.
>>>
>>>
>>
>> ok
>>
>> Thank you
>>
>
> I have few queries and notes
>
> 1. Why first patch holds Gin related functionality? Can be it separated?
>
> Yes, it can be easily separated. Attached archive with separated GIN patch
> no.2.
>
> 2. Why Json path functions starts by "_" ? These functions are not removed
> by other patches.
>
> Originally, these functions were created only for testing purposes and
> should
> be treated as "internal". But with introduction of jsonpath operators
> jsonpath
> tests can be completely rewritten using this operators.
>

yes - it should be removed.

Probably separation to jsonpath and sqljson is not happy (or sqljson part
should not contains JSON_QUERY and related functions).

Why this code is in patch?

*+/********************Example functions for
JsonPath***************************/++static Datum+returnDATUM(void *arg,
bool *isNull)+{+<->*isNull =
false;+<->return<>PointerGetDatum(arg);+}++static Datum+returnNULL(void
*arg, bool *isNull)+{+<->*isNull = true;+<->return Int32GetDatum(0);+}+*
Regards

Pavel

> 3. What is base for jsonpath-extensions? ANSI/SQL?
>
> Our jsonpath extensions are not based on any standards, so they are quite
> dangerous because they can conflict with the standard in the future.
>
> This patch is pretty big - so I propose to push JSONPath and SQL/JSON
> related patches first, and then in next iteration to push JSON_TABLE patch.
> Is it acceptable strategy?
>
> I think it's acceptable. And this was the main reason for the separation
> of patches.
>
> I am sure so JSON_TABLE is pretty important function, but it is pretty
> complex too (significantly more complex than XMLTABLE), so it can be
> practiacal to move this function to separate project. I hope so all patches
> will be merged in release 11 time.
>
>
> --
> Nikita Glukhov
> Postgres Professional: http://www.postgrespro.com
> The Russian Postgres Company
>

Hi, hackers!

I have a question about transformation of JSON constructors into executor nodes.

In first letter in this thread we wrote:
JSON_OBJECT(), JSON_ARRAY() constructors and IS JSON predicate are
transformed into raw function calls.

Here is an example explaining what it means:

=# CREATE VIEW json_object_view AS
SELECT JSON_OBJECT('foo': 1, 'bar': '[1,2]' FORMAT JSON RETURNING text);
CREATE VIEW
=# \sv json_object_view
CREATE OR REPLACE VIEW public.json_object_view AS
SELECT json_build_object_ext(false, false, 'foo', 1, 'bar', '[1,2]'::text::json)::text

As you can see JSON_OBJECT() was transformed into a call on new function
json_build_object_ext(), which shares a code with existing json_build_object()
but differs from it only by two additional boolean parameters for
representation of {WITH|WITHOUT} UNIQUE [KEYS] and {NULL|ABSENT} ON NULL
clauses. Information about FORMAT, RETURNING clauses was lost, since they
were transformed into casts.

Other constructors are transformed similary:
JSON_ARRAY() => json[b]_build_array_ext(boolean, VARIADIC any)
JSON_OBJECTAGG() => json[b]_objectagg(any, any, boolean, boolean)
JSON_ARRAYAGG() => json[b]_agg[_strict](any)

Also there is a variant of JSON_ARRAY() with subquery which transformed into a
subselect with json[b]_agg():
=# CREATE VIEW json_array_view AS SELECT JSON_ARRAY(SELECT generate_series(1,3));
CREATE VIEW
=# \sv json_array_view
CREATE OR REPLACE VIEW public.json_array_view AS
SELECT ( SELECT json_agg_strict(q.a)
FROM ( SELECT generate_series(1, 3) AS generate_series) q(a))

And here is my question: is it acceptable to do such transformations?
And if is not acceptable (it seemed unacceptable to us from the beginning,
but we did not have time for correct implementation), how should JSON
constructor nodes look like?

The simplest solution that I can propose is to save both transformed
expressions in existing JsonObjectCtor/JsonArrayCtor nodes which exist
now only in untransformed trees. Whole untransformed JsonXxxCtor node
will be used for displaying, transformed expression -- for execution only.

But it will not work for aggregates, because they are transformed into a
Aggref/WindowFunc node. Information needed for correct displaying should be
saved somewhere in these standard nodes.

And for subquery variant of JSON_ARRAY I can only offer to leave transformation
into a subselect with JSON_ARRAYAGG():
JSON_ARRAY(query) => (SELECT JSON_ARRAYAGG(bar) FROM (query) foo(bar))

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

Could someone clarify the status of this patch set? It has been in
"Waiting" mode since the previous CF and no new patch, just a few
questions from the author.

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

Hi

2017-11-02 3:39 GMT+01:00 Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com
>:

> Could someone clarify the status of this patch set? It has been in
> "Waiting" mode since the previous CF and no new patch, just a few
> questions from the author.
>

There was a state "needs review". I looked to the patch, and found some
issues, so I sent mail about these issues and switched state to "waiting on
author"

Regards

Pavel

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

On 2017-02-28 20:08, Oleg Bartunov wrote:
> Attached patch is an implementation of SQL/JSON data model from SQL-2016
> standard (ISO/IEC 9075-2:2016(E))

I've faintly started looking into this.

> We created repository for reviewing (ask for write access) -
> https://github.com/postgrespro/sqljson/tree/sqljson

> Examples of usage can be found in src/test/regress/sql/sql_json.sql

> The whole documentation about json support should be reorganized and added,
> and we plan to do this before release. We need help of community here.

> The standard describes SQL/JSON path language, which used by SQL/JSON query
> operators to query JSON. It defines path language as string literal. We
> implemented the path language as JSONPATH data type, since other
> approaches are not friendly to planner and executor.

I was a bit sad to discover that I can't
PREPARE jsq AS SELECT JSON_QUERY('{}', $1);
I assume because of this part of the updated grammar:
json_path_specification:
Sconst { $$ = $1; }
;

Would it make sense, fundamentally, to allow variables there? After
Andrew Gierth's analysis of this grammar problem, I understand that it's
not reasonable to expect JSON_TABLE() to support variable jsonpaths, but
maybe it would be feasible for everything else? From Andrew's changes to
the new grammar (see attached) it seems to me that at least that part is
possible. Or should I forget about trying to implement the other part?

On 03.11.2017 00:32, Piotr Stefaniak wrote:

> On 2017-02-28 20:08, Oleg Bartunov wrote:
>> The standard describes SQL/JSON path language, which used by SQL/JSON query
>> operators to query JSON. It defines path language as string literal. We
>> implemented the path language as JSONPATH data type, since other
>> approaches are not friendly to planner and executor.
> I was a bit sad to discover that I can't
> PREPARE jsq AS SELECT JSON_QUERY('{}', $1);
> I assume because of this part of the updated grammar:
> json_path_specification:
> Sconst { $$ = $1; }
> ;
>
> Would it make sense, fundamentally, to allow variables there? After
> Andrew Gierth's analysis of this grammar problem, I understand that it's
> not reasonable to expect JSON_TABLE() to support variable jsonpaths, but
> maybe it would be feasible for everything else? From Andrew's changes to
> the new grammar (see attached) it seems to me that at least that part is
> possible. Or should I forget about trying to implement the other part?
By standard only string literals can be used in JSON path specifications.
But of course it is possible to allow to use variable jsonpath
expressions in
SQL/JSON functions.

Attached patch implements this feature for JSON query functions,
JSON_TABLE is
not supported now because it needs some refactoring.

I have pushed this commit to the separate branch because it is not
finished yet:
https://github.com/postgrespro/sqljson/tree/sqljson_variable_json_path

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

On Fri, Nov 3, 2017 at 11:29 AM, Nikita Glukhov <n(dot)gluhov(at)postgrespro(dot)ru> wrote:
> By standard only string literals can be used in JSON path specifications.
> But of course it is possible to allow to use variable jsonpath expressions
> in
> SQL/JSON functions.
>
> Attached patch implements this feature for JSON query functions, JSON_TABLE
> is
> not supported now because it needs some refactoring.
>
> I have pushed this commit to the separate branch because it is not finished
> yet:
> https://github.com/postgrespro/sqljson/tree/sqljson_variable_json_path

The patch sent previously does not directly apply on HEAD, and as far
as I can see the last patch set published on
/message-id/2361ae4a-66b1-c6c5-ea6a-84851a1c08b5@postgrespro.ru
has rotten. Could you send a new patch set?

About the patch set, I had a look at the first patch which is not that
heavy, however it provides zero documentation, close to zero comments,
but adds more than 500 lines of code. I find that a bit hard to give
an opinion on, having commit messages associated to each patch would
be also nice. This way, reviewers can figure what's going out in this
mess and provide feedback. Making things incremental is welcome as
well, for example in the first patch I have a hard way finding out why
timestamps are touched to begin with.

The patch is already marked as "waiting on author" for more than one month.
--
Michael

On Fri, Nov 3, 2017 at 12:07 PM, Michael Paquier
<michael(dot)paquier(at)gmail(dot)com> wrote:
> The patch sent previously does not directly apply on HEAD, and as far
> as I can see the last patch set published on
> /message-id/2361ae4a-66b1-c6c5-ea6a-84851a1c08b5@postgrespro.ru
> has rotten. Could you send a new patch set?
>
> About the patch set, I had a look at the first patch which is not that
> heavy, however it provides zero documentation, close to zero comments,
> but adds more than 500 lines of code. I find that a bit hard to give
> an opinion on, having commit messages associated to each patch would
> be also nice. This way, reviewers can figure what's going out in this
> mess and provide feedback. Making things incremental is welcome as
> well, for example in the first patch I have a hard way finding out why
> timestamps are touched to begin with.

My mistake here, only the first patch adds 8,200 lines of code. This
makes the lack of comments and docs even worse.
--
Michael

On 03.11.2017 15:07, Michael Paquier wrote:

> On Fri, Nov 3, 2017 at 11:29 AM, Nikita Glukhov <n(dot)gluhov(at)postgrespro(dot)ru> wrote:
>> By standard only string literals can be used in JSON path specifications.
>> But of course it is possible to allow to use variable jsonpath expressions
>> in
>> SQL/JSON functions.
>>
>> Attached patch implements this feature for JSON query functions, JSON_TABLE
>> is
>> not supported now because it needs some refactoring.
>>
>> I have pushed this commit to the separate branch because it is not finished
>> yet:
>> https://github.com/postgrespro/sqljson/tree/sqljson_variable_json_path
> The patch sent previously does not directly apply on HEAD, and as far
> as I can see the last patch set published on
> /message-id/2361ae4a-66b1-c6c5-ea6a-84851a1c08b5@postgrespro.ru
> has rotten. Could you send a new patch set?
Attached patch set rebased onto current master.
Branches in our github repository also updated.

> About the patch set, I had a look at the first patch which is not that
> heavy, however it provides zero documentation, close to zero comments,
> but adds more than 500 lines of code. I find that a bit hard to give
> an opinion on, having commit messages associated to each patch would
> be also nice. This way, reviewers can figure what's going out in this
> mess and provide feedback.
Sorry that comments and commit messages are still absent. I am going to
do it
in the next version of these patches where SQL/JSON constructors displaying
will be fixed.

> Making things incremental is welcome as
> well, for example in the first patch I have a hard way finding out why
> timestamps are touched to begin with.
Timestamp's code was touched to add support of two features needed for
SQL/JSON
 .datetime() item method by standard:
 - TZH and TZM template patterns
 - datetime components recognition

I absolutely agree that this should be in a separate patch.

> The patch is already marked as "waiting on author" for more than one month.
--
Nikita Glukhov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company

On 04.11.2017 01:52, Nikita Glukhov wrote:

>> On 03.11.2017 15:07, Michael Paquier wrote:
>>
>> About the patch set, I had a look at the first patch which is not that
>> heavy, however it provides zero documentation, close to zero comments,
>> but adds more than 500 lines of code. I find that a bit hard to give
>> an opinion on, having commit messages associated to each patch would
>> be also nice. This way, reviewers can figure what's going out in this
>> mess and provide feedback.
> Sorry that comments and commit messages are still absent. I am going
> to do it
> in the next version of these patches where SQL/JSON constructors
> displaying
> will be fixed.
>
Attached the new version of the patches where displaying of SQL/JSON
constructor nodes was fixed.  I decided not to invent new nodes but to
extend
existing FuncExpr, Aggref, WindowFunc nodes with new formatting fields that
give us ability to override default displaying in ruleutils.c.  Also new
invisible CoercionForm was added for hiding casts in which FORMAT and
RETUNING
clauses are transformed.

>>   Making things incremental is welcome as
>> well, for example in the first patch I have a hard way finding out why
>> timestamps are touched to begin with.
> Timestamp's code was touched to add support of two features needed for
> SQL/JSON
>  .datetime() item method by standard:
>  - TZH and TZM template patterns
>  - datetime components recognition
>
> I absolutely agree that this should be in a separate patch.
>
TZH and TZM template patterns were moved into a separate patch.

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

On Wed, Nov 15, 2017 at 10:17 AM, Nikita Glukhov
<n(dot)gluhov(at)postgrespro(dot)ru> wrote:
> Attached the new version of the patches where displaying of SQL/JSON
> constructor nodes was fixed. I decided not to invent new nodes but to
> extend
> existing FuncExpr, Aggref, WindowFunc nodes with new formatting fields that
> give us ability to override default displaying in ruleutils.c. Also new
> invisible CoercionForm was added for hiding casts in which FORMAT and
> RETUNING
> clauses are transformed.

Okay, I can see that the patch is still in the same situation two
weeks after I looked at it first, so I am marking it as returned with
feedback. This needs to be broken down, and get documentation. At this
point getting a review out of this patch is not something I'd
recommend until it is put in a shape that makes it easier. Please also
help in reviewing other's patches, yours here is very large.
--
Michael

On 29.11.2017 05:24, Michael Paquier wrote:

> On Wed, Nov 15, 2017 at 10:17 AM, Nikita Glukhov
> <n(dot)gluhov(at)postgrespro(dot)ru> wrote:
>> Attached the new version of the patches where displaying of SQL/JSON
>> constructor nodes was fixed. I decided not to invent new nodes but to
>> extend
>> existing FuncExpr, Aggref, WindowFunc nodes with new formatting fields that
>> give us ability to override default displaying in ruleutils.c. Also new
>> invisible CoercionForm was added for hiding casts in which FORMAT and
>> RETUNING
>> clauses are transformed.
> Okay, I can see that the patch is still in the same situation two
> weeks after I looked at it first, so I am marking it as returned with
> feedback. This needs to be broken down, and get documentation. At this
> point getting a review out of this patch is not something I'd
> recommend until it is put in a shape that makes it easier. Please also
> help in reviewing other's patches, yours here is very large.

Attached new version of patches:

1. Added some comments for the jsonpath code and some documentation for
jsonpath operators and opclasses.  Sorry that complete documentation for
jsonpath itself is still missing.

2. Added support for custom user-defined item methods and functions in
jsonpath.
This feature allows us to move our non-standard methods (map, reduce,
fold, min,
max) to the extension contrib/jsnopathx. Now user can implement all standard
JavaScript array methods by himself.

3. Added variable jsonpath specifications in SQL/JSON functions.

4. Added subtransactions inside PG_TRY/PG_CATCH block that is used for
ON ERROR
clause support in JSON_EXISTS, JSON_VALUE, JSON_QUERY (see ExecEvalJson() in
src/backend/executor/execExpeInterp.c).

The last addition is the most questionable.  By using standard ON ERROR
сlause
we can specify default value for expression when an error occurs during
casting
JSON data to the target SQL type.  Cast expressions can contain
arbitrary user
functions, so they should be executed inside own subtransaction like it
is done
in plpgsql (see exec_stmt_block()).  Subtransaction start obviously
introduces
significant performance overhead (except the case when ERROR ON ERROR is
used and error handling is unnecessary):

=# EXPLAIN (ANALYZE)
   SELECT JSON_VALUE(jsonb '1', '$' RETURNING int ERROR ON ERROR)
   FROM generate_series(1, 1000000);
 ...
 Execution time: 395.238 ms

=# EXPLAIN (ANALYZE)
   SELECT JSON_VALUE(jsonb '1', '$' RETURNING int)
   FROM generate_series(1, 1000000);
 ...
 Execution time: 914.850 ms

To partially eliminate this overhead, I'm trying to examine cast-expression
volatility:
 * mutable -- subtransaction is started
 * stable -- subtransaction is not started, only new ResourceOwner is
created
 * immutable -- ResourceOwner is not even created
But don't know if it is correct to rely on volatility here.  And also I
doubt
that we can start multiple subtransactions (for each SQL/JSON function
 evaluation) within a single SELECT statement.

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

2018-01-02 3:04 GMT+01:00 Nikita Glukhov <n(dot)gluhov(at)postgrespro(dot)ru>:

> On 29.11.2017 05:24, Michael Paquier wrote:
>
> On Wed, Nov 15, 2017 at 10:17 AM, Nikita Glukhov
>> <n(dot)gluhov(at)postgrespro(dot)ru> wrote:
>>
>>> Attached the new version of the patches where displaying of SQL/JSON
>>> constructor nodes was fixed. I decided not to invent new nodes but to
>>> extend
>>> existing FuncExpr, Aggref, WindowFunc nodes with new formatting fields
>>> that
>>> give us ability to override default displaying in ruleutils.c. Also new
>>> invisible CoercionForm was added for hiding casts in which FORMAT and
>>> RETUNING
>>> clauses are transformed.
>>>
>> Okay, I can see that the patch is still in the same situation two
>> weeks after I looked at it first, so I am marking it as returned with
>> feedback. This needs to be broken down, and get documentation. At this
>> point getting a review out of this patch is not something I'd
>> recommend until it is put in a shape that makes it easier. Please also
>> help in reviewing other's patches, yours here is very large.
>>
>
> Attached new version of patches:
>
> 1. Added some comments for the jsonpath code and some documentation for
> jsonpath operators and opclasses. Sorry that complete documentation for
> jsonpath itself is still missing.
>
> 2. Added support for custom user-defined item methods and functions in
> jsonpath.
> This feature allows us to move our non-standard methods (map, reduce,
> fold, min,
> max) to the extension contrib/jsnopathx. Now user can implement all
> standard
> JavaScript array methods by himself.
>
> 3. Added variable jsonpath specifications in SQL/JSON functions.
>
> 4. Added subtransactions inside PG_TRY/PG_CATCH block that is used for ON
> ERROR
> clause support in JSON_EXISTS, JSON_VALUE, JSON_QUERY (see ExecEvalJson()
> in
> src/backend/executor/execExpeInterp.c).
>
>
> The last addition is the most questionable. By using standard ON ERROR
> сlause
> we can specify default value for expression when an error occurs during
> casting
> JSON data to the target SQL type. Cast expressions can contain arbitrary
> user
> functions, so they should be executed inside own subtransaction like it is
> done
> in plpgsql (see exec_stmt_block()). Subtransaction start obviously
> introduces
> significant performance overhead (except the case when ERROR ON ERROR is
> used and error handling is unnecessary):
>
> =# EXPLAIN (ANALYZE)
> SELECT JSON_VALUE(jsonb '1', '$' RETURNING int ERROR ON ERROR)
> FROM generate_series(1, 1000000);
> ...
> Execution time: 395.238 ms
>
> =# EXPLAIN (ANALYZE)
> SELECT JSON_VALUE(jsonb '1', '$' RETURNING int)
> FROM generate_series(1, 1000000);
> ...
> Execution time: 914.850 ms
>

Subtransactions are pretty expensive - so using subtransaction for every
output row doesn't look like good solution.

In this case the exception is created in your code, so it should not be
PostgreSQL exception necessary.

Without ERROR ON ERROR clause it ignore errors by default?

>
> To partially eliminate this overhead, I'm trying to examine cast-expression
> volatility:
> * mutable -- subtransaction is started
> * stable -- subtransaction is not started, only new ResourceOwner is
> created
> * immutable -- ResourceOwner is not even created
> But don't know if it is correct to rely on volatility here. And also I
> doubt
> that we can start multiple subtransactions (for each SQL/JSON function
> evaluation) within a single SELECT statement.
>
>
> --
> Nikita Glukhov
> Postgres Professional: http://www.postgrespro.com
> The Russian Postgres Company
>

2018-01-02 3:04 GMT+01:00 Nikita Glukhov <n(dot)gluhov(at)postgrespro(dot)ru>:

> On 29.11.2017 05:24, Michael Paquier wrote:
>
> On Wed, Nov 15, 2017 at 10:17 AM, Nikita Glukhov
>> <n(dot)gluhov(at)postgrespro(dot)ru> wrote:
>>
>>> Attached the new version of the patches where displaying of SQL/JSON
>>> constructor nodes was fixed. I decided not to invent new nodes but to
>>> extend
>>> existing FuncExpr, Aggref, WindowFunc nodes with new formatting fields
>>> that
>>> give us ability to override default displaying in ruleutils.c. Also new
>>> invisible CoercionForm was added for hiding casts in which FORMAT and
>>> RETUNING
>>> clauses are transformed.
>>>
>> Okay, I can see that the patch is still in the same situation two
>> weeks after I looked at it first, so I am marking it as returned with
>> feedback. This needs to be broken down, and get documentation. At this
>> point getting a review out of this patch is not something I'd
>> recommend until it is put in a shape that makes it easier. Please also
>> help in reviewing other's patches, yours here is very large.
>>
>
> Attached new version of patches:
>
> 1. Added some comments for the jsonpath code and some documentation for
> jsonpath operators and opclasses. Sorry that complete documentation for
> jsonpath itself is still missing.
>
> 2. Added support for custom user-defined item methods and functions in
> jsonpath.
> This feature allows us to move our non-standard methods (map, reduce,
> fold, min,
> max) to the extension contrib/jsnopathx. Now user can implement all
> standard
> JavaScript array methods by himself.
>
> 3. Added variable jsonpath specifications in SQL/JSON functions.
>
> 4. Added subtransactions inside PG_TRY/PG_CATCH block that is used for ON
> ERROR
> clause support in JSON_EXISTS, JSON_VALUE, JSON_QUERY (see ExecEvalJson()
> in
> src/backend/executor/execExpeInterp.c).
>
>
> The last addition is the most questionable. By using standard ON ERROR
> сlause
> we can specify default value for expression when an error occurs during
> casting
> JSON data to the target SQL type. Cast expressions can contain arbitrary
> user
> functions, so they should be executed inside own subtransaction like it is
> done
> in plpgsql (see exec_stmt_block()). Subtransaction start obviously
> introduces
> significant performance overhead (except the case when ERROR ON ERROR is
> used and error handling is unnecessary):
>
> =# EXPLAIN (ANALYZE)
> SELECT JSON_VALUE(jsonb '1', '$' RETURNING int ERROR ON ERROR)
> FROM generate_series(1, 1000000);
> ...
> Execution time: 395.238 ms
>
> =# EXPLAIN (ANALYZE)
> SELECT JSON_VALUE(jsonb '1', '$' RETURNING int)
> FROM generate_series(1, 1000000);
> ...
> Execution time: 914.850 ms
>
> To partially eliminate this overhead, I'm trying to examine cast-expression
> volatility:
> * mutable -- subtransaction is started
> * stable -- subtransaction is not started, only new ResourceOwner is
> created
> * immutable -- ResourceOwner is not even created
> But don't know if it is correct to rely on volatility here. And also I
> doubt
> that we can start multiple subtransactions (for each SQL/JSON function
> evaluation) within a single SELECT statement.
>
>
I am looking on this patch set and it looks very well.

Personally I dislike any extensions against SQL/JSON in this patch. And
there is lot of extensions there. It doesn't mean so these extensions are
bad, but it should be passed as next step and there should be separate
discussion related to these extensions.

Please, can you reduce this patch to only SQL/JSON part?

Regards

Pavel

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

Hi

regress tests fails

Regards

Pavel

On Tue, Jan 2, 2018 at 10:47 AM, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> wrote:
>
>
> 2018-01-02 3:04 GMT+01:00 Nikita Glukhov <n(dot)gluhov(at)postgrespro(dot)ru>:
>>
>> On 29.11.2017 05:24, Michael Paquier wrote:
>>
>>> On Wed, Nov 15, 2017 at 10:17 AM, Nikita Glukhov
>>> <n(dot)gluhov(at)postgrespro(dot)ru> wrote:
>>>>
>>>> Attached the new version of the patches where displaying of SQL/JSON
>>>> constructor nodes was fixed. I decided not to invent new nodes but to
>>>> extend
>>>> existing FuncExpr, Aggref, WindowFunc nodes with new formatting fields
>>>> that
>>>> give us ability to override default displaying in ruleutils.c. Also new
>>>> invisible CoercionForm was added for hiding casts in which FORMAT and
>>>> RETUNING
>>>> clauses are transformed.
>>>
>>> Okay, I can see that the patch is still in the same situation two
>>> weeks after I looked at it first, so I am marking it as returned with
>>> feedback. This needs to be broken down, and get documentation. At this
>>> point getting a review out of this patch is not something I'd
>>> recommend until it is put in a shape that makes it easier. Please also
>>> help in reviewing other's patches, yours here is very large.
>>
>>
>> Attached new version of patches:
>>
>> 1. Added some comments for the jsonpath code and some documentation for
>> jsonpath operators and opclasses. Sorry that complete documentation for
>> jsonpath itself is still missing.
>>
>> 2. Added support for custom user-defined item methods and functions in
>> jsonpath.
>> This feature allows us to move our non-standard methods (map, reduce,
>> fold, min,
>> max) to the extension contrib/jsnopathx. Now user can implement all
>> standard
>> JavaScript array methods by himself.
>>
>> 3. Added variable jsonpath specifications in SQL/JSON functions.
>>
>> 4. Added subtransactions inside PG_TRY/PG_CATCH block that is used for ON
>> ERROR
>> clause support in JSON_EXISTS, JSON_VALUE, JSON_QUERY (see ExecEvalJson()
>> in
>> src/backend/executor/execExpeInterp.c).
>>
>>
>> The last addition is the most questionable. By using standard ON ERROR
>> сlause
>> we can specify default value for expression when an error occurs during
>> casting
>> JSON data to the target SQL type. Cast expressions can contain arbitrary
>> user
>> functions, so they should be executed inside own subtransaction like it is
>> done
>> in plpgsql (see exec_stmt_block()). Subtransaction start obviously
>> introduces
>> significant performance overhead (except the case when ERROR ON ERROR is
>> used and error handling is unnecessary):
>>
>> =# EXPLAIN (ANALYZE)
>> SELECT JSON_VALUE(jsonb '1', '$' RETURNING int ERROR ON ERROR)
>> FROM generate_series(1, 1000000);
>> ...
>> Execution time: 395.238 ms
>>
>> =# EXPLAIN (ANALYZE)
>> SELECT JSON_VALUE(jsonb '1', '$' RETURNING int)
>> FROM generate_series(1, 1000000);
>> ...
>> Execution time: 914.850 ms
>>
>> To partially eliminate this overhead, I'm trying to examine
>> cast-expression
>> volatility:
>> * mutable -- subtransaction is started
>> * stable -- subtransaction is not started, only new ResourceOwner is
>> created
>> * immutable -- ResourceOwner is not even created
>> But don't know if it is correct to rely on volatility here. And also I
>> doubt
>> that we can start multiple subtransactions (for each SQL/JSON function
>> evaluation) within a single SELECT statement.
>>
>
> I am looking on this patch set and it looks very well.
>
> Personally I dislike any extensions against SQL/JSON in this patch. And
> there is lot of extensions there. It doesn't mean so these extensions are
> bad, but it should be passed as next step and there should be separate
> discussion related to these extensions.
>
> Please, can you reduce this patch to only SQL/JSON part?

+1, our goal is to push the standard to PG 11, which is more or less realistic.
Nikita will rearrange the patch set, so patches 1, 2, 4, 7, 8, 9, 10,
11, 12, which
implement SQL/JSON could be applied without extra patches.

Patches 5,6 are desirable, since we can implement custom operators. This is
very important for postgres, which is known as extensible database with rich set
of extensions. Think about geojson with spatial operators or array
operators, for
example. But I agree, it's subject of separate thread.

In very extreme case, we could commit for PG 11 only jsonpath-related patches
1,2 and probably 4. I think, that jsonpath is what we really miss in postgres.

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

On 01/02/2018 02:44 PM, Oleg Bartunov wrote:
> On Tue, Jan 2, 2018 at 10:47 AM, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> wrote:

>> I am looking on this patch set and it looks very well.
>>
>> Personally I dislike any extensions against SQL/JSON in this patch. And
>> there is lot of extensions there. It doesn't mean so these extensions are
>> bad, but it should be passed as next step and there should be separate
>> discussion related to these extensions.
>>
>> Please, can you reduce this patch to only SQL/JSON part?
> +1, our goal is to push the standard to PG 11, which is more or less realistic.
> Nikita will rearrange the patch set, so patches 1, 2, 4, 7, 8, 9, 10,
> 11, 12, which
> implement SQL/JSON could be applied without extra patches.
>
> Patches 5,6 are desirable, since we can implement custom operators. This is
> very important for postgres, which is known as extensible database with rich set
> of extensions. Think about geojson with spatial operators or array
> operators, for
> example. But I agree, it's subject of separate thread.
>
> In very extreme case, we could commit for PG 11 only jsonpath-related patches
> 1,2 and probably 4. I think, that jsonpath is what we really miss in postgres.

That seems a bit pessimistic. I hope we can do lots better.

It looks to me like patches 1, 7 and 8 can stand alone, and should be
submitted separately, and we should try to get them committed early.
These are all small patches - a couple of hundred lines each.

Patches 2, 3, and 4 should come next - I included patch 3 because I
think GIN indexing is going to be critical to success.

After that 9, 10, 11 and 12.

I don't have a problem with the rest, but they should probably have a
lower priority. If we can get to them well and good.

We should stop use the word 'extension' when we don't mean what Postgres
calls an extension (which is only patch 14 in this case). Call it an
addition or extra feature or something else. Otherwise it gets confusing.

I'm not 100% clear on why we're adding jsonpathx as an extension,
though. Do we not think most json users will want to use map, reduce etc.?

cheers

andrew

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

2018-01-02 21:39 GMT+01:00 Andrew Dunstan <andrew(dot)dunstan(at)2ndquadrant(dot)com>:

>
>
> On 01/02/2018 02:44 PM, Oleg Bartunov wrote:
> > On Tue, Jan 2, 2018 at 10:47 AM, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
> wrote:
>
> >> I am looking on this patch set and it looks very well.
> >>
> >> Personally I dislike any extensions against SQL/JSON in this patch. And
> >> there is lot of extensions there. It doesn't mean so these extensions
> are
> >> bad, but it should be passed as next step and there should be separate
> >> discussion related to these extensions.
> >>
> >> Please, can you reduce this patch to only SQL/JSON part?
> > +1, our goal is to push the standard to PG 11, which is more or less
> realistic.
> > Nikita will rearrange the patch set, so patches 1, 2, 4, 7, 8, 9, 10,
> > 11, 12, which
> > implement SQL/JSON could be applied without extra patches.
> >
> > Patches 5,6 are desirable, since we can implement custom operators. This
> is
> > very important for postgres, which is known as extensible database with
> rich set
> > of extensions. Think about geojson with spatial operators or array
> > operators, for
> > example. But I agree, it's subject of separate thread.
> >
> > In very extreme case, we could commit for PG 11 only jsonpath-related
> patches
> > 1,2 and probably 4. I think, that jsonpath is what we really miss in
> postgres.
>
>
> That seems a bit pessimistic. I hope we can do lots better.
>
> It looks to me like patches 1, 7 and 8 can stand alone, and should be
> submitted separately, and we should try to get them committed early.
> These are all small patches - a couple of hundred lines each.
>
> Patches 2, 3, and 4 should come next - I included patch 3 because I
> think GIN indexing is going to be critical to success.
>
> After that 9, 10, 11 and 12.
>
> I don't have a problem with the rest, but they should probably have a
> lower priority. If we can get to them well and good.
>
> We should stop use the word 'extension' when we don't mean what Postgres
> calls an extension (which is only patch 14 in this case). Call it an
> addition or extra feature or something else. Otherwise it gets confusing.
>
> I'm not 100% clear on why we're adding jsonpathx as an extension,
> though. Do we not think most json users will want to use map, reduce etc.?
>

In this moment, there is lot of code, and we should be concentrated to
merging the core of this feature. I am sure, so discussion about extra
features will come, and will be more realistic and less nervous if SQL/JSON
will be merged already.

I looked to patch - and It is big, really big - we should to start with
some important subset that we can understand and test well.

Regards

Pavel

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

On 01/02/2018 03:48 PM, Pavel Stehule wrote:
>
>
> 2018-01-02 21:39 GMT+01:00 Andrew Dunstan
> <andrew(dot)dunstan(at)2ndquadrant(dot)com <mailto:andrew(dot)dunstan(at)2ndquadrant(dot)com>>:
>
>
>
> On 01/02/2018 02:44 PM, Oleg Bartunov wrote:
> > On Tue, Jan 2, 2018 at 10:47 AM, Pavel Stehule
> <pavel(dot)stehule(at)gmail(dot)com <mailto:pavel(dot)stehule(at)gmail(dot)com>> wrote:
>
> >> I am looking on this patch set and it looks very well.
> >>
> >> Personally I dislike any extensions against SQL/JSON in this
> patch. And
> >> there is lot of extensions there. It doesn't mean so these
> extensions are
> >> bad, but it should be passed as next step and there should be
> separate
> >> discussion related to these extensions.
> >>
> >> Please, can you reduce this patch to only SQL/JSON part?
> > +1, our goal is to push the standard to PG 11, which is more or
> less realistic.
> > Nikita will rearrange the patch set, so patches 1, 2, 4, 7, 8,
> 9, 10,
> > 11, 12, which
> > implement SQL/JSON could be applied without extra patches.
> >
> > Patches 5,6 are desirable, since we can implement custom
> operators. This is
> > very important for postgres, which is known as extensible
> database with rich set
> > of extensions. Think about geojson with spatial operators or  array
> > operators, for
> > example. But I agree, it's subject of separate thread.
> >
> > In very extreme case, we could commit for PG 11 only
> jsonpath-related patches
> > 1,2 and probably 4.  I think, that jsonpath is what we really
> miss in postgres.
>
>
> That seems a bit pessimistic. I hope we can do lots better.
>
> It looks to me like patches 1, 7 and 8 can stand alone, and should be
> submitted separately, and we should try to get them committed early.
> These are all small patches - a couple of hundred lines each.
>
> Patches 2, 3, and 4 should come next - I included patch 3 because I
> think GIN indexing is going to be critical to success.
>
> After that 9, 10, 11 and 12.
>
> I don't have a problem with the rest, but they should probably have a
> lower priority. If we can get to them well and good.
>
> We should stop use the word 'extension' when we don't mean what
> Postgres
> calls an extension (which is only patch 14 in this case). Call it an
> addition or extra feature or something else. Otherwise it gets
> confusing.
>
> I'm not 100% clear on why we're adding jsonpathx as an extension,
> though. Do we not think most json users will want to use map,
> reduce etc.?
>
>
> In this moment, there is lot of code, and we should be concentrated to
> merging the core of this feature. I am sure, so discussion about extra
> features will come, and will be more realistic and less nervous if
> SQL/JSON will be merged already.
>
> I looked to patch - and It is big, really big - we should to start
> with some important subset that we can understand and test well.

Sure, I agree, we should start with jsonpath, and then move on to 
SQL/JSON and then the json_table patches. Patches 5, 6, 13 and 14 would
come last. That's the order I suggested above. My question was whether
or not, when we finally get to jsonpathx we really want to make it an
extension. I can't see a very good reason for doing so.

cheers

andrew

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

On Tue, Jan 2, 2018 at 8:39 PM, Andrew Dunstan
<andrew(dot)dunstan(at)2ndquadrant(dot)com> wrote:
>
>
> On 01/02/2018 02:44 PM, Oleg Bartunov wrote:
>> On Tue, Jan 2, 2018 at 10:47 AM, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> wrote:
>
>>> I am looking on this patch set and it looks very well.
>>>
>>> Personally I dislike any extensions against SQL/JSON in this patch. And
>>> there is lot of extensions there. It doesn't mean so these extensions are
>>> bad, but it should be passed as next step and there should be separate
>>> discussion related to these extensions.
>>>
>>> Please, can you reduce this patch to only SQL/JSON part?
>> +1, our goal is to push the standard to PG 11, which is more or less realistic.
>> Nikita will rearrange the patch set, so patches 1, 2, 4, 7, 8, 9, 10,
>> 11, 12, which
>> implement SQL/JSON could be applied without extra patches.
>>
>> Patches 5,6 are desirable, since we can implement custom operators. This is
>> very important for postgres, which is known as extensible database with rich set
>> of extensions. Think about geojson with spatial operators or array
>> operators, for
>> example. But I agree, it's subject of separate thread.
>>
>> In very extreme case, we could commit for PG 11 only jsonpath-related patches
>> 1,2 and probably 4. I think, that jsonpath is what we really miss in postgres.
>
>
> That seems a bit pessimistic. I hope we can do lots better.

Would love too !

>
> It looks to me like patches 1, 7 and 8 can stand alone, and should be
> submitted separately, and we should try to get them committed early.
> These are all small patches - a couple of hundred lines each.

+1

>
> Patches 2, 3, and 4 should come next - I included patch 3 because I
> think GIN indexing is going to be critical to success.

agree, we can consider patch 4 later

>
> After that 9, 10, 11 and 12.

again, 10 , 12 may be considered later

>
> I don't have a problem with the rest, but they should probably have a
> lower priority. If we can get to them well and good.
>
> We should stop use the word 'extension' when we don't mean what Postgres
> calls an extension (which is only patch 14 in this case). Call it an
> addition or extra feature or something else. Otherwise it gets confusing.

+1, lets call 'extra'

>
> I'm not 100% clear on why we're adding jsonpathx as an extension,
> though. Do we not think most json users will want to use map, reduce etc.?

We decided to do that, since the whole patch set is already big.

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

On 03.01.2018 00:38, Oleg Bartunov wrote:
> On Tue, Jan 2, 2018 at 8:39 PM, Andrew Dunstan
> <andrew(dot)dunstan(at)2ndquadrant(dot)com> wrote:
>>
>> On 01/02/2018 02:44 PM, Oleg Bartunov wrote:
>>> On Tue, Jan 2, 2018 at 10:47 AM, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> wrote:
>>>> I am looking on this patch set and it looks very well.
>>>>
>>>> Personally I dislike any extensions against SQL/JSON in this patch. And
>>>> there is lot of extensions there. It doesn't mean so these extensions are
>>>> bad, but it should be passed as next step and there should be separate
>>>> discussion related to these extensions.
>>>>
>>>> Please, can you reduce this patch to only SQL/JSON part?
>>> +1, our goal is to push the standard to PG 11, which is more or less realistic.
>>> Nikita will rearrange the patch set, so patches 1, 2, 4, 7, 8, 9, 10,
>>> 11, 12, which
>>> implement SQL/JSON could be applied without extra patches.
>>>
>>> Patches 5,6 are desirable, since we can implement custom operators. This is
>>> very important for postgres, which is known as extensible database with rich set
>>> of extensions. Think about geojson with spatial operators or array
>>> operators, for
>>> example. But I agree, it's subject of separate thread.
>>>
>>> In very extreme case, we could commit for PG 11 only jsonpath-related patches
>>> 1,2 and probably 4. I think, that jsonpath is what we really miss in postgres.
>>
>> That seems a bit pessimistic. I hope we can do lots better.
> Would love too !
>
>> It looks to me like patches 1, 7 and 8 can stand alone, and should be
>> submitted separately, and we should try to get them committed early.
>> These are all small patches - a couple of hundred lines each.
> +1
>
>> Patches 2, 3, and 4 should come next - I included patch 3 because I
>> think GIN indexing is going to be critical to success.
> agree, we can consider patch 4 later
>
>> After that 9, 10, 11 and 12.
>
> again, 10 , 12 may be considered later
>
>> I don't have a problem with the rest, but they should probably have a
>> lower priority. If we can get to them well and good.
>>
>> We should stop use the word 'extension' when we don't mean what Postgres
>> calls an extension (which is only patch 14 in this case). Call it an
>> addition or extra feature or something else. Otherwise it gets confusing.
> +1, lets call 'extra'
>
>> I'm not 100% clear on why we're adding jsonpathx as an extension,
>> though. Do we not think most json users will want to use map, reduce etc.?
> We decided to do that, since the whole patch set is already big.
>
>>
>>
>> cheers
>>
>> andrew
>>
>> --
>> Andrew Dunstan https://www.2ndQuadrant.com
>> PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
>>

I have removed all extra features from the patch set, they can be found
in our
github repository: https://github.com/postgrespro/sqljson/tree/sqljson_ext.

Now there are 10 patches which have the following dependencies:

1:
2: 1
3: 2
4: 2
5:
6:
7: 2, 5, 6
8: 7, 4
9: 7
10: 8, 9

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

On 01/02/2018 05:04 PM, Nikita Glukhov wrote:
>
> I have removed all extra features from the patch set, they can be
> found in our
> github repository:
> https://github.com/postgrespro/sqljson/tree/sqljson_ext.
>
> Now there are 10 patches which have the following dependencies:
>
> 1:
> 2: 1
> 3: 2
> 4: 2
> 5:
> 6:
> 7: 2, 5, 6
> 8: 7, 4
> 9: 7
> 10: 8, 9
>

OK. We need to get this into the commitfest. Preferably not all in one
piece. I would do:

1, 5, and 6  independently
2, 3 and 4 together
7 and 8 together
9 and 10 together

Those seem like digestible pieces.

Also, there is a spurious BOM at the start of
src/test/regress/sql/sqljson.sql in patch 7. Please fix that.

cheers

andrew

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

On 3 Jan 2018 00:23, "Andrew Dunstan" <andrew(dot)dunstan(at)2ndquadrant(dot)com>
wrote:

On 01/02/2018 05:04 PM, Nikita Glukhov wrote:
>
> I have removed all extra features from the patch set, they can be
> found in our
> github repository:
> https://github.com/postgrespro/sqljson/tree/sqljson_ext.
>
> Now there are 10 patches which have the following dependencies:
>
> 1:
> 2: 1
> 3: 2
> 4: 2
> 5:
> 6:
> 7: 2, 5, 6
> 8: 7, 4
> 9: 7
> 10: 8, 9
>

OK. We need to get this into the commitfest. Preferably not all in one
piece. I would do:

1, 5, and 6 independently
2, 3 and 4 together
7 and 8 together
9 and 10 together

What's about 11,12 ? They are about json_table.

Those seem like digestible pieces.

Also, there is a spurious BOM at the start of
src/test/regress/sql/sqljson.sql in patch 7. Please fix that.

cheers

andrew

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

On 01/03/2018 11:04 AM, Oleg Bartunov wrote:
>
>
> On 3 Jan 2018 00:23, "Andrew Dunstan" <andrew(dot)dunstan(at)2ndquadrant(dot)com
> <mailto:andrew(dot)dunstan(at)2ndquadrant(dot)com>> wrote:
>
>
>
> On 01/02/2018 05:04 PM, Nikita Glukhov wrote:
> >
> > I have removed all extra features from the patch set, they can be
> > found in our
> > github repository:
> > https://github.com/postgrespro/sqljson/tree/sqljson_ext
> <https://github.com/postgrespro/sqljson/tree/sqljson_ext>.
> >
> > Now there are 10 patches which have the following dependencies:
> >
> > 1:
> > 2: 1
> > 3: 2
> > 4: 2
> > 5:
> > 6:
> > 7: 2, 5, 6
> > 8: 7, 4
> > 9: 7
> > 10: 8, 9
> >
>
>
> OK. We need to get this into the commitfest. Preferably not all in one
> piece. I would do:
>
> 1, 5, and 6  independently
> 2, 3 and 4 together
> 7 and 8 together
> 9 and 10 together
>
>
> What's about 11,12 ? They are about json_table.

No, those are 9 and 10 in Nikita's new patchset.

cheers

andrew

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

I have removed all extra features from the patch set, they can be found in
our

> github repository: https://github.com/postgrespro/sqljson/tree/sqljson_ext
> .
>
> Now there are 10 patches which have the following dependencies:
>
> 1:
> 2: 1
> 3: 2
> 4: 2
> 5:
> 6:
> 7: 2, 5, 6
> 8: 7, 4
> 9: 7
> 10: 8, 9
>
>
looks better. Maybe two regress tests are unstable. I had to fix two.

please, can you check it?

json_sqljson ... FAILED
jsonb_sqljson ... FAILED

Regards

Pavel

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

2018-01-03 17:04 GMT+01:00 Oleg Bartunov <obartunov(at)gmail(dot)com>:

>
>
> On 3 Jan 2018 00:23, "Andrew Dunstan" <andrew(dot)dunstan(at)2ndquadrant(dot)com>
> wrote:
>
>
>
> On 01/02/2018 05:04 PM, Nikita Glukhov wrote:
> >
> > I have removed all extra features from the patch set, they can be
> > found in our
> > github repository:
> > https://github.com/postgrespro/sqljson/tree/sqljson_ext.
> >
> > Now there are 10 patches which have the following dependencies:
> >
> > 1:
> > 2: 1
> > 3: 2
> > 4: 2
> > 5:
> > 6:
> > 7: 2, 5, 6
> > 8: 7, 4
> > 9: 7
> > 10: 8, 9
> >
>
>
> OK. We need to get this into the commitfest. Preferably not all in one
> piece. I would do:
>
> 1, 5, and 6 independently
> 2, 3 and 4 together
> 7 and 8 together
> 9 and 10 together
>
>
> What's about 11,12 ? They are about json_table.
>

I did some tests of json_table and it is working well.

good work

Pavel

> Those seem like digestible pieces.
>
> Also, there is a spurious BOM at the start of
> src/test/regress/sql/sqljson.sql in patch 7. Please fix that.
>
> cheers
>
> andrew
>
> --
> Andrew Dunstan https://www.2ndQuadrant.com
> PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
>
>
>

Hi

I am checking the JSONPath related code

Questions, notes:

1. jsonpath operators are not consistent with any other .. json, xml .. I
am missing ?, @> operátors
2. documentation issue - there is "'{"a":[1,2,3,4,5]}'::json *? '$.a[*] ?
(@ > 2)'" - operator *? doesn't exists
3. operator @~ looks like too aggressive shortcut - should be better
commented

What is not clean, if jsonpath should to create some new operators for
json, jsonb types? It is special filter, defined by type, so from my
perspective the special operators are not necessary.

Regards

Pavel

On Sat, Jan 6, 2018 at 8:22 AM, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> wrote:
> Hi
>
> I am checking the JSONPath related code
>
> Questions, notes:
>
> 1. jsonpath operators are not consistent with any other .. json, xml .. I am
> missing ?, @> operátors

I have slides about jsonpath
http://www.sai.msu.su/~megera/postgres/talks/sqljson-pgconf.eu-2017.pdf

> 2. documentation issue - there is "'{"a":[1,2,3,4,5]}'::json *? '$.a[*] ? (@
>> 2)'" - operator *? doesn't exists

There are should be @? operator

> 3. operator @~ looks like too aggressive shortcut - should be better
> commented
>
> What is not clean, if jsonpath should to create some new operators for json,
> jsonb types? It is special filter, defined by type, so from my perspective
> the special operators are not necessary.

It's impossible to distinguish jsonpath from text, so introducing new operators
are easier than everytime explicitly specify jsonpath datatype.

>
> Regards
>
> Pavel
>
>

2018-01-06 22:02 GMT+01:00 Oleg Bartunov <obartunov(at)gmail(dot)com>:

> On Sat, Jan 6, 2018 at 8:22 AM, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
> wrote:
> > Hi
> >
> > I am checking the JSONPath related code
> >
> > Questions, notes:
> >
> > 1. jsonpath operators are not consistent with any other .. json, xml ..
> I am
> > missing ?, @> operátors
>
> I have slides about jsonpath
> http://www.sai.msu.su/~megera/postgres/talks/sqljson-pgconf.eu-2017.pdf
>
> > 2. documentation issue - there is "'{"a":[1,2,3,4,5]}'::json *? '$.a[*]
> ? (@
> >> 2)'" - operator *? doesn't exists
>
> There are should be @? operator
>
> > 3. operator @~ looks like too aggressive shortcut - should be better
> > commented
> >
> > What is not clean, if jsonpath should to create some new operators for
> json,
> > jsonb types? It is special filter, defined by type, so from my
> perspective
> > the special operators are not necessary.
>
> It's impossible to distinguish jsonpath from text, so introducing new
> operators
> are easier than everytime explicitly specify jsonpath datatype.
>

There are two possible solutions - special operator or explicit casting. In
this case I am not sure if special operator for this case is good solution.
Probably nobody will use it - because there SQL/JSON functions, but I don't
think so this inconsistency is correct.

I have not strong opinion about it - it will be hidden feature for almost
all users.

> >
> > Regards
> >
> > Pavel
> >
> >
>

Hi

I try jsonpath on json

{
"book":
[
{
"title": "Beginning JSON",
"author": "Ben Smith",
"price": 49.99
},

{
"title": "JSON at Work",
"author": "Tom Marrs",
"price": 29.99
},

{
"title": "Learn JSON in a DAY",
"author": "Acodemy",
"price": 8.99
},

{
"title": "JSON: Questions and Answers",
"author": "George Duckett",
"price": 6.00
}
],

"price range":
{
"cheap": 10.00,
"medium": 20.00
}
}

I am not jsonpath expert, so I can be bad

How I can get title of book with cost 6?

postgres=# select j @* '$.book[*] ? (@.price==6)' from test;
┌─────────────────────────────────────────────────────┐
│ ?column? │
╞═════════════════════════════════════════════════════╡
│ { ↵│
│ "title": "JSON: Questions and Answers",↵│
│ "author": "George Duckett", ↵│
│ "price": 6.00 ↵│
│ } ↵│
│ │
└─────────────────────────────────────────────────────┘
(1 row)

-- not sure, if it is correct
postgres=# select j @* '$.book[*].title ? (@.price==6)' from test;
┌──────────┐
│ ?column? │
╞══════════╡
└──────────┘
(0 rows)

I found some examples, where the filter has bigger sense, but it is not
supported

LINE 1: select j @* '$.book[?(@.price==6.00)].title' from test;
^
DETAIL: syntax error, unexpected '?' at or near "?"

Regards

Pavel

On 07.01.2018 00:22, Pavel Stehule wrote:

> Hi
>
> I try jsonpath on json
>
> {
>     "book":
>     [
>         {
>             "title": "Beginning JSON",
>             "author": "Ben Smith",
>             "price": 49.99
>         },
>
>         {
>             "title": "JSON at Work",
>             "author": "Tom Marrs",
>             "price": 29.99
>         },
>
>         {
>             "title": "Learn JSON in a DAY",
>             "author": "Acodemy",
>             "price": 8.99
>         },
>
>         {
>             "title": "JSON: Questions and Answers",
>             "author": "George Duckett",
>             "price": 6.00
>         }
>     ],
>
>     "price range":
>     {
>         "cheap": 10.00,
>         "medium": 20.00
>     }
> }
>
>
> I am not jsonpath expert, so I can be bad
>
> How I can get title of book with cost 6?
>
> postgres=# select j @* '$.book[*] ? (@.price==6)' from test;
> ┌─────────────────────────────────────────────────────┐
> │                      ?column?                       │
> ╞═════════════════════════════════════════════════════╡
> │ {                                                  ↵│
> │             "title": "JSON: Questions and Answers",↵│
> │             "author": "George Duckett",            ↵│
> │             "price": 6.00                          ↵│
> │         }                                          ↵│
> │                                                     │
> └─────────────────────────────────────────────────────┘
> (1 row)
>
> -- not sure, if it is correct
> postgres=# select j @* '$.book[*].title ? (@.price==6)' from test;
> ┌──────────┐
> │ ?column? │
> ╞══════════╡
> └──────────┘
> (0 rows)
>
> I found some examples, where the filter has bigger sense, but it is
> not supported
>
>
> LINE 1: select j @* '$.book[?(@.price==6.00)].title' from test;
>                     ^
> DETAIL:  syntax error, unexpected '?' at or near "?"
>
".title" simply should go after the filter:

select j @* '$.book[*] ? (@.price==6.00).title' from test;

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

2018-01-06 22:23 GMT+01:00 Nikita Glukhov <n(dot)gluhov(at)postgrespro(dot)ru>:

> On 07.01.2018 00:22, Pavel Stehule wrote:
>
> Hi
>
> I try jsonpath on json
>
> {
> "book":
> [
> {
> "title": "Beginning JSON",
> "author": "Ben Smith",
> "price": 49.99
> },
>
> {
> "title": "JSON at Work",
> "author": "Tom Marrs",
> "price": 29.99
> },
>
> {
> "title": "Learn JSON in a DAY",
> "author": "Acodemy",
> "price": 8.99
> },
>
> {
> "title": "JSON: Questions and Answers",
> "author": "George Duckett",
> "price": 6.00
> }
> ],
>
> "price range":
> {
> "cheap": 10.00,
> "medium": 20.00
> }
> }
>
>
> I am not jsonpath expert, so I can be bad
>
> How I can get title of book with cost 6?
>
> postgres=# select j @* '$.book[*] ? (@.price==6)' from test;
> ┌─────────────────────────────────────────────────────┐
> │ ?column? │
> ╞═════════════════════════════════════════════════════╡
> │ { ↵│
> │ "title": "JSON: Questions and Answers",↵│
> │ "author": "George Duckett", ↵│
> │ "price": 6.00 ↵│
> │ } ↵│
> │ │
> └─────────────────────────────────────────────────────┘
> (1 row)
>
> -- not sure, if it is correct
> postgres=# select j @* '$.book[*].title ? (@.price==6)' from test;
> ┌──────────┐
> │ ?column? │
> ╞══════════╡
> └──────────┘
> (0 rows)
>
> I found some examples, where the filter has bigger sense, but it is not
> supported
>
>
> LINE 1: select j @* '$.book[?(@.price==6.00)].title' from test;
> ^
> DETAIL: syntax error, unexpected '?' at or near "?"
>
> ".title" simply should go after the filter:
>
> select j @* '$.book[*] ? (@.price==6.00).title' from test;
>
>
It is working, thank you.

and the form "$.book[?(@.price==6.00)].title" ? I found this example in
some other SQL/JSON implementations.

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

On 07.01.2018 00:33, Pavel Stehule wrote:

> 2018-01-06 22:23 GMT+01:00 Nikita Glukhov <n(dot)gluhov(at)postgrespro(dot)ru
> <mailto:n(dot)gluhov(at)postgrespro(dot)ru>>:
>
> On 07.01.2018 00:22, Pavel Stehule wrote:
>
>> Hi
>>
>> I try jsonpath on json
>>
>> {
>>     "book":
>>     [
>>         {
>>             "title": "Beginning JSON",
>>             "author": "Ben Smith",
>>             "price": 49.99
>>         },
>>
>>         {
>>             "title": "JSON at Work",
>>             "author": "Tom Marrs",
>>             "price": 29.99
>>         },
>>
>>         {
>>             "title": "Learn JSON in a DAY",
>>             "author": "Acodemy",
>>             "price": 8.99
>>         },
>>
>>         {
>>             "title": "JSON: Questions and Answers",
>>             "author": "George Duckett",
>>             "price": 6.00
>>         }
>>     ],
>>
>>     "price range":
>>     {
>>         "cheap": 10.00,
>>         "medium": 20.00
>>     }
>> }
>>
>>
>> I am not jsonpath expert, so I can be bad
>>
>> How I can get title of book with cost 6?
>>
>> postgres=# select j @* '$.book[*] ? (@.price==6)' from test;
>> ┌─────────────────────────────────────────────────────┐
>> │ ?column?                       │
>> ╞═════════════════════════════════════════════════════╡
>> │ { ↵│
>> │             "title": "JSON: Questions and Answers",↵│
>> │             "author": "George Duckett",            ↵│
>> │             "price": 6.00                          ↵│
>> │         } ↵│
>> │ │
>> └─────────────────────────────────────────────────────┘
>> (1 row)
>>
>> -- not sure, if it is correct
>> postgres=# select j @* '$.book[*].title ? (@.price==6)' from test;
>> ┌──────────┐
>> │ ?column? │
>> ╞══════════╡
>> └──────────┘
>> (0 rows)
>>
>> I found some examples, where the filter has bigger sense, but it
>> is not supported
>>
>>
>> LINE 1: select j @* '$.book[?(@.price==6.00)].title' from test;
>>                     ^
>> DETAIL:  syntax error, unexpected '?' at or near "?"
>>
> ".title" simply should go after the filter:
>
> select j @* '$.book[*] ? (@.price==6.00).title' from test;
>
>
> It is working, thank you.
>
> and the form "$.book[?(@.price==6.00)].title" ? I found this example
> in some other SQL/JSON implementations.
>
This is non-standard feature, but it can be easily added for
compatibility with other implementations.

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

On 07.01.2018 00:11, Pavel Stehule wrote:

> 2018-01-06 22:02 GMT+01:00 Oleg Bartunov <obartunov(at)gmail(dot)com
> <mailto:obartunov(at)gmail(dot)com>>:
>
> On Sat, Jan 6, 2018 at 8:22 AM, Pavel Stehule
> <pavel(dot)stehule(at)gmail(dot)com <mailto:pavel(dot)stehule(at)gmail(dot)com>> wrote:
> > Hi
> >
> > I am checking the JSONPath related code
> >
> > Questions, notes:
> >
> > 1. jsonpath operators are not consistent with any other .. json,
> xml .. I am
> > missing ?, @> operátors
>
> I have slides about jsonpath
> http://www.sai.msu.su/~megera/postgres/talks/sqljson-pgconf.eu-2017.pdf
> <http://www.sai.msu.su/%7Emegera/postgres/talks/sqljson-pgconf.eu-2017.pdf>
>
> > 2. documentation issue - there is "'{"a":[1,2,3,4,5]}'::json *?
> '$.a[*] ? (@
> >> 2)'" - operator *? doesn't exists
>
> There are should be @? operator
>
> > 3. operator @~ looks like too aggressive shortcut - should be better
> > commented
> >
> > What is not clean, if jsonpath should to create some new
> operators for json,
> > jsonb types? It is special filter, defined by type, so from my
> perspective
> > the special operators are not necessary.
>
> It's impossible to distinguish jsonpath from text, so introducing
> new operators
> are easier than everytime explicitly specify jsonpath datatype.
>
>
> There are two possible solutions - special operator or explicit
> casting. In this case I am not sure if special operator for this case
> is good solution. Probably nobody will use it - because there SQL/JSON
> functions, but I don't think so this inconsistency is correct.
>
> I have not strong opinion about it - it will be hidden feature for
> almost all users.
>
Operators are necessary for index support now.

Operators allows us to use a more concise syntax in simple cases, when
we extract JSON item(s) without error handling:
js @* '$.key'
vs
JSON_QUERY(js, '$.key' RETURNING jsonb ERROR ON ERROR)

Also @* оperator gives us ability to extract a set of JSON items.
JSON_QUERY can only wrap extracted item sequence into JSON array which
we have to unwrap with our json[b]_array_elements() function. I also
thought about returning setof-types in JSON_VALUE/JSON_QUERY:

JSON_QUERY(jsonb '[1,2,3]', '$[*]' RETURNING SETOF jsonb)

But it is not so easy to implement now, because we should introduce new
node like TableFunc (or also we can try to use existing JSON_TABLE
infrastructure).

Set-returning expressions are not allowed in every context, so for
returning singleton items there should be additional operator.

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

On 01/02/2018 05:23 PM, Andrew Dunstan wrote:
>
> On 01/02/2018 05:04 PM, Nikita Glukhov wrote:
>> I have removed all extra features from the patch set, they can be
>> found in our
>> github repository:
>> https://github.com/postgrespro/sqljson/tree/sqljson_ext.
>>
>> Now there are 10 patches which have the following dependencies:
>>
>> 1:
>> 2: 1
>> 3: 2
>> 4: 2
>> 5:
>> 6:
>> 7: 2, 5, 6
>> 8: 7, 4
>> 9: 7
>> 10: 8, 9
>>
>
> OK. We need to get this into the commitfest. Preferably not all in one
> piece. I would do:
>
> 1, 5, and 6  independently
> 2, 3 and 4 together
> 7 and 8 together
> 9 and 10 together
>
> Those seem like digestible pieces.
>
> Also, there is a spurious BOM at the start of
> src/test/regress/sql/sqljson.sql in patch 7. Please fix that.
>

OK, an extended version of patch 1 has been committed. I'm working on
patches 2, 3, and 4 (the jsonpath patches) as time permits (and right
now time is very tight). Here are some very preliminary comments:

* The documentation needs improvement. A para with contents of "TODO"
is not acceptable.
* I'm not generally a fan of using flex/bison for small languages like
this. Something similar to what we're using for json itself (a
simple lexer and a recursive descent parser) would be more suitable.
Others might have different views.
* The timestamp handling refactoring is a good thing but would
probably be better done as a separate patch.
* the code is pretty sparsely commented. Quite apart from other
considerations that makes it harder to review.

I also note that the later patches have no documentation whatsoever.
That needs serious work, and if you want to get these patches in then
please supply some documentation ASAP. If you need help with English we
can work on that, but just throwing patches of this size and complexity
over the wall into the commitfest without any documentation is not the
way to proceed.

cheers

andrew

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

2018-01-09 21:44 GMT+01:00 Andrew Dunstan <andrew(dot)dunstan(at)2ndquadrant(dot)com>:

>
>
> On 01/02/2018 05:23 PM, Andrew Dunstan wrote:
> >
> > On 01/02/2018 05:04 PM, Nikita Glukhov wrote:
> >> I have removed all extra features from the patch set, they can be
> >> found in our
> >> github repository:
> >> https://github.com/postgrespro/sqljson/tree/sqljson_ext.
> >>
> >> Now there are 10 patches which have the following dependencies:
> >>
> >> 1:
> >> 2: 1
> >> 3: 2
> >> 4: 2
> >> 5:
> >> 6:
> >> 7: 2, 5, 6
> >> 8: 7, 4
> >> 9: 7
> >> 10: 8, 9
> >>
> >
> > OK. We need to get this into the commitfest. Preferably not all in one
> > piece. I would do:
> >
> > 1, 5, and 6 independently
> > 2, 3 and 4 together
> > 7 and 8 together
> > 9 and 10 together
> >
> > Those seem like digestible pieces.
> >
> > Also, there is a spurious BOM at the start of
> > src/test/regress/sql/sqljson.sql in patch 7. Please fix that.
> >
>
>
> OK, an extended version of patch 1 has been committed. I'm working on
> patches 2, 3, and 4 (the jsonpath patches) as time permits (and right
> now time is very tight). Here are some very preliminary comments:
>
> * The documentation needs improvement. A para with contents of "TODO"
> is not acceptable.
>

There should be introduction to JSONPath expressions

* I'm not generally a fan of using flex/bison for small languages like
> this. Something similar to what we're using for json itself (a
> simple lexer and a recursive descent parser) would be more suitable.
> Others might have different views.
>

I am think so flex/bison in this case is correct - JSONPath expression is
more complex language than JSON self

Regards

Pavel

> * The timestamp handling refactoring is a good thing but would
> probably be better done as a separate patch.
> * the code is pretty sparsely commented. Quite apart from other
> considerations that makes it harder to review.
>
> I also note that the later patches have no documentation whatsoever.
> That needs serious work, and if you want to get these patches in then
> please supply some documentation ASAP. If you need help with English we
> can work on that, but just throwing patches of this size and complexity
> over the wall into the commitfest without any documentation is not the
> way to proceed.
>
> cheers
>
> andrew
>
> --
> Andrew Dunstan https://www.2ndQuadrant.com
> PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
>
>

On 9 Jan 2018 23:44, "Andrew Dunstan" <andrew(dot)dunstan(at)2ndquadrant(dot)com>
wrote:

On 01/02/2018 05:23 PM, Andrew Dunstan wrote:
>
> On 01/02/2018 05:04 PM, Nikita Glukhov wrote:
>> I have removed all extra features from the patch set, they can be
>> found in our
>> github repository:
>> https://github.com/postgrespro/sqljson/tree/sqljson_ext.
>>
>> Now there are 10 patches which have the following dependencies:
>>
>> 1:
>> 2: 1
>> 3: 2
>> 4: 2
>> 5:
>> 6:
>> 7: 2, 5, 6
>> 8: 7, 4
>> 9: 7
>> 10: 8, 9
>>
>
> OK. We need to get this into the commitfest. Preferably not all in one
> piece. I would do:
>
> 1, 5, and 6 independently
> 2, 3 and 4 together
> 7 and 8 together
> 9 and 10 together
>
> Those seem like digestible pieces.
>
> Also, there is a spurious BOM at the start of
> src/test/regress/sql/sqljson.sql in patch 7. Please fix that.
>

OK, an extended version of patch 1 has been committed. I'm working on
patches 2, 3, and 4 (the jsonpath patches) as time permits (and right
now time is very tight). Here are some very preliminary comments:

* The documentation needs improvement. A para with contents of "TODO"
is not acceptable.
* I'm not generally a fan of using flex/bison for small languages like
this. Something similar to what we're using for json itself (a
simple lexer and a recursive descent parser) would be more suitable.

flex/bison is right tool for jsonpath, which is complex thing.

Others might have different views.
* The timestamp handling refactoring is a good thing but would
probably be better done as a separate patch.

Agree

* the code is pretty sparsely commented. Quite apart from other
considerations that makes it harder to review.

I also note that the later patches have no documentation whatsoever.
That needs serious work, and if you want to get these patches in then
please supply some documentation ASAP. If you need help with English we
can work on that, but just throwing patches of this size and complexity
over the wall into the commitfest without any documentation is not the
way to proceed.

Andrew, we are back from holidays and I will start writing on
documentation. I have difficulty with design of documentation, since it's
unclear to me how detailed it should be. I'm inclining to follow xml style
of documentation, which is quite formal and could be more easy to write.

cheers

andrew

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

On 01/10/2018 01:37 AM, Oleg Bartunov wrote:
>
>
>
>     this. Something similar to what we're using for json itself (a
>     simple lexer and a recursive descent parser) would be more
> suitable.
>
>
> flex/bison is right tool for jsonpath, which is complex thing. 

It's not really very complex. The bison grammar has 21 non-terminals. As
languages go that's not huge.

May main concerns are code size and performance. RD parsers are
typically very fast and compact unless they are badly written. There are
reasons that years ago gcc switched from using bison to using a hand cut
RD parser. I guess we wouldn't expect for the most part that jsonpath
expressions would need to be compiled per row, so maybe performance
isn't that critical. But if we do expect really dynamic jsonpath
expressions then we need to make sure we are as fast as we can get.

I guess if all you have is a hammer everything looks like a nail, but we
should not assume that bison is the answer to every parsing problem we have.

I'm not going to hold up the patch over this issue. I should probably
have looked closer and raised it months ago. But if and when I get time
I will look into some benchmarking.

> I also note that the later patches have no documentation whatsoever.
> That needs serious work, and if you want to get these patches in then
> please supply some documentation ASAP. If you need help with
> English we
> can work on that, but just throwing patches of this size and
> complexity
> over the wall into the commitfest without any documentation is not the
> way to proceed.
>
>
> Andrew, we are back from holidays and I will start writing on
> documentation. I have difficulty with design of documentation, since
> it's unclear to me how detailed it should be. I'm inclining to follow
> xml style of documentation, which is quite formal and could be more
> easy to write.

OK, good. The sooner the better though. Err on the side of more detail
please.

cheers

andrew

--

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

On 10 Jan 2018 20:14, "Andrew Dunstan" <andrew(dot)dunstan(at)2ndquadrant(dot)com>
wrote:

On 01/10/2018 01:37 AM, Oleg Bartunov wrote:
>
>
>
> this. Something similar to what we're using for json itself (a
> simple lexer and a recursive descent parser) would be more
> suitable.
>
>
> flex/bison is right tool for jsonpath, which is complex thing.

It's not really very complex. The bison grammar has 21 non-terminals. As
languages go that's not huge.

May main concerns are code size and performance. RD parsers are
typically very fast and compact unless they are badly written. There are
reasons that years ago gcc switched from using bison to using a hand cut
RD parser. I guess we wouldn't expect for the most part that jsonpath
expressions would need to be compiled per row, so maybe performance
isn't that critical. But if we do expect really dynamic jsonpath
expressions then we need to make sure we are as fast as we can get.

Good point, our jsonpath can be expression ( the standard describes it as
constant literal).

I guess if all you have is a hammer everything looks like a nail, but we
should not assume that bison is the answer to every parsing problem we have.

I'm not going to hold up the patch over this issue. I should probably
have looked closer and raised it months ago. But if and when I get time
I will look into some benchmarking.

I think it's not important right now and we could always replace parser
later.

> I also note that the later patches have no documentation whatsoever.
> That needs serious work, and if you want to get these patches in then
> please supply some documentation ASAP. If you need help with
> English we
> can work on that, but just throwing patches of this size and
> complexity
> over the wall into the commitfest without any documentation is not the
> way to proceed.
>
>
> Andrew, we are back from holidays and I will start writing on
> documentation. I have difficulty with design of documentation, since
> it's unclear to me how detailed it should be. I'm inclining to follow
> xml style of documentation, which is quite formal and could be more
> easy to write.

OK, good. The sooner the better though. Err on the side of more detail
please.

cheers

andrew

--

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

Hi,

This patchset currently has multiple CF entries:
https://commitfest.postgresql.org/17/1063/

and then subordinate ones like
https://commitfest.postgresql.org/17/1471/
https://commitfest.postgresql.org/17/1472/
https://commitfest.postgresql.org/17/1473/

Thus I'm marking this entry as returned with feedback.

- Andres