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

> On Tue, Oct 29, 2019 at 5:23 PM btfujiitkp <btfujiitkp@oss.nttdata.com> 
> wrote:
>> > Thomas Munro <thomas.munro@gmail.com> writes:
>> >> On Sun, Sep 1, 2019 at 5:04 PM Thomas Munro <thomas.munro@gmail.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