Re: [HACKERS] SQL/JSON in PostgreSQL

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