Change seconds argument of make_*() functions to numeric
Started by Peter Eisentrautabout 5 years ago3 messages
Along with the discussed change of the return type of EXTRACT from
float8 to numeric [0]/messages/by-id/42b73d2d-da12-ba9f-570a-420e0cce19d9@phystech.edu, I was looking around what other date/time APIs
might be using float arguments or return values. The only thing left
appears to be the functions make_time, make_timestamp, make_timestamptz,
and make_interval, which take an argument specifying the seconds, which
has type float8 right now. I'm proposing the attached patch to change
that to numeric.
Can we change the arguments, as proposed here, or do we need to add
separate overloaded versions and leave the existing versions in place?
[0]: /messages/by-id/42b73d2d-da12-ba9f-570a-420e0cce19d9@phystech.edu
/messages/by-id/42b73d2d-da12-ba9f-570a-420e0cce19d9@phystech.edu
Attachments:
0001-Change-seconds-argument-of-make_-functions-to-numeri.patchtext/plain; charset=UTF-8; name=0001-Change-seconds-argument-of-make_-functions-to-numeri.patch; x-mac-creator=0; x-mac-type=0Download
From 4b9f7e7d46d26f2c163abdfbb40dabcb6d2927e9 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter@eisentraut.org>
Date: Wed, 16 Dec 2020 18:08:57 +0100
Subject: [PATCH] Change seconds argument of make_*() functions to numeric
Change the seconds argument of
- make_time
- make_timestamp
- make_timestamptz
- make_interval
from float8 to numeric.
---
doc/src/sgml/func.sgml | 8 +++---
src/backend/catalog/system_views.sql | 2 +-
src/backend/utils/adt/date.c | 38 +++++++++++++++-----------
src/backend/utils/adt/numeric.c | 16 +++++++----
src/backend/utils/adt/timestamp.c | 40 +++++++++++++++-------------
src/include/catalog/pg_proc.dat | 10 +++----
src/include/utils/date.h | 3 ++-
src/include/utils/numeric.h | 1 +
8 files changed, 67 insertions(+), 51 deletions(-)
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index df29af6371..cc5e5a257e 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -8925,7 +8925,7 @@ <title>Date/Time Functions</title>
<optional>, <parameter>days</parameter> <type>int</type>
<optional>, <parameter>hours</parameter> <type>int</type>
<optional>, <parameter>mins</parameter> <type>int</type>
- <optional>, <parameter>secs</parameter> <type>double precision</type>
+ <optional>, <parameter>secs</parameter> <type>numeric</type>
</optional></optional></optional></optional></optional></optional></optional> )
<returnvalue>interval</returnvalue>
</para>
@@ -8946,7 +8946,7 @@ <title>Date/Time Functions</title>
</indexterm>
<function>make_time</function> ( <parameter>hour</parameter> <type>int</type>,
<parameter>min</parameter> <type>int</type>,
- <parameter>sec</parameter> <type>double precision</type> )
+ <parameter>sec</parameter> <type>numeric</type> )
<returnvalue>time</returnvalue>
</para>
<para>
@@ -8968,7 +8968,7 @@ <title>Date/Time Functions</title>
<parameter>day</parameter> <type>int</type>,
<parameter>hour</parameter> <type>int</type>,
<parameter>min</parameter> <type>int</type>,
- <parameter>sec</parameter> <type>double precision</type> )
+ <parameter>sec</parameter> <type>numeric</type> )
<returnvalue>timestamp</returnvalue>
</para>
<para>
@@ -8991,7 +8991,7 @@ <title>Date/Time Functions</title>
<parameter>day</parameter> <type>int</type>,
<parameter>hour</parameter> <type>int</type>,
<parameter>min</parameter> <type>int</type>,
- <parameter>sec</parameter> <type>double precision</type>
+ <parameter>sec</parameter> <type>numeric</type>
<optional>, <parameter>timezone</parameter> <type>text</type> </optional> )
<returnvalue>timestamp with time zone</returnvalue>
</para>
diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql
index b140c210bc..ec1fd22004 100644
--- a/src/backend/catalog/system_views.sql
+++ b/src/backend/catalog/system_views.sql
@@ -1309,7 +1309,7 @@ CREATE OR REPLACE FUNCTION pg_create_logical_replication_slot(
CREATE OR REPLACE FUNCTION
make_interval(years int4 DEFAULT 0, months int4 DEFAULT 0, weeks int4 DEFAULT 0,
days int4 DEFAULT 0, hours int4 DEFAULT 0, mins int4 DEFAULT 0,
- secs double precision DEFAULT 0.0)
+ secs numeric DEFAULT 0)
RETURNS interval
LANGUAGE INTERNAL
STRICT IMMUTABLE PARALLEL SAFE
diff --git a/src/backend/utils/adt/date.c b/src/backend/utils/adt/date.c
index a470cf890a..c966637c4e 100644
--- a/src/backend/utils/adt/date.c
+++ b/src/backend/utils/adt/date.c
@@ -31,6 +31,7 @@
#include "utils/builtins.h"
#include "utils/date.h"
#include "utils/datetime.h"
+#include "utils/numeric.h"
#include "utils/sortsupport.h"
/*
@@ -1266,26 +1267,30 @@ time_overflows(int hour, int min, int sec, fsec_t fsec)
return false;
}
-/* float_time_overflows()
- * Same, when we have seconds + fractional seconds as one "double" value.
+/* numeric_time_overflows()
+ * Same, when we have seconds + fractional seconds as one "numeric" value.
*/
bool
-float_time_overflows(int hour, int min, double sec)
+numeric_time_overflows(int hour, int min, Numeric sec)
{
+ int32 usec;
+
/* Range-check the fields individually. */
if (hour < 0 || hour > HOURS_PER_DAY ||
min < 0 || min >= MINS_PER_HOUR)
return true;
/*
- * "sec", being double, requires extra care. Cope with NaN, and round off
- * before applying the range check to avoid unexpected errors due to
- * imprecise input. (We assume rint() behaves sanely with infinities.)
+ * "sec", being double, requires extra care. Cope with NaN and infinity,
+ * and round off before applying the range check to avoid unexpected
+ * errors due to imprecise input.
*/
- if (isnan(sec))
+ if (numeric_is_nan(sec))
+ return true;
+ if (numeric_is_inf(sec))
return true;
- sec = rint(sec * USECS_PER_SEC);
- if (sec < 0 || sec > SECS_PER_MINUTE * USECS_PER_SEC)
+ usec = numeric_int4_opt_error(numeric_mul_opt_error(sec, int64_to_numeric(USECS_PER_SEC), NULL), NULL);
+ if (usec < 0 || usec > SECS_PER_MINUTE * USECS_PER_SEC)
return true;
/*
@@ -1294,7 +1299,7 @@ float_time_overflows(int hour, int min, double sec)
* way that callers will convert the fields to a time.
*/
if (((((hour * MINS_PER_HOUR + min) * SECS_PER_MINUTE)
- * USECS_PER_SEC) + (int64) sec) > USECS_PER_DAY)
+ * USECS_PER_SEC) + (int64) usec) > USECS_PER_DAY)
return true;
return false;
@@ -1402,19 +1407,20 @@ make_time(PG_FUNCTION_ARGS)
{
int tm_hour = PG_GETARG_INT32(0);
int tm_min = PG_GETARG_INT32(1);
- double sec = PG_GETARG_FLOAT8(2);
+ Numeric sec = PG_GETARG_NUMERIC(2);
TimeADT time;
/* Check for time overflow */
- if (float_time_overflows(tm_hour, tm_min, sec))
+ if (numeric_time_overflows(tm_hour, tm_min, sec))
ereport(ERROR,
(errcode(ERRCODE_DATETIME_FIELD_OVERFLOW),
- errmsg("time field value out of range: %d:%02d:%02g",
- tm_hour, tm_min, sec)));
+ errmsg("time field value out of range: %d:%02d:%s",
+ tm_hour, tm_min, numeric_normalize(sec))));
/* This should match tm2time */
- time = (((tm_hour * MINS_PER_HOUR + tm_min) * SECS_PER_MINUTE)
- * USECS_PER_SEC) + (int64) rint(sec * USECS_PER_SEC);
+ time = (((tm_hour * MINS_PER_HOUR + tm_min) * SECS_PER_MINUTE)* USECS_PER_SEC)
+ /* round(sec * USECS_PER_SEC, 0) */
+ + numeric_int4_opt_error(numeric_mul_opt_error(sec, int64_to_numeric(USECS_PER_SEC), NULL), NULL);
PG_RETURN_TIMEADT(time);
}
diff --git a/src/backend/utils/adt/numeric.c b/src/backend/utils/adt/numeric.c
index 20c9cac2fa..6da42aa406 100644
--- a/src/backend/utils/adt/numeric.c
+++ b/src/backend/utils/adt/numeric.c
@@ -4186,11 +4186,9 @@ int8_numeric(PG_FUNCTION_ARGS)
PG_RETURN_NUMERIC(int64_to_numeric(val));
}
-
-Datum
-numeric_int8(PG_FUNCTION_ARGS)
+int64
+numeric_int8_opt_error(Numeric num, bool *error)
{
- Numeric num = PG_GETARG_NUMERIC(0);
NumericVar x;
int64 result;
@@ -4214,7 +4212,15 @@ numeric_int8(PG_FUNCTION_ARGS)
(errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
errmsg("bigint out of range")));
- PG_RETURN_INT64(result);
+ return result;
+}
+
+Datum
+numeric_int8(PG_FUNCTION_ARGS)
+{
+ Numeric num = PG_GETARG_NUMERIC(0);
+
+ PG_RETURN_INT64(numeric_int8_opt_error(num, NULL));
}
diff --git a/src/backend/utils/adt/timestamp.c b/src/backend/utils/adt/timestamp.c
index 2dbd309122..5a1c08e96b 100644
--- a/src/backend/utils/adt/timestamp.c
+++ b/src/backend/utils/adt/timestamp.c
@@ -35,6 +35,7 @@
#include "utils/date.h"
#include "utils/datetime.h"
#include "utils/float.h"
+#include "utils/numeric.h"
/*
* gcc's -ffast-math switch breaks routines that expect exact results from
@@ -550,7 +551,7 @@ parse_sane_timezone(struct pg_tm *tm, text *zone)
*/
static Timestamp
make_timestamp_internal(int year, int month, int day,
- int hour, int min, double sec)
+ int hour, int min, Numeric sec)
{
struct pg_tm tm;
TimeOffset date;
@@ -587,24 +588,25 @@ make_timestamp_internal(int year, int month, int day,
date = date2j(tm.tm_year, tm.tm_mon, tm.tm_mday) - POSTGRES_EPOCH_JDATE;
/* Check for time overflow */
- if (float_time_overflows(hour, min, sec))
+ if (numeric_time_overflows(hour, min, sec))
ereport(ERROR,
(errcode(ERRCODE_DATETIME_FIELD_OVERFLOW),
- errmsg("time field value out of range: %d:%02d:%02g",
- hour, min, sec)));
+ errmsg("time field value out of range: %d:%02d:%s",
+ hour, min, numeric_normalize(sec))));
/* This should match tm2time */
- time = (((hour * MINS_PER_HOUR + min) * SECS_PER_MINUTE)
- * USECS_PER_SEC) + (int64) rint(sec * USECS_PER_SEC);
+ time = (((hour * MINS_PER_HOUR + min) * SECS_PER_MINUTE) * USECS_PER_SEC)
+ /* round(sec * USECS_PER_SEC, 0) */
+ + numeric_int4_opt_error(numeric_mul_opt_error(sec, int64_to_numeric(USECS_PER_SEC), NULL), NULL);
result = date * USECS_PER_DAY + time;
/* check for major overflow */
if ((result - time) / USECS_PER_DAY != date)
ereport(ERROR,
(errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
- errmsg("timestamp out of range: %d-%02d-%02d %d:%02d:%02g",
+ errmsg("timestamp out of range: %d-%02d-%02d %d:%02d:%s",
year, month, day,
- hour, min, sec)));
+ hour, min, numeric_normalize(sec))));
/* check for just-barely overflow (okay except time-of-day wraps) */
/* caution: we want to allow 1999-12-31 24:00:00 */
@@ -612,17 +614,17 @@ make_timestamp_internal(int year, int month, int day,
(result > 0 && date < -1))
ereport(ERROR,
(errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
- errmsg("timestamp out of range: %d-%02d-%02d %d:%02d:%02g",
+ errmsg("timestamp out of range: %d-%02d-%02d %d:%02d:%s",
year, month, day,
- hour, min, sec)));
+ hour, min, numeric_normalize(sec))));
/* final range check catches just-out-of-range timestamps */
if (!IS_VALID_TIMESTAMP(result))
ereport(ERROR,
(errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
- errmsg("timestamp out of range: %d-%02d-%02d %d:%02d:%02g",
+ errmsg("timestamp out of range: %d-%02d-%02d %d:%02d:%s",
year, month, day,
- hour, min, sec)));
+ hour, min, numeric_normalize(sec))));
return result;
}
@@ -638,7 +640,7 @@ make_timestamp(PG_FUNCTION_ARGS)
int32 mday = PG_GETARG_INT32(2);
int32 hour = PG_GETARG_INT32(3);
int32 min = PG_GETARG_INT32(4);
- float8 sec = PG_GETARG_FLOAT8(5);
+ Numeric sec = PG_GETARG_NUMERIC(5);
Timestamp result;
result = make_timestamp_internal(year, month, mday,
@@ -658,7 +660,7 @@ make_timestamptz(PG_FUNCTION_ARGS)
int32 mday = PG_GETARG_INT32(2);
int32 hour = PG_GETARG_INT32(3);
int32 min = PG_GETARG_INT32(4);
- float8 sec = PG_GETARG_FLOAT8(5);
+ Numeric sec = PG_GETARG_NUMERIC(5);
Timestamp result;
result = make_timestamp_internal(year, month, mday,
@@ -679,7 +681,7 @@ make_timestamptz_at_timezone(PG_FUNCTION_ARGS)
int32 mday = PG_GETARG_INT32(2);
int32 hour = PG_GETARG_INT32(3);
int32 min = PG_GETARG_INT32(4);
- float8 sec = PG_GETARG_FLOAT8(5);
+ Numeric sec = PG_GETARG_NUMERIC(5);
text *zone = PG_GETARG_TEXT_PP(6);
TimestampTz result;
Timestamp timestamp;
@@ -1500,14 +1502,14 @@ make_interval(PG_FUNCTION_ARGS)
int32 days = PG_GETARG_INT32(3);
int32 hours = PG_GETARG_INT32(4);
int32 mins = PG_GETARG_INT32(5);
- double secs = PG_GETARG_FLOAT8(6);
+ Numeric secs = PG_GETARG_NUMERIC(6);
Interval *result;
/*
* Reject out-of-range inputs. We really ought to check the integer
* inputs as well, but it's not entirely clear what limits to apply.
*/
- if (isinf(secs) || isnan(secs))
+ if (numeric_is_inf(secs) || numeric_is_nan(secs))
ereport(ERROR,
(errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
errmsg("interval out of range")));
@@ -1516,10 +1518,10 @@ make_interval(PG_FUNCTION_ARGS)
result->month = years * MONTHS_PER_YEAR + months;
result->day = weeks * 7 + days;
- secs = rint(secs * USECS_PER_SEC);
result->time = hours * ((int64) SECS_PER_HOUR * USECS_PER_SEC) +
mins * ((int64) SECS_PER_MINUTE * USECS_PER_SEC) +
- (int64) secs;
+ /* round(sec * USECS_PER_SEC, 0) */
+ numeric_int8_opt_error(numeric_mul_opt_error(secs, int64_to_numeric(USECS_PER_SEC), NULL), NULL);
PG_RETURN_INTERVAL_P(result);
}
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index e6c7b070f6..63dd511a5e 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -9996,26 +9996,26 @@
proargnames => '{year,month,day}', prosrc => 'make_date' },
{ oid => '3847', descr => 'construct time',
proname => 'make_time', prorettype => 'time',
- proargtypes => 'int4 int4 float8', proargnames => '{hour,min,sec}',
+ proargtypes => 'int4 int4 numeric', proargnames => '{hour,min,sec}',
prosrc => 'make_time' },
{ oid => '3461', descr => 'construct timestamp',
proname => 'make_timestamp', prorettype => 'timestamp',
- proargtypes => 'int4 int4 int4 int4 int4 float8',
+ proargtypes => 'int4 int4 int4 int4 int4 numeric',
proargnames => '{year,month,mday,hour,min,sec}', prosrc => 'make_timestamp' },
{ oid => '3462', descr => 'construct timestamp with time zone',
proname => 'make_timestamptz', provolatile => 's',
- prorettype => 'timestamptz', proargtypes => 'int4 int4 int4 int4 int4 float8',
+ prorettype => 'timestamptz', proargtypes => 'int4 int4 int4 int4 int4 numeric',
proargnames => '{year,month,mday,hour,min,sec}',
prosrc => 'make_timestamptz' },
{ oid => '3463', descr => 'construct timestamp with time zone',
proname => 'make_timestamptz', provolatile => 's',
prorettype => 'timestamptz',
- proargtypes => 'int4 int4 int4 int4 int4 float8 text',
+ proargtypes => 'int4 int4 int4 int4 int4 numeric text',
proargnames => '{year,month,mday,hour,min,sec,timezone}',
prosrc => 'make_timestamptz_at_timezone' },
{ oid => '3464', descr => 'construct interval',
proname => 'make_interval', prorettype => 'interval',
- proargtypes => 'int4 int4 int4 int4 int4 int4 float8',
+ proargtypes => 'int4 int4 int4 int4 int4 int4 numeric',
proargnames => '{years,months,weeks,days,hours,mins,secs}',
prosrc => 'make_interval' },
diff --git a/src/include/utils/date.h b/src/include/utils/date.h
index 6fc491e6a6..b901b81cc1 100644
--- a/src/include/utils/date.h
+++ b/src/include/utils/date.h
@@ -19,6 +19,7 @@
#include "datatype/timestamp.h"
#include "fmgr.h"
#include "pgtime.h"
+#include "utils/numeric.h"
typedef int32 DateADT;
@@ -84,7 +85,7 @@ extern int timetz2tm(TimeTzADT *time, struct pg_tm *tm, fsec_t *fsec, int *tzp);
extern int tm2time(struct pg_tm *tm, fsec_t fsec, TimeADT *result);
extern int tm2timetz(struct pg_tm *tm, fsec_t fsec, int tz, TimeTzADT *result);
extern bool time_overflows(int hour, int min, int sec, fsec_t fsec);
-extern bool float_time_overflows(int hour, int min, double sec);
+extern bool numeric_time_overflows(int hour, int min, Numeric sec);
extern void AdjustTimeForTypmod(TimeADT *time, int32 typmod);
#endif /* DATE_H */
diff --git a/src/include/utils/numeric.h b/src/include/utils/numeric.h
index 2a768b9a04..04000d9330 100644
--- a/src/include/utils/numeric.h
+++ b/src/include/utils/numeric.h
@@ -75,5 +75,6 @@ extern Numeric numeric_div_opt_error(Numeric num1, Numeric num2,
extern Numeric numeric_mod_opt_error(Numeric num1, Numeric num2,
bool *have_error);
extern int32 numeric_int4_opt_error(Numeric num, bool *error);
+extern int64 numeric_int8_opt_error(Numeric num, bool *error);
#endif /* _PG_NUMERIC_H_ */
--
2.29.2
Re: Change seconds argument of make_*() functions to numeric
čt 17. 12. 2020 v 17:43 odesílatel Peter Eisentraut <
peter.eisentraut@enterprisedb.com> napsal:
Along with the discussed change of the return type of EXTRACT from
float8 to numeric [0], I was looking around what other date/time APIs
might be using float arguments or return values. The only thing left
appears to be the functions make_time, make_timestamp, make_timestamptz,
and make_interval, which take an argument specifying the seconds, which
has type float8 right now. I'm proposing the attached patch to change
that to numeric.Can we change the arguments, as proposed here, or do we need to add
separate overloaded versions and leave the existing versions in place?
What this change does with views. Can it break upgrade by pg_upgrade?
Regards
Pavel
Show quoted text
[0]:
/messages/by-id/42b73d2d-da12-ba9f-570a-420e0cce19d9@phystech.edu
Re: Change seconds argument of make_*() functions to numeric
Peter Eisentraut <peter.eisentraut@enterprisedb.com> writes:
Along with the discussed change of the return type of EXTRACT from
float8 to numeric [0], I was looking around what other date/time APIs
might be using float arguments or return values. The only thing left
appears to be the functions make_time, make_timestamp, make_timestamptz,
and make_interval, which take an argument specifying the seconds, which
has type float8 right now. I'm proposing the attached patch to change
that to numeric.
I don't really see the point here. Since the seconds value is constrained
to 0..60 and will be rounded off to microseconds, you would have to work
seriously hard to find an example where float8 roundoff error could be
a problem. I don't think we should take whatever speed and compatibility
hit is implied by using numeric instead of float8.
(make_interval in theory could be an exception, since it doesn't constrain
the range of seconds values. But I still don't believe there's a problem
in practice.)
Can we change the arguments, as proposed here, or do we need to add
separate overloaded versions and leave the existing versions in place?
Since there's no implicit float8 to numeric cast, removing the existing
versions could quite easily cause failures of queries that work today.
regards, tom lane