Peter Eisentraut <peter.eisentr...@2ndquadrant.com> writes: > Here is a new patch series version. > I have created a new internal function for converting integers to > numeric, to make the implementation a bit more elegant and compact.
I reviewed the 0002 patch, finding one bug (in int8_sum) and a few more calls of int8_numeric that could be converted. I think the attached updated version is committable, and I'd recommend going ahead with that regardless of the rest of this. I hadn't realized how many random calls of int8_numeric and int4_numeric we'd grown, but there are a lot, so this is nice cleanup. I continue to think that we can't commit 0003 in this form, because of the breakage that will ensure in stored views. As I said upthread, we should leave the existing SQL-exposed functions alone, invent new ones that return numeric, and alter the parser to translate EXTRACT constructs to the new functions. This approach would also provide an "out" for anyone who does complain about the performance cost --- they can just continue to use the old functions. regards, tom lane
diff --git a/contrib/btree_gist/btree_numeric.c b/contrib/btree_gist/btree_numeric.c index d66901680e..35e466cdd9 100644 --- a/contrib/btree_gist/btree_numeric.c +++ b/contrib/btree_gist/btree_numeric.c @@ -195,7 +195,7 @@ gbt_numeric_penalty(PG_FUNCTION_ARGS) } else { - Numeric nul = DatumGetNumeric(DirectFunctionCall1(int4_numeric, Int32GetDatum(0))); + Numeric nul = int64_to_numeric(0); *result = 0.0; diff --git a/contrib/jsonb_plperl/jsonb_plperl.c b/contrib/jsonb_plperl/jsonb_plperl.c index b81ba54b80..22e90afe1b 100644 --- a/contrib/jsonb_plperl/jsonb_plperl.c +++ b/contrib/jsonb_plperl/jsonb_plperl.c @@ -216,9 +216,7 @@ SV_to_JsonbValue(SV *in, JsonbParseState **jsonb_state, bool is_elem) IV ival = SvIV(in); out.type = jbvNumeric; - out.val.numeric = - DatumGetNumeric(DirectFunctionCall1(int8_numeric, - Int64GetDatum((int64) ival))); + out.val.numeric = int64_to_numeric(ival); } else if (SvNOK(in)) { diff --git a/src/backend/utils/adt/cash.c b/src/backend/utils/adt/cash.c index 6515fc8ec6..d093ce8038 100644 --- a/src/backend/utils/adt/cash.c +++ b/src/backend/utils/adt/cash.c @@ -1042,7 +1042,7 @@ cash_numeric(PG_FUNCTION_ARGS) fpoint = 2; /* convert the integral money value to numeric */ - result = DirectFunctionCall1(int8_numeric, Int64GetDatum(money)); + result = NumericGetDatum(int64_to_numeric(money)); /* scale appropriately, if needed */ if (fpoint > 0) @@ -1056,8 +1056,7 @@ cash_numeric(PG_FUNCTION_ARGS) scale = 1; for (i = 0; i < fpoint; i++) scale *= 10; - numeric_scale = DirectFunctionCall1(int8_numeric, - Int64GetDatum(scale)); + numeric_scale = NumericGetDatum(int64_to_numeric(scale)); /* * Given integral inputs approaching INT64_MAX, select_div_scale() @@ -1107,7 +1106,7 @@ numeric_cash(PG_FUNCTION_ARGS) scale *= 10; /* multiply the input amount by scale factor */ - numeric_scale = DirectFunctionCall1(int8_numeric, Int64GetDatum(scale)); + 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 */ diff --git a/src/backend/utils/adt/dbsize.c b/src/backend/utils/adt/dbsize.c index 2320c06a9b..7def7392b9 100644 --- a/src/backend/utils/adt/dbsize.c +++ b/src/backend/utils/adt/dbsize.c @@ -579,14 +579,6 @@ numeric_to_cstring(Numeric n) return DatumGetCString(DirectFunctionCall1(numeric_out, d)); } -static Numeric -int64_to_numeric(int64 v) -{ - Datum d = Int64GetDatum(v); - - return DatumGetNumeric(DirectFunctionCall1(int8_numeric, d)); -} - static bool numeric_is_less(Numeric a, Numeric b) { @@ -615,9 +607,9 @@ numeric_half_rounded(Numeric n) Datum two; Datum result; - zero = DirectFunctionCall1(int8_numeric, Int64GetDatum(0)); - one = DirectFunctionCall1(int8_numeric, Int64GetDatum(1)); - two = DirectFunctionCall1(int8_numeric, Int64GetDatum(2)); + zero = NumericGetDatum(int64_to_numeric(0)); + one = NumericGetDatum(int64_to_numeric(1)); + two = NumericGetDatum(int64_to_numeric(2)); if (DatumGetBool(DirectFunctionCall2(numeric_ge, d, zero))) d = DirectFunctionCall2(numeric_add, d, one); @@ -632,12 +624,10 @@ static Numeric numeric_shift_right(Numeric n, unsigned count) { Datum d = NumericGetDatum(n); - Datum divisor_int64; Datum divisor_numeric; Datum result; - divisor_int64 = Int64GetDatum((int64) (1 << count)); - divisor_numeric = DirectFunctionCall1(int8_numeric, divisor_int64); + divisor_numeric = NumericGetDatum(int64_to_numeric(1 << count)); result = DirectFunctionCall2(numeric_div_trunc, d, divisor_numeric); return DatumGetNumeric(result); } @@ -832,8 +822,7 @@ pg_size_bytes(PG_FUNCTION_ARGS) { Numeric mul_num; - mul_num = DatumGetNumeric(DirectFunctionCall1(int8_numeric, - Int64GetDatum(multiplier))); + mul_num = int64_to_numeric(multiplier); num = DatumGetNumeric(DirectFunctionCall2(numeric_mul, NumericGetDatum(mul_num), diff --git a/src/backend/utils/adt/formatting.c b/src/backend/utils/adt/formatting.c index 7d09537d82..f9aa968f09 100644 --- a/src/backend/utils/adt/formatting.c +++ b/src/backend/utils/adt/formatting.c @@ -6070,10 +6070,8 @@ numeric_to_number(PG_FUNCTION_ARGS) if (IS_MULTI(&Num)) { Numeric x; - Numeric a = DatumGetNumeric(DirectFunctionCall1(int4_numeric, - Int32GetDatum(10))); - Numeric b = DatumGetNumeric(DirectFunctionCall1(int4_numeric, - Int32GetDatum(-Num.multi))); + Numeric a = int64_to_numeric(10); + Numeric b = int64_to_numeric(-Num.multi); x = DatumGetNumeric(DirectFunctionCall2(numeric_power, NumericGetDatum(a), @@ -6162,10 +6160,8 @@ numeric_to_char(PG_FUNCTION_ARGS) if (IS_MULTI(&Num)) { - Numeric a = DatumGetNumeric(DirectFunctionCall1(int4_numeric, - Int32GetDatum(10))); - Numeric b = DatumGetNumeric(DirectFunctionCall1(int4_numeric, - Int32GetDatum(Num.multi))); + Numeric a = int64_to_numeric(10); + Numeric b = int64_to_numeric(Num.multi); x = DatumGetNumeric(DirectFunctionCall2(numeric_power, NumericGetDatum(a), @@ -6339,11 +6335,8 @@ int8_to_char(PG_FUNCTION_ARGS) else if (IS_EEEE(&Num)) { /* to avoid loss of precision, must go via numeric not float8 */ - Numeric val; - - val = DatumGetNumeric(DirectFunctionCall1(int8_numeric, - Int64GetDatum(value))); - orgnum = numeric_out_sci(val, Num.post); + orgnum = numeric_out_sci(int64_to_numeric(value), + Num.post); /* * numeric_out_sci() does not emit a sign for positive numbers. We diff --git a/src/backend/utils/adt/jsonpath_exec.c b/src/backend/utils/adt/jsonpath_exec.c index f146767bfc..7403c760b4 100644 --- a/src/backend/utils/adt/jsonpath_exec.c +++ b/src/backend/utils/adt/jsonpath_exec.c @@ -842,9 +842,7 @@ executeItemOptUnwrapTarget(JsonPathExecContext *cxt, JsonPathItem *jsp, lastjbv = hasNext ? &tmpjbv : palloc(sizeof(*lastjbv)); lastjbv->type = jbvNumeric; - lastjbv->val.numeric = - DatumGetNumeric(DirectFunctionCall1(int4_numeric, - Int32GetDatum(last))); + lastjbv->val.numeric = int64_to_numeric(last); res = executeNextItem(cxt, jsp, &elem, lastjbv, found, hasNext); @@ -1012,9 +1010,7 @@ executeItemOptUnwrapTarget(JsonPathExecContext *cxt, JsonPathItem *jsp, jb = palloc(sizeof(*jb)); jb->type = jbvNumeric; - jb->val.numeric = - DatumGetNumeric(DirectFunctionCall1(int4_numeric, - Int32GetDatum(size))); + jb->val.numeric = int64_to_numeric(size); res = executeNextItem(cxt, jsp, NULL, jb, found, false); } @@ -1979,8 +1975,7 @@ executeKeyValueMethod(JsonPathExecContext *cxt, JsonPathItem *jsp, id += (int64) cxt->baseObject.id * INT64CONST(10000000000); idval.type = jbvNumeric; - idval.val.numeric = DatumGetNumeric(DirectFunctionCall1(int8_numeric, - Int64GetDatum(id))); + idval.val.numeric = int64_to_numeric(id); it = JsonbIteratorInit(jbc); diff --git a/src/backend/utils/adt/numeric.c b/src/backend/utils/adt/numeric.c index ed825a1fdd..d2cc74b284 100644 --- a/src/backend/utils/adt/numeric.c +++ b/src/backend/utils/adt/numeric.c @@ -4073,23 +4073,29 @@ numeric_trim_scale(PG_FUNCTION_ARGS) * ---------------------------------------------------------------------- */ - -Datum -int4_numeric(PG_FUNCTION_ARGS) +Numeric +int64_to_numeric(int64 val) { - int32 val = PG_GETARG_INT32(0); Numeric res; NumericVar result; init_var(&result); - int64_to_numericvar((int64) val, &result); + int64_to_numericvar(val, &result); res = make_result(&result); free_var(&result); - PG_RETURN_NUMERIC(res); + return res; +} + +Datum +int4_numeric(PG_FUNCTION_ARGS) +{ + int32 val = PG_GETARG_INT32(0); + + PG_RETURN_NUMERIC(int64_to_numeric(val)); } int32 @@ -4174,18 +4180,8 @@ Datum int8_numeric(PG_FUNCTION_ARGS) { int64 val = PG_GETARG_INT64(0); - Numeric res; - NumericVar result; - init_var(&result); - - int64_to_numericvar(val, &result); - - res = make_result(&result); - - free_var(&result); - - PG_RETURN_NUMERIC(res); + PG_RETURN_NUMERIC(int64_to_numeric(val)); } @@ -4224,18 +4220,8 @@ Datum int2_numeric(PG_FUNCTION_ARGS) { int16 val = PG_GETARG_INT16(0); - Numeric res; - NumericVar result; - - init_var(&result); - - int64_to_numericvar((int64) val, &result); - - res = make_result(&result); - free_var(&result); - - PG_RETURN_NUMERIC(res); + PG_RETURN_NUMERIC(int64_to_numeric(val)); } @@ -5290,11 +5276,7 @@ int2_accum(PG_FUNCTION_ARGS) #ifdef HAVE_INT128 do_int128_accum(state, (int128) PG_GETARG_INT16(1)); #else - Numeric newval; - - newval = DatumGetNumeric(DirectFunctionCall1(int2_numeric, - PG_GETARG_DATUM(1))); - do_numeric_accum(state, newval); + do_numeric_accum(state, int64_to_numeric(PG_GETARG_INT16(1))); #endif } @@ -5317,11 +5299,7 @@ int4_accum(PG_FUNCTION_ARGS) #ifdef HAVE_INT128 do_int128_accum(state, (int128) PG_GETARG_INT32(1)); #else - Numeric newval; - - newval = DatumGetNumeric(DirectFunctionCall1(int4_numeric, - PG_GETARG_DATUM(1))); - do_numeric_accum(state, newval); + do_numeric_accum(state, int64_to_numeric(PG_GETARG_INT32(1))); #endif } @@ -5340,13 +5318,7 @@ int8_accum(PG_FUNCTION_ARGS) state = makeNumericAggState(fcinfo, true); if (!PG_ARGISNULL(1)) - { - Numeric newval; - - newval = DatumGetNumeric(DirectFunctionCall1(int8_numeric, - PG_GETARG_DATUM(1))); - do_numeric_accum(state, newval); - } + do_numeric_accum(state, int64_to_numeric(PG_GETARG_INT64(1))); PG_RETURN_POINTER(state); } @@ -5570,11 +5542,7 @@ int8_avg_accum(PG_FUNCTION_ARGS) #ifdef HAVE_INT128 do_int128_accum(state, (int128) PG_GETARG_INT64(1)); #else - Numeric newval; - - newval = DatumGetNumeric(DirectFunctionCall1(int8_numeric, - PG_GETARG_DATUM(1))); - do_numeric_accum(state, newval); + do_numeric_accum(state, int64_to_numeric(PG_GETARG_INT64(1))); #endif } @@ -5767,13 +5735,8 @@ int2_accum_inv(PG_FUNCTION_ARGS) #ifdef HAVE_INT128 do_int128_discard(state, (int128) PG_GETARG_INT16(1)); #else - Numeric newval; - - newval = DatumGetNumeric(DirectFunctionCall1(int2_numeric, - PG_GETARG_DATUM(1))); - /* Should never fail, all inputs have dscale 0 */ - if (!do_numeric_discard(state, newval)) + if (!do_numeric_discard(state, int64_to_numeric(PG_GETARG_INT16(1)))) elog(ERROR, "do_numeric_discard failed unexpectedly"); #endif } @@ -5797,13 +5760,8 @@ int4_accum_inv(PG_FUNCTION_ARGS) #ifdef HAVE_INT128 do_int128_discard(state, (int128) PG_GETARG_INT32(1)); #else - Numeric newval; - - newval = DatumGetNumeric(DirectFunctionCall1(int4_numeric, - PG_GETARG_DATUM(1))); - /* Should never fail, all inputs have dscale 0 */ - if (!do_numeric_discard(state, newval)) + if (!do_numeric_discard(state, int64_to_numeric(PG_GETARG_INT32(1)))) elog(ERROR, "do_numeric_discard failed unexpectedly"); #endif } @@ -5824,13 +5782,8 @@ int8_accum_inv(PG_FUNCTION_ARGS) if (!PG_ARGISNULL(1)) { - Numeric newval; - - newval = DatumGetNumeric(DirectFunctionCall1(int8_numeric, - PG_GETARG_DATUM(1))); - /* Should never fail, all inputs have dscale 0 */ - if (!do_numeric_discard(state, newval)) + if (!do_numeric_discard(state, int64_to_numeric(PG_GETARG_INT64(1)))) elog(ERROR, "do_numeric_discard failed unexpectedly"); } @@ -5853,13 +5806,8 @@ int8_avg_accum_inv(PG_FUNCTION_ARGS) #ifdef HAVE_INT128 do_int128_discard(state, (int128) PG_GETARG_INT64(1)); #else - Numeric newval; - - newval = DatumGetNumeric(DirectFunctionCall1(int8_numeric, - PG_GETARG_DATUM(1))); - /* Should never fail, all inputs have dscale 0 */ - if (!do_numeric_discard(state, newval)) + if (!do_numeric_discard(state, int64_to_numeric(PG_GETARG_INT64(1)))) elog(ERROR, "do_numeric_discard failed unexpectedly"); #endif } @@ -5914,8 +5862,7 @@ numeric_poly_avg(PG_FUNCTION_ARGS) int128_to_numericvar(state->sumX, &result); - countd = DirectFunctionCall1(int8_numeric, - Int64GetDatumFast(state->N)); + countd = NumericGetDatum(int64_to_numeric(state->N)); sumd = NumericGetDatum(make_result(&result)); free_var(&result); @@ -5951,7 +5898,7 @@ numeric_avg(PG_FUNCTION_ARGS) if (state->nInfcount > 0) PG_RETURN_NUMERIC(make_result(&const_ninf)); - N_datum = DirectFunctionCall1(int8_numeric, Int64GetDatum(state->N)); + N_datum = NumericGetDatum(int64_to_numeric(state->N)); init_var(&sumX_var); accum_sum_final(&state->sumX, &sumX_var); @@ -6411,7 +6358,6 @@ Datum int8_sum(PG_FUNCTION_ARGS) { Numeric oldsum; - Datum newval; if (PG_ARGISNULL(0)) { @@ -6419,8 +6365,7 @@ int8_sum(PG_FUNCTION_ARGS) if (PG_ARGISNULL(1)) PG_RETURN_NULL(); /* still no non-null */ /* This is the first non-null input. */ - newval = DirectFunctionCall1(int8_numeric, PG_GETARG_DATUM(1)); - PG_RETURN_DATUM(newval); + PG_RETURN_NUMERIC(int64_to_numeric(PG_GETARG_INT64(1))); } /* @@ -6436,10 +6381,9 @@ int8_sum(PG_FUNCTION_ARGS) PG_RETURN_NUMERIC(oldsum); /* OK to do the addition. */ - newval = DirectFunctionCall1(int8_numeric, PG_GETARG_DATUM(1)); - PG_RETURN_DATUM(DirectFunctionCall2(numeric_add, - NumericGetDatum(oldsum), newval)); + NumericGetDatum(oldsum), + NumericGetDatum(int64_to_numeric(PG_GETARG_INT64(1))))); } @@ -6618,10 +6562,8 @@ int8_avg(PG_FUNCTION_ARGS) if (transdata->count == 0) PG_RETURN_NULL(); - countd = DirectFunctionCall1(int8_numeric, - Int64GetDatumFast(transdata->count)); - sumd = DirectFunctionCall1(int8_numeric, - Int64GetDatumFast(transdata->sum)); + countd = NumericGetDatum(int64_to_numeric(transdata->count)); + sumd = NumericGetDatum(int64_to_numeric(transdata->sum)); PG_RETURN_DATUM(DirectFunctionCall2(numeric_div, sumd, countd)); } diff --git a/src/include/utils/numeric.h b/src/include/utils/numeric.h index 0b7d4ba3c4..2a768b9a04 100644 --- a/src/include/utils/numeric.h +++ b/src/include/utils/numeric.h @@ -62,6 +62,8 @@ int32 numeric_maximum_size(int32 typmod); extern char *numeric_out_sci(Numeric num, int scale); extern char *numeric_normalize(Numeric num); +extern Numeric int64_to_numeric(int64 val); + extern Numeric numeric_add_opt_error(Numeric num1, Numeric num2, bool *have_error); extern Numeric numeric_sub_opt_error(Numeric num1, Numeric num2,