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

On 11/3/19 2:43 PM, Thomas Munro wrote:
> 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)
>
>> 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