From: | Fujii Masao <masao(dot)fujii(at)oss(dot)nttdata(dot)com> |
---|---|
To: | Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com> |
Cc: | robertmhaas(at)gmail(dot)com, jgdr(at)dalibo(dot)com, michael(at)paquier(dot)xyz, pgsql-hackers(at)postgresql(dot)org |
Subject: | Re: +(pg_lsn, int8) and -(pg_lsn, int8) operators |
Date: | 2020-05-09 14:40:15 |
Message-ID: | 20cd4627-672b-32e8-730f-633e30c60f1f@oss.nttdata.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On 2020/05/08 12:10, Kyotaro Horiguchi wrote:
> At Fri, 8 May 2020 11:31:42 +0900, Fujii Masao <masao(dot)fujii(at)oss(dot)nttdata(dot)com> wrote in
>>>> You mean that pg_lsn_pli() and pg_lsn_mii() should emit an error like
>>>> "the number of bytes to add/subtract cannnot be NaN" when NaN is
>>>> specified?
>>> The function is called while executing an expression, so "NaN cannot
>>> be used in this expression" or something like that would work.
>>
>> This sounds ambiguous. I like to use clearer messages like
>>
>> cannot add NaN to pg_lsn
>> cannot subtract NaN from pg_lsn
>
> They works fine to me.
Ok, I updated pg_lsn_pli() and pg_lsn_mii() so that they emit an error
when NaN is specified as the number of bytes.
>>>> I'm not sure if int128 is available in every environments.
>>> In second thought, I found that we don't have enough substitute
>>> functions for the platforms without a native implement. Instead,
>>> there are some overflow-safe uint64 math functions, that is,
>>> pg_add/sub_u64_overflow. This patch defines numeric_pg_lsn which is
>>> substantially numeric_uint64. By using them, for example, we can make
>>> pg_lsn_pli mainly with integer arithmetic as follows.
>>
>> Sorry, I'm not sure what the benefit of this approach...
>
> (If we don't allow negative nbytes,)
> We accept numeric so that the operators can accept values out of range
> of int64, but we don't need to perform all arithmetic in numeric. That
> approach does less numeric arithmetic, that is, faster and simpler.
> We don't need to string'ify LSN with it. That avoid stack consumption.
>
>>> If invalid values are given as the addend, the following message would
>>> make sense.
>>> =# select '1/1::pg_lsn + 'NaN'::numeric;
>>> ERROR: cannot use NaN in this expression
>>> =# select '1/1::pg_lsn + '-1'::numeric;
>>> ERROR: numeric value out of range for this expression
>>
>> Could you tell me why we should reject this calculation?
>> IMO it's ok to add the negative number, and which is possible
>> with the latest patch.
>
> Sorry, I misread the patch as it rejected -1 for *nbytes*, by seeing
> numeric_pg_lsn.
>
> Finally, I'm convinced that we lack required integer arithmetic
> infrastructure to perform the objective.
>
> The patch looks good to me except the size of buf[], but I don't
> strongly object to that.
Ok, I changed the size of buf[] to 32.
Attached is the updated version of the patch.
Regards,
--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
Attachment | Content-Type | Size |
---|---|---|
pg_lsn_operators_v4.patch | text/plain | 12.4 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Tom Lane | 2020-05-09 15:17:26 | Re: Logical replication subscription owner |
Previous Message | Etsuro Fujita | 2020-05-09 11:35:04 | Re: PG 13 release notes, first draft |