Re: +(pg_lsn, int8) and -(pg_lsn, int8) operators
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 tested The 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
Re: +(pg_lsn, int8) and -(pg_lsn, int8) operators
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
Re: +(pg_lsn, int8) and -(pg_lsn, int8) operators
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
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 = and
. Two LSNs can be subtracted using the
- 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
++(pg_lsn,numeric) and
+-(pg_lsn,numeric) operators, respectively. Note that
+the calculated LSN should be in the range of pg_lsn type,
+i.e., between 0/0 and
+/.
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, );
+
+ if (!numericvar_to_uint64(, (uint64 *) ))
+ 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();
+ set_var_from_var(var, );
+ round_var(, 0);
+
+ /* Check for zero input */
+ strip_var();
+ ndigits = rounded.ndigits;
+ if (ndigits == 0)
+ {
+ *result = 0;
+ free_var();
+ return true;
+ }
+
+ /* Check for negative input */
+ if (rounded.sign == NUMERIC_NEG)
+ {
+ free_var();
+ return false;
+ }
+
+ /*
+* For input like 100, 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, )))
+ {
+ free_var();
+ return false;
+ }
+
+ if (i < ndigits)
+ {
+ if (unlikely(pg_add_u64_overflow(val, digits[i], )))
+ {
+ free_var();
+ return false;
+ }
+ }
+ }
+
+ free_var();
+
+ *result = val;
+
+ return true;
+}
+
#ifdef HAVE_INT128
/*
Re: +(pg_lsn, int8) and -(pg_lsn, int8) operators
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
Re: +(pg_lsn, int8) and -(pg_lsn, int8) operators
At Sat, 9 May 2020 23:40:15 +0900, Fujii Masao wrote in > > > On 2020/05/08 12:10, Kyotaro Horiguchi wrote: > > At Fri, 8 May 2020 11:31:42 +0900, Fujii Masao > > 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. 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
Re: +(pg_lsn, int8) and -(pg_lsn, int8) operators
On 2020/05/08 12:10, Kyotaro Horiguchi wrote:
At Fri, 8 May 2020 11:31:42 +0900, Fujii Masao
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
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 = and
. Two LSNs can be subtracted using the
- 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
++(pg_lsn,numeric) and
+-(pg_lsn,numeric) operators, respectively. Note that
+the calculated LSN should be in the range of pg_lsn type,
+i.e., between 0/0 and
+/.
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, );
+
+ if (!numericvar_to_uint64(, (uint64 *) ))
+ 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 -
Re: +(pg_lsn, int8) and -(pg_lsn, int8) operators
At Fri, 8 May 2020 11:31:42 +0900, Fujii Masao 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 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. regards. -- Kyotaro Horiguchi NTT Open Source Software Center
Re: +(pg_lsn, int8) and -(pg_lsn, int8) operators
On 2020/05/08 10:00, Kyotaro Horiguchi wrote:
At Thu, 7 May 2020 13:17:01 +0900, Fujii Masao
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
+ charbuf[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, ))
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
Re: +(pg_lsn, int8) and -(pg_lsn, int8) operators
At Thu, 7 May 2020 13:17:01 +0900, Fujii Masao
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.
> > + charbuf[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, ))
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
Re: +(pg_lsn, int8) and -(pg_lsn, int8) operators
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
+(pg_lsn,numeric) and -(pg_lsn,numeric)
operators, respectively. Note that the calculated LSN should be in the range
of pg_lsn type, i.e., between 0/0 and
/.
-
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
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 = and
. Two LSNs can be subtracted using the
- 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
++(pg_lsn,numeric) and
+-(pg_lsn,numeric) operators, respectively. Note that
+the calculated LSN should be in the range of pg_lsn type,
+i.e., between 0/0 and
+/.
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, );
+
+ if (!numericvar_to_uint64(, (uint64 *) ))
+ 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();
+ set_var_from_var(var, );
+ round_var(, 0);
+
+ /* Check for zero input */
+ strip_var();
+ ndigits = rounded.ndigits;
+ if (ndigits == 0)
+ {
+ *result = 0;
+ free_var();
+ return true;
+ }
+
+ /* Check for negative input */
+ if (rounded.sign == NUMERIC_NEG)
+ {
+ free_var();
+ return false;
+ }
+
+ /*
+* For input like 100, we must treat stripped digits as real. So
+* the loop assumes there are weight+1 digits before the decimal point.
+
Re: +(pg_lsn, int8) and -(pg_lsn, int8) operators
On 2020/05/07 11:21, Kyotaro Horiguchi wrote:
At Tue, 28 Apr 2020 12:56:19 +0900, Fujii Masao
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 + 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?
+ charbuf[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
Re: +(pg_lsn, int8) and -(pg_lsn, int8) operators
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
+(pg_lsn,numeric) and -(pg_lsn,numeric)
operators, respectively. Note that the calculated LSN should be in the range
of pg_lsn type, i.e., between 0/0 and
/.
-
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
Re: +(pg_lsn, int8) and -(pg_lsn, int8) operators
At Tue, 28 Apr 2020 12:56:19 +0900, Fujii Masao
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 + 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.
+ charbuf[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
Re: +(pg_lsn, int8) and -(pg_lsn, int8) operators
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
> +(pg_lsn,numeric) and -(pg_lsn,numeric)
> operators, respectively. Note that the calculated LSN should be in the range
> of pg_lsn type, i.e., between 0/0 and
> /.
> -
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
signature.asc
Description: PGP signature
Re: +(pg_lsn, int8) and -(pg_lsn, int8) operators
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 + and
+- 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
+(pg_lsn,numeric) and -(pg_lsn,numeric)
operators, respectively. Note that the calculated LSN should be in the range
of pg_lsn type, i.e., between 0/0 and
/.
-
+ /* 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
Re: +(pg_lsn, int8) and -(pg_lsn, int8) operators
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 + and
+- 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
signature.asc
Description: PGP signature
Re: +(pg_lsn, int8) and -(pg_lsn, int8) operators
On 2020/04/28 1:24, Robert Haas wrote:
On Sun, Apr 26, 2020 at 9:41 PM Kyotaro Horiguchi
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
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 = and
. Two LSNs can be subtracted using the
- 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 + and
+- operators, respectively.
+
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, );
+
+ if (!numericvar_to_uint64(, (uint64 *) ))
+ 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();
+ set_var_from_var(var, );
+ round_var(, 0);
+
+ /* Check for zero input */
+ strip_var();
+ ndigits = rounded.ndigits;
+ if (ndigits == 0)
+ {
+ *result = 0;
+ free_var();
+ return true;
+ }
+
+ /* Check for negative input */
+ if (rounded.sign == NUMERIC_NEG)
+ {
+ free_var();
+ return false;
+ }
+
+ /*
+* For input like 100, 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, )))
+ {
+ free_var();
+ return false;
+ }
+
+ if (i < ndigits)
+ {
+ if (unlikely(pg_add_u64_overflow(val, digits[i], )))
+ {
+ free_var();
+ return false;
+ }
+ }
+ }
Re: +(pg_lsn, int8) and -(pg_lsn, int8) operators
On Sun, Apr 26, 2020 at 9:41 PM Kyotaro Horiguchi 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
Re: +(pg_lsn, int8) and -(pg_lsn, int8) operators
At Fri, 24 Apr 2020 12:15:26 +0200, Jehan-Guillaume de Rorthais
wrote in
> On Fri, 24 Apr 2020 16:24:14 +0900
> Michael Paquier 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:
> > >
> > > https://twitter.com/fujii_masao/status/1252652020487487488
> >
> > (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
Re: +(pg_lsn, int8) and -(pg_lsn, int8) operators
On Fri, 24 Apr 2020 16:24:14 +0900 Michael Paquier 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: > > > > https://twitter.com/fujii_masao/status/1252652020487487488 > > (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.
Re: +(pg_lsn, int8) and -(pg_lsn, int8) operators
On Thu, Apr 23, 2020 at 08:09:22AM -0400, Robert Haas wrote: > For anyone who missed it, this idea was popular on Twitter: > > https://twitter.com/fujii_masao/status/1252652020487487488 (For the sake of the archives) To which Alvaro, Robert, Fabrízio de Royes Mello, Julien Rouhaud and I answered positively to. -- Michael signature.asc Description: PGP signature
Re: +(pg_lsn, int8) and -(pg_lsn, int8) operators
On Thu, Apr 23, 2020 at 12:28 PM Ashutosh Bapat 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 > +/- = 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
Re: +(pg_lsn, int8) and -(pg_lsn, int8) operators
On Thu, Apr 23, 2020 at 2:51 PM Fujii Masao 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 +/- = LSN of N+/- n th log record that would be interesting. :) -- Best Wishes, Ashutosh Bapat
Re: +(pg_lsn, int8) and -(pg_lsn, int8) operators
On Thu, Apr 23, 2020 at 5:21 AM Fujii Masao 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
+(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
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 = and
. Two LSNs can be subtracted using the
- 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 + and
+- operators, respectively.
+
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 '/FFFE'::pg_lsn + 1::bigint;
+ ?column?
+---
+ /
+(1 row)
+
+SELECT 'F
