Re: [BUG] failed assertion in EnsurePortalSnapshotExists()

On 9/29/21 8:01 PM, Tom Lane wrote:
> Alvaro Herrera <alvherre@2ndquadrant.com> writes:
>> On 2021-Sep-29, Ranier Vilela wrote:
>>> Em qua., 29 de set. de 2021 às 08:12, Drouvot, Bertrand <bdrouvot@amazon.com>
>>> escreveu:
>>> Duplicating functions is very bad for maintenance and bloats the code
>>> unnecessarily, IMHO.
>> Well, there are 42 calls of PushActiveSnapshot currently, and only 6 are
>> updated in the patch.  Given that six sevenths of the calls continue to
>> use the existing function and that it is less verbose than the new one,
>> that seems sufficient argument to keep it.
> Seeing that we have to back-patch this, changing the ABI of
> PushActiveSnapshot seems like a complete non-starter.
>
> The idea I'd had to avoid code duplication was to make
> PushActiveSnapshot a wrapper for the extended function:
>
> void
> PushActiveSnapshot(Snapshot snap)
> {
>      PushActiveSnapshotWithLevel(snap, GetCurrentTransactionNestLevel());
> }

Implemented into the new attached patch.

> So more like
>
>      Assert(ActiveSnapshot == NULL ||
>             snap_level >= ActiveSnapshot->as_level);

Implemented into the new attached patch.

But make check is now failing on join_hash.sql, I have been able to 
repro with:

create table bdt (a int);
begin;
savepoint a;
rollback to a;
explain select count(*) from bdt;

Which triggers a failed assertion on the new one:

TRAP: FailedAssertion("ActiveSnapshot == NULL || as_level >= 
ActiveSnapshot->as_level"

because we have as_level = 2 while ActiveSnapshot->as_level = 3.

Thanks

Bertrand