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

On 11/3/19 2:43 PM, Thomas Munro wrote:
> 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)
> 
>> 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).
> 
> In the attached I also took Tom's advice and used unused_oids script
> to pick random OIDs >= 8000 for all new objects (ignoring nearby
> comments about the range of OIDs used in different sections of the
> file).
> 

These two patches (v3) no longer apply cleanly.  Could you please
rebase?

-- 
Mark Dilger