On Wed, May 22, 2019 at 5:47 PM Robert Haas <robertmh...@gmail.com> wrote: > > On Wed, May 22, 2019 at 7:17 AM Amit Kapila <amit.kapil...@gmail.com> wrote: > > > > +/* Extract xid from a value comprised of epoch and xid */ > > > > +#define GetXidFromEpochXid(epochxid) \ > > > > + ((uint32) (epochxid) & 0XFFFFFFFF) > > > > + > > > > +/* Extract epoch from a value comprised of epoch and xid */ > > > > +#define GetEpochFromEpochXid(epochxid) \ > > > > + ((uint32) ((epochxid) >> 32)) > > > > + > > > > > > Why do these exist? > > > > > > > We don't need the second one (GetEpochFromEpochXid), but the first one > > is required. Basically, the oldestXidHavingUndo computation does > > consider oldestXmin (which is still a TransactionId) as we can't > > retain undo which is 2^31 transactions old due to other limitations > > like clog/snapshots still has a limit of 4-byte transaction ids. > > Slightly unrelated, but we do want to improve the undo retention in a > > subsequent version such that we won't allow pending undo for > > transaction whose age is more than 2^31. > > The point is that we now have EpochFromFullTransactionId and > XidFromFullTransactionId. You shouldn't be inventing your own version > of that infrastructure. Use FullTransactionId, not a uint64, and then > use the functions for dealing with full transaction IDs from > transam.h. >
Okay, I misunderstood the comment. I'll change accordingly. Thanks for pointing out. -- With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com