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