+(pg_lsn, int8) and -(pg_lsn, int8) operators
Hi,
I'd like to propose to introduce the +(pg_lsn, int8) and -(pg_lsn, int8)
operators. The + operator allows us to add the number of bytes into pg_lsn,
resulting new pg_lsn. The - operator allows us to substract the number
of bytes from pg_lsn, resulting new pg_lsn. Thought?
I sometimes need these features for debuging purpose.
Attached is the patch implementing those operators.
Of course, this is the dev item for v14.
Regards,
--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
Attachments:
pg_lsn_operators_v1.patchtext/plain; charset=UTF-8; name=pg_lsn_operators_v1.patch; x-mac-creator=0; x-mac-type=0Download
diff --git a/doc/src/sgml/datatype.sgml b/doc/src/sgml/datatype.sgml
index c2e42f31c0..c5469e4678 100644
--- a/doc/src/sgml/datatype.sgml
+++ b/doc/src/sgml/datatype.sgml
@@ -4776,7 +4776,10 @@ SELECT * FROM pg_attribute
standard comparison operators, like <literal>=</literal> and
<literal>></literal>. Two LSNs can be subtracted using the
<literal>-</literal> operator; the result is the number of bytes separating
- 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.
+
</para>
</sect1>
diff --git a/src/backend/utils/adt/pg_lsn.c b/src/backend/utils/adt/pg_lsn.c
index d9754a7778..e4bdad3ef7 100644
--- a/src/backend/utils/adt/pg_lsn.c
+++ b/src/backend/utils/adt/pg_lsn.c
@@ -248,3 +248,49 @@ pg_lsn_mi(PG_FUNCTION_ARGS)
return result;
}
+
+/*
+ * Add the number of bytes to pg_lsn, giving a new pg_lsn.
+ * Must handle both positive and negative numbers of bytes.
+ */
+Datum
+pg_lsn_pli(PG_FUNCTION_ARGS)
+{
+ XLogRecPtr lsn = PG_GETARG_LSN(0);
+ int64 nbytes = PG_GETARG_INT64(1);
+ XLogRecPtr result;
+
+ result = lsn + nbytes;
+
+ /* Check for pg_lsn overflow */
+ if ((nbytes >= 0 && result < lsn) ||
+ (nbytes < 0 && result > lsn))
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("pg_lsn out of range")));
+
+ PG_RETURN_LSN(result);
+}
+
+/*
+ * Substract the number of bytes from pg_lsn, giving a new pg_lsn.
+ * Must handle both positive and negative numbers of bytes.
+ */
+Datum
+pg_lsn_mii(PG_FUNCTION_ARGS)
+{
+ XLogRecPtr lsn = PG_GETARG_LSN(0);
+ int64 nbytes = PG_GETARG_INT64(1);
+ XLogRecPtr result;
+
+ result = lsn - nbytes;
+
+ /* Check for pg_lsn overflow */
+ if ((nbytes >= 0 && result > lsn) ||
+ (nbytes < 0 && result < lsn))
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("pg_lsn out of range")));
+
+ PG_RETURN_LSN(result);
+}
diff --git a/src/include/catalog/pg_operator.dat b/src/include/catalog/pg_operator.dat
index 00ada7e48f..dc3dcc30c5 100644
--- a/src/include/catalog/pg_operator.dat
+++ b/src/include/catalog/pg_operator.dat
@@ -2909,6 +2909,12 @@
{ oid => '3228', descr => 'minus',
oprname => '-', oprleft => 'pg_lsn', oprright => 'pg_lsn',
oprresult => 'numeric', oprcode => 'pg_lsn_mi' },
+{ oid => '4179', descr => 'add',
+ oprname => '+', oprleft => 'pg_lsn', oprright => 'int8',
+ oprresult => 'pg_lsn', oprcode => 'pg_lsn_pli' },
+{ oid => '4180', descr => 'subtract',
+ oprname => '-', oprleft => 'pg_lsn', oprright => 'int8',
+ oprresult => 'pg_lsn', oprcode => 'pg_lsn_mii' },
# enum operators
{ oid => '3516', descr => 'equal',
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 4bce3ad8de..198feddc91 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -8578,6 +8578,12 @@
{ oid => '4188', descr => 'smaller of two',
proname => 'pg_lsn_smaller', prorettype => 'pg_lsn',
proargtypes => 'pg_lsn pg_lsn', prosrc => 'pg_lsn_smaller' },
+{ oid => '4198',
+ proname => 'pg_lsn_pli', prorettype => 'pg_lsn',
+ proargtypes => 'pg_lsn int8', prosrc => 'pg_lsn_pli' },
+{ oid => '4199',
+ proname => 'pg_lsn_mii', prorettype => 'pg_lsn',
+ proargtypes => 'pg_lsn int8', prosrc => 'pg_lsn_mii' },
# enum related procs
{ oid => '3504', descr => 'I/O',
diff --git a/src/test/regress/expected/pg_lsn.out b/src/test/regress/expected/pg_lsn.out
index 64d41dfdad..175b5dc9a7 100644
--- a/src/test/regress/expected/pg_lsn.out
+++ b/src/test/regress/expected/pg_lsn.out
@@ -71,6 +71,34 @@ SELECT '0/16AE7F8'::pg_lsn - '0/16AE7F7'::pg_lsn;
1
(1 row)
+SELECT '0/16AE7F7'::pg_lsn + 16::bigint;
+ ?column?
+-----------
+ 0/16AE807
+(1 row)
+
+SELECT '0/16AE7F7'::pg_lsn - 16::bigint;
+ ?column?
+-----------
+ 0/16AE7E7
+(1 row)
+
+SELECT 'FFFFFFFF/FFFFFFFE'::pg_lsn + 1::bigint;
+ ?column?
+-------------------
+ FFFFFFFF/FFFFFFFF
+(1 row)
+
+SELECT 'FFFFFFFF/FFFFFFFE'::pg_lsn + 2::bigint; -- out of range error
+ERROR: pg_lsn out of range
+SELECT '0/1'::pg_lsn - 1::bigint;
+ ?column?
+----------
+ 0/0
+(1 row)
+
+SELECT '0/1'::pg_lsn - 2::bigint; -- out of range error
+ERROR: pg_lsn out of range
-- Check btree and hash opclasses
EXPLAIN (COSTS OFF)
SELECT DISTINCT (i || '/' || j)::pg_lsn f
diff --git a/src/test/regress/sql/pg_lsn.sql b/src/test/regress/sql/pg_lsn.sql
index 2c143c82ff..d64ac852c4 100644
--- a/src/test/regress/sql/pg_lsn.sql
+++ b/src/test/regress/sql/pg_lsn.sql
@@ -27,6 +27,12 @@ SELECT '0/16AE7F7' < '0/16AE7F8'::pg_lsn;
SELECT '0/16AE7F8' > pg_lsn '0/16AE7F7';
SELECT '0/16AE7F7'::pg_lsn - '0/16AE7F8'::pg_lsn;
SELECT '0/16AE7F8'::pg_lsn - '0/16AE7F7'::pg_lsn;
+SELECT '0/16AE7F7'::pg_lsn + 16::bigint;
+SELECT '0/16AE7F7'::pg_lsn - 16::bigint;
+SELECT 'FFFFFFFF/FFFFFFFE'::pg_lsn + 1::bigint;
+SELECT 'FFFFFFFF/FFFFFFFE'::pg_lsn + 2::bigint; -- out of range error
+SELECT '0/1'::pg_lsn - 1::bigint;
+SELECT '0/1'::pg_lsn - 2::bigint; -- out of range error
-- Check btree and hash opclasses
EXPLAIN (COSTS OFF)
On Thu, Apr 23, 2020 at 5:21 AM Fujii Masao <masao.fujii@oss.nttdata.com> wrote:
I'd like to propose to introduce the +(pg_lsn, int8) and -(pg_lsn, int8)
operators. The + operator allows us to add the number of bytes into pg_lsn,
resulting new pg_lsn. The - operator allows us to substract the number
of bytes from pg_lsn, resulting new pg_lsn. Thought?
I sometimes need these features for debuging purpose.
For anyone who missed it, this idea was popular on Twitter:
https://twitter.com/fujii_masao/status/1252652020487487488
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
On Thu, Apr 23, 2020 at 2:51 PM Fujii Masao <masao.fujii@oss.nttdata.com> wrote:
Hi,
I'd like to propose to introduce the +(pg_lsn, int8) and -(pg_lsn, int8)
operators. The + operator allows us to add the number of bytes into pg_lsn,
resulting new pg_lsn. The - operator allows us to substract the number
of bytes from pg_lsn, resulting new pg_lsn. Thought?
I sometimes need these features for debuging purpose.
As it's presented in the patch I don't see much value in calling it as
LSN arithmetic. If we could do something like LSN of Nth WAL record
+/- <number of WAL records, n> = LSN of N+/- n th log record that
would be interesting. :)
--
Best Wishes,
Ashutosh Bapat
On Thu, Apr 23, 2020 at 12:28 PM Ashutosh Bapat
<ashutosh.bapat.oss@gmail.com> wrote:
As it's presented in the patch I don't see much value in calling it as
LSN arithmetic. If we could do something like LSN of Nth WAL record
+/- <number of WAL records, n> = LSN of N+/- n th log record that
would be interesting. :)
Well, that would mean that the value of x + 1 would depend not only on
x but on the contents of WAL, and that it would be uncomputable
without having the WAL available, and that adding large values would
be quite expensive.
I much prefer Fujii Masao's proposal.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
On Thu, Apr 23, 2020 at 08:09:22AM -0400, Robert Haas wrote:
For anyone who missed it, this idea was popular on Twitter:
(For the sake of the archives)
To which Alvaro, Robert, Fabrízio de Royes Mello, Julien Rouhaud and I
answered positively to.
--
Michael
On Fri, 24 Apr 2020 16:24:14 +0900
Michael Paquier <michael@paquier.xyz> wrote:
On Thu, Apr 23, 2020 at 08:09:22AM -0400, Robert Haas wrote:
For anyone who missed it, this idea was popular on Twitter:
(For the sake of the archives)
To which Alvaro, Robert, Fabrízio de Royes Mello, Julien Rouhaud and I
answered positively to.
And me, discretely, with a little heart.
At Fri, 24 Apr 2020 12:15:26 +0200, Jehan-Guillaume de Rorthais <jgdr@dalibo.com> wrote in
On Fri, 24 Apr 2020 16:24:14 +0900
Michael Paquier <michael@paquier.xyz> wrote:On Thu, Apr 23, 2020 at 08:09:22AM -0400, Robert Haas wrote:
For anyone who missed it, this idea was popular on Twitter:
(For the sake of the archives)
To which Alvaro, Robert, Fabrízio de Royes Mello, Julien Rouhaud and I
answered positively to.And me, discretely, with a little heart.
+1. I actually sometimes need it.
y the way, -(pg_lsn, pg_lsn) yields a numeric. I feel that it could be
confusing that the new operators takes a bigint. We need to cast the
second term to bigint in the following expression.
'2/20'::pg_lsn + ('1/10'::pg_lsn - '1/5'::pg_lsn)
The new + operator is not commutative. I'm not sure it is the right
desgin to make it commutative, but it would be irritatibe if it is
not. (Or maybe we should implement them as functions rather than
operators..)
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
On Sun, Apr 26, 2020 at 9:41 PM Kyotaro Horiguchi
<horikyota.ntt@gmail.com> wrote:
+1. I actually sometimes need it.
y the way, -(pg_lsn, pg_lsn) yields a numeric.
It might be a good idea to use numeric here, too. Because int8 is
signed, it's not big enough to cover the whole range of LSNs.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
On 2020/04/28 1:24, Robert Haas wrote:
On Sun, Apr 26, 2020 at 9:41 PM Kyotaro Horiguchi
<horikyota.ntt@gmail.com> wrote:+1. I actually sometimes need it.
y the way, -(pg_lsn, pg_lsn) yields a numeric.
It might be a good idea to use numeric here, too. Because int8 is
signed, it's not big enough to cover the whole range of LSNs.
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.
Regards,
--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
Attachments:
pg_lsn_operators_v2.patchtext/plain; charset=UTF-8; name=pg_lsn_operators_v2.patch; x-mac-creator=0; x-mac-type=0Download
diff --git a/doc/src/sgml/datatype.sgml b/doc/src/sgml/datatype.sgml
index 22eda0f4e9..b747571af5 100644
--- a/doc/src/sgml/datatype.sgml
+++ b/doc/src/sgml/datatype.sgml
@@ -4782,7 +4782,10 @@ SELECT * FROM pg_attribute
standard comparison operators, like <literal>=</literal> and
<literal>></literal>. Two LSNs can be subtracted using the
<literal>-</literal> operator; the result is the number of bytes separating
- 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.
+
</para>
</sect1>
diff --git a/src/backend/utils/adt/numeric.c b/src/backend/utils/adt/numeric.c
index 9986132b45..19f300205b 100644
--- a/src/backend/utils/adt/numeric.c
+++ b/src/backend/utils/adt/numeric.c
@@ -41,6 +41,7 @@
#include "utils/guc.h"
#include "utils/int8.h"
#include "utils/numeric.h"
+#include "utils/pg_lsn.h"
#include "utils/sortsupport.h"
/* ----------
@@ -472,6 +473,7 @@ static void apply_typmod(NumericVar *var, int32 typmod);
static bool numericvar_to_int32(const NumericVar *var, int32 *result);
static bool numericvar_to_int64(const NumericVar *var, int64 *result);
static void int64_to_numericvar(int64 val, NumericVar *var);
+static bool numericvar_to_uint64(const NumericVar *var, uint64 *result);
#ifdef HAVE_INT128
static bool numericvar_to_int128(const NumericVar *var, int128 *result);
static void int128_to_numericvar(int128 val, NumericVar *var);
@@ -3688,6 +3690,31 @@ numeric_float4(PG_FUNCTION_ARGS)
}
+Datum
+numeric_pg_lsn(PG_FUNCTION_ARGS)
+{
+ Numeric num = PG_GETARG_NUMERIC(0);
+ NumericVar x;
+ XLogRecPtr 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")));
+
+ /* Convert to variable format and thence to pg_lsn */
+ init_var_from_num(num, &x);
+
+ if (!numericvar_to_uint64(&x, (uint64 *) &result))
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("pg_lsn out of range")));
+
+ PG_RETURN_LSN(result);
+}
+
+
/* ----------------------------------------------------------------------
*
* Aggregate functions
@@ -6739,6 +6766,78 @@ int64_to_numericvar(int64 val, NumericVar *var)
var->weight = ndigits - 1;
}
+/*
+ * Convert numeric to uint64, rounding if needed.
+ *
+ * If overflow, return false (no error is raised). Return true if okay.
+ */
+static bool
+numericvar_to_uint64(const NumericVar *var, uint64 *result)
+{
+ NumericDigit *digits;
+ int ndigits;
+ int weight;
+ int i;
+ uint64 val;
+ NumericVar rounded;
+
+ /* Round to nearest integer */
+ init_var(&rounded);
+ set_var_from_var(var, &rounded);
+ round_var(&rounded, 0);
+
+ /* Check for zero input */
+ strip_var(&rounded);
+ ndigits = rounded.ndigits;
+ if (ndigits == 0)
+ {
+ *result = 0;
+ free_var(&rounded);
+ return true;
+ }
+
+ /* Check for negative input */
+ if (rounded.sign == NUMERIC_NEG)
+ {
+ free_var(&rounded);
+ return false;
+ }
+
+ /*
+ * For input like 10000000000, we must treat stripped digits as real. So
+ * the loop assumes there are weight+1 digits before the decimal point.
+ */
+ weight = rounded.weight;
+ Assert(weight >= 0 && ndigits <= weight + 1);
+
+ /* Construct the result */
+ digits = rounded.digits;
+ val = digits[0];
+ for (i = 1; i <= weight; i++)
+ {
+ if (unlikely(pg_mul_u64_overflow(val, NBASE, &val)))
+ {
+ free_var(&rounded);
+ return false;
+ }
+
+ if (i < ndigits)
+ {
+ if (unlikely(pg_add_u64_overflow(val, digits[i], &val)))
+ {
+ free_var(&rounded);
+ return false;
+ }
+ }
+ }
+
+ free_var(&rounded);
+
+ *result = val;
+
+ return true;
+}
+
#ifdef HAVE_INT128
/*
* Convert numeric to int128, rounding if needed.
diff --git a/src/backend/utils/adt/pg_lsn.c b/src/backend/utils/adt/pg_lsn.c
index d9754a7778..d3e289964c 100644
--- a/src/backend/utils/adt/pg_lsn.c
+++ b/src/backend/utils/adt/pg_lsn.c
@@ -16,6 +16,7 @@
#include "funcapi.h"
#include "libpq/pqformat.h"
#include "utils/builtins.h"
+#include "utils/numeric.h"
#include "utils/pg_lsn.h"
#define MAXPG_LSNLEN 17
@@ -248,3 +249,59 @@ pg_lsn_mi(PG_FUNCTION_ARGS)
return result;
}
+
+/*
+ * Add the number of bytes to pg_lsn, giving a new pg_lsn.
+ * Must handle both positive and negative numbers of bytes.
+ */
+Datum
+pg_lsn_pli(PG_FUNCTION_ARGS)
+{
+ XLogRecPtr lsn = PG_GETARG_LSN(0);
+ Datum nbytes = PG_GETARG_DATUM(1);
+ Datum num;
+ Datum res;
+ char buf[256];
+
+ /* Convert to numeric */
+ snprintf(buf, sizeof(buf), UINT64_FORMAT, lsn);
+ num = DirectFunctionCall3(numeric_in,
+ CStringGetDatum(buf),
+ ObjectIdGetDatum(0),
+ Int32GetDatum(-1));
+
+ /* Add two numerics */
+ res = DirectFunctionCall2(numeric_add,
+ NumericGetDatum(num), nbytes);
+
+ /* Convert to pg_lsn */
+ return DirectFunctionCall1(numeric_pg_lsn, res);
+}
+
+/*
+ * Substract the number of bytes from pg_lsn, giving a new pg_lsn.
+ * Must handle both positive and negative numbers of bytes.
+ */
+Datum
+pg_lsn_mii(PG_FUNCTION_ARGS)
+{
+ XLogRecPtr lsn = PG_GETARG_LSN(0);
+ Datum nbytes = PG_GETARG_DATUM(1);
+ Datum num;
+ Datum res;
+ char buf[256];
+
+ /* Convert to numeric */
+ snprintf(buf, sizeof(buf), UINT64_FORMAT, lsn);
+ num = DirectFunctionCall3(numeric_in,
+ CStringGetDatum(buf),
+ ObjectIdGetDatum(0),
+ Int32GetDatum(-1));
+
+ /* Add two numerics */
+ res = DirectFunctionCall2(numeric_sub,
+ NumericGetDatum(num), nbytes);
+
+ /* Convert to pg_lsn */
+ return DirectFunctionCall1(numeric_pg_lsn, res);
+}
diff --git a/src/include/catalog/pg_operator.dat b/src/include/catalog/pg_operator.dat
index 00ada7e48f..15dfa1497a 100644
--- a/src/include/catalog/pg_operator.dat
+++ b/src/include/catalog/pg_operator.dat
@@ -2909,6 +2909,17 @@
{ oid => '3228', descr => 'minus',
oprname => '-', oprleft => 'pg_lsn', oprright => 'pg_lsn',
oprresult => 'numeric', oprcode => 'pg_lsn_mi' },
+{ oid => '5025', descr => 'add',
+ oprname => '+', oprleft => 'pg_lsn', oprright => 'numeric',
+ oprresult => 'pg_lsn', oprcom => '+(numeric,pg_lsn)',
+ oprcode => 'pg_lsn_pli' },
+{ oid => '5026', descr => 'add',
+ oprname => '+', oprleft => 'numeric', oprright => 'pg_lsn',
+ oprresult => 'pg_lsn', oprcom => '+(pg_lsn,numeric)',
+ oprcode => 'numeric_pl_pg_lsn' },
+{ oid => '5027', descr => 'subtract',
+ oprname => '-', oprleft => 'pg_lsn', oprright => 'numeric',
+ oprresult => 'pg_lsn', oprcode => 'pg_lsn_mii' },
# enum operators
{ oid => '3516', descr => 'equal',
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 4bce3ad8de..0eb94d92e3 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -4398,6 +4398,9 @@
{ oid => '1783', descr => 'convert numeric to int2',
proname => 'int2', prorettype => 'int2', proargtypes => 'numeric',
prosrc => 'numeric_int2' },
+{ oid => '6103', descr => 'convert numeric to pg_lsn',
+ proname => 'pg_lsn', prorettype => 'pg_lsn', proargtypes => 'numeric',
+ prosrc => 'numeric_pg_lsn' },
{ oid => '3556', descr => 'convert jsonb to boolean',
proname => 'bool', prorettype => 'bool', proargtypes => 'jsonb',
@@ -8578,6 +8581,15 @@
{ oid => '4188', descr => 'smaller of two',
proname => 'pg_lsn_smaller', prorettype => 'pg_lsn',
proargtypes => 'pg_lsn pg_lsn', prosrc => 'pg_lsn_smaller' },
+{ oid => '5022',
+ proname => 'pg_lsn_pli', prorettype => 'pg_lsn',
+ proargtypes => 'pg_lsn numeric', prosrc => 'pg_lsn_pli' },
+{ oid => '5023',
+ proname => 'numeric_pl_pg_lsn', prolang => 'sql', prorettype => 'pg_lsn',
+ proargtypes => 'numeric pg_lsn', prosrc => 'select $2 + $1' },
+{ oid => '5024',
+ proname => 'pg_lsn_mii', prorettype => 'pg_lsn',
+ proargtypes => 'pg_lsn numeric', prosrc => 'pg_lsn_mii' },
# enum related procs
{ oid => '3504', descr => 'I/O',
diff --git a/src/test/regress/expected/pg_lsn.out b/src/test/regress/expected/pg_lsn.out
index 64d41dfdad..7eb6a387b1 100644
--- a/src/test/regress/expected/pg_lsn.out
+++ b/src/test/regress/expected/pg_lsn.out
@@ -71,6 +71,52 @@ SELECT '0/16AE7F8'::pg_lsn - '0/16AE7F7'::pg_lsn;
1
(1 row)
+SELECT '0/16AE7F7'::pg_lsn + 16::numeric;
+ ?column?
+-----------
+ 0/16AE807
+(1 row)
+
+SELECT 16::numeric + '0/16AE7F7'::pg_lsn;
+ ?column?
+-----------
+ 0/16AE807
+(1 row)
+
+SELECT '0/16AE7F7'::pg_lsn - 16::numeric;
+ ?column?
+-----------
+ 0/16AE7E7
+(1 row)
+
+SELECT 'FFFFFFFF/FFFFFFFE'::pg_lsn + 1::numeric;
+ ?column?
+-------------------
+ FFFFFFFF/FFFFFFFF
+(1 row)
+
+SELECT 'FFFFFFFF/FFFFFFFE'::pg_lsn + 2::numeric; -- out of range error
+ERROR: pg_lsn out of range
+SELECT '0/1'::pg_lsn - 1::numeric;
+ ?column?
+----------
+ 0/0
+(1 row)
+
+SELECT '0/1'::pg_lsn - 2::numeric; -- out of range error
+ERROR: pg_lsn out of range
+SELECT '0/0'::pg_lsn + ('FFFFFFFF/FFFFFFFF'::pg_lsn - '0/0'::pg_lsn);
+ ?column?
+-------------------
+ FFFFFFFF/FFFFFFFF
+(1 row)
+
+SELECT 'FFFFFFFF/FFFFFFFF'::pg_lsn - ('FFFFFFFF/FFFFFFFF'::pg_lsn - '0/0'::pg_lsn);
+ ?column?
+----------
+ 0/0
+(1 row)
+
-- Check btree and hash opclasses
EXPLAIN (COSTS OFF)
SELECT DISTINCT (i || '/' || j)::pg_lsn f
diff --git a/src/test/regress/sql/pg_lsn.sql b/src/test/regress/sql/pg_lsn.sql
index 2c143c82ff..74d9fb4e6a 100644
--- a/src/test/regress/sql/pg_lsn.sql
+++ b/src/test/regress/sql/pg_lsn.sql
@@ -27,6 +27,15 @@ SELECT '0/16AE7F7' < '0/16AE7F8'::pg_lsn;
SELECT '0/16AE7F8' > pg_lsn '0/16AE7F7';
SELECT '0/16AE7F7'::pg_lsn - '0/16AE7F8'::pg_lsn;
SELECT '0/16AE7F8'::pg_lsn - '0/16AE7F7'::pg_lsn;
+SELECT '0/16AE7F7'::pg_lsn + 16::numeric;
+SELECT 16::numeric + '0/16AE7F7'::pg_lsn;
+SELECT '0/16AE7F7'::pg_lsn - 16::numeric;
+SELECT 'FFFFFFFF/FFFFFFFE'::pg_lsn + 1::numeric;
+SELECT 'FFFFFFFF/FFFFFFFE'::pg_lsn + 2::numeric; -- out of range error
+SELECT '0/1'::pg_lsn - 1::numeric;
+SELECT '0/1'::pg_lsn - 2::numeric; -- out of range error
+SELECT '0/0'::pg_lsn + ('FFFFFFFF/FFFFFFFF'::pg_lsn - '0/0'::pg_lsn);
+SELECT 'FFFFFFFF/FFFFFFFF'::pg_lsn - ('FFFFFFFF/FFFFFFFF'::pg_lsn - '0/0'::pg_lsn);
-- Check btree and hash opclasses
EXPLAIN (COSTS OFF)
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?
+ /* 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.
--
Michael
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
On Thu, Apr 30, 2020 at 11:40:59PM +0900, Fujii Masao wrote:
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>.
-----------------
That reads fine.
+ /* 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?
Yes, that looks worth making sure to track, especially if the behavior
of this code changes in the future.
--
Michael
At Tue, 28 Apr 2020 12:56:19 +0900, Fujii Masao <masao.fujii@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)
+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.
+ /* 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.
+ 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.
By the way coudln't we use int128 instead for internal arithmetic? I
think that makes the code simpler.
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
On 2020/05/02 11:29, Michael Paquier wrote:
On Thu, Apr 30, 2020 at 11:40:59PM +0900, Fujii Masao wrote:
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>.
-----------------That reads fine.
Ok, I will update the docs in that way.
+ /* 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?Yes, that looks worth making sure to track, especially if the behavior
of this code changes in the future.
Ok, I will add that regression test.
Regards,
--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
On 2020/05/07 11:21, Kyotaro Horiguchi wrote:
At Tue, 28 Apr 2020 12:56:19 +0900, Fujii Masao <masao.fujii@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_lsnThis 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
On 2020/05/07 13:15, Fujii Masao wrote:
On 2020/05/02 11:29, Michael Paquier wrote:
On Thu, Apr 30, 2020 at 11:40:59PM +0900, Fujii Masao wrote:
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>.
-----------------That reads fine.
Ok, I will update the docs in that way.
Done.
+�� /* 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?Yes, that looks worth making sure to track, especially if the behavior
of this code changes in the future.Ok, I will add that regression test.
Done. Attached is the updated version of the patch!
Regards,
--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
Attachments:
pg_lsn_operators_v3.patchtext/plain; charset=UTF-8; name=pg_lsn_operators_v3.patch; x-mac-creator=0; x-mac-type=0Download
diff --git a/doc/src/sgml/datatype.sgml b/doc/src/sgml/datatype.sgml
index a8d0780387..a86b794ce0 100644
--- a/doc/src/sgml/datatype.sgml
+++ b/doc/src/sgml/datatype.sgml
@@ -4823,7 +4823,13 @@ SELECT * FROM pg_attribute
standard comparison operators, like <literal>=</literal> and
<literal>></literal>. Two LSNs can be subtracted using the
<literal>-</literal> operator; the result is the number of bytes separating
- those write-ahead log locations.
+ those write-ahead log locations. Also the number of bytes can be
+ added into and subtracted 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>.
</para>
</sect1>
diff --git a/src/backend/utils/adt/numeric.c b/src/backend/utils/adt/numeric.c
index 9986132b45..19f300205b 100644
--- a/src/backend/utils/adt/numeric.c
+++ b/src/backend/utils/adt/numeric.c
@@ -41,6 +41,7 @@
#include "utils/guc.h"
#include "utils/int8.h"
#include "utils/numeric.h"
+#include "utils/pg_lsn.h"
#include "utils/sortsupport.h"
/* ----------
@@ -472,6 +473,7 @@ static void apply_typmod(NumericVar *var, int32 typmod);
static bool numericvar_to_int32(const NumericVar *var, int32 *result);
static bool numericvar_to_int64(const NumericVar *var, int64 *result);
static void int64_to_numericvar(int64 val, NumericVar *var);
+static bool numericvar_to_uint64(const NumericVar *var, uint64 *result);
#ifdef HAVE_INT128
static bool numericvar_to_int128(const NumericVar *var, int128 *result);
static void int128_to_numericvar(int128 val, NumericVar *var);
@@ -3688,6 +3690,31 @@ numeric_float4(PG_FUNCTION_ARGS)
}
+Datum
+numeric_pg_lsn(PG_FUNCTION_ARGS)
+{
+ Numeric num = PG_GETARG_NUMERIC(0);
+ NumericVar x;
+ XLogRecPtr 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")));
+
+ /* Convert to variable format and thence to pg_lsn */
+ init_var_from_num(num, &x);
+
+ if (!numericvar_to_uint64(&x, (uint64 *) &result))
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("pg_lsn out of range")));
+
+ PG_RETURN_LSN(result);
+}
+
+
/* ----------------------------------------------------------------------
*
* Aggregate functions
@@ -6739,6 +6766,78 @@ int64_to_numericvar(int64 val, NumericVar *var)
var->weight = ndigits - 1;
}
+/*
+ * Convert numeric to uint64, rounding if needed.
+ *
+ * If overflow, return false (no error is raised). Return true if okay.
+ */
+static bool
+numericvar_to_uint64(const NumericVar *var, uint64 *result)
+{
+ NumericDigit *digits;
+ int ndigits;
+ int weight;
+ int i;
+ uint64 val;
+ NumericVar rounded;
+
+ /* Round to nearest integer */
+ init_var(&rounded);
+ set_var_from_var(var, &rounded);
+ round_var(&rounded, 0);
+
+ /* Check for zero input */
+ strip_var(&rounded);
+ ndigits = rounded.ndigits;
+ if (ndigits == 0)
+ {
+ *result = 0;
+ free_var(&rounded);
+ return true;
+ }
+
+ /* Check for negative input */
+ if (rounded.sign == NUMERIC_NEG)
+ {
+ free_var(&rounded);
+ return false;
+ }
+
+ /*
+ * For input like 10000000000, we must treat stripped digits as real. So
+ * the loop assumes there are weight+1 digits before the decimal point.
+ */
+ weight = rounded.weight;
+ Assert(weight >= 0 && ndigits <= weight + 1);
+
+ /* Construct the result */
+ digits = rounded.digits;
+ val = digits[0];
+ for (i = 1; i <= weight; i++)
+ {
+ if (unlikely(pg_mul_u64_overflow(val, NBASE, &val)))
+ {
+ free_var(&rounded);
+ return false;
+ }
+
+ if (i < ndigits)
+ {
+ if (unlikely(pg_add_u64_overflow(val, digits[i], &val)))
+ {
+ free_var(&rounded);
+ return false;
+ }
+ }
+ }
+
+ free_var(&rounded);
+
+ *result = val;
+
+ return true;
+}
+
#ifdef HAVE_INT128
/*
* Convert numeric to int128, rounding if needed.
diff --git a/src/backend/utils/adt/pg_lsn.c b/src/backend/utils/adt/pg_lsn.c
index d9754a7778..d3e289964c 100644
--- a/src/backend/utils/adt/pg_lsn.c
+++ b/src/backend/utils/adt/pg_lsn.c
@@ -16,6 +16,7 @@
#include "funcapi.h"
#include "libpq/pqformat.h"
#include "utils/builtins.h"
+#include "utils/numeric.h"
#include "utils/pg_lsn.h"
#define MAXPG_LSNLEN 17
@@ -248,3 +249,59 @@ pg_lsn_mi(PG_FUNCTION_ARGS)
return result;
}
+
+/*
+ * Add the number of bytes to pg_lsn, giving a new pg_lsn.
+ * Must handle both positive and negative numbers of bytes.
+ */
+Datum
+pg_lsn_pli(PG_FUNCTION_ARGS)
+{
+ XLogRecPtr lsn = PG_GETARG_LSN(0);
+ Datum nbytes = PG_GETARG_DATUM(1);
+ Datum num;
+ Datum res;
+ char buf[256];
+
+ /* Convert to numeric */
+ snprintf(buf, sizeof(buf), UINT64_FORMAT, lsn);
+ num = DirectFunctionCall3(numeric_in,
+ CStringGetDatum(buf),
+ ObjectIdGetDatum(0),
+ Int32GetDatum(-1));
+
+ /* Add two numerics */
+ res = DirectFunctionCall2(numeric_add,
+ NumericGetDatum(num), nbytes);
+
+ /* Convert to pg_lsn */
+ return DirectFunctionCall1(numeric_pg_lsn, res);
+}
+
+/*
+ * Substract the number of bytes from pg_lsn, giving a new pg_lsn.
+ * Must handle both positive and negative numbers of bytes.
+ */
+Datum
+pg_lsn_mii(PG_FUNCTION_ARGS)
+{
+ XLogRecPtr lsn = PG_GETARG_LSN(0);
+ Datum nbytes = PG_GETARG_DATUM(1);
+ Datum num;
+ Datum res;
+ char buf[256];
+
+ /* Convert to numeric */
+ snprintf(buf, sizeof(buf), UINT64_FORMAT, lsn);
+ num = DirectFunctionCall3(numeric_in,
+ CStringGetDatum(buf),
+ ObjectIdGetDatum(0),
+ Int32GetDatum(-1));
+
+ /* Add two numerics */
+ res = DirectFunctionCall2(numeric_sub,
+ NumericGetDatum(num), nbytes);
+
+ /* Convert to pg_lsn */
+ return DirectFunctionCall1(numeric_pg_lsn, res);
+}
diff --git a/src/include/catalog/pg_operator.dat b/src/include/catalog/pg_operator.dat
index 00ada7e48f..15dfa1497a 100644
--- a/src/include/catalog/pg_operator.dat
+++ b/src/include/catalog/pg_operator.dat
@@ -2909,6 +2909,17 @@
{ oid => '3228', descr => 'minus',
oprname => '-', oprleft => 'pg_lsn', oprright => 'pg_lsn',
oprresult => 'numeric', oprcode => 'pg_lsn_mi' },
+{ oid => '5025', descr => 'add',
+ oprname => '+', oprleft => 'pg_lsn', oprright => 'numeric',
+ oprresult => 'pg_lsn', oprcom => '+(numeric,pg_lsn)',
+ oprcode => 'pg_lsn_pli' },
+{ oid => '5026', descr => 'add',
+ oprname => '+', oprleft => 'numeric', oprright => 'pg_lsn',
+ oprresult => 'pg_lsn', oprcom => '+(pg_lsn,numeric)',
+ oprcode => 'numeric_pl_pg_lsn' },
+{ oid => '5027', descr => 'subtract',
+ oprname => '-', oprleft => 'pg_lsn', oprright => 'numeric',
+ oprresult => 'pg_lsn', oprcode => 'pg_lsn_mii' },
# enum operators
{ oid => '3516', descr => 'equal',
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 4bce3ad8de..0eb94d92e3 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -4398,6 +4398,9 @@
{ oid => '1783', descr => 'convert numeric to int2',
proname => 'int2', prorettype => 'int2', proargtypes => 'numeric',
prosrc => 'numeric_int2' },
+{ oid => '6103', descr => 'convert numeric to pg_lsn',
+ proname => 'pg_lsn', prorettype => 'pg_lsn', proargtypes => 'numeric',
+ prosrc => 'numeric_pg_lsn' },
{ oid => '3556', descr => 'convert jsonb to boolean',
proname => 'bool', prorettype => 'bool', proargtypes => 'jsonb',
@@ -8578,6 +8581,15 @@
{ oid => '4188', descr => 'smaller of two',
proname => 'pg_lsn_smaller', prorettype => 'pg_lsn',
proargtypes => 'pg_lsn pg_lsn', prosrc => 'pg_lsn_smaller' },
+{ oid => '5022',
+ proname => 'pg_lsn_pli', prorettype => 'pg_lsn',
+ proargtypes => 'pg_lsn numeric', prosrc => 'pg_lsn_pli' },
+{ oid => '5023',
+ proname => 'numeric_pl_pg_lsn', prolang => 'sql', prorettype => 'pg_lsn',
+ proargtypes => 'numeric pg_lsn', prosrc => 'select $2 + $1' },
+{ oid => '5024',
+ proname => 'pg_lsn_mii', prorettype => 'pg_lsn',
+ proargtypes => 'pg_lsn numeric', prosrc => 'pg_lsn_mii' },
# enum related procs
{ oid => '3504', descr => 'I/O',
diff --git a/src/test/regress/expected/numeric.out b/src/test/regress/expected/numeric.out
index c7fe63d037..978d38b729 100644
--- a/src/test/regress/expected/numeric.out
+++ b/src/test/regress/expected/numeric.out
@@ -2315,3 +2315,30 @@ FROM (VALUES (0::numeric, 0::numeric),
SELECT lcm(9999 * (10::numeric)^131068 + (10::numeric^131068 - 1), 2); -- overflow
ERROR: value overflows numeric format
+--
+-- Tests for pg_lsn()
+--
+SELECT pg_lsn(23783416::numeric);
+ pg_lsn
+-----------
+ 0/16AE7F8
+(1 row)
+
+SELECT pg_lsn(0::numeric);
+ pg_lsn
+--------
+ 0/0
+(1 row)
+
+SELECT pg_lsn(18446744073709551615::numeric);
+ pg_lsn
+-------------------
+ FFFFFFFF/FFFFFFFF
+(1 row)
+
+SELECT pg_lsn(-1::numeric);
+ERROR: pg_lsn out of range
+SELECT pg_lsn(18446744073709551616::numeric);
+ERROR: pg_lsn out of range
+SELECT pg_lsn('NaN'::numeric);
+ERROR: cannot convert NaN to pg_lsn
diff --git a/src/test/regress/expected/pg_lsn.out b/src/test/regress/expected/pg_lsn.out
index 64d41dfdad..7eb6a387b1 100644
--- a/src/test/regress/expected/pg_lsn.out
+++ b/src/test/regress/expected/pg_lsn.out
@@ -71,6 +71,52 @@ SELECT '0/16AE7F8'::pg_lsn - '0/16AE7F7'::pg_lsn;
1
(1 row)
+SELECT '0/16AE7F7'::pg_lsn + 16::numeric;
+ ?column?
+-----------
+ 0/16AE807
+(1 row)
+
+SELECT 16::numeric + '0/16AE7F7'::pg_lsn;
+ ?column?
+-----------
+ 0/16AE807
+(1 row)
+
+SELECT '0/16AE7F7'::pg_lsn - 16::numeric;
+ ?column?
+-----------
+ 0/16AE7E7
+(1 row)
+
+SELECT 'FFFFFFFF/FFFFFFFE'::pg_lsn + 1::numeric;
+ ?column?
+-------------------
+ FFFFFFFF/FFFFFFFF
+(1 row)
+
+SELECT 'FFFFFFFF/FFFFFFFE'::pg_lsn + 2::numeric; -- out of range error
+ERROR: pg_lsn out of range
+SELECT '0/1'::pg_lsn - 1::numeric;
+ ?column?
+----------
+ 0/0
+(1 row)
+
+SELECT '0/1'::pg_lsn - 2::numeric; -- out of range error
+ERROR: pg_lsn out of range
+SELECT '0/0'::pg_lsn + ('FFFFFFFF/FFFFFFFF'::pg_lsn - '0/0'::pg_lsn);
+ ?column?
+-------------------
+ FFFFFFFF/FFFFFFFF
+(1 row)
+
+SELECT 'FFFFFFFF/FFFFFFFF'::pg_lsn - ('FFFFFFFF/FFFFFFFF'::pg_lsn - '0/0'::pg_lsn);
+ ?column?
+----------
+ 0/0
+(1 row)
+
-- Check btree and hash opclasses
EXPLAIN (COSTS OFF)
SELECT DISTINCT (i || '/' || j)::pg_lsn f
diff --git a/src/test/regress/sql/numeric.sql b/src/test/regress/sql/numeric.sql
index 41475a9a24..fdfe6fa3c7 100644
--- a/src/test/regress/sql/numeric.sql
+++ b/src/test/regress/sql/numeric.sql
@@ -1111,3 +1111,13 @@ FROM (VALUES (0::numeric, 0::numeric),
(4232.820::numeric, 132.72000::numeric)) AS v(a, b);
SELECT lcm(9999 * (10::numeric)^131068 + (10::numeric^131068 - 1), 2); -- overflow
+
+--
+-- Tests for pg_lsn()
+--
+SELECT pg_lsn(23783416::numeric);
+SELECT pg_lsn(0::numeric);
+SELECT pg_lsn(18446744073709551615::numeric);
+SELECT pg_lsn(-1::numeric);
+SELECT pg_lsn(18446744073709551616::numeric);
+SELECT pg_lsn('NaN'::numeric);
diff --git a/src/test/regress/sql/pg_lsn.sql b/src/test/regress/sql/pg_lsn.sql
index 2c143c82ff..74d9fb4e6a 100644
--- a/src/test/regress/sql/pg_lsn.sql
+++ b/src/test/regress/sql/pg_lsn.sql
@@ -27,6 +27,15 @@ SELECT '0/16AE7F7' < '0/16AE7F8'::pg_lsn;
SELECT '0/16AE7F8' > pg_lsn '0/16AE7F7';
SELECT '0/16AE7F7'::pg_lsn - '0/16AE7F8'::pg_lsn;
SELECT '0/16AE7F8'::pg_lsn - '0/16AE7F7'::pg_lsn;
+SELECT '0/16AE7F7'::pg_lsn + 16::numeric;
+SELECT 16::numeric + '0/16AE7F7'::pg_lsn;
+SELECT '0/16AE7F7'::pg_lsn - 16::numeric;
+SELECT 'FFFFFFFF/FFFFFFFE'::pg_lsn + 1::numeric;
+SELECT 'FFFFFFFF/FFFFFFFE'::pg_lsn + 2::numeric; -- out of range error
+SELECT '0/1'::pg_lsn - 1::numeric;
+SELECT '0/1'::pg_lsn - 2::numeric; -- out of range error
+SELECT '0/0'::pg_lsn + ('FFFFFFFF/FFFFFFFF'::pg_lsn - '0/0'::pg_lsn);
+SELECT 'FFFFFFFF/FFFFFFFF'::pg_lsn - ('FFFFFFFF/FFFFFFFF'::pg_lsn - '0/0'::pg_lsn);
-- Check btree and hash opclasses
EXPLAIN (COSTS OFF)
At Thu, 7 May 2020 13:17:01 +0900, Fujii Masao <masao.fujii@oss.nttdata.com> wrote in
On 2020/05/07 11:21, Kyotaro Horiguchi wrote:
+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);
Mmm.
+ /* 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?
The function is called while executing an expression, so "NaN cannot
be used in this expression" or something like that would work.
+ 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?
It's just a waste of stack depth by over 200 bytes. I doesn't lead to
an actual problem but it is evidently useless.
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.
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.
Datum
pg_lsn_pli(..)
{
XLogRecPtr lsn = PG_GETARG_LSN(0);
Datum num_nbytes = PG_GETARG_DATUM(1);
Datum u64_nbytes =
DatumGetInt64(DirectFunctionCall1(numeric_pg_lsn, num_nbytes));
XLogRecPtr result;
if (pg_add_u64_overflow(lsn, u64_nbytes, &result))
elog(ERROR, "result out of range");
PG_RETURN_LSN(result);
}
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
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
On 2020/05/08 10:00, Kyotaro Horiguchi wrote:
At Thu, 7 May 2020 13:17:01 +0900, Fujii Masao <masao.fujii@oss.nttdata.com> wrote in
On 2020/05/07 11:21, Kyotaro Horiguchi wrote:
+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);Mmm.
+ /* 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?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
+ 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?
It's just a waste of stack depth by over 200 bytes. I doesn't lead to
an actual problem but it is evidently useless.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.
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...
Datum
pg_lsn_pli(..)
{
XLogRecPtr lsn = PG_GETARG_LSN(0);
Datum num_nbytes = PG_GETARG_DATUM(1);
Datum u64_nbytes =
DatumGetInt64(DirectFunctionCall1(numeric_pg_lsn, num_nbytes));
XLogRecPtr result;if (pg_add_u64_overflow(lsn, u64_nbytes, &result))
elog(ERROR, "result out of range");PG_RETURN_LSN(result);
}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.
Regards,
--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
At Fri, 8 May 2020 11:31:42 +0900, Fujii Masao <masao.fujii@oss.nttdata.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.
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 expressionCould 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.
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
On 2020/05/08 12:10, Kyotaro Horiguchi wrote:
At Fri, 8 May 2020 11:31:42 +0900, Fujii Masao <masao.fujii@oss.nttdata.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_lsnThey 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 expressionCould 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
Attachments:
pg_lsn_operators_v4.patchtext/plain; charset=UTF-8; name=pg_lsn_operators_v4.patch; x-mac-creator=0; x-mac-type=0Download
diff --git a/doc/src/sgml/datatype.sgml b/doc/src/sgml/datatype.sgml
index a8d0780387..a86b794ce0 100644
--- a/doc/src/sgml/datatype.sgml
+++ b/doc/src/sgml/datatype.sgml
@@ -4823,7 +4823,13 @@ SELECT * FROM pg_attribute
standard comparison operators, like <literal>=</literal> and
<literal>></literal>. Two LSNs can be subtracted using the
<literal>-</literal> operator; the result is the number of bytes separating
- those write-ahead log locations.
+ those write-ahead log locations. Also the number of bytes can be
+ added into and subtracted 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>.
</para>
</sect1>
diff --git a/src/backend/utils/adt/numeric.c b/src/backend/utils/adt/numeric.c
index 9986132b45..94593c7f63 100644
--- a/src/backend/utils/adt/numeric.c
+++ b/src/backend/utils/adt/numeric.c
@@ -41,6 +41,7 @@
#include "utils/guc.h"
#include "utils/int8.h"
#include "utils/numeric.h"
+#include "utils/pg_lsn.h"
#include "utils/sortsupport.h"
/* ----------
@@ -472,6 +473,7 @@ static void apply_typmod(NumericVar *var, int32 typmod);
static bool numericvar_to_int32(const NumericVar *var, int32 *result);
static bool numericvar_to_int64(const NumericVar *var, int64 *result);
static void int64_to_numericvar(int64 val, NumericVar *var);
+static bool numericvar_to_uint64(const NumericVar *var, uint64 *result);
#ifdef HAVE_INT128
static bool numericvar_to_int128(const NumericVar *var, int128 *result);
static void int128_to_numericvar(int128 val, NumericVar *var);
@@ -3688,6 +3690,30 @@ numeric_float4(PG_FUNCTION_ARGS)
}
+Datum
+numeric_pg_lsn(PG_FUNCTION_ARGS)
+{
+ Numeric num = PG_GETARG_NUMERIC(0);
+ NumericVar x;
+ XLogRecPtr result;
+
+ if (NUMERIC_IS_NAN(num))
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("cannot convert NaN to pg_lsn")));
+
+ /* Convert to variable format and thence to pg_lsn */
+ init_var_from_num(num, &x);
+
+ if (!numericvar_to_uint64(&x, (uint64 *) &result))
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("pg_lsn out of range")));
+
+ PG_RETURN_LSN(result);
+}
+
+
/* ----------------------------------------------------------------------
*
* Aggregate functions
@@ -6739,6 +6765,78 @@ int64_to_numericvar(int64 val, NumericVar *var)
var->weight = ndigits - 1;
}
+/*
+ * Convert numeric to uint64, rounding if needed.
+ *
+ * If overflow, return false (no error is raised). Return true if okay.
+ */
+static bool
+numericvar_to_uint64(const NumericVar *var, uint64 *result)
+{
+ NumericDigit *digits;
+ int ndigits;
+ int weight;
+ int i;
+ uint64 val;
+ NumericVar rounded;
+
+ /* Round to nearest integer */
+ init_var(&rounded);
+ set_var_from_var(var, &rounded);
+ round_var(&rounded, 0);
+
+ /* Check for zero input */
+ strip_var(&rounded);
+ ndigits = rounded.ndigits;
+ if (ndigits == 0)
+ {
+ *result = 0;
+ free_var(&rounded);
+ return true;
+ }
+
+ /* Check for negative input */
+ if (rounded.sign == NUMERIC_NEG)
+ {
+ free_var(&rounded);
+ return false;
+ }
+
+ /*
+ * For input like 10000000000, we must treat stripped digits as real. So
+ * the loop assumes there are weight+1 digits before the decimal point.
+ */
+ weight = rounded.weight;
+ Assert(weight >= 0 && ndigits <= weight + 1);
+
+ /* Construct the result */
+ digits = rounded.digits;
+ val = digits[0];
+ for (i = 1; i <= weight; i++)
+ {
+ if (unlikely(pg_mul_u64_overflow(val, NBASE, &val)))
+ {
+ free_var(&rounded);
+ return false;
+ }
+
+ if (i < ndigits)
+ {
+ if (unlikely(pg_add_u64_overflow(val, digits[i], &val)))
+ {
+ free_var(&rounded);
+ return false;
+ }
+ }
+ }
+
+ free_var(&rounded);
+
+ *result = val;
+
+ return true;
+}
+
#ifdef HAVE_INT128
/*
* Convert numeric to int128, rounding if needed.
diff --git a/src/backend/utils/adt/pg_lsn.c b/src/backend/utils/adt/pg_lsn.c
index d9754a7778..ad0a7bd869 100644
--- a/src/backend/utils/adt/pg_lsn.c
+++ b/src/backend/utils/adt/pg_lsn.c
@@ -16,6 +16,7 @@
#include "funcapi.h"
#include "libpq/pqformat.h"
#include "utils/builtins.h"
+#include "utils/numeric.h"
#include "utils/pg_lsn.h"
#define MAXPG_LSNLEN 17
@@ -248,3 +249,71 @@ pg_lsn_mi(PG_FUNCTION_ARGS)
return result;
}
+
+/*
+ * Add the number of bytes to pg_lsn, giving a new pg_lsn.
+ * Must handle both positive and negative numbers of bytes.
+ */
+Datum
+pg_lsn_pli(PG_FUNCTION_ARGS)
+{
+ XLogRecPtr lsn = PG_GETARG_LSN(0);
+ Numeric nbytes = PG_GETARG_NUMERIC(1);
+ Datum num;
+ Datum res;
+ char buf[32];
+
+ if (numeric_is_nan(nbytes))
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("cannot add NaN to pg_lsn")));
+
+ /* Convert to numeric */
+ snprintf(buf, sizeof(buf), UINT64_FORMAT, lsn);
+ num = DirectFunctionCall3(numeric_in,
+ CStringGetDatum(buf),
+ ObjectIdGetDatum(0),
+ Int32GetDatum(-1));
+
+ /* Add two numerics */
+ res = DirectFunctionCall2(numeric_add,
+ NumericGetDatum(num),
+ NumericGetDatum(nbytes));
+
+ /* Convert to pg_lsn */
+ return DirectFunctionCall1(numeric_pg_lsn, res);
+}
+
+/*
+ * Subtract the number of bytes from pg_lsn, giving a new pg_lsn.
+ * Must handle both positive and negative numbers of bytes.
+ */
+Datum
+pg_lsn_mii(PG_FUNCTION_ARGS)
+{
+ XLogRecPtr lsn = PG_GETARG_LSN(0);
+ Numeric nbytes = PG_GETARG_NUMERIC(1);
+ Datum num;
+ Datum res;
+ char buf[32];
+
+ if (numeric_is_nan(nbytes))
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("cannot subtract NaN from pg_lsn")));
+
+ /* Convert to numeric */
+ snprintf(buf, sizeof(buf), UINT64_FORMAT, lsn);
+ num = DirectFunctionCall3(numeric_in,
+ CStringGetDatum(buf),
+ ObjectIdGetDatum(0),
+ Int32GetDatum(-1));
+
+ /* Subtract two numerics */
+ res = DirectFunctionCall2(numeric_sub,
+ NumericGetDatum(num),
+ NumericGetDatum(nbytes));
+
+ /* Convert to pg_lsn */
+ return DirectFunctionCall1(numeric_pg_lsn, res);
+}
diff --git a/src/include/catalog/pg_operator.dat b/src/include/catalog/pg_operator.dat
index 00ada7e48f..15dfa1497a 100644
--- a/src/include/catalog/pg_operator.dat
+++ b/src/include/catalog/pg_operator.dat
@@ -2909,6 +2909,17 @@
{ oid => '3228', descr => 'minus',
oprname => '-', oprleft => 'pg_lsn', oprright => 'pg_lsn',
oprresult => 'numeric', oprcode => 'pg_lsn_mi' },
+{ oid => '5025', descr => 'add',
+ oprname => '+', oprleft => 'pg_lsn', oprright => 'numeric',
+ oprresult => 'pg_lsn', oprcom => '+(numeric,pg_lsn)',
+ oprcode => 'pg_lsn_pli' },
+{ oid => '5026', descr => 'add',
+ oprname => '+', oprleft => 'numeric', oprright => 'pg_lsn',
+ oprresult => 'pg_lsn', oprcom => '+(pg_lsn,numeric)',
+ oprcode => 'numeric_pl_pg_lsn' },
+{ oid => '5027', descr => 'subtract',
+ oprname => '-', oprleft => 'pg_lsn', oprright => 'numeric',
+ oprresult => 'pg_lsn', oprcode => 'pg_lsn_mii' },
# enum operators
{ oid => '3516', descr => 'equal',
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 4bce3ad8de..0eb94d92e3 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -4398,6 +4398,9 @@
{ oid => '1783', descr => 'convert numeric to int2',
proname => 'int2', prorettype => 'int2', proargtypes => 'numeric',
prosrc => 'numeric_int2' },
+{ oid => '6103', descr => 'convert numeric to pg_lsn',
+ proname => 'pg_lsn', prorettype => 'pg_lsn', proargtypes => 'numeric',
+ prosrc => 'numeric_pg_lsn' },
{ oid => '3556', descr => 'convert jsonb to boolean',
proname => 'bool', prorettype => 'bool', proargtypes => 'jsonb',
@@ -8578,6 +8581,15 @@
{ oid => '4188', descr => 'smaller of two',
proname => 'pg_lsn_smaller', prorettype => 'pg_lsn',
proargtypes => 'pg_lsn pg_lsn', prosrc => 'pg_lsn_smaller' },
+{ oid => '5022',
+ proname => 'pg_lsn_pli', prorettype => 'pg_lsn',
+ proargtypes => 'pg_lsn numeric', prosrc => 'pg_lsn_pli' },
+{ oid => '5023',
+ proname => 'numeric_pl_pg_lsn', prolang => 'sql', prorettype => 'pg_lsn',
+ proargtypes => 'numeric pg_lsn', prosrc => 'select $2 + $1' },
+{ oid => '5024',
+ proname => 'pg_lsn_mii', prorettype => 'pg_lsn',
+ proargtypes => 'pg_lsn numeric', prosrc => 'pg_lsn_mii' },
# enum related procs
{ oid => '3504', descr => 'I/O',
diff --git a/src/test/regress/expected/numeric.out b/src/test/regress/expected/numeric.out
index c7fe63d037..978d38b729 100644
--- a/src/test/regress/expected/numeric.out
+++ b/src/test/regress/expected/numeric.out
@@ -2315,3 +2315,30 @@ FROM (VALUES (0::numeric, 0::numeric),
SELECT lcm(9999 * (10::numeric)^131068 + (10::numeric^131068 - 1), 2); -- overflow
ERROR: value overflows numeric format
+--
+-- Tests for pg_lsn()
+--
+SELECT pg_lsn(23783416::numeric);
+ pg_lsn
+-----------
+ 0/16AE7F8
+(1 row)
+
+SELECT pg_lsn(0::numeric);
+ pg_lsn
+--------
+ 0/0
+(1 row)
+
+SELECT pg_lsn(18446744073709551615::numeric);
+ pg_lsn
+-------------------
+ FFFFFFFF/FFFFFFFF
+(1 row)
+
+SELECT pg_lsn(-1::numeric);
+ERROR: pg_lsn out of range
+SELECT pg_lsn(18446744073709551616::numeric);
+ERROR: pg_lsn out of range
+SELECT pg_lsn('NaN'::numeric);
+ERROR: cannot convert NaN to pg_lsn
diff --git a/src/test/regress/expected/pg_lsn.out b/src/test/regress/expected/pg_lsn.out
index 64d41dfdad..99a748a6a7 100644
--- a/src/test/regress/expected/pg_lsn.out
+++ b/src/test/regress/expected/pg_lsn.out
@@ -71,6 +71,56 @@ SELECT '0/16AE7F8'::pg_lsn - '0/16AE7F7'::pg_lsn;
1
(1 row)
+SELECT '0/16AE7F7'::pg_lsn + 16::numeric;
+ ?column?
+-----------
+ 0/16AE807
+(1 row)
+
+SELECT 16::numeric + '0/16AE7F7'::pg_lsn;
+ ?column?
+-----------
+ 0/16AE807
+(1 row)
+
+SELECT '0/16AE7F7'::pg_lsn - 16::numeric;
+ ?column?
+-----------
+ 0/16AE7E7
+(1 row)
+
+SELECT 'FFFFFFFF/FFFFFFFE'::pg_lsn + 1::numeric;
+ ?column?
+-------------------
+ FFFFFFFF/FFFFFFFF
+(1 row)
+
+SELECT 'FFFFFFFF/FFFFFFFE'::pg_lsn + 2::numeric; -- out of range error
+ERROR: pg_lsn out of range
+SELECT '0/1'::pg_lsn - 1::numeric;
+ ?column?
+----------
+ 0/0
+(1 row)
+
+SELECT '0/1'::pg_lsn - 2::numeric; -- out of range error
+ERROR: pg_lsn out of range
+SELECT '0/0'::pg_lsn + ('FFFFFFFF/FFFFFFFF'::pg_lsn - '0/0'::pg_lsn);
+ ?column?
+-------------------
+ FFFFFFFF/FFFFFFFF
+(1 row)
+
+SELECT 'FFFFFFFF/FFFFFFFF'::pg_lsn - ('FFFFFFFF/FFFFFFFF'::pg_lsn - '0/0'::pg_lsn);
+ ?column?
+----------
+ 0/0
+(1 row)
+
+SELECT '0/16AE7F7'::pg_lsn + 'NaN'::numeric;
+ERROR: cannot add NaN to pg_lsn
+SELECT '0/16AE7F7'::pg_lsn - 'NaN'::numeric;
+ERROR: cannot subtract NaN from pg_lsn
-- Check btree and hash opclasses
EXPLAIN (COSTS OFF)
SELECT DISTINCT (i || '/' || j)::pg_lsn f
diff --git a/src/test/regress/sql/numeric.sql b/src/test/regress/sql/numeric.sql
index 41475a9a24..fdfe6fa3c7 100644
--- a/src/test/regress/sql/numeric.sql
+++ b/src/test/regress/sql/numeric.sql
@@ -1111,3 +1111,13 @@ FROM (VALUES (0::numeric, 0::numeric),
(4232.820::numeric, 132.72000::numeric)) AS v(a, b);
SELECT lcm(9999 * (10::numeric)^131068 + (10::numeric^131068 - 1), 2); -- overflow
+
+--
+-- Tests for pg_lsn()
+--
+SELECT pg_lsn(23783416::numeric);
+SELECT pg_lsn(0::numeric);
+SELECT pg_lsn(18446744073709551615::numeric);
+SELECT pg_lsn(-1::numeric);
+SELECT pg_lsn(18446744073709551616::numeric);
+SELECT pg_lsn('NaN'::numeric);
diff --git a/src/test/regress/sql/pg_lsn.sql b/src/test/regress/sql/pg_lsn.sql
index 2c143c82ff..615368ba96 100644
--- a/src/test/regress/sql/pg_lsn.sql
+++ b/src/test/regress/sql/pg_lsn.sql
@@ -27,6 +27,17 @@ SELECT '0/16AE7F7' < '0/16AE7F8'::pg_lsn;
SELECT '0/16AE7F8' > pg_lsn '0/16AE7F7';
SELECT '0/16AE7F7'::pg_lsn - '0/16AE7F8'::pg_lsn;
SELECT '0/16AE7F8'::pg_lsn - '0/16AE7F7'::pg_lsn;
+SELECT '0/16AE7F7'::pg_lsn + 16::numeric;
+SELECT 16::numeric + '0/16AE7F7'::pg_lsn;
+SELECT '0/16AE7F7'::pg_lsn - 16::numeric;
+SELECT 'FFFFFFFF/FFFFFFFE'::pg_lsn + 1::numeric;
+SELECT 'FFFFFFFF/FFFFFFFE'::pg_lsn + 2::numeric; -- out of range error
+SELECT '0/1'::pg_lsn - 1::numeric;
+SELECT '0/1'::pg_lsn - 2::numeric; -- out of range error
+SELECT '0/0'::pg_lsn + ('FFFFFFFF/FFFFFFFF'::pg_lsn - '0/0'::pg_lsn);
+SELECT 'FFFFFFFF/FFFFFFFF'::pg_lsn - ('FFFFFFFF/FFFFFFFF'::pg_lsn - '0/0'::pg_lsn);
+SELECT '0/16AE7F7'::pg_lsn + 'NaN'::numeric;
+SELECT '0/16AE7F7'::pg_lsn - 'NaN'::numeric;
-- Check btree and hash opclasses
EXPLAIN (COSTS OFF)
At Sat, 9 May 2020 23:40:15 +0900, Fujii Masao <masao.fujii@oss.nttdata.com> wrote in
On 2020/05/08 12:10, Kyotaro Horiguchi wrote:
At Fri, 8 May 2020 11:31:42 +0900, Fujii Masao
<masao.fujii@oss.nttdata.com> wrote inYou 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_lsnThey 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.
It's fine with me.
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.
Thank you very much! The patch looks good to me.
regard.
--
Kyotaro Horiguchi
NTT Open Source Software Center
Hi,
The patch looks fine to me, however there is one hunk failing for the test case, so it needs to be rebased.
The new status of this patch is: Waiting on Author
On 2020/06/30 1:03, Asif Rehman wrote:
Hi,
The patch looks fine to me, however there is one hunk failing for the test case, so it needs to be rebased.
Thanks for the check! Attached is the updated version of the patch.
The new status of this patch is: Waiting on Author
I will change the status back to Needs Review.
Regards,
--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
Attachments:
pg_lsn_operators_v5.patchtext/plain; charset=UTF-8; name=pg_lsn_operators_v5.patch; x-mac-creator=0; x-mac-type=0Download
diff --git a/doc/src/sgml/datatype.sgml b/doc/src/sgml/datatype.sgml
index 49fb19ff91..7027758d28 100644
--- a/doc/src/sgml/datatype.sgml
+++ b/doc/src/sgml/datatype.sgml
@@ -4801,7 +4801,13 @@ SELECT * FROM pg_attribute
standard comparison operators, like <literal>=</literal> and
<literal>></literal>. Two LSNs can be subtracted using the
<literal>-</literal> operator; the result is the number of bytes separating
- those write-ahead log locations.
+ those write-ahead log locations. Also the number of bytes can be
+ added into and subtracted 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>.
</para>
</sect1>
diff --git a/src/backend/utils/adt/numeric.c b/src/backend/utils/adt/numeric.c
index 5f23f2afac..1773fa292e 100644
--- a/src/backend/utils/adt/numeric.c
+++ b/src/backend/utils/adt/numeric.c
@@ -41,6 +41,7 @@
#include "utils/guc.h"
#include "utils/int8.h"
#include "utils/numeric.h"
+#include "utils/pg_lsn.h"
#include "utils/sortsupport.h"
/* ----------
@@ -472,6 +473,7 @@ static void apply_typmod(NumericVar *var, int32 typmod);
static bool numericvar_to_int32(const NumericVar *var, int32 *result);
static bool numericvar_to_int64(const NumericVar *var, int64 *result);
static void int64_to_numericvar(int64 val, NumericVar *var);
+static bool numericvar_to_uint64(const NumericVar *var, uint64 *result);
#ifdef HAVE_INT128
static bool numericvar_to_int128(const NumericVar *var, int128 *result);
static void int128_to_numericvar(int128 val, NumericVar *var);
@@ -3692,6 +3694,30 @@ numeric_float4(PG_FUNCTION_ARGS)
}
+Datum
+numeric_pg_lsn(PG_FUNCTION_ARGS)
+{
+ Numeric num = PG_GETARG_NUMERIC(0);
+ NumericVar x;
+ XLogRecPtr result;
+
+ if (NUMERIC_IS_NAN(num))
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("cannot convert NaN to pg_lsn")));
+
+ /* Convert to variable format and thence to pg_lsn */
+ init_var_from_num(num, &x);
+
+ if (!numericvar_to_uint64(&x, (uint64 *) &result))
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("pg_lsn out of range")));
+
+ PG_RETURN_LSN(result);
+}
+
+
/* ----------------------------------------------------------------------
*
* Aggregate functions
@@ -6742,6 +6768,78 @@ int64_to_numericvar(int64 val, NumericVar *var)
var->weight = ndigits - 1;
}
+/*
+ * Convert numeric to uint64, rounding if needed.
+ *
+ * If overflow, return false (no error is raised). Return true if okay.
+ */
+static bool
+numericvar_to_uint64(const NumericVar *var, uint64 *result)
+{
+ NumericDigit *digits;
+ int ndigits;
+ int weight;
+ int i;
+ uint64 val;
+ NumericVar rounded;
+
+ /* Round to nearest integer */
+ init_var(&rounded);
+ set_var_from_var(var, &rounded);
+ round_var(&rounded, 0);
+
+ /* Check for zero input */
+ strip_var(&rounded);
+ ndigits = rounded.ndigits;
+ if (ndigits == 0)
+ {
+ *result = 0;
+ free_var(&rounded);
+ return true;
+ }
+
+ /* Check for negative input */
+ if (rounded.sign == NUMERIC_NEG)
+ {
+ free_var(&rounded);
+ return false;
+ }
+
+ /*
+ * For input like 10000000000, we must treat stripped digits as real. So
+ * the loop assumes there are weight+1 digits before the decimal point.
+ */
+ weight = rounded.weight;
+ Assert(weight >= 0 && ndigits <= weight + 1);
+
+ /* Construct the result */
+ digits = rounded.digits;
+ val = digits[0];
+ for (i = 1; i <= weight; i++)
+ {
+ if (unlikely(pg_mul_u64_overflow(val, NBASE, &val)))
+ {
+ free_var(&rounded);
+ return false;
+ }
+
+ if (i < ndigits)
+ {
+ if (unlikely(pg_add_u64_overflow(val, digits[i], &val)))
+ {
+ free_var(&rounded);
+ return false;
+ }
+ }
+ }
+
+ free_var(&rounded);
+
+ *result = val;
+
+ return true;
+}
+
#ifdef HAVE_INT128
/*
* Convert numeric to int128, rounding if needed.
diff --git a/src/backend/utils/adt/pg_lsn.c b/src/backend/utils/adt/pg_lsn.c
index d9754a7778..ad0a7bd869 100644
--- a/src/backend/utils/adt/pg_lsn.c
+++ b/src/backend/utils/adt/pg_lsn.c
@@ -16,6 +16,7 @@
#include "funcapi.h"
#include "libpq/pqformat.h"
#include "utils/builtins.h"
+#include "utils/numeric.h"
#include "utils/pg_lsn.h"
#define MAXPG_LSNLEN 17
@@ -248,3 +249,71 @@ pg_lsn_mi(PG_FUNCTION_ARGS)
return result;
}
+
+/*
+ * Add the number of bytes to pg_lsn, giving a new pg_lsn.
+ * Must handle both positive and negative numbers of bytes.
+ */
+Datum
+pg_lsn_pli(PG_FUNCTION_ARGS)
+{
+ XLogRecPtr lsn = PG_GETARG_LSN(0);
+ Numeric nbytes = PG_GETARG_NUMERIC(1);
+ Datum num;
+ Datum res;
+ char buf[32];
+
+ if (numeric_is_nan(nbytes))
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("cannot add NaN to pg_lsn")));
+
+ /* Convert to numeric */
+ snprintf(buf, sizeof(buf), UINT64_FORMAT, lsn);
+ num = DirectFunctionCall3(numeric_in,
+ CStringGetDatum(buf),
+ ObjectIdGetDatum(0),
+ Int32GetDatum(-1));
+
+ /* Add two numerics */
+ res = DirectFunctionCall2(numeric_add,
+ NumericGetDatum(num),
+ NumericGetDatum(nbytes));
+
+ /* Convert to pg_lsn */
+ return DirectFunctionCall1(numeric_pg_lsn, res);
+}
+
+/*
+ * Subtract the number of bytes from pg_lsn, giving a new pg_lsn.
+ * Must handle both positive and negative numbers of bytes.
+ */
+Datum
+pg_lsn_mii(PG_FUNCTION_ARGS)
+{
+ XLogRecPtr lsn = PG_GETARG_LSN(0);
+ Numeric nbytes = PG_GETARG_NUMERIC(1);
+ Datum num;
+ Datum res;
+ char buf[32];
+
+ if (numeric_is_nan(nbytes))
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("cannot subtract NaN from pg_lsn")));
+
+ /* Convert to numeric */
+ snprintf(buf, sizeof(buf), UINT64_FORMAT, lsn);
+ num = DirectFunctionCall3(numeric_in,
+ CStringGetDatum(buf),
+ ObjectIdGetDatum(0),
+ Int32GetDatum(-1));
+
+ /* Subtract two numerics */
+ res = DirectFunctionCall2(numeric_sub,
+ NumericGetDatum(num),
+ NumericGetDatum(nbytes));
+
+ /* Convert to pg_lsn */
+ return DirectFunctionCall1(numeric_pg_lsn, res);
+}
diff --git a/src/include/catalog/pg_operator.dat b/src/include/catalog/pg_operator.dat
index 59771f606d..5b0e063655 100644
--- a/src/include/catalog/pg_operator.dat
+++ b/src/include/catalog/pg_operator.dat
@@ -2909,6 +2909,17 @@
{ oid => '3228', descr => 'minus',
oprname => '-', oprleft => 'pg_lsn', oprright => 'pg_lsn',
oprresult => 'numeric', oprcode => 'pg_lsn_mi' },
+{ oid => '5025', descr => 'add',
+ oprname => '+', oprleft => 'pg_lsn', oprright => 'numeric',
+ oprresult => 'pg_lsn', oprcom => '+(numeric,pg_lsn)',
+ oprcode => 'pg_lsn_pli' },
+{ oid => '5026', descr => 'add',
+ oprname => '+', oprleft => 'numeric', oprright => 'pg_lsn',
+ oprresult => 'pg_lsn', oprcom => '+(pg_lsn,numeric)',
+ oprcode => 'numeric_pl_pg_lsn' },
+{ oid => '5027', descr => 'subtract',
+ oprname => '-', oprleft => 'pg_lsn', oprright => 'numeric',
+ oprresult => 'pg_lsn', oprcode => 'pg_lsn_mii' },
# enum operators
{ oid => '3516', descr => 'equal',
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 61f2c2f5b4..38295aca48 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -4398,6 +4398,9 @@
{ oid => '1783', descr => 'convert numeric to int2',
proname => 'int2', prorettype => 'int2', proargtypes => 'numeric',
prosrc => 'numeric_int2' },
+{ oid => '6103', descr => 'convert numeric to pg_lsn',
+ proname => 'pg_lsn', prorettype => 'pg_lsn', proargtypes => 'numeric',
+ prosrc => 'numeric_pg_lsn' },
{ oid => '3556', descr => 'convert jsonb to boolean',
proname => 'bool', prorettype => 'bool', proargtypes => 'jsonb',
@@ -8576,6 +8579,15 @@
{ oid => '4188', descr => 'smaller of two',
proname => 'pg_lsn_smaller', prorettype => 'pg_lsn',
proargtypes => 'pg_lsn pg_lsn', prosrc => 'pg_lsn_smaller' },
+{ oid => '5022',
+ proname => 'pg_lsn_pli', prorettype => 'pg_lsn',
+ proargtypes => 'pg_lsn numeric', prosrc => 'pg_lsn_pli' },
+{ oid => '5023',
+ proname => 'numeric_pl_pg_lsn', prolang => 'sql', prorettype => 'pg_lsn',
+ proargtypes => 'numeric pg_lsn', prosrc => 'select $2 + $1' },
+{ oid => '5024',
+ proname => 'pg_lsn_mii', prorettype => 'pg_lsn',
+ proargtypes => 'pg_lsn numeric', prosrc => 'pg_lsn_mii' },
# enum related procs
{ oid => '3504', descr => 'I/O',
diff --git a/src/test/regress/expected/numeric.out b/src/test/regress/expected/numeric.out
index 2f3ecb50a7..81a0c5d40f 100644
--- a/src/test/regress/expected/numeric.out
+++ b/src/test/regress/expected/numeric.out
@@ -2348,3 +2348,30 @@ SELECT -4!;
ERROR: factorial of a negative number is undefined
SELECT factorial(-4);
ERROR: factorial of a negative number is undefined
+--
+-- Tests for pg_lsn()
+--
+SELECT pg_lsn(23783416::numeric);
+ pg_lsn
+-----------
+ 0/16AE7F8
+(1 row)
+
+SELECT pg_lsn(0::numeric);
+ pg_lsn
+--------
+ 0/0
+(1 row)
+
+SELECT pg_lsn(18446744073709551615::numeric);
+ pg_lsn
+-------------------
+ FFFFFFFF/FFFFFFFF
+(1 row)
+
+SELECT pg_lsn(-1::numeric);
+ERROR: pg_lsn out of range
+SELECT pg_lsn(18446744073709551616::numeric);
+ERROR: pg_lsn out of range
+SELECT pg_lsn('NaN'::numeric);
+ERROR: cannot convert NaN to pg_lsn
diff --git a/src/test/regress/expected/pg_lsn.out b/src/test/regress/expected/pg_lsn.out
index 64d41dfdad..99a748a6a7 100644
--- a/src/test/regress/expected/pg_lsn.out
+++ b/src/test/regress/expected/pg_lsn.out
@@ -71,6 +71,56 @@ SELECT '0/16AE7F8'::pg_lsn - '0/16AE7F7'::pg_lsn;
1
(1 row)
+SELECT '0/16AE7F7'::pg_lsn + 16::numeric;
+ ?column?
+-----------
+ 0/16AE807
+(1 row)
+
+SELECT 16::numeric + '0/16AE7F7'::pg_lsn;
+ ?column?
+-----------
+ 0/16AE807
+(1 row)
+
+SELECT '0/16AE7F7'::pg_lsn - 16::numeric;
+ ?column?
+-----------
+ 0/16AE7E7
+(1 row)
+
+SELECT 'FFFFFFFF/FFFFFFFE'::pg_lsn + 1::numeric;
+ ?column?
+-------------------
+ FFFFFFFF/FFFFFFFF
+(1 row)
+
+SELECT 'FFFFFFFF/FFFFFFFE'::pg_lsn + 2::numeric; -- out of range error
+ERROR: pg_lsn out of range
+SELECT '0/1'::pg_lsn - 1::numeric;
+ ?column?
+----------
+ 0/0
+(1 row)
+
+SELECT '0/1'::pg_lsn - 2::numeric; -- out of range error
+ERROR: pg_lsn out of range
+SELECT '0/0'::pg_lsn + ('FFFFFFFF/FFFFFFFF'::pg_lsn - '0/0'::pg_lsn);
+ ?column?
+-------------------
+ FFFFFFFF/FFFFFFFF
+(1 row)
+
+SELECT 'FFFFFFFF/FFFFFFFF'::pg_lsn - ('FFFFFFFF/FFFFFFFF'::pg_lsn - '0/0'::pg_lsn);
+ ?column?
+----------
+ 0/0
+(1 row)
+
+SELECT '0/16AE7F7'::pg_lsn + 'NaN'::numeric;
+ERROR: cannot add NaN to pg_lsn
+SELECT '0/16AE7F7'::pg_lsn - 'NaN'::numeric;
+ERROR: cannot subtract NaN from pg_lsn
-- Check btree and hash opclasses
EXPLAIN (COSTS OFF)
SELECT DISTINCT (i || '/' || j)::pg_lsn f
diff --git a/src/test/regress/sql/numeric.sql b/src/test/regress/sql/numeric.sql
index 1332a9cf07..5dc80f686f 100644
--- a/src/test/regress/sql/numeric.sql
+++ b/src/test/regress/sql/numeric.sql
@@ -1122,3 +1122,13 @@ SELECT 100000!;
SELECT 0!;
SELECT -4!;
SELECT factorial(-4);
+
+--
+-- Tests for pg_lsn()
+--
+SELECT pg_lsn(23783416::numeric);
+SELECT pg_lsn(0::numeric);
+SELECT pg_lsn(18446744073709551615::numeric);
+SELECT pg_lsn(-1::numeric);
+SELECT pg_lsn(18446744073709551616::numeric);
+SELECT pg_lsn('NaN'::numeric);
diff --git a/src/test/regress/sql/pg_lsn.sql b/src/test/regress/sql/pg_lsn.sql
index 2c143c82ff..615368ba96 100644
--- a/src/test/regress/sql/pg_lsn.sql
+++ b/src/test/regress/sql/pg_lsn.sql
@@ -27,6 +27,17 @@ SELECT '0/16AE7F7' < '0/16AE7F8'::pg_lsn;
SELECT '0/16AE7F8' > pg_lsn '0/16AE7F7';
SELECT '0/16AE7F7'::pg_lsn - '0/16AE7F8'::pg_lsn;
SELECT '0/16AE7F8'::pg_lsn - '0/16AE7F7'::pg_lsn;
+SELECT '0/16AE7F7'::pg_lsn + 16::numeric;
+SELECT 16::numeric + '0/16AE7F7'::pg_lsn;
+SELECT '0/16AE7F7'::pg_lsn - 16::numeric;
+SELECT 'FFFFFFFF/FFFFFFFE'::pg_lsn + 1::numeric;
+SELECT 'FFFFFFFF/FFFFFFFE'::pg_lsn + 2::numeric; -- out of range error
+SELECT '0/1'::pg_lsn - 1::numeric;
+SELECT '0/1'::pg_lsn - 2::numeric; -- out of range error
+SELECT '0/0'::pg_lsn + ('FFFFFFFF/FFFFFFFF'::pg_lsn - '0/0'::pg_lsn);
+SELECT 'FFFFFFFF/FFFFFFFF'::pg_lsn - ('FFFFFFFF/FFFFFFFF'::pg_lsn - '0/0'::pg_lsn);
+SELECT '0/16AE7F7'::pg_lsn + 'NaN'::numeric;
+SELECT '0/16AE7F7'::pg_lsn - 'NaN'::numeric;
-- Check btree and hash opclasses
EXPLAIN (COSTS OFF)
The following review has been posted through the commitfest application:
make installcheck-world: tested, passed
Implements feature: tested, passed
Spec compliant: tested, passed
Documentation: not tested
The patch looks good to me.
The new status of this patch is: Ready for Committer
On 2020/06/30 19:54, Asif Rehman wrote:
The following review has been posted through the commitfest application:
make installcheck-world: tested, passed
Implements feature: tested, passed
Spec compliant: tested, passed
Documentation: not testedThe patch looks good to me.
The new status of this patch is: Ready for Committer
Thanks for the review! Pushed.
Regards,
--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION