Re: [BUG] failed assertion in EnsurePortalSnapshotExists() - Mailing list pgsql-hackers

From Drouvot, Bertrand
Subject Re: [BUG] failed assertion in EnsurePortalSnapshotExists()
Date
Msg-id 8d17ca2e-cb7a-2f34-e55f-f51190b706b0@amazon.com
Whole thread Raw
In response to Re: [BUG] failed assertion in EnsurePortalSnapshotExists()  (Ranier Vilela <ranier.vf@gmail.com>)
List pgsql-hackers


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

pgsql-hackers by date:

Previous
From: Ranier Vilela
Date:
Subject: Re: [BUG] failed assertion in EnsurePortalSnapshotExists()
Next
From: Amit Kapila
Date:
Subject: Re: Some thoughts about the TAP tests' wait_for_catchup()