Re: Eval expression R/O once time (src/backend/executor/execExpr.c)

Andres Freund <andres(at)anarazel(dot)de> writes:
> On 2021-09-21 15:09:11 -0300, Ranier Vilela wrote:
>> Currently when determining where CoerceToDomainValue can be read,
>> it evaluates every step in a loop.
>> But, I think that the expression is immutable and should be solved only
>> once.

> What is immutable here?

I think Ranier has a point here. The clear intent of this bit:

/*
* If first time through, determine where CoerceToDomainValue
* nodes should read from.
*/
if (domainval == NULL)
{

is that we only need to emit the EEOP_MAKE_READONLY once when there are
multiple CHECK constraints. But because domainval has the wrong lifespan,
that test is constant-true, and we'll do it over each time to little
purpose.

> And it has to, the allocation intentionally is separate for each
> constraint. As the comment even explicitly says:
> /*
> * Since value might be read multiple times, force to R/O
> * - but only if it could be an expanded datum.
> */

No, what that's on about is that each constraint might contain multiple
VALUE symbols. But once we've R/O-ified the datum, we can keep using
it across VALUE symbols in different CHECK expressions, not just one.

(AFAICS anyway)

I'm unexcited by the proposed move of the save_innermost_domainval/null
variables, though. It adds no correctness and it forces an additional
level of indentation of a good deal of code, as the patch fails to show.

regards, tom lane