On 23 Aug 2016 16:02, "Petr Jelinek" <p...@2ndquadrant.com> wrote: > > On 23/08/16 02:55, Craig Ringer wrote: >> >> On 23 August 2016 at 01:03, Robert Haas <robertmh...@gmail.com >> <mailto:robertmh...@gmail.com>> wrote: >> >> >> >> I think you should use underscores to separate all of the words >> instead of only some of them. >> >> >> ifassigned => if_assigned >> >> ifrecent=> if_recent >> >> Updated patch series attached. As before, 0-4 intended for commit, 5 >> just because it'll be handy to have around for people doing wraparound >> related testing. > > > I guess you mean 0-3 for commit and 4 is just handy?
Er. Right. 1-3. 4 just as handy test/tool. 1 most important and useful. Then 2. Then 3. > From the point of code this patch seems good to me. Thanks. > I do wonder about the 3rd patch though. I wonder if it would not be better to have the opposite function instead - converting xid to txid as that will always work and does not have to have the NULL case and would be simpler in terms of code. Yeah, but it wouldn't solve the need to take txid_current() output and do stuff with it other than ordinal comparison. Like pass to commit ts functions and others that take xid. If we extend all funcs that take xid to take bigint instead, they just get to use the same epoch logic in them, complete with some way to deal with wrapped xids sensibly. It has to be done somewhere. Though it's prettier if hidden from the user. More importantly imo, txid => bigint has to assume the current epoch. We have no way to make sure the user doesn't try to use something already wrapped. I don't mind if everyone decides it's better to make xid go away and use bigint everywhere user facing. Or even a new bigxid type. More work than I can really afford but can manage; shouldn't block #1 and #2 though as they already use bigint.