From 6a9b93a0e1ba3ea068045b5cae2495f9a322c4fb Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Tue, 25 Oct 2005 17:13:07 +0000 Subject: [PATCH] Remove justify_hours call from interval_mul and interval_div, and make some small stylistic improvements in these functions. Also fix several places where TMODULO() was being used with wrong-sized quotient argument, creating a risk of overflow --- interval2tm was actually capable of going into an infinite loop because of this. --- src/backend/utils/adt/timestamp.c | 122 ++++++++++++++++++++------------- src/test/regress/expected/interval.out | 2 +- 2 files changed, 76 insertions(+), 48 deletions(-) diff --git a/src/backend/utils/adt/timestamp.c b/src/backend/utils/adt/timestamp.c index d3090413c4..b4a518a1da 100644 --- a/src/backend/utils/adt/timestamp.c +++ b/src/backend/utils/adt/timestamp.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/utils/adt/timestamp.c,v 1.155 2005/10/15 02:49:30 momjian Exp $ + * $PostgreSQL: pgsql/src/backend/utils/adt/timestamp.c,v 1.156 2005/10/25 17:13:07 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -1224,8 +1224,10 @@ interval2tm(Interval span, struct pg_tm * tm, fsec_t *fsec) { #ifdef HAVE_INT64_TIMESTAMP int64 time; + int64 tfrac; #else double time; + double tfrac; #endif tm->tm_year = span.month / MONTHS_PER_YEAR; @@ -1234,17 +1236,23 @@ interval2tm(Interval span, struct pg_tm * tm, fsec_t *fsec) time = span.time; #ifdef HAVE_INT64_TIMESTAMP - tm->tm_hour = time / USECS_PER_HOUR; - time -= tm->tm_hour * USECS_PER_HOUR; - tm->tm_min = time / USECS_PER_MINUTE; - time -= tm->tm_min * USECS_PER_MINUTE; - tm->tm_sec = time / USECS_PER_SEC; - *fsec = time - (tm->tm_sec * USECS_PER_SEC); + tfrac = time / USECS_PER_HOUR; + time -= tfrac * USECS_PER_HOUR; + tm->tm_hour = tfrac; /* could overflow ... */ + tfrac = time / USECS_PER_MINUTE; + time -= tfrac * USECS_PER_MINUTE; + tm->tm_min = tfrac; + tfrac = time / USECS_PER_SEC; + *fsec = time - (tfrac * USECS_PER_SEC); + tm->tm_sec = tfrac; #else recalc: - TMODULO(time, tm->tm_hour, (double) SECS_PER_HOUR); - TMODULO(time, tm->tm_min, (double) SECS_PER_MINUTE); - TMODULO(time, tm->tm_sec, 1.0); + TMODULO(time, tfrac, (double) SECS_PER_HOUR); + tm->tm_hour = tfrac; /* could overflow ... */ + TMODULO(time, tfrac, (double) SECS_PER_MINUTE); + tm->tm_min = tfrac; + TMODULO(time, tfrac, 1.0); + tm->tm_sec = tfrac; time = TSROUND(time); /* roundoff may need to propagate to higher-order fields */ if (time >= 1.0) @@ -1935,55 +1943,68 @@ timestamp_mi(PG_FUNCTION_ARGS) result->month = 0; result->day = 0; + /* this is wrong, but removing it breaks a lot of regression tests */ result = DatumGetIntervalP(DirectFunctionCall1(interval_justify_hours, IntervalPGetDatum(result))); + PG_RETURN_INTERVAL_P(result); } -/* interval_justify_hours() - * Adjust interval so 'time' contains less than a whole day, and - * 'day' contains an integral number of days. This is useful for +/* + * interval_justify_hours() + * + * Adjust interval so 'time' contains less than a whole day, adding + * the excess to 'day'. This is useful for * situations (such as non-TZ) where '1 day' = '24 hours' is valid, - * e.g. interval subtraction and division. The SQL standard requires - * such conversion in these cases, but not the conversion of days to months. + * e.g. interval subtraction and division. */ Datum interval_justify_hours(PG_FUNCTION_ARGS) { Interval *span = PG_GETARG_INTERVAL_P(0); Interval *result; +#ifdef HAVE_INT64_TIMESTAMP + int64 wholeday; +#else + double wholeday; +#endif result = (Interval *) palloc(sizeof(Interval)); result->month = span->month; + result->day = span->day; result->time = span->time; #ifdef HAVE_INT64_TIMESTAMP - result->time += span->day * USECS_PER_DAY; - TMODULO(result->time, result->day, USECS_PER_DAY); + TMODULO(result->time, wholeday, USECS_PER_DAY); #else - result->time += span->day * (double) SECS_PER_DAY; - TMODULO(result->time, result->day, (double) SECS_PER_DAY); + TMODULO(result->time, wholeday, (double) SECS_PER_DAY); #endif + result->day += wholeday; /* could overflow... */ PG_RETURN_INTERVAL_P(result); } -/* interval_justify_days() - * Adjust interval so 'time' contains less than 30 days, and - * adds as months. +/* + * interval_justify_days() + * + * Adjust interval so 'day' contains less than 30 days, adding + * the excess to 'month'. */ Datum interval_justify_days(PG_FUNCTION_ARGS) { Interval *span = PG_GETARG_INTERVAL_P(0); Interval *result; + int32 wholemonth; result = (Interval *) palloc(sizeof(Interval)); + result->month = span->month; result->day = span->day; result->time = span->time; - result->day += span->month * DAYS_PER_MONTH; - TMODULO(result->day, result->month, DAYS_PER_MONTH); + wholemonth = result->day / DAYS_PER_MONTH; + result->day -= wholemonth * DAYS_PER_MONTH; + result->month += wholemonth; PG_RETURN_INTERVAL_P(result); } @@ -2282,19 +2303,28 @@ interval_mul(PG_FUNCTION_ARGS) result = (Interval *) palloc(sizeof(Interval)); - result->month = span->month * factor; - result->day = span->day * factor; + month_remainder = span->month * factor; + day_remainder = span->day * factor; + result->month = (int32) month_remainder; + result->day = (int32) day_remainder; + month_remainder -= result->month; + day_remainder -= result->day; - /* Compute remainders */ - month_remainder = span->month * factor - result->month; - day_remainder = span->day * factor - result->day; + /* + * The above correctly handles the whole-number part of the month and + * day products, but we have to do something with any fractional part + * resulting when the factor is nonintegral. We cascade the fractions + * down to lower units using the conversion factors DAYS_PER_MONTH and + * SECS_PER_DAY. Note we do NOT cascade up, since we are not forced to + * do so by the representation. The user can choose to cascade up later, + * using justify_hours and/or justify_days. + */ - /* Cascade fractions to lower units */ /* fractional months full days into days */ month_remainder_days = month_remainder * DAYS_PER_MONTH; - result->day += month_remainder_days; + result->day += (int32) month_remainder_days; /* fractional months partial days into time */ - day_remainder += month_remainder_days - (int) month_remainder_days; + day_remainder += month_remainder_days - (int32) month_remainder_days; #ifdef HAVE_INT64_TIMESTAMP result->time = rint(span->time * factor + day_remainder * USECS_PER_DAY); @@ -2302,8 +2332,6 @@ interval_mul(PG_FUNCTION_ARGS) result->time = span->time * factor + day_remainder * SECS_PER_DAY; #endif - result = DatumGetIntervalP(DirectFunctionCall1(interval_justify_hours, - IntervalPGetDatum(result))); PG_RETURN_INTERVAL_P(result); } @@ -2334,29 +2362,29 @@ interval_div(PG_FUNCTION_ARGS) (errcode(ERRCODE_DIVISION_BY_ZERO), errmsg("division by zero"))); - result->month = span->month / factor; - result->day = span->day / factor; - result->time = span->time / factor; + month_remainder = span->month / factor; + day_remainder = span->day / factor; + result->month = (int32) month_remainder; + result->day = (int32) day_remainder; + month_remainder -= result->month; + day_remainder -= result->day; - /* Compute remainders */ - month_remainder = span->month / factor - result->month; - day_remainder = span->day / factor - result->day; + /* + * Handle any fractional parts the same way as in interval_mul. + */ - /* Cascade fractions to lower units */ /* fractional months full days into days */ month_remainder_days = month_remainder * DAYS_PER_MONTH; - result->day += month_remainder_days; + result->day += (int32) month_remainder_days; /* fractional months partial days into time */ - day_remainder += month_remainder_days - (int) month_remainder_days; + day_remainder += month_remainder_days - (int32) month_remainder_days; #ifdef HAVE_INT64_TIMESTAMP - result->time += rint(day_remainder * USECS_PER_DAY); + result->time = rint(span->time / factor + day_remainder * USECS_PER_DAY); #else - result->time += day_remainder * SECS_PER_DAY; + result->time = span->time / factor + day_remainder * SECS_PER_DAY; #endif - result = DatumGetIntervalP(DirectFunctionCall1(interval_justify_hours, - IntervalPGetDatum(result))); PG_RETURN_INTERVAL_P(result); } diff --git a/src/test/regress/expected/interval.out b/src/test/regress/expected/interval.out index 9efcdf8139..d07dd3013d 100644 --- a/src/test/regress/expected/interval.out +++ b/src/test/regress/expected/interval.out @@ -218,7 +218,7 @@ SELECT '' AS ten, * FROM INTERVAL_TBL; select avg(f1) from interval_tbl; avg ------------------------------------------------- - @ 4 years 1 mon 10 days 4 hours 18 mins 23 secs + @ 4 years 1 mon 9 days 28 hours 18 mins 23 secs (1 row) -- test long interval input -- 2.11.0