Re: [PATCH] Allow queries in WHEN expression of FOR EACH STATEMENT triggers

Hi Surafel,

On 3 Sep 2020, at 19:22, Surafel Temesgen wrote:

> This is my review of your patch

Thanks for the review.

> subqueries in row trigger's is not supported in your patch so the the
> documentation have to reflect it

It is still the case that the documentation says this. But, that may 
have been unclear as the documentation wouldn't compile (as you noted), 
so it wasn't possible to read it in the rendered form.

>
> + </literal>UPDATE</literal> triggers are able to refer to both
> </literal>OLD</literal>
>
> + and <literal>NEW</literal>
>
> Opening and ending tag mismatch on UPDATE and OLD literal so 
> documentation
> build fails and please update the documentation on server programming
> section too

Fixed.

I've also amended the server programming section to accurately reflect 
how WHEN conditions can be used.

> Instead of planning every time the trigger fire I suggest to store 
> plan or
> prepared statement node so planning time can be saved

Yes, that would make sense. I'll look in to what needs to be done.

Do you know if there are other areas of the code that cache plans that 
could act as a guide as to how best to achieve it?

> There are server crash on the following sequence of command
>
> ...
>
> INSERT INTO main_table (a, b) VALUES
>
> (101, 498),
>
> (102, 499);
>
> server crashed

Thanks. It was an incorrect Assert about NULL returns. Fixed.

-Joe