Re: [BUG] failed assertion in EnsurePortalSnapshotExists()

On 9/29/21 1:23 PM, Ranier Vilela wrote:
>
> Em qua., 29 de set. de 2021 às 08:12, Drouvot, Bertrand 
> <bdrouvot@amazon.com> escreveu:
>
>     Hi,
>
>     On 9/29/21 12:59 PM, Ranier Vilela wrote:
>>
>>     Em qua., 29 de set. de 2021 às 06:55, Drouvot, Bertrand
>>     <bdrouvot@amazon.com> escreveu:
>>
>>         I'm also inclined to #1.
>>
>>     I have a stupid question, why duplicate PushActiveSnapshot?
>>     Wouldn't one function be better?
>>
>>     PushActiveSnapshot(Snapshot snap, int as_level);
>>
>>     Sample calls:
>>     PushActiveSnapshot(GetTransactionSnapshot(),
>>     GetCurrentTransactionNestLevel());
>>     PushActiveSnapshot(queryDesc->snapshot,
>>     GetCurrentTransactionNestLevel());
>>     PushActiveSnapshot(GetTransactionSnapshot(), portal->createSubid);
>
>     I would say because that could "break" existing extensions for
>     example.
>
>     Adding a new function prevents "updating" existing extensions
>     making use of PushActiveSnapshot().
>
> Valid argument of course.
> But the extensions should also fit the core code.
> Duplicating functions is very bad for maintenance and bloats the code 
> unnecessarily, IMHO.
>
Right. I don't have a strong opinion about this.

Let's see what Tom, Alvaro or others arguments/opinions are (should they 
also want to go with option #1).

Thanks

Bertrand