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

Joe Wildish" <joe(at)lateraljoin(dot)com> writes:
> On Wed, 22 Sep 2021, at 17:09, Tom Lane wrote:
> The main change is a switch to using SPI for expression evaluation. The plans are also cached along the same lines as the RI trigger plans.
>>
>> I really dislike this implementation technique. Aside from the likely
>> performance hit for existing triggers, I think it opens serious security
>> holes, because we can't fully guarantee that deparse-and-reparse doesn't
>> change the semantics.

> Where do you consider the performance hit to be?

The deparse/reparse cost might not be negligible, and putting SPI into
the equation where it was not before is certainly going to add overhead.
Now maybe those things are negligible in context, but I wouldn't believe
it without seeing some performance numbers.

> Do you have a suggestion for an alternative? I guess it would be go to the planner/executor directly with the node tree?

What I'd be thinking about is what it'd take to extend expression_planner
and related infrastructure to allow expressions containing SubLinks.
I fear there are a lot of moving parts there though, since the restriction
has been in place so long.

>> (Relevant to that, I wonder why this patch is only concerned with
>> WHEN clauses and not all the other places where we forbid subqueries
>> for implementation simplicity.)

> I don't know how many other places WHEN clauses are used. Rules, perhaps?

I'm thinking of things like CHECK constraints. Grepping for calls to
expression_planner would give you a clearer idea of the scope.

> The short answer is this patch was written to solve a specific problem I had rather than it being a more general attempt to remove all "subquery forbidden" restrictions.

I won't carp too much if the initial patch only removes the restriction
for WHEN; but I'd like to see it lay the groundwork to remove the
restriction elsewhere as well.

regards, tom lane