Re: Review: Fix snapshot taking inconsistencies

On 2010-10-13 1:21 AM +0300, Tom Lane wrote:
> Marko Tiikkaja<marko.tiikkaja@cs.helsinki.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