On Wed, Jun 4, 2014 at 7:10 PM, Andres Freund <andres@2ndquadrant.com> wrote:
> On 2014-06-04 19:03:15 -0400, Robert Haas wrote:
>> On Mon, Jun 2, 2014 at 1:35 PM, Andres Freund <andres@2ndquadrant.com> wrote:
>> > Robert: Do you remember that case?
>> >
>> > Alvaro: In the end it'd not be very harmful - if it happens
>> > TransactionIdDidCommit() will return false (there's special case code
>> > for it).
>>
>> Not specifically, but I'd be surprised if it isn't possible.
>
> Wouldn't that mean that every single visibility routine in tqual.c is
> buggy?
>
> I am not convinced there aren't further bugs in some corner cases in
> tqual.c. But even a low likelihood scenarious of
> xmax = InvalidTransactionId && (infomask & XMAX_INVALID) == 0
> would have become visible by now given how widespread/central those
> tests are?
What specifically do you think will break? For example, in
HeapTupleSatisfiesMVCC(), if no xmax-related hint bits are set, it
looks to me like we'll end up here:
if (!TransactionIdDidCommit(HeapTupleHeaderGetRawXmax(tuple)))
{
/* it must have aborted or crashed */
SetHintBits(tuple, buffer, HEAP_XMAX_INVALID,
InvalidTransactionId);
return true;
}
Well, TransactionIdDidCommit() will call TransactionLogFetch() which
has a special case coding to handle non-normal transaction IDs, which
for InvalidTransactionId will return TRANSACTION_STATUS_ABORTED. Now
maybe that is dead code and always has been, but that wouldn't be my
bet going into it: I bet that got there for a reason.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company