Em ter., 21 de set. de 2021 às 20:12, Ranier Vilela <ranier.vf@gmail.com> escreveu:
Em ter., 21 de set. de 2021 às 19:21, Tom Lane <tgl@sss.pgh.pa.us> escreveu:
Andres Freund <andres@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.
Exactly, thanks for the clear explanation.
> 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.
Ok, but I think that still has a value in reducing the scope.
save_innermost_domainval and save_innermost_domainnull,
only are needed with DOM_CONSTRAINT_CHECK expressions,
and both are declared even when they will not be used.
Anyway, the v1 patch fixes only the expression eval.
