On 8/19/22 10:11 AM, Jonathan S. Katz wrote:
> Hi,
>
> On 8/17/22 11:45 PM, Nikita Glukhov wrote:
>> Hi,
>>
>> On 17.08.2022 04:45, Jonathan S. Katz wrote:
>>>
>>> On 8/15/22 10:14 PM, Andres Freund wrote:
>>>
>>>> I pushed a few cleanups to
>>>> https://github.com/anarazel/postgres/commits/json
>>>> while I was hacking on this (ignore that it's based on the meson
>>>> tree, that's
>>>> just faster for me). Some of them might not be applicable anymore,
>>>> but it
>>>> might still make sense for you to look at.
>>>
>>> With RMT hat on, this appears to be making progress. A few questions
>>> / comments for the group:
>>>
>>> 1. Nikita: Did you have a chance to review Andres's changes as well?
>>
>> Yes, I have reviewed Andres's changes, they all are ok.
>
> Thank you!
>
>> Then I started to do on the top of it other fixes that help to avoid
>> subtransactions when they are not needed. And it ended in the new
>> refactoring of coercion code. Also I moved here from v6-0003 fix of
>> ExecEvalJsonNeedSubtransaction() which considers more cases.
>
> Great.
>
> Andres, Robert: Do these changes address your concerns about the use of
> substransactions and reduce the risk of xid wraparound?
>
>> On 16.08.2022 05:14, Andres Freund wrote:
>>>> But for JIT I still had to construct additional ExprState with a
>>>> function compiled from subexpression steps.
>>
>>> Why did you have to do this?
>>
>> I simply did not dare to implement compilation of recursively-callable
>> function with additional parameter stepno. In the v8 patch I did it
>> by adding a switch with all possible jump addresses of EEOP_SUBTRANS
>> steps in the beginning of the function. And it really seems to work
>> faster, but needs more exploration. See patch 0003, where both
>> variants preserved using #ifdef.
>>
>>
>> The desciprion of the v7 patches:
>>
>> 0001 Simplify JsonExpr execution
>> Andres's changes + mine:
>> - Added JsonCoercionType enum, fields like via_io replaced with it
>> - Emit only context item steps in JSON_TABLE_OP case
>> - Skip coercion of NULLs to non-domain types (is it correct?)
>>
>> 0002 Fix returning of json[b] domains in JSON_VALUE:
>> simply rebase of v6 onto 0001
>>
>> 0003 Add EEOP_SUBTRANS executor step
>> v6 + new recursive JIT
>>
>> 0004 Split JsonExpr execution into steps
>> simply rebase of v6 + used LLMBuildSwitch() in EEOP_JSONEXPR
>
> What do folks think of these patches?
Andres, Andrew, Amit, Robert -- as you have either worked on this or
expressed opinions -- any thoughts on this current patch set?
Thanks,
Jonathan
