On Dec14, 2010, at 21:52 , Tom Lane wrote:
> Florian Pflug <fgp@phlo.org> writes:
>> Could you explain what seems to be wrong with my patch?
>
> I'm unconvinced that this is the proper response to whatever the problem
> is;
Well, you didn't comment on the part of my previous e-mail that *did*
explain why I believe this is the proper response...
> and if it is the right response, it seems to still need a good bit
> more work. You didn't even update the functions' header comments, let
> alone look at their callers to see how they'd be affected by an
> InvalidTransactionId result.
Well, I hit this while re-verifying the serializable lock consistency stuff
with a current HEAD, so I didn't really want to spend more time on this than
necessary. Especially since that patch replaces the assert in question anyway.
So I moved the assert to a safe place and HeapTupleHeaderGetXmax() with
it, since if the assert fails HeapTupleHeaderGetXmax() will return garbage
anyway (read: a multi-xid instead of an xid in some cases!).
For non-assert-enabled builds, the only effect of the patch is thus to
consistently return InvalidTransactionId if the crosscheck snapshot turns
HeapTupleMayBeUpdated into HeapTupleUpdated. Which certainly seems to be
an improvement over sometimes returning InvalidTransactionId, sometimes
a locker's xid, and sometime's a multi-xid.
best regards,
Florian Pflug
