Andres Freund <and...@anarazel.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