Re: Should we add xid_current() or a int8->xid cast?

> On Tue, Oct 29, 2019 at 5:23 PM btfujiitkp <btfujiitkp(at)oss(dot)nttdata(dot)com>
> wrote:
>> > Thomas Munro <thomas(dot)munro(at)gmail(dot)com> writes:
>> >> On Sun, Sep 1, 2019 at 5:04 PM Thomas Munro <thomas(dot)munro(at)gmail(dot)com>
>> >> wrote:
>> >>> Adding to CF.
>> >
>> >> Rebased. An OID clashed so re-roll the dice. Also spotted a typo.
>> >
>>
>> I have some questions in this code.
>
> Thanks for looking at the patch.
>
>> First,
>> "FullTransactionIdPrecedes(xmax, val)" is not equal to "val >= xmax"
>> of
>> the previous code. "FullTransactionIdPrecedes(xmax, val)" expresses
>> "val > xmax". Is it all right?
>>
>> @@ -384,15 +324,17 @@ parse_snapshot(const char *str)
>> while (*str != '\0')
>> {
>> /* read next value */
>> - val = str2txid(str, &endp);
>> + val = FullTransactionIdFromU64(pg_strtouint64(str,
>> &endp, 10));
>> str = endp;
>>
>> /* require the input to be in order */
>> - if (val < xmin || val >= xmax || val < last_val)
>> + if (FullTransactionIdPrecedes(val, xmin) ||
>> + FullTransactionIdPrecedes(xmax, val) ||
>> + FullTransactionIdPrecedes(val, last_val))
>>
>> In addition to it, as to current TransactionId(not FullTransactionId)
>> comparison, when we express ">=" of TransactionId, we use
>> "TransactionIdFollowsOrEquals". this method is referred by some
>> methods.
>> On the other hand, FullTransactionIdFollowsOrEquals has not
>> implemented
>> yet. So, how about implementing this method?
>
> Good idea. I added the missing variants:
>
> +#define FullTransactionIdPrecedesOrEquals(a, b) ((a).value <=
> (b).value)
> +#define FullTransactionIdFollows(a, b) ((a).value > (b).value)
> +#define FullTransactionIdFollowsOrEquals(a, b) ((a).value >=
> (b).value)
>

Thank you for your patch.
It looks good.

>> Second,
>> About naming rule, "8" of xid8 means 8 bytes, but "8" has different
>> meaning in each situation. For example, int8 of PostgreSQL means 8
>> bytes, int8 of C language means 8 bits. If 64 is used, it just means
>> 64
>> bits. how about xid64()?
>
> In C, the typenames use bits, by happy coincidence similar to the C99
> stdint.h typenames (int32_t etc) that we should perhaps eventually
> switch to.
>
> In SQL, the types have names based on the number of bytes: int2, int4,
> int8, float4, float8, not conforming to any standard but established
> over 3 decades ago and also understood by a few other SQL systems.
>
> That's unfortunate, but I can't see that ever changing. I thought
> that it would make most sense for the SQL type to be called xid8,
> though admittedly it doesn't quite fit the pattern because xid is not
> called xid4. There is another example a bit like that: macaddr (6
> bytes) and macaccdr8 (8 bytes). As for the C type, we use
> TransactionId and FullTransactionId (rather than, say, xid32 and
> xid64).

That makes sense.

Anyway,
In the pg_proc.dat, "xid_snapshot_xip" should be "xid8_snapshot_xip".
And some parts of 0002 patch are rejected when I patch 0002 after
patching 0001.

regards