Re: SQL/JSON: functions

On Mon, Mar 23, 2020 at 8:28 PM Nikita Glukhov <n(dot)gluhov(at)postgrespro(dot)ru> wrote:
> Attached 47th version of the patches.
>
> On 21.03.2020 22:38, Pavel Stehule wrote:
>
>
> On 21. 3. 2020 v 11:07 Nikita Glukhov <n(dot)gluhov(at)postgrespro(dot)ru> wrote:
>>
>> Attached 46th version of the patches.
>>
>> On 20.03.2020 22:34, Pavel Stehule wrote:
>>
>>
>> On 19.03.2020 23:57 Nikita Glukhov <n(dot)gluhov(at)postgrespro(dot)ru> wrote:
>>>
>>> Attached 45th version of the patches.
>>>
>>> Nodes JsonFormat, JsonReturning, JsonPassing, JsonBehavior were fixed.
>>>
>>> On 17.03.2020 21:35, Pavel Stehule wrote:
>>>>
>>>> User functions json[b]_build_object_ext() and json[b]_build_array_ext() also
>>>> can be easily removed. But it seems harder to remove new aggregate functions
>>>> json[b]_objectagg() and json[b]_agg_strict(), because they can't be called
>>>> directly from JsonCtorExpr node.
>>>
>>>
>>> I don't see reasons for another reduction now. Can be great if you can finalize work what you plan for pg13.
>>>
>> I have removed json[b]_build_object_ext() and json[b]_build_array_ext().
>>
>> But json[b]_objectagg() and json[b]_agg_strict() are still present.
>> It seems that removing them requires majors refactoring of the execution
>> of Aggref and WindowFunc nodes.
>
> I have replaced aggregate function
>
> json[b]_objectagg(key any, val any, absent_on_null boolean, unique_keys boolean)
>
> with three separate functions:
>
> json[b]_object_agg_strict(any, any)
> json[b]_object_agg_unique(any, any)
> json[b]_object_agg_unique_strict(any, any)
>
>
> This should be more correct than single aggregate with additional parameters.

I've following notes about this patchset.

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

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