Em qua., 23 de jun. de 2021 às 21:45, Greg Nancarrow <gregn4...@gmail.com>
escreveu:
> On Wed, Jun 23, 2021 at 11:01 PM Tom Lane <t...@sss.pgh.pa.us> wrote:
> >
> >
> > The code is correct as-is; the proposed change would result in taking
> > more snapshots than needed. Perhaps the comment needs revision, since
> > you both misread it. The comment is written in terms of "when can we
> > skip taking a snapshot", while the test in the code is written for
> > the inverse condition "when do we need a snapshot".
>
> Yes, you're right.
> Even though I did realise that the comment was talking about the
> inverse, the condition for needing a snapshot still seemed too narrow,
> based on the comment, but checking the cases again, it is correct.
>
I still don't agree. But we leave the code like that, to see how it
behaves.
> Perhaps that code could have been written as the following, to better
> align with the comments:
>
> skip_snapshot = (!expr->expr_simple_mutable || estate->readonly_func);
> if (!skip_snapshot)
> {
> ...
> }
>
> ...
>
> if (!skip_snapshot)
> PopActiveSnapshot();
>
-1. That way it's readability is much worse, more complicated and IMHO it
generates worse asm.
regards,
Ranier Vilela
