Subtle bug in clog.c

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),
shouldn't the byte remaing the same as before? That is, changing from
SUBCOMMITTED to either COMMIT or ABORT does not actually do anything?

I wonder why this works. In the phantom Xid patch I had to set the
bytes to zero first, and then set the new bits ... it took me quite a
while to figure it out and I'm still wondering why the current code
doesn't break.

In the phantom patch I had to add this line before the OR-ing:

+ *byteptr &= ~(TRANSACTION_STATUS_SUB_COMMITTED << bshift);

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 ... An
aborted subxact only works because we don't mark it SUBCOMMIT, so
committed subxacts with aborted parent _always_ have to recurse up the
tree subtrans tree. (An experiment to prove this theory could involve
marking SUBCOMMIT all subtransaction at start -- things will break all
around and they shouldn't.)

--
Alvaro Herrera (<alvherre[a]dcc.uchile.cl>)
"Always assume the user will do much worse than the stupidest thing
you can imagine." (Julien PUYDT)