Re: Review: Fix snapshot taking inconsistencies

On 2010-10-13 1:21 AM +0300, Tom Lane wrote:
> Marko Tiikkaja<marko(dot)tiikkaja(at)cs(dot)helsinki(dot)fi> writes:
>> Here's a new version of the patch, deprecating pg_parse_and_rewrite.
>
> I started looking at this patch, and I'm wondering why you inserted all
> the Register/UnregisterSnapshot calls that weren't there before (eg, why
> did spi.c have to change at all?). ISTM either these are not necessary,
> or there is a pre-existing snapshot management bug that's independent
> of the question of just when to take new snapshots. I couldn't find
> anything in the thread claiming the latter though.

That's actually just my ignorance I forgot to mention. As I understand
it, our code currently first pushes one snapshot and then does multiple
PushActiveSnapshot (or PushUpdatedSnapshot)/PopActiveSnapshot rounds
before popping the oldest snapshot off the stack (and releasing it). So
in the patch, I would've had to push the snapshot twice the first time
to avoid it being released.

Thinking about it now, that might be a better option. Or maybe we
should change the snapshot API to make this more convenient?

The spi.c change also changes the logic; the SPI code currently takes a
new snapshot for every query if the caller doesn't provide a snapshot.

Regards,
Marko Tiikkaja