pg.ddx.io pgsql-bugs@postgresql.org mailing list archive
help / color / mirror / Atom feedtime values past 24:00:00 (or rather 23:59:60)
4+ messages / 2 participants
[nested] [flat]
Commits from this thread:
2020-06-04 a958b07bc4 6490376e56 b2c64f571d a9632830bb Reject "23:59:60.nnn" in datetime input.
* time values past 24:00:00 (or rather 23:59:60)
@ 2020-05-20 12:58 Christoph Berg <myon@debian.org>
0 siblings, 1 reply; 4+ messages in thread
From: Christoph Berg @ 2020-05-20 12:58 UTC (permalink / raw)
To: pgsql-bugs
This is ok:
# select '23:59:60'::time;
time
──────────
24:00:00
# select '23:59:61'::time;
ERROR: 22008: date/time field value out of range: "23:59:61"
This is not ok:
# select '23:59:60.999'::time;
time
──────────────
24:00:00.999
-- value isn't read back:
# select '24:00:00.999'::time;
ERROR: 22008: date/time field value out of range: "24:00:00.999"
-- we can even go to :01
# select '23:59:60.9999999'::time;
time
──────────
24:00:01
PG 12.3.
Christoph
^ permalink raw reply [nested|flat] 4+ messages in thread
* Re: time values past 24:00:00 (or rather 23:59:60)
@ 2020-05-21 05:53 Tom Lane <tgl@sss.pgh.pa.us>
parent: Christoph Berg <myon@debian.org>
0 siblings, 1 reply; 4+ messages in thread
From: Tom Lane @ 2020-05-21 05:53 UTC (permalink / raw)
To: Christoph Berg <myon@debian.org>; +Cc: pgsql-bugs
Christoph Berg <myon@debian.org> writes:
> This is not ok:
> # select '23:59:60.999'::time;
> time
> --------------
> 24:00:00.999
Yeah. Here's a draft patch for that (sans any regression tests as yet).
I found several places with largely the same logic, so I tried to
centralize the tests; although it turns out we need a minimum of
two versions, because some places deal with fractional seconds
differently.
The places using float seconds had their own bugs: one forgot to worry
about NaN, and neither was being careful about imprecise input.
It's slightly annoying to have the check functions duplicate the
calculation that some level of caller is going to do, but avoiding that
would take more refactoring than I think is warranted. In practice a
few extra multiplications aren't likely to be noticeable here.
regards, tom lane
Attachments:
handle-maximum-time-of-day-better.patch (text/x-diff, 6.7K) download
diff --git a/src/backend/utils/adt/date.c b/src/backend/utils/adt/date.c
index 0c55b68..eaaffa7 100644
--- a/src/backend/utils/adt/date.c
+++ b/src/backend/utils/adt/date.c
@@ -18,6 +18,7 @@
#include <ctype.h>
#include <limits.h>
#include <float.h>
+#include <math.h>
#include <time.h>
#include "access/xact.h"
@@ -1270,6 +1271,65 @@ tm2time(struct pg_tm *tm, fsec_t fsec, TimeADT *result)
return 0;
}
+/* time_overflows()
+ * Check to see if a broken-down time-of-day is out of range.
+ */
+bool
+time_overflows(int hour, int min, int sec, fsec_t fsec)
+{
+ /* Range-check the fields individually. */
+ if (hour < 0 || hour > HOURS_PER_DAY ||
+ min < 0 || min >= MINS_PER_HOUR ||
+ sec < 0 || sec > SECS_PER_MINUTE ||
+ fsec < 0 || fsec > USECS_PER_SEC)
+ return true;
+
+ /*
+ * Because we allow, eg, hour = 24 or sec = 60, we must check separately
+ * that the total time value doesn't exceed 24:00:00.
+ */
+ if ((((((hour * MINS_PER_HOUR + min) * SECS_PER_MINUTE)
+ + sec) * USECS_PER_SEC) + fsec) > USECS_PER_DAY)
+ return true;
+
+ return false;
+}
+
+/* float_time_overflows()
+ * Same, when we have seconds + fractional seconds as one "double" value.
+ */
+bool
+float_time_overflows(int hour, int min, double sec)
+{
+ /* 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.)
+ */
+ if (isnan(sec))
+ return true;
+ sec = rint(sec * USECS_PER_SEC);
+ if (sec < 0 || sec > SECS_PER_MINUTE * USECS_PER_SEC)
+ return true;
+
+ /*
+ * Because we allow, eg, hour = 24 or sec = 60, we must check separately
+ * that the total time value doesn't exceed 24:00:00. This must match the
+ * 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)
+ return true;
+
+ return false;
+}
+
+
/* time2tm()
* Convert time data type to POSIX time structure.
*
@@ -1374,12 +1434,8 @@ make_time(PG_FUNCTION_ARGS)
double sec = PG_GETARG_FLOAT8(2);
TimeADT time;
- /* This should match the checks in DecodeTimeOnly */
- if (tm_hour < 0 || tm_min < 0 || tm_min > MINS_PER_HOUR - 1 ||
- sec < 0 || sec > SECS_PER_MINUTE ||
- tm_hour > HOURS_PER_DAY ||
- /* test for > 24:00:00 */
- (tm_hour == HOURS_PER_DAY && (tm_min > 0 || sec > 0)))
+ /* Check for time overflow */
+ if (float_time_overflows(tm_hour, tm_min, sec))
ereport(ERROR,
(errcode(ERRCODE_DATETIME_FIELD_OVERFLOW),
errmsg("time field value out of range: %d:%02d:%02g",
@@ -1387,7 +1443,7 @@ make_time(PG_FUNCTION_ARGS)
/* This should match tm2time */
time = (((tm_hour * MINS_PER_HOUR + tm_min) * SECS_PER_MINUTE)
- * USECS_PER_SEC) + rint(sec * USECS_PER_SEC);
+ * USECS_PER_SEC) + (int64) rint(sec * USECS_PER_SEC);
PG_RETURN_TIMEADT(time);
}
diff --git a/src/backend/utils/adt/datetime.c b/src/backend/utils/adt/datetime.c
index 1914138..0b6dfb2 100644
--- a/src/backend/utils/adt/datetime.c
+++ b/src/backend/utils/adt/datetime.c
@@ -936,14 +936,9 @@ DecodeDateTime(char **field, int *ftype, int nf,
if (dterr)
return dterr;
- /*
- * Check upper limit on hours; other limits checked in
- * DecodeTime()
- */
- /* test for > 24:00:00 */
- if (tm->tm_hour > HOURS_PER_DAY ||
- (tm->tm_hour == HOURS_PER_DAY &&
- (tm->tm_min > 0 || tm->tm_sec > 0 || *fsec > 0)))
+ /* check for time overflow */
+ if (time_overflows(tm->tm_hour, tm->tm_min, tm->tm_sec,
+ *fsec))
return DTERR_FIELD_OVERFLOW;
break;
@@ -2218,16 +2213,8 @@ DecodeTimeOnly(char **field, int *ftype, int nf,
else if (mer == PM && tm->tm_hour != HOURS_PER_DAY / 2)
tm->tm_hour += HOURS_PER_DAY / 2;
- /*
- * This should match the checks in make_timestamp_internal
- */
- if (tm->tm_hour < 0 || tm->tm_min < 0 || tm->tm_min > MINS_PER_HOUR - 1 ||
- tm->tm_sec < 0 || tm->tm_sec > SECS_PER_MINUTE ||
- tm->tm_hour > HOURS_PER_DAY ||
- /* test for > 24:00:00 */
- (tm->tm_hour == HOURS_PER_DAY &&
- (tm->tm_min > 0 || tm->tm_sec > 0 || *fsec > 0)) ||
- *fsec < INT64CONST(0) || *fsec > USECS_PER_SEC)
+ /* check for time overflow */
+ if (time_overflows(tm->tm_hour, tm->tm_min, tm->tm_sec, *fsec))
return DTERR_FIELD_OVERFLOW;
if ((fmask & DTK_TIME_M) != DTK_TIME_M)
diff --git a/src/backend/utils/adt/timestamp.c b/src/backend/utils/adt/timestamp.c
index 7ea97d0..5fe304c 100644
--- a/src/backend/utils/adt/timestamp.c
+++ b/src/backend/utils/adt/timestamp.c
@@ -32,6 +32,7 @@
#include "parser/scansup.h"
#include "utils/array.h"
#include "utils/builtins.h"
+#include "utils/date.h"
#include "utils/datetime.h"
#include "utils/float.h"
@@ -581,18 +582,8 @@ make_timestamp_internal(int year, int month, int day,
date = date2j(tm.tm_year, tm.tm_mon, tm.tm_mday) - POSTGRES_EPOCH_JDATE;
- /*
- * This should match the checks in DecodeTimeOnly, except that since we're
- * dealing with a float "sec" value, we also explicitly reject NaN. (An
- * infinity input should get rejected by the range comparisons, but we
- * can't be sure how those will treat a NaN.)
- */
- if (hour < 0 || min < 0 || min > MINS_PER_HOUR - 1 ||
- isnan(sec) ||
- sec < 0 || sec > SECS_PER_MINUTE ||
- hour > HOURS_PER_DAY ||
- /* test for > 24:00:00 */
- (hour == HOURS_PER_DAY && (min > 0 || sec > 0)))
+ /* Check for time overflow */
+ if (float_time_overflows(hour, min, sec))
ereport(ERROR,
(errcode(ERRCODE_DATETIME_FIELD_OVERFLOW),
errmsg("time field value out of range: %d:%02d:%02g",
@@ -600,7 +591,7 @@ make_timestamp_internal(int year, int month, int day,
/* This should match tm2time */
time = (((hour * MINS_PER_HOUR + min) * SECS_PER_MINUTE)
- * USECS_PER_SEC) + rint(sec * USECS_PER_SEC);
+ * USECS_PER_SEC) + (int64) rint(sec * USECS_PER_SEC);
result = date * USECS_PER_DAY + time;
/* check for major overflow */
diff --git a/src/include/utils/date.h b/src/include/utils/date.h
index 91c8b36..4cdb1f9 100644
--- a/src/include/utils/date.h
+++ b/src/include/utils/date.h
@@ -80,6 +80,8 @@ extern int time2tm(TimeADT time, struct pg_tm *tm, fsec_t *fsec);
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 void AdjustTimeForTypmod(TimeADT *time, int32 typmod);
#endif /* DATE_H */
* Re: time values past 24:00:00 (or rather 23:59:60)
@ 2020-06-04 20:54 Tom Lane <tgl@sss.pgh.pa.us>
parent: Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 1 reply; 4+ messages in thread
From: Tom Lane @ 2020-06-04 20:54 UTC (permalink / raw)
To: Christoph Berg <myon@debian.org>; +Cc: pgsql-bugs
I wrote:
> Christoph Berg <myon@debian.org> writes:
>> This is not ok:
>> # select '23:59:60.999'::time;
>> time
>> --------------
>> 24:00:00.999
> Yeah. Here's a draft patch for that (sans any regression tests as yet).
I pushed this fix, but only as far back as v10. To apply it in the
9.x branches, we'd need to make the code support float timestamps,
and I didn't think it was worth the trouble.
regards, tom lane
^ permalink raw reply [nested|flat] 4+ messages in thread
* Re: time values past 24:00:00 (or rather 23:59:60)
@ 2020-06-05 10:27 Christoph Berg <myon@debian.org>
parent: Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 0 replies; 4+ messages in thread
From: Christoph Berg @ 2020-06-05 10:27 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: pgsql-bugs
Re: Tom Lane
> I pushed this fix, but only as far back as v10. To apply it in the
> 9.x branches, we'd need to make the code support float timestamps,
> and I didn't think it was worth the trouble.
Aye. Thanks!
Christoph
^ permalink raw reply [nested|flat] 4+ messages in thread
end of thread, other threads:[~2020-06-05 10:27 UTC | newest]
Thread overview: 4+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2020-05-20 12:58 time values past 24:00:00 (or rather 23:59:60) Christoph Berg <myon@debian.org>
2020-05-21 05:53 ` Tom Lane <tgl@sss.pgh.pa.us>
2020-06-04 20:54 ` Tom Lane <tgl@sss.pgh.pa.us>
2020-06-05 10:27 ` Christoph Berg <myon@debian.org>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox