Re: [PATCH] bigint txids vs 'xid' type, new txid_recent(bigint) => xid

On 19 August 2016 at 02:35, Jim Nasby <Jim(dot)Nasby(at)bluetreble(dot)com> wrote:

> On 8/18/16 5:46 AM, Amit Kapila wrote:
>
>> I think there is a value in exposing such a variant which takes bigint
>> and internally converts it to xid. I am not sure the semantics for
>>
>
> I think that's a bad idea because you have the exact same problems we have
> now: bigint is signed, epoch is not.

Eh. A distant future problem IMO. txid_current will already start returning
negative values if the epoch crosses INT32_MAX.

the other proposal txid_recent() is equivalent to what we have for
>> txid_current(). One thing which is different is that txid_current()
>> allocates a new transaction if there is currently none. If you
>>
>
> Right, and it would be nice to be able to tell if an XID has been assigned
> to your transaction or not; something you currently can't do.

It's trivial to expose GetTopTransactionIdIfAny() . Basically copy and
paste txid_current() to txid_current_ifassigned() and replace the
GetTopTransactionId() call with GetTopTransactionIdIfAny() .

Or add a bool argument to txid_current() to not assign one. But I'd rather
a new function in this case, and it's so short that the duplication is no
concern.

plainly want to convert it to 32 bit xid, then may be txid_convert or
>> something like that is more suitable.
>>
>
> I think we need to either add real types for handling XID/epoch/TXID or
> finally create uint types. It's *way* too easy to screw things up the way
> they are today.

Hm. Large scope increase there. Especially introducing unsigned types.
There's a reason that hasn't been done already - it's not just copying a
whole pile of code, it's also defining all the signed/unsigned interactions
and conversions carefully. People mix signed and unsigned types incorrectly
in C all the time, and often don't notice the problems. It also only gains
you an extra bit. Unsigned types would be nice when interacting with
outside systems that use them and Pg innards, but that's about all they're
good for IMO. For everything else you should be using numeric if you're
worried about fitting in a bigint.

I'm not against adding a 'bigxid' or 'epoch_xid' or something, internally a
uint64. It wouldn't need all the opclasses, casts, function overloads, etc
that uint8 would. It's likely to break code that expects txid_current() to
return a bigint, but since it looks like most of that code is already
silently broken I'm not too upset by that.

Separately to all that, though, we should still have a way to get the
32-bit xid from an xid with epoch that doesn't require the user to know its
internal structure and bitshift it, especially since they can't check the
epoch. Maybe call it txid_convert_ifrecent(bigint). IMO the "recent" part
is important because of the returns-null-if-xid-is-old behaviour. It's not
a straight conversion.

--
Craig Ringer http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services