Re: [PATCH] bigint txids vs 'xid' type, new txid_recent(bigint) => xid - Mailing list pgsql-hackers

On Tue, Aug 16, 2016 at 2:45 PM, Craig Ringer <craig@2ndquadrant.com> wrote:
> Hi all
>
> While implementing support for traceable transactions (finding out after the
> fact whether an xact committed or aborted), I've found that Pg is very
> inconsistent with what it considers a transaction ID from a user facing
> point of view, to the point where I think it's hard for users to write
> correct queries.
>
> txid_current() returns a 64-bit xid in which the higher 32 bits are the xid
> epoch. This providers users with wraparound protection and means they don't
> have to deal with the moving xid threshold.
>
> Many other functions accept and return 'xid', the 32-bit type that isn't
> wraparound protected. Presumably they assume you'll only use them with
> recent transaction IDs, but there are a couple of problems with this:
>
> * We can't ensure they're only used with recent XIDs and can't detect if
> they're passed a wrapped around xid
>
> * There's no good way to _get_ a 32-bit xid for the current xact since
> txid_current() returns a 64-bit bigint xid.
>
> (I have to admit that in the past I always blindly assumed that
> txid_current() returned bigint for historical reasons, because we don't have
> a uint32 type and the xid type didn't exist yet. So I'd do things like get
> the value of txid_current() and pass it to pg_xact_commit_timestamp() later
> on. This turns out to be wrong, it just happens to work until the epoch
> counter increments for the first time. Similarly, working around the seeming
> oversight of a missing bigint to xid cast with ::text::xid is wrong but will
> seem fine at first.)
>
> I'm surprised the 32-bit xid was ever exposed to the user, rather than a
> 64-bit epoch-extended xid.
>
> It's not clear to me how a user is supposed to correctly pass the result of
> txid_current() to anything like pg_xact_commit_timestamp(xid). They'd have
> to get the epoch from a new txid_current() call, split both into two 32-bit
> values, and do wraparound checking. Exceedingly unwieldy and hard to get
> right.
>
> Since I don't think we can get rid of the 32-bit xid, I think we need a
> function to get the 32-bit xid from a 64-bit epoch-and-xid with wraparound
> protection.
>
> Here's a patch for that, adding a function txid_recent(bigint) => xid that
> returns the low 32 bits of a 64-bit xid like that returned from txid_current
> if the xid isn't wrapped around. If it's past the wraparound threshold the
> function returns null, since most functions that take xid are strict and
> will in turn return null. The alternative, an ERROR, seems harder for users
> to handle without resorting to plpgsql. It does ERROR on XIDs in the future
> though, since there's no good reason to see those. The epoch is ignored for
> permanent XIDs.
>
> I don't like the name much, but haven't come up with a better one yet.
>
> Thoughts?
>
>
> IMO some functions that take 'xid' should be considered for a bigint
> variant:
>
>  age    (as txid_age(bigint))
>  pg_xact_commit_timestamp
>

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
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
plainly want to convert it to 32 bit xid, then may be txid_convert or
something like that is more suitable.

-- 
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com