On 2020/05/07 11:21, Kyotaro Horiguchi wrote:
At Tue, 28 Apr 2020 12:56:19 +0900, Fujii Masao <masao.fu...@oss.nttdata.com>
wrote in
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.
+ into and substracted from LSN using the <literal>+</literal> and
s/substracted/subtracted/
(This still remains in the latest version)
Thanks! Will fix this.
+static bool
+numericvar_to_uint64(const NumericVar *var, uint64 *result)
Other numricvar_to_xxx() functions return an integer value that means
success by 0 and failure by -1, which is one of standard signature of
this kind of functions. I don't see a reason for this function to
have different signatures from them.
Unless I'm missing something, other functions also return boolean.
For example,
static bool numericvar_to_int32(const NumericVar *var, int32 *result);
static bool numericvar_to_int64(const NumericVar *var, int64 *result);
+ /* 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")));
The ERROR seems perfect to me since NaN is out of the domain of
LSN. log(-1) results in a similar error.
On the other hand, the code above makes the + operator behave as the
follows.
=# SELECT '1/1'::pg_lsn + 'NaN'::numeric;
ERROR: cannot convert NaN to pg_lsn
This looks somewhat different from what actually wrong is.
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?
+ char buf[256];
+
+ /* Convert to numeric */
+ snprintf(buf, sizeof(buf), UINT64_FORMAT, lsn);
The values larger than 2^64 is useless. So 32 (or any value larger
than 21) is enough for the buffer length.
Could you tell me what the actual problem is when buf[256] is used?
By the way coudln't we use int128 instead for internal arithmetic? I
think that makes the code simpler.
I'm not sure if int128 is available in every environments.
Regards,
--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
