Re: Subtle bug in clog.c

Alvaro Herrera <alvherre(at)dcc(dot)uchile(dot)cl> writes:
> I'm wondering about the following statement in TransactionIdSetStatus():
> *byteptr |= (status << bshift);

> When the status is SUBCOMMITTED, the bytemask will have 11 for those two
> bytes; therefore, when OR-ing a 10 or 01 mask (committed or aborted),

Um. That code is assuming that the initial state was either 00 or the
target value (per the former state of the Assert above it). You're
right, we need to change that.

At one time this behavior was important because there wasn't
synchronization between writers and readers of clog; so transformations
could only set bits and not clear them. That doesn't matter anymore
though, because there are page-level locks taken in clog.c.

> I wonder why this works.

It doesn't --- the state remains SUBCOMMITTED.

> My theory is that the current code is a no-op and that it works because
> the server recurses up the subtrans tree to find the parent state ...

Right.

I'm off to dinner but will commit a fix for this when I return.

regards, tom lane