On Fri, Mar 18, 2022 at 1:17 AM Greg Stark <st...@mit.edu> wrote:
>
> On Fri, 11 Mar 2022 at 15:17, Tom Lane <t...@sss.pgh.pa.us> wrote:
> >
> > Amul Sul <sula...@gmail.com> writes:
>
> > >  Yeah, that's true, I am too not sure if we really need to refactor
> > >  all those; If we want, I can give it a try.
> >
> > The patch as-presented isn't very compelling for
> > lack of callers of the new function
>
> Tom, are you saying you think we're not interested in just adding this
> function unless it's part of this refactoring?
>
> Amul, do you think if we did numeric_to_int64/numeric_to_uint64 as a
> refactored API and a second patch that made numeric_pg_lsn and other
> consumers use it it would clean up the code significantly?

Sorry for the long absence.

Yes, I think we can do cleanup to some extent.  Attaching the
following patches that first intend to remove DirectFunctionCall as
much as possible:

0001:
Refactors and moves numeric_pg_lsn to pg_lsn.c file. Function gut is
in numeric.c as numeric_to_uint64_type() generalised function that can
be used elsewhere too.

0002:
Does little more cleanup to pg_lsn.c file -- replace few
DirectFunctionCall1() by the numeric_to_uint64_type().

0003:
Refactors numeric_int8() function and replace few
DirectFunctionCall1() to numeric_int8 by the newly added
numeric_to_int64() and numeric_to_int64_type().
numeric_to_int64_type() version can be called only when you want to
refer to the specific type name in the error message like
numeric_to_uint64_type, e.g.MONEY type.

0004:
Adding float8_to_numeric and numeric_to_float8 by refactoring
float8_numeric and numeric_float8 respectively. I am a bit confused
about whether the type should be referred to as float8 or double.
Replaces a few DirectFunctionCall() calls by these c functions.

0005:
This one replaces all DirectFunctionCall needed for the numeric
arithmetic operations.

Regards,
Amul
From fd5e1ccffe2c1aa1c619fb0f31389f35fe554e5b Mon Sep 17 00:00:00 2001
From: Amul Sul <amul.sul@enterprisedb.com>
Date: Fri, 22 Apr 2022 06:17:51 -0400
Subject: [PATCH v2 5/5] Call numeric arithmetic functions directly

---
 contrib/btree_gist/btree_numeric.c          | 17 +++------
 src/backend/access/brin/brin_minmax_multi.c | 14 ++++----
 src/backend/utils/adt/cash.c                | 15 ++++----
 src/backend/utils/adt/dbsize.c              | 33 +++++++++--------
 src/backend/utils/adt/formatting.c          | 15 ++++----
 src/backend/utils/adt/jsonb.c               |  7 ++--
 src/backend/utils/adt/numeric.c             | 39 +++++++++++----------
 src/backend/utils/adt/pg_lsn.c              | 16 ++++-----
 src/backend/utils/adt/rangetypes.c          | 10 +++---
 9 files changed, 80 insertions(+), 86 deletions(-)

diff --git a/contrib/btree_gist/btree_numeric.c b/contrib/btree_gist/btree_numeric.c
index 35e466cdd94..6c50cfa41f2 100644
--- a/contrib/btree_gist/btree_numeric.c
+++ b/contrib/btree_gist/btree_numeric.c
@@ -174,17 +174,10 @@ gbt_numeric_penalty(PG_FUNCTION_ARGS)
 	ok = gbt_var_key_readable(org);
 	uk = gbt_var_key_readable((GBT_VARKEY *) DatumGetPointer(uni));
 
-	us = DatumGetNumeric(DirectFunctionCall2(numeric_sub,
-											 PointerGetDatum(uk.upper),
-											 PointerGetDatum(uk.lower)));
+	us = numeric_sub_opt_error((Numeric) uk.upper, (Numeric) uk.lower, NULL);
+	os = numeric_sub_opt_error((Numeric) ok.upper, (Numeric) ok.lower, NULL);
 
-	os = DatumGetNumeric(DirectFunctionCall2(numeric_sub,
-											 PointerGetDatum(ok.upper),
-											 PointerGetDatum(ok.lower)));
-
-	ds = DatumGetNumeric(DirectFunctionCall2(numeric_sub,
-											 NumericGetDatum(us),
-											 NumericGetDatum(os)));
+	ds = numeric_sub_opt_error(us, os, NULL);
 
 	if (numeric_is_nan(us))
 	{
@@ -202,9 +195,7 @@ gbt_numeric_penalty(PG_FUNCTION_ARGS)
 		if (DirectFunctionCall2(numeric_gt, NumericGetDatum(ds), NumericGetDatum(nul)))
 		{
 			*result += FLT_MIN;
-			os = DatumGetNumeric(DirectFunctionCall2(numeric_div,
-													 NumericGetDatum(ds),
-													 NumericGetDatum(us)));
+			os = numeric_div_opt_error(ds, us, NULL);
 			*result += (float4) DatumGetFloat8(DirectFunctionCall1(numeric_float8_no_overflow, NumericGetDatum(os)));
 		}
 	}
diff --git a/src/backend/access/brin/brin_minmax_multi.c b/src/backend/access/brin/brin_minmax_multi.c
index 764efa381f2..c84a70d1ad9 100644
--- a/src/backend/access/brin/brin_minmax_multi.c
+++ b/src/backend/access/brin/brin_minmax_multi.c
@@ -2008,19 +2008,21 @@ brin_minmax_multi_distance_tid(PG_FUNCTION_ARGS)
 Datum
 brin_minmax_multi_distance_numeric(PG_FUNCTION_ARGS)
 {
-	Datum		d;
-	Datum		a1 = PG_GETARG_DATUM(0);
-	Datum		a2 = PG_GETARG_DATUM(1);
+	Numeric		res;
+	Numeric		a1 = PG_GETARG_NUMERIC(0);
+	Numeric		a2 = PG_GETARG_NUMERIC(1);
 
 	/*
 	 * We know the values are range boundaries, but the range may be collapsed
 	 * (i.e. single points), with equal values.
 	 */
-	Assert(DatumGetBool(DirectFunctionCall2(numeric_le, a1, a2)));
+	Assert(DatumGetBool(DirectFunctionCall2(numeric_le,
+											NumericGetDatum(a1),
+											NumericGetDatum(a2))));
 
-	d = DirectFunctionCall2(numeric_sub, a2, a1);	/* a2 - a1 */
+	res = numeric_sub_opt_error(a2, a1, NULL);	/* a2 - a1 */
 
-	PG_RETURN_FLOAT8(numeric_to_float8(DatumGetNumeric(d)));
+	PG_RETURN_FLOAT8(numeric_to_float8(res));
 }
 
 /*
diff --git a/src/backend/utils/adt/cash.c b/src/backend/utils/adt/cash.c
index d05335dbd47..81f4eceee9c 100644
--- a/src/backend/utils/adt/cash.c
+++ b/src/backend/utils/adt/cash.c
@@ -1069,7 +1069,10 @@ cash_numeric(PG_FUNCTION_ARGS)
 											Int32GetDatum(fpoint));
 
 		/* Now we can safely divide ... */
-		quotient = DirectFunctionCall2(numeric_div, result, numeric_scale);
+		quotient =
+			NumericGetDatum(numeric_div_opt_error(DatumGetNumeric(result),
+												  DatumGetNumeric(numeric_scale),
+												  NULL));
 
 		/* ... and forcibly round to exactly the intended number of digits */
 		result = DirectFunctionCall2(numeric_round,
@@ -1086,12 +1089,12 @@ cash_numeric(PG_FUNCTION_ARGS)
 Datum
 numeric_cash(PG_FUNCTION_ARGS)
 {
-	Datum		amount = PG_GETARG_DATUM(0);
+	Numeric		amount = PG_GETARG_NUMERIC(0);
 	Cash		result;
 	int			fpoint;
 	int64		scale;
 	int			i;
-	Datum		numeric_scale;
+	Numeric		numeric_scale;
 	struct lconv *lconvert = PGLC_localeconv();
 
 	/* see comments about frac_digits in cash_in() */
@@ -1105,11 +1108,11 @@ numeric_cash(PG_FUNCTION_ARGS)
 		scale *= 10;
 
 	/* multiply the input amount by scale factor */
-	numeric_scale = NumericGetDatum(int64_to_numeric(scale));
-	amount = DirectFunctionCall2(numeric_mul, amount, numeric_scale);
+	numeric_scale = int64_to_numeric(scale);
+	amount = numeric_mul_opt_error(amount, numeric_scale, NULL);
 
 	/* note that numeric_to_int64 will round to nearest integer for us */
-	result = numeric_to_int64_type(DatumGetNumeric(amount), "money");
+	result = numeric_to_int64_type(amount, "money");
 
 	PG_RETURN_CASH(result);
 }
diff --git a/src/backend/utils/adt/dbsize.c b/src/backend/utils/adt/dbsize.c
index afd6a7ab623..10675d19bcb 100644
--- a/src/backend/utils/adt/dbsize.c
+++ b/src/backend/utils/adt/dbsize.c
@@ -620,22 +620,29 @@ numeric_absolute(Numeric n)
 static Numeric
 numeric_half_rounded(Numeric n)
 {
-	Datum		d = NumericGetDatum(n);
-	Datum		zero;
-	Datum		one;
-	Datum		two;
+	Numeric		d;
+	Numeric		zero;
+	Numeric		one;
+	Numeric		two;
 	Datum		result;
+	bool		gt;
+
+	zero = int64_to_numeric(0);
+	one = int64_to_numeric(1);
+	two = int64_to_numeric(2);
 
-	zero = NumericGetDatum(int64_to_numeric(0));
-	one = NumericGetDatum(int64_to_numeric(1));
-	two = NumericGetDatum(int64_to_numeric(2));
+	gt = DatumGetBool(DirectFunctionCall2(numeric_ge,
+										  NumericGetDatum(n),
+										  NumericGetDatum(zero)));
 
-	if (DatumGetBool(DirectFunctionCall2(numeric_ge, d, zero)))
-		d = DirectFunctionCall2(numeric_add, d, one);
+	if (gt)
+		d = numeric_add_opt_error(n, one, NULL);
 	else
-		d = DirectFunctionCall2(numeric_sub, d, one);
+		d = numeric_sub_opt_error(n, one, NULL);
 
-	result = DirectFunctionCall2(numeric_div_trunc, d, two);
+	result = DirectFunctionCall2(numeric_div_trunc,
+								 NumericGetDatum(d),
+								 NumericGetDatum(two));
 	return DatumGetNumeric(result);
 }
 
@@ -819,9 +826,7 @@ pg_size_bytes(PG_FUNCTION_ARGS)
 
 			mul_num = int64_to_numeric(multiplier);
 
-			num = DatumGetNumeric(DirectFunctionCall2(numeric_mul,
-													  NumericGetDatum(mul_num),
-													  NumericGetDatum(num)));
+			num = numeric_mul_opt_error(mul_num, num, NULL);
 		}
 	}
 
diff --git a/src/backend/utils/adt/formatting.c b/src/backend/utils/adt/formatting.c
index 97a4544ffc6..f19f94b8e5f 100644
--- a/src/backend/utils/adt/formatting.c
+++ b/src/backend/utils/adt/formatting.c
@@ -6178,9 +6178,9 @@ numeric_to_number(PG_FUNCTION_ARGS)
 		x = DatumGetNumeric(DirectFunctionCall2(numeric_power,
 												NumericGetDatum(a),
 												NumericGetDatum(b)));
-		result = DirectFunctionCall2(numeric_mul,
-									 result,
-									 NumericGetDatum(x));
+		result = NumericGetDatum(numeric_mul_opt_error(DatumGetNumeric(result),
+													   x,
+													   NULL));
 	}
 
 	pfree(numstr);
@@ -6217,9 +6217,7 @@ numeric_to_char(PG_FUNCTION_ARGS)
 		x = DatumGetNumeric(DirectFunctionCall2(numeric_round,
 												NumericGetDatum(value),
 												Int32GetDatum(0)));
-		numstr =
-			int_to_roman(DatumGetInt32(DirectFunctionCall1(numeric_int4,
-														   NumericGetDatum(x))));
+		numstr = int_to_roman(numeric_int4_opt_error(x, NULL));
 	}
 	else if (IS_EEEE(&Num))
 	{
@@ -6268,9 +6266,8 @@ numeric_to_char(PG_FUNCTION_ARGS)
 			x = DatumGetNumeric(DirectFunctionCall2(numeric_power,
 													NumericGetDatum(a),
 													NumericGetDatum(b)));
-			val = DatumGetNumeric(DirectFunctionCall2(numeric_mul,
-													  NumericGetDatum(value),
-													  NumericGetDatum(x)));
+			val = numeric_mul_opt_error(value, x, NULL);
+
 			Num.pre += Num.multi;
 		}
 
diff --git a/src/backend/utils/adt/jsonb.c b/src/backend/utils/adt/jsonb.c
index 7b295c199d8..d595d99a03c 100644
--- a/src/backend/utils/adt/jsonb.c
+++ b/src/backend/utils/adt/jsonb.c
@@ -2156,17 +2156,16 @@ jsonb_int4(PG_FUNCTION_ARGS)
 {
 	Jsonb	   *in = PG_GETARG_JSONB_P(0);
 	JsonbValue	v;
-	Datum		retValue;
+	int32		retValue;
 
 	if (!JsonbExtractScalar(&in->root, &v) || v.type != jbvNumeric)
 		cannotCastJsonbValue(v.type, "integer");
 
-	retValue = DirectFunctionCall1(numeric_int4,
-								   NumericGetDatum(v.val.numeric));
+	retValue = numeric_int4_opt_error(v.val.numeric, NULL);
 
 	PG_FREE_IF_COPY(in, 0);
 
-	PG_RETURN_DATUM(retValue);
+	PG_RETURN_INT32(retValue);
 }
 
 Datum
diff --git a/src/backend/utils/adt/numeric.c b/src/backend/utils/adt/numeric.c
index d83fe5a326d..c42e72c3193 100644
--- a/src/backend/utils/adt/numeric.c
+++ b/src/backend/utils/adt/numeric.c
@@ -5960,8 +5960,8 @@ numeric_poly_avg(PG_FUNCTION_ARGS)
 #ifdef HAVE_INT128
 	PolyNumAggState *state;
 	NumericVar	result;
-	Datum		countd,
-				sumd;
+	Numeric		count,
+				sum;
 
 	state = PG_ARGISNULL(0) ? NULL : (PolyNumAggState *) PG_GETARG_POINTER(0);
 
@@ -5973,12 +5973,12 @@ numeric_poly_avg(PG_FUNCTION_ARGS)
 
 	int128_to_numericvar(state->sumX, &result);
 
-	countd = NumericGetDatum(int64_to_numeric(state->N));
-	sumd = NumericGetDatum(make_result(&result));
+	count = int64_to_numeric(state->N);
+	sum = make_result(&result);
 
 	free_var(&result);
 
-	PG_RETURN_DATUM(DirectFunctionCall2(numeric_div, sumd, countd));
+	PG_RETURN_NUMERIC(numeric_div_opt_error(sum, count, NULL));
 #else
 	return numeric_avg(fcinfo);
 #endif
@@ -5988,8 +5988,7 @@ Datum
 numeric_avg(PG_FUNCTION_ARGS)
 {
 	NumericAggState *state;
-	Datum		N_datum;
-	Datum		sumX_datum;
+	Numeric		sumX;
 	NumericVar	sumX_var;
 
 	state = PG_ARGISNULL(0) ? NULL : (NumericAggState *) PG_GETARG_POINTER(0);
@@ -6009,14 +6008,14 @@ numeric_avg(PG_FUNCTION_ARGS)
 	if (state->nInfcount > 0)
 		PG_RETURN_NUMERIC(make_result(&const_ninf));
 
-	N_datum = NumericGetDatum(int64_to_numeric(state->N));
-
 	init_var(&sumX_var);
 	accum_sum_final(&state->sumX, &sumX_var);
-	sumX_datum = NumericGetDatum(make_result(&sumX_var));
+	sumX = make_result(&sumX_var);
 	free_var(&sumX_var);
 
-	PG_RETURN_DATUM(DirectFunctionCall2(numeric_div, sumX_datum, N_datum));
+	PG_RETURN_NUMERIC(numeric_div_opt_error(sumX,
+											int64_to_numeric(state->N),
+											NULL));
 }
 
 Datum
@@ -6469,6 +6468,7 @@ Datum
 int8_sum(PG_FUNCTION_ARGS)
 {
 	Numeric		oldsum;
+	Numeric		result;
 
 	if (PG_ARGISNULL(0))
 	{
@@ -6492,9 +6492,10 @@ int8_sum(PG_FUNCTION_ARGS)
 		PG_RETURN_NUMERIC(oldsum);
 
 	/* OK to do the addition. */
-	PG_RETURN_DATUM(DirectFunctionCall2(numeric_add,
-										NumericGetDatum(oldsum),
-										NumericGetDatum(int64_to_numeric(PG_GETARG_INT64(1)))));
+	result = numeric_add_opt_error(oldsum,
+								   int64_to_numeric(PG_GETARG_INT64(1)),
+								   NULL);
+	PG_RETURN_NUMERIC(result);
 }
 
 
@@ -6661,8 +6662,8 @@ int8_avg(PG_FUNCTION_ARGS)
 {
 	ArrayType  *transarray = PG_GETARG_ARRAYTYPE_P(0);
 	Int8TransTypeData *transdata;
-	Datum		countd,
-				sumd;
+	Numeric		count,
+				sum;
 
 	if (ARR_HASNULL(transarray) ||
 		ARR_SIZE(transarray) != ARR_OVERHEAD_NONULLS(1) + sizeof(Int8TransTypeData))
@@ -6673,10 +6674,10 @@ int8_avg(PG_FUNCTION_ARGS)
 	if (transdata->count == 0)
 		PG_RETURN_NULL();
 
-	countd = NumericGetDatum(int64_to_numeric(transdata->count));
-	sumd = NumericGetDatum(int64_to_numeric(transdata->sum));
+	count = int64_to_numeric(transdata->count);
+	sum = int64_to_numeric(transdata->sum);
 
-	PG_RETURN_DATUM(DirectFunctionCall2(numeric_div, sumd, countd));
+	PG_RETURN_NUMERIC(numeric_div_opt_error(sum, count, NULL));
 }
 
 /*
diff --git a/src/backend/utils/adt/pg_lsn.c b/src/backend/utils/adt/pg_lsn.c
index e0991d4d2f6..9ee31859d07 100644
--- a/src/backend/utils/adt/pg_lsn.c
+++ b/src/backend/utils/adt/pg_lsn.c
@@ -270,7 +270,7 @@ pg_lsn_pli(PG_FUNCTION_ARGS)
 	XLogRecPtr	lsn = PG_GETARG_LSN(0);
 	Numeric		nbytes = PG_GETARG_NUMERIC(1);
 	Datum		num;
-	Datum		res;
+	Numeric		res;
 	char		buf[32];
 
 	if (numeric_is_nan(nbytes))
@@ -286,12 +286,10 @@ pg_lsn_pli(PG_FUNCTION_ARGS)
 							  Int32GetDatum(-1));
 
 	/* Add two numerics */
-	res = DirectFunctionCall2(numeric_add,
-							  NumericGetDatum(num),
-							  NumericGetDatum(nbytes));
+	res = numeric_add_opt_error(DatumGetNumeric(num), nbytes, NULL);
 
 	/* Convert to pg_lsn */
-	PG_RETURN_LSN(numeric_to_pg_lsn(DatumGetNumeric(res)));
+	PG_RETURN_LSN(numeric_to_pg_lsn(res));
 }
 
 /*
@@ -304,7 +302,7 @@ pg_lsn_mii(PG_FUNCTION_ARGS)
 	XLogRecPtr	lsn = PG_GETARG_LSN(0);
 	Numeric		nbytes = PG_GETARG_NUMERIC(1);
 	Datum		num;
-	Datum		res;
+	Numeric		res;
 	char		buf[32];
 
 	if (numeric_is_nan(nbytes))
@@ -320,10 +318,8 @@ pg_lsn_mii(PG_FUNCTION_ARGS)
 							  Int32GetDatum(-1));
 
 	/* Subtract two numerics */
-	res = DirectFunctionCall2(numeric_sub,
-							  NumericGetDatum(num),
-							  NumericGetDatum(nbytes));
+	res = numeric_sub_opt_error(DatumGetNumeric(num), nbytes, NULL);
 
 	/* Convert to pg_lsn */
-	PG_RETURN_LSN(numeric_to_pg_lsn(DatumGetNumeric(res)));
+	PG_RETURN_LSN(numeric_to_pg_lsn(res));
 }
diff --git a/src/backend/utils/adt/rangetypes.c b/src/backend/utils/adt/rangetypes.c
index 8fb67387cc5..e0b339ab1f7 100644
--- a/src/backend/utils/adt/rangetypes.c
+++ b/src/backend/utils/adt/rangetypes.c
@@ -1571,14 +1571,14 @@ int8range_subdiff(PG_FUNCTION_ARGS)
 Datum
 numrange_subdiff(PG_FUNCTION_ARGS)
 {
-	Datum		v1 = PG_GETARG_DATUM(0);
-	Datum		v2 = PG_GETARG_DATUM(1);
-	Datum		numresult;
+	Numeric		v1 = PG_GETARG_NUMERIC(0);
+	Numeric		v2 = PG_GETARG_NUMERIC(1);
+	Numeric		numresult;
 	float8		floatresult;
 
-	numresult = DirectFunctionCall2(numeric_sub, v1, v2);
+	numresult = numeric_sub_opt_error(v1, v2, NULL);
 
-	floatresult = numeric_to_float8(DatumGetNumeric(numresult));
+	floatresult = numeric_to_float8(numresult);
 
 	PG_RETURN_FLOAT8(floatresult);
 }
-- 
2.18.0

From 4ae6045b62da403f76ed8cbb9a28a5523d99c298 Mon Sep 17 00:00:00 2001
From: Amul Sul <amul.sul@enterprisedb.com>
Date: Fri, 22 Apr 2022 00:35:18 -0400
Subject: [PATCH v2 2/5] More cleanup to pg_lsn.c

---
 src/backend/utils/adt/pg_lsn.c | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/src/backend/utils/adt/pg_lsn.c b/src/backend/utils/adt/pg_lsn.c
index 90ecd0e7590..e0991d4d2f6 100644
--- a/src/backend/utils/adt/pg_lsn.c
+++ b/src/backend/utils/adt/pg_lsn.c
@@ -22,6 +22,8 @@
 #define MAXPG_LSNLEN			17
 #define MAXPG_LSNCOMPONENT	8
 
+static inline XLogRecPtr numeric_to_pg_lsn(Numeric num);
+
 /*----------------------------------------------------------
  * Formatting and conversion routines.
  *---------------------------------------------------------*/
@@ -60,15 +62,18 @@ pg_lsn_in_internal(const char *str, bool *have_error)
 	return result;
 }
 
+static inline XLogRecPtr
+numeric_to_pg_lsn(Numeric num)
+{
+	return (XLogRecPtr) numeric_to_uint64_type(num, "pg_lsn");
+}
+
 Datum
 numeric_pg_lsn(PG_FUNCTION_ARGS)
 {
 	Numeric		num = PG_GETARG_NUMERIC(0);
-	XLogRecPtr	result;
-
-	result = (XLogRecPtr) numeric_to_uint64_type(num, "pg_lsn");
 
-	PG_RETURN_LSN(result);
+	PG_RETURN_LSN(numeric_to_pg_lsn(num));
 }
 
 Datum
@@ -286,7 +291,7 @@ pg_lsn_pli(PG_FUNCTION_ARGS)
 							  NumericGetDatum(nbytes));
 
 	/* Convert to pg_lsn */
-	return DirectFunctionCall1(numeric_pg_lsn, res);
+	PG_RETURN_LSN(numeric_to_pg_lsn(DatumGetNumeric(res)));
 }
 
 /*
@@ -320,5 +325,5 @@ pg_lsn_mii(PG_FUNCTION_ARGS)
 							  NumericGetDatum(nbytes));
 
 	/* Convert to pg_lsn */
-	return DirectFunctionCall1(numeric_pg_lsn, res);
+	PG_RETURN_LSN(numeric_to_pg_lsn(DatumGetNumeric(res)));
 }
-- 
2.18.0

From 64065814fd30cddb12b41674f01d7e505f793e19 Mon Sep 17 00:00:00 2001
From: Amul Sul <amul.sul@enterprisedb.com>
Date: Tue, 15 Feb 2022 23:57:26 -0500
Subject: [PATCH v2 1/5] Move numeric_pg_lsn to pg_lsn.c file.

---
 src/backend/utils/adt/numeric.c | 64 +++++++++++++++++----------------
 src/backend/utils/adt/pg_lsn.c  | 11 ++++++
 src/include/utils/numeric.h     |  1 +
 3 files changed, 45 insertions(+), 31 deletions(-)

diff --git a/src/backend/utils/adt/numeric.c b/src/backend/utils/adt/numeric.c
index 45547f6ae7f..3157df7cd7b 100644
--- a/src/backend/utils/adt/numeric.c
+++ b/src/backend/utils/adt/numeric.c
@@ -4211,6 +4211,39 @@ int64_div_fast_to_numeric(int64 val1, int log10val2)
 	return res;
 }
 
+/*
+ * typeName is the user-visible type name of uint64 used for the error
+ * reporting.
+ */
+uint64
+numeric_to_uint64_type(Numeric num, char *typeName)
+{
+	NumericVar	x;
+	uint64		result;
+
+	if (NUMERIC_IS_SPECIAL(num))
+	{
+		if (NUMERIC_IS_NAN(num))
+			ereport(ERROR,
+					(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+					 errmsg("cannot convert NaN to %s", typeName)));
+		else
+			ereport(ERROR,
+					(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+					 errmsg("cannot convert infinity to %s", typeName)));
+	}
+
+	/* Convert to variable format and thence to pg_lsn */
+	init_var_from_num(num, &x);
+
+	if (!numericvar_to_uint64(&x, &result))
+		ereport(ERROR,
+				(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+				 errmsg("%s out of range", typeName)));
+
+	return result;
+}
+
 Datum
 int4_numeric(PG_FUNCTION_ARGS)
 {
@@ -4543,37 +4576,6 @@ 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_SPECIAL(num))
-	{
-		if (NUMERIC_IS_NAN(num))
-			ereport(ERROR,
-					(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-					 errmsg("cannot convert NaN to %s", "pg_lsn")));
-		else
-			ereport(ERROR,
-					(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-					 errmsg("cannot convert infinity to %s", "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
diff --git a/src/backend/utils/adt/pg_lsn.c b/src/backend/utils/adt/pg_lsn.c
index 45408787da9..90ecd0e7590 100644
--- a/src/backend/utils/adt/pg_lsn.c
+++ b/src/backend/utils/adt/pg_lsn.c
@@ -60,6 +60,17 @@ pg_lsn_in_internal(const char *str, bool *have_error)
 	return result;
 }
 
+Datum
+numeric_pg_lsn(PG_FUNCTION_ARGS)
+{
+	Numeric		num = PG_GETARG_NUMERIC(0);
+	XLogRecPtr	result;
+
+	result = (XLogRecPtr) numeric_to_uint64_type(num, "pg_lsn");
+
+	PG_RETURN_LSN(result);
+}
+
 Datum
 pg_lsn_in(PG_FUNCTION_ARGS)
 {
diff --git a/src/include/utils/numeric.h b/src/include/utils/numeric.h
index 21cd5a5cbf5..1b9ad3b5b58 100644
--- a/src/include/utils/numeric.h
+++ b/src/include/utils/numeric.h
@@ -74,6 +74,7 @@ extern char *numeric_normalize(Numeric num);
 
 extern Numeric int64_to_numeric(int64 val);
 extern Numeric int64_div_fast_to_numeric(int64 val1, int log10val2);
+extern uint64 numeric_to_uint64_type(Numeric num, char *typeName);
 
 extern Numeric numeric_add_opt_error(Numeric num1, Numeric num2,
 									 bool *have_error);
-- 
2.18.0

From b0eb5c832864e6f754d1fcdd28134025861035bd Mon Sep 17 00:00:00 2001
From: Amul Sul <amul.sul@enterprisedb.com>
Date: Fri, 22 Apr 2022 02:54:20 -0400
Subject: [PATCH v2 4/5] Add float8_to_numeric and numeric_to_float8 by code
 refactoring

---
 contrib/jsonb_plperl/jsonb_plperl.c         |   4 +-
 src/backend/access/brin/brin_minmax_multi.c |   2 +-
 src/backend/utils/adt/jsonb.c               |   7 +-
 src/backend/utils/adt/jsonpath_exec.c       |   4 +-
 src/backend/utils/adt/numeric.c             | 106 +++++++++++---------
 src/backend/utils/adt/rangetypes.c          |   4 +-
 src/include/utils/numeric.h                 |   2 +
 7 files changed, 71 insertions(+), 58 deletions(-)

diff --git a/contrib/jsonb_plperl/jsonb_plperl.c b/contrib/jsonb_plperl/jsonb_plperl.c
index 22e90afe1b6..cd645e57b22 100644
--- a/contrib/jsonb_plperl/jsonb_plperl.c
+++ b/contrib/jsonb_plperl/jsonb_plperl.c
@@ -238,9 +238,7 @@ SV_to_JsonbValue(SV *in, JsonbParseState **jsonb_state, bool is_elem)
 							 errmsg("cannot convert NaN to jsonb")));
 
 				out.type = jbvNumeric;
-				out.val.numeric =
-					DatumGetNumeric(DirectFunctionCall1(float8_numeric,
-														Float8GetDatum(nval)));
+				out.val.numeric = float8_to_numeric(nval);
 			}
 			else if (SvPOK(in))
 			{
diff --git a/src/backend/access/brin/brin_minmax_multi.c b/src/backend/access/brin/brin_minmax_multi.c
index 10d4f17bc6f..764efa381f2 100644
--- a/src/backend/access/brin/brin_minmax_multi.c
+++ b/src/backend/access/brin/brin_minmax_multi.c
@@ -2020,7 +2020,7 @@ brin_minmax_multi_distance_numeric(PG_FUNCTION_ARGS)
 
 	d = DirectFunctionCall2(numeric_sub, a2, a1);	/* a2 - a1 */
 
-	PG_RETURN_FLOAT8(DirectFunctionCall1(numeric_float8, d));
+	PG_RETURN_FLOAT8(numeric_to_float8(DatumGetNumeric(d)));
 }
 
 /*
diff --git a/src/backend/utils/adt/jsonb.c b/src/backend/utils/adt/jsonb.c
index 58f45afb9e2..7b295c199d8 100644
--- a/src/backend/utils/adt/jsonb.c
+++ b/src/backend/utils/adt/jsonb.c
@@ -2209,17 +2209,16 @@ jsonb_float8(PG_FUNCTION_ARGS)
 {
 	Jsonb	   *in = PG_GETARG_JSONB_P(0);
 	JsonbValue	v;
-	Datum		retValue;
+	float8		retValue;
 
 	if (!JsonbExtractScalar(&in->root, &v) || v.type != jbvNumeric)
 		cannotCastJsonbValue(v.type, "double precision");
 
-	retValue = DirectFunctionCall1(numeric_float8,
-								   NumericGetDatum(v.val.numeric));
+	retValue = numeric_to_float8(v.val.numeric);
 
 	PG_FREE_IF_COPY(in, 0);
 
-	PG_RETURN_DATUM(retValue);
+	PG_RETURN_FLOAT8(retValue);
 }
 
 /*
diff --git a/src/backend/utils/adt/jsonpath_exec.c b/src/backend/utils/adt/jsonpath_exec.c
index 2544c6b1551..8320a7d8249 100644
--- a/src/backend/utils/adt/jsonpath_exec.c
+++ b/src/backend/utils/adt/jsonpath_exec.c
@@ -1150,8 +1150,8 @@ executeItemOptUnwrapTarget(JsonPathExecContext *cxt, JsonPathItem *jsp,
 
 					jb = &jbv;
 					jb->type = jbvNumeric;
-					jb->val.numeric = DatumGetNumeric(DirectFunctionCall1(float8_numeric,
-																		  Float8GetDatum(val)));
+					jb->val.numeric = float8_to_numeric(val);
+
 					res = jperOk;
 				}
 
diff --git a/src/backend/utils/adt/numeric.c b/src/backend/utils/adt/numeric.c
index d4970e572aa..d83fe5a326d 100644
--- a/src/backend/utils/adt/numeric.c
+++ b/src/backend/utils/adt/numeric.c
@@ -4283,6 +4283,64 @@ numeric_to_uint64_type(Numeric num, char *typeName)
 	return result;
 }
 
+Numeric
+float8_to_numeric(float8 val)
+{
+	Numeric		res;
+	NumericVar	result;
+	char		buf[DBL_DIG + 100];
+
+	if (isnan(val))
+		return make_result(&const_nan);
+
+	if (isinf(val))
+	{
+		if (val < 0)
+			return make_result(&const_ninf);
+		else
+			return make_result(&const_pinf);
+	}
+
+	snprintf(buf, sizeof(buf), "%.*g", DBL_DIG, val);
+
+	init_var(&result);
+
+	/* Assume we need not worry about leading/trailing spaces */
+	(void) set_var_from_str(buf, buf, &result);
+
+	res = make_result(&result);
+
+	free_var(&result);
+
+	return res;
+}
+
+float8
+numeric_to_float8(Numeric num)
+{
+	char	   *tmp;
+	Datum		result;
+
+	if (NUMERIC_IS_SPECIAL(num))
+	{
+		if (NUMERIC_IS_PINF(num))
+			return get_float8_infinity();
+		else if (NUMERIC_IS_NINF(num))
+			return -get_float8_infinity();
+		else
+			return get_float8_nan();
+	}
+
+	tmp = DatumGetCString(DirectFunctionCall1(numeric_out,
+											  NumericGetDatum(num)));
+
+	result = DirectFunctionCall1(float8in, CStringGetDatum(tmp));
+
+	pfree(tmp);
+
+	return DatumGetFloat8(result);
+}
+
 Datum
 int4_numeric(PG_FUNCTION_ARGS)
 {
@@ -4464,33 +4522,8 @@ Datum
 float8_numeric(PG_FUNCTION_ARGS)
 {
 	float8		val = PG_GETARG_FLOAT8(0);
-	Numeric		res;
-	NumericVar	result;
-	char		buf[DBL_DIG + 100];
-
-	if (isnan(val))
-		PG_RETURN_NUMERIC(make_result(&const_nan));
-
-	if (isinf(val))
-	{
-		if (val < 0)
-			PG_RETURN_NUMERIC(make_result(&const_ninf));
-		else
-			PG_RETURN_NUMERIC(make_result(&const_pinf));
-	}
 
-	snprintf(buf, sizeof(buf), "%.*g", DBL_DIG, val);
-
-	init_var(&result);
-
-	/* Assume we need not worry about leading/trailing spaces */
-	(void) set_var_from_str(buf, buf, &result);
-
-	res = make_result(&result);
-
-	free_var(&result);
-
-	PG_RETURN_NUMERIC(res);
+	PG_RETURN_NUMERIC(float8_to_numeric(val));
 }
 
 
@@ -4498,27 +4531,8 @@ Datum
 numeric_float8(PG_FUNCTION_ARGS)
 {
 	Numeric		num = PG_GETARG_NUMERIC(0);
-	char	   *tmp;
-	Datum		result;
 
-	if (NUMERIC_IS_SPECIAL(num))
-	{
-		if (NUMERIC_IS_PINF(num))
-			PG_RETURN_FLOAT8(get_float8_infinity());
-		else if (NUMERIC_IS_NINF(num))
-			PG_RETURN_FLOAT8(-get_float8_infinity());
-		else
-			PG_RETURN_FLOAT8(get_float8_nan());
-	}
-
-	tmp = DatumGetCString(DirectFunctionCall1(numeric_out,
-											  NumericGetDatum(num)));
-
-	result = DirectFunctionCall1(float8in, CStringGetDatum(tmp));
-
-	pfree(tmp);
-
-	PG_RETURN_DATUM(result);
+	PG_RETURN_FLOAT8(numeric_to_float8(num));
 }
 
 
diff --git a/src/backend/utils/adt/rangetypes.c b/src/backend/utils/adt/rangetypes.c
index aa4c53e0ae1..8fb67387cc5 100644
--- a/src/backend/utils/adt/rangetypes.c
+++ b/src/backend/utils/adt/rangetypes.c
@@ -41,6 +41,7 @@
 #include "utils/lsyscache.h"
 #include "utils/rangetypes.h"
 #include "utils/timestamp.h"
+#include "utils/numeric.h"
 
 
 /* fn_extra cache entry for one of the range I/O functions */
@@ -1577,8 +1578,7 @@ numrange_subdiff(PG_FUNCTION_ARGS)
 
 	numresult = DirectFunctionCall2(numeric_sub, v1, v2);
 
-	floatresult = DatumGetFloat8(DirectFunctionCall1(numeric_float8,
-													 numresult));
+	floatresult = numeric_to_float8(DatumGetNumeric(numresult));
 
 	PG_RETURN_FLOAT8(floatresult);
 }
diff --git a/src/include/utils/numeric.h b/src/include/utils/numeric.h
index 11814ad828e..16aba38034e 100644
--- a/src/include/utils/numeric.h
+++ b/src/include/utils/numeric.h
@@ -77,6 +77,8 @@ extern int64 numeric_to_int64(Numeric num);
 extern int64 numeric_to_int64_type(Numeric num, char *typeName);
 extern Numeric int64_div_fast_to_numeric(int64 val1, int log10val2);
 extern uint64 numeric_to_uint64_type(Numeric num, char *typeName);
+extern Numeric float8_to_numeric(float8 val);
+extern float8 numeric_to_float8(Numeric num);
 
 extern Numeric numeric_add_opt_error(Numeric num1, Numeric num2,
 									 bool *have_error);
-- 
2.18.0

From 51ba68f09bfe6a729140c8f7e77c51f2b0af15a9 Mon Sep 17 00:00:00 2001
From: Amul Sul <amul.sul@enterprisedb.com>
Date: Fri, 22 Apr 2022 01:26:29 -0400
Subject: [PATCH v2 3/5] Add numeric_to_int64 by refactoring numeric_int8

---
 src/backend/utils/adt/cash.c    |  4 ++--
 src/backend/utils/adt/dbsize.c  |  6 +----
 src/backend/utils/adt/jsonb.c   |  7 +++---
 src/backend/utils/adt/numeric.c | 39 +++++++++++++++++++++++++++++++++
 src/include/utils/numeric.h     |  2 ++
 5 files changed, 47 insertions(+), 11 deletions(-)

diff --git a/src/backend/utils/adt/cash.c b/src/backend/utils/adt/cash.c
index f7e78fa1056..d05335dbd47 100644
--- a/src/backend/utils/adt/cash.c
+++ b/src/backend/utils/adt/cash.c
@@ -1108,8 +1108,8 @@ numeric_cash(PG_FUNCTION_ARGS)
 	numeric_scale = NumericGetDatum(int64_to_numeric(scale));
 	amount = DirectFunctionCall2(numeric_mul, amount, numeric_scale);
 
-	/* note that numeric_int8 will round to nearest integer for us */
-	result = DatumGetInt64(DirectFunctionCall1(numeric_int8, amount));
+	/* note that numeric_to_int64 will round to nearest integer for us */
+	result = numeric_to_int64_type(DatumGetNumeric(amount), "money");
 
 	PG_RETURN_CASH(result);
 }
diff --git a/src/backend/utils/adt/dbsize.c b/src/backend/utils/adt/dbsize.c
index 0576764ac4b..afd6a7ab623 100644
--- a/src/backend/utils/adt/dbsize.c
+++ b/src/backend/utils/adt/dbsize.c
@@ -701,7 +701,6 @@ pg_size_bytes(PG_FUNCTION_ARGS)
 			   *endptr;
 	char		saved_char;
 	Numeric		num;
-	int64		result;
 	bool		have_digits = false;
 
 	str = text_to_cstring(arg);
@@ -826,10 +825,7 @@ pg_size_bytes(PG_FUNCTION_ARGS)
 		}
 	}
 
-	result = DatumGetInt64(DirectFunctionCall1(numeric_int8,
-											   NumericGetDatum(num)));
-
-	PG_RETURN_INT64(result);
+	PG_RETURN_INT64(numeric_to_int64(num));
 }
 
 /*
diff --git a/src/backend/utils/adt/jsonb.c b/src/backend/utils/adt/jsonb.c
index 26d81366c9f..58f45afb9e2 100644
--- a/src/backend/utils/adt/jsonb.c
+++ b/src/backend/utils/adt/jsonb.c
@@ -2174,17 +2174,16 @@ jsonb_int8(PG_FUNCTION_ARGS)
 {
 	Jsonb	   *in = PG_GETARG_JSONB_P(0);
 	JsonbValue	v;
-	Datum		retValue;
+	int64		retValue;
 
 	if (!JsonbExtractScalar(&in->root, &v) || v.type != jbvNumeric)
 		cannotCastJsonbValue(v.type, "bigint");
 
-	retValue = DirectFunctionCall1(numeric_int8,
-								   NumericGetDatum(v.val.numeric));
+	retValue = numeric_to_int64(v.val.numeric);
 
 	PG_FREE_IF_COPY(in, 0);
 
-	PG_RETURN_DATUM(retValue);
+	PG_RETURN_INT64(retValue);
 }
 
 Datum
diff --git a/src/backend/utils/adt/numeric.c b/src/backend/utils/adt/numeric.c
index 3157df7cd7b..d4970e572aa 100644
--- a/src/backend/utils/adt/numeric.c
+++ b/src/backend/utils/adt/numeric.c
@@ -4150,6 +4150,45 @@ int64_to_numeric(int64 val)
 	return res;
 }
 
+int64
+numeric_to_int64(Numeric num)
+{
+	return numeric_to_int64_type(num, "bigint");
+}
+
+/*
+ * typeName is the user-visible type name of uint64 used for the error
+ * reporting.
+ */
+int64
+numeric_to_int64_type(Numeric num, char *typeName)
+{
+	NumericVar	x;
+	int64		result;
+
+	if (NUMERIC_IS_SPECIAL(num))
+	{
+		if (NUMERIC_IS_NAN(num))
+			ereport(ERROR,
+					(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+					 errmsg("cannot convert NaN to %s", typeName)));
+		else
+			ereport(ERROR,
+					(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+					 errmsg("cannot convert infinity to %s", typeName)));
+	}
+
+	/* Convert to variable format and thence to int8 */
+	init_var_from_num(num, &x);
+
+	if (!numericvar_to_int64(&x, &result))
+		ereport(ERROR,
+				(errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
+				 errmsg("%s out of range", typeName)));
+
+	return result;
+}
+
 /*
  * Convert val1/(10**val2) to numeric.  This is much faster than normal
  * numeric division.
diff --git a/src/include/utils/numeric.h b/src/include/utils/numeric.h
index 1b9ad3b5b58..11814ad828e 100644
--- a/src/include/utils/numeric.h
+++ b/src/include/utils/numeric.h
@@ -73,6 +73,8 @@ extern char *numeric_out_sci(Numeric num, int scale);
 extern char *numeric_normalize(Numeric num);
 
 extern Numeric int64_to_numeric(int64 val);
+extern int64 numeric_to_int64(Numeric num);
+extern int64 numeric_to_int64_type(Numeric num, char *typeName);
 extern Numeric int64_div_fast_to_numeric(int64 val1, int log10val2);
 extern uint64 numeric_to_uint64_type(Numeric num, char *typeName);
 
-- 
2.18.0

Reply via email to