Re: Assertion failure in HEAD and 13 after calling COMMIT in a stored proc

From: Ranier Vilela <ranier(dot)vf(at)gmail(dot)com>
To: Greg Nancarrow <gregn4422(at)gmail(dot)com>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, nasbyj(at)amazon(dot)com, Pg Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Assertion failure in HEAD and 13 after calling COMMIT in a stored proc
Date: 2021-06-24 16:02:49
Message-ID: CAEudQArab1xw9_GvzjTLkxKxdeV+utdHruY5LQ-Cw6VfoM=_9g@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Em qua., 23 de jun. de 2021 às 21:45, Greg Nancarrow <gregn4422(at)gmail(dot)com>
escreveu:

> On Wed, Jun 23, 2021 at 11:01 PM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)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

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Ranier Vilela 2021-06-24 16:05:12 Re: Assertion failure in HEAD and 13 after calling COMMIT in a stored proc
Previous Message Daniel Gustafsson 2021-06-24 15:56:39 Re: [Patch] change the default value of shared_buffers in postgresql.conf.sample