On 2020/04/28 15:03, Michael Paquier wrote:
> On Tue, Apr 28, 2020 at 12:56:19PM +0900, Fujii Masao wrote:
>> Yes. Attached is the updated version of the patch, which introduces
>> +(pg_lsn, numeric) and -(pg_lsn, numeric) operators.
>> To implement them, I added also numeric_pg_lsn() function that converts
>> numeric to pg_lsn.
>
> - those write-ahead log locations.
> + those write-ahead log locations. Also the number of bytes can be added
> + into and substracted from LSN using the <literal>+</literal> and
> + <literal>-</literal> operators, respectively.
> That's short. Should this mention the restriction with numeric (or
> just recommend its use) because we don't have a 64b unsigned type
> internally, basically Robert's point?
Thanks for the review! What about the following description?
-----------------
Also the number of bytes can be added into and substracted from LSN using the
<literal>+(pg_lsn,numeric)</literal> and <literal>-(pg_lsn,numeric)</literal>
operators, respectively. Note that the calculated LSN should be in the range
of <type>pg_lsn</type> type, i.e., between <literal>0/0</literal> and
<literal>FFFFFFFF/FFFFFFFF</literal>.
-----------------
>
> + /* XXX would it be better to return NULL? */
> + if (NUMERIC_IS_NAN(num))
> + ereport(ERROR,
> + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
> + errmsg("cannot convert NaN to pg_lsn")));
> That would be good to test, and an error sounds fine to me.
You mean that we should add the test that goes through this code block,
into the regression test?
Regards,
--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION