Re: SQL/JSON features for v15

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?

Thanks,

Jonathan