Re: [PATCH] bigint txids vs 'xid' type, new txid_recent(bigint) => xid - Mailing list pgsql-hackers
| From | Craig Ringer |
|---|---|
| Subject | Re: [PATCH] bigint txids vs 'xid' type, new txid_recent(bigint) => xid |
| Date | |
| Msg-id | CAMsr+YEjcxNoEzFzD4y_u3PjMKUYE7mJnYy00ntNFJ8Z+LvW_A@mail.gmail.com Whole thread Raw |
| In response to | Re: [PATCH] bigint txids vs 'xid' type, new txid_recent(bigint) => xid (Jim Nasby <Jim.Nasby@BlueTreble.com>) |
| Responses |
Re: [PATCH] bigint txids vs 'xid' type, new
txid_recent(bigint) => xid
(Peter Eisentraut <peter.eisentraut@2ndquadrant.com>)
|
| List | pgsql-hackers |
On 19 August 2016 at 02:35, Jim Nasby <Jim.Nasby@bluetreble.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.
pgsql-hackers by date: