Re: small typo in src/backend/access/transam/xlog.c

Andres Freund <andres(at)2ndquadrant(dot)com> writes:
> On 2013-07-22 15:55:46 -0400, Robert Haas wrote:
>> And why is that?

> The comment above tells: "while the lower half is the XOR of tv_sec and
> tv_usec."

Yeah, the code doesn't match the comment; this mistake seems to be
aboriginal.

> I don't think it really matters. the bitwise OR has the tenency to
> collect too many set bits, but ... who cares?

This is making the value less unique than intended, so I think it's
worth doing something about. However, it's also worth noting that the
intended behavior (as described by the comment) was designed to allow
for the possibility that "uint64" is really only 32 bits --- a
possibility we stopped supporting several versions ago. So rather than
just quickly s/|/^/, maybe we should step back and think about whether
we want to change the sysid generation algorithm altogether.

We could for instance keep the high half as tv_sec, while making the low
half be something like (tv_usec << 12) | (getpid() & 0xfff). This would
restore the intended ability to reverse-engineer the exact creation time
from the sysidentifier, and also add a little more uniqueness by way of
the creating process's PID. (Note tv_usec must fit in 20 bits.)

regards, tom lane