Re: [PATCH] Transaction traceability - txid_status(bigint)

On 23 Aug 2016 16:02, "Petr Jelinek" <petr(at)2ndquadrant(dot)com> wrote:
>
> On 23/08/16 02:55, Craig Ringer wrote:
>>
>> On 23 August 2016 at 01:03, Robert Haas <robertmhaas(at)gmail(dot)com
>> <mailto:robertmhaas(at)gmail(dot)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.