Applied. ---------------------------------------------------------------------------
Bruce Momjian wrote: > It has bothered me that many of our time routines use special magic > constants for time values, e.g. 24, 12, 60, etc. > > The attached patch changes these magic constants to macros to clarify > the code. I would like to apply this for 9.1 as a cleanup. > > -- > Bruce Momjian <br...@momjian.us> http://momjian.us > EnterpriseDB http://enterprisedb.com > > + It's impossible for everything to be true. + [ text/x-diff is unsupported, treating like TEXT/PLAIN ] > diff --git a/src/backend/utils/adt/date.c b/src/backend/utils/adt/date.c > index 80dd10b..f96fa6c 100644 > --- a/src/backend/utils/adt/date.c > +++ b/src/backend/utils/adt/date.c > @@ -2612,7 +2612,7 @@ timetz_zone(PG_FUNCTION_ARGS) > type = DecodeSpecial(0, lowzone, &val); > > if (type == TZ || type == DTZ) > - tz = val * 60; > + tz = val * MINS_PER_HOUR; > else > { > tzp = pg_tzset(tzname); > diff --git a/src/backend/utils/adt/datetime.c > b/src/backend/utils/adt/datetime.c > index 85f0206..f0fe2e3 100644 > --- a/src/backend/utils/adt/datetime.c > +++ b/src/backend/utils/adt/datetime.c > @@ -342,7 +342,7 @@ j2date(int jd, int *year, int *month, int *day) > *year = y - 4800; > quad = julian * 2141 / 65536; > *day = julian - 7834 * quad / 256; > - *month = (quad + 10) % 12 + 1; > + *month = (quad + 10) % MONTHS_PER_YEAR + 1; > > return; > } /* j2date() */ > @@ -952,8 +952,8 @@ DecodeDateTime(char **field, int *ftype, int nf, > * DecodeTime() > */ > /* test for > 24:00:00 */ > - if (tm->tm_hour > 24 || > - (tm->tm_hour == 24 && > + if (tm->tm_hour > HOURS_PER_DAY || > + (tm->tm_hour == HOURS_PER_DAY && > (tm->tm_min > 0 || tm->tm_sec > 0 || > *fsec > 0))) > return DTERR_FIELD_OVERFLOW; > break; > @@ -1371,12 +1371,12 @@ DecodeDateTime(char **field, int *ftype, int nf, > return dterr; > > /* handle AM/PM */ > - if (mer != HR24 && tm->tm_hour > 12) > + if (mer != HR24 && tm->tm_hour > HOURS_PER_DAY / 2) > return DTERR_FIELD_OVERFLOW; > - if (mer == AM && tm->tm_hour == 12) > + if (mer == AM && tm->tm_hour == HOURS_PER_DAY / 2) > tm->tm_hour = 0; > - else if (mer == PM && tm->tm_hour != 12) > - tm->tm_hour += 12; > + else if (mer == PM && tm->tm_hour != HOURS_PER_DAY / 2) > + tm->tm_hour += HOURS_PER_DAY / 2; > > /* do additional checking for full date specs... */ > if (*dtype == DTK_DATE) > @@ -2058,17 +2058,18 @@ DecodeTimeOnly(char **field, int *ftype, int nf, > return dterr; > > /* handle AM/PM */ > - if (mer != HR24 && tm->tm_hour > 12) > + if (mer != HR24 && tm->tm_hour > HOURS_PER_DAY / 2) > return DTERR_FIELD_OVERFLOW; > - if (mer == AM && tm->tm_hour == 12) > + if (mer == AM && tm->tm_hour == HOURS_PER_DAY / 2) > tm->tm_hour = 0; > - else if (mer == PM && tm->tm_hour != 12) > - tm->tm_hour += 12; > + else if (mer == PM && tm->tm_hour != HOURS_PER_DAY / 2) > + tm->tm_hour += HOURS_PER_DAY / 2; > > - if (tm->tm_hour < 0 || tm->tm_min < 0 || tm->tm_min > 59 || > - tm->tm_sec < 0 || tm->tm_sec > 60 || tm->tm_hour > 24 || > + 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 == 24 && > + (tm->tm_hour == HOURS_PER_DAY && > (tm->tm_min > 0 || tm->tm_sec > 0 || *fsec > 0)) || > #ifdef HAVE_INT64_TIMESTAMP > *fsec < INT64CONST(0) || *fsec > USECS_PER_SEC > @@ -2396,13 +2397,15 @@ DecodeTime(char *str, int fmask, int range, > > /* do a sanity check */ > #ifdef HAVE_INT64_TIMESTAMP > - if (tm->tm_hour < 0 || tm->tm_min < 0 || tm->tm_min > 59 || > - tm->tm_sec < 0 || tm->tm_sec > 60 || *fsec < INT64CONST(0) || > + 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 || > + *fsec < INT64CONST(0) || > *fsec > USECS_PER_SEC) > return DTERR_FIELD_OVERFLOW; > #else > - if (tm->tm_hour < 0 || tm->tm_min < 0 || tm->tm_min > 59 || > - tm->tm_sec < 0 || tm->tm_sec > 60 || *fsec < 0 || *fsec > 1) > + 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 || > + *fsec < 0 || *fsec > 1) > return DTERR_FIELD_OVERFLOW; > #endif > > @@ -2748,9 +2751,9 @@ DecodeTimezone(char *str, int *tzp) > > if (hr < 0 || hr > 14) > return DTERR_TZDISP_OVERFLOW; > - if (min < 0 || min >= 60) > + if (min < 0 || min >= MINS_PER_HOUR) > return DTERR_TZDISP_OVERFLOW; > - if (sec < 0 || sec >= 60) > + if (sec < 0 || sec >= SECS_PER_MINUTE) > return DTERR_TZDISP_OVERFLOW; > > tz = (hr * MINS_PER_HOUR + min) * SECS_PER_MINUTE + sec; > @@ -3324,7 +3327,7 @@ DecodeISO8601Interval(char *str, > { > case 'Y': > tm->tm_year += val; > - tm->tm_mon += (fval * 12); > + tm->tm_mon += (fval * MONTHS_PER_YEAR); > break; > case 'M': > tm->tm_mon += val; > @@ -3359,7 +3362,7 @@ DecodeISO8601Interval(char *str, > return DTERR_BAD_FORMAT; > > tm->tm_year += val; > - tm->tm_mon += (fval * 12); > + tm->tm_mon += (fval * MONTHS_PER_YEAR); > if (unit == '\0') > return 0; > if (unit == 'T') > @@ -4155,7 +4158,7 @@ InstallTimeZoneAbbrevs(tzEntry *abbrevs, int n) > { > strncpy(newtbl[i].token, abbrevs[i].abbrev, TOKMAXLEN); > newtbl[i].type = abbrevs[i].is_dst ? DTZ : TZ; > - TOVAL(&newtbl[i], abbrevs[i].offset / 60); > + TOVAL(&newtbl[i], abbrevs[i].offset / MINS_PER_HOUR); > } > > /* Check the ordering, if testing */ > diff --git a/src/backend/utils/adt/formatting.c > b/src/backend/utils/adt/formatting.c > index f90d36d..aba1145 100644 > --- a/src/backend/utils/adt/formatting.c > +++ b/src/backend/utils/adt/formatting.c > @@ -2129,7 +2129,7 @@ DCH_to_char(FormatNode *node, bool is_interval, > TmToChar *in, char *out, Oid col > * intervals > */ > sprintf(s, "%0*d", S_FM(n->suffix) ? 0 : 2, > - tm->tm_hour % (HOURS_PER_DAY / > 2) == 0 ? 12 : > + tm->tm_hour % (HOURS_PER_DAY / > 2) == 0 ? HOURS_PER_DAY / 2 : > tm->tm_hour % (HOURS_PER_DAY / > 2)); > if (S_THth(n->suffix)) > str_numth(s, s, S_TH_TYPE(n->suffix)); > @@ -2486,14 +2486,14 @@ DCH_to_char(FormatNode *node, bool is_interval, > TmToChar *in, char *out, Oid col > if (!tm->tm_mon) > break; > sprintf(s, "%*s", S_FM(n->suffix) ? 0 : -4, > - rm_months_upper[12 - > tm->tm_mon]); > + rm_months_upper[MONTHS_PER_YEAR > - tm->tm_mon]); > s += strlen(s); > break; > case DCH_rm: > if (!tm->tm_mon) > break; > sprintf(s, "%*s", S_FM(n->suffix) ? 0 : -4, > - rm_months_lower[12 - > tm->tm_mon]); > + rm_months_lower[MONTHS_PER_YEAR > - tm->tm_mon]); > s += strlen(s); > break; > case DCH_W: > @@ -2779,12 +2779,12 @@ DCH_from_char(FormatNode *node, char *in, TmFromChar > *out) > case DCH_RM: > from_char_seq_search(&value, &s, > rm_months_upper, > > ALL_UPPER, MAX_RM_LEN, n); > - from_char_set_int(&out->mm, 12 - value, n); > + from_char_set_int(&out->mm, MONTHS_PER_YEAR - > value, n); > break; > case DCH_rm: > from_char_seq_search(&value, &s, > rm_months_lower, > > ALL_LOWER, MAX_RM_LEN, n); > - from_char_set_int(&out->mm, 12 - value, n); > + from_char_set_int(&out->mm, MONTHS_PER_YEAR - > value, n); > break; > case DCH_W: > from_char_parse_int(&out->w, &s, n); > @@ -3236,16 +3236,16 @@ do_to_timestamp(text *date_txt, text *fmt, > > if (tmfc.clock == CLOCK_12_HOUR) > { > - if (tm->tm_hour < 1 || tm->tm_hour > 12) > + if (tm->tm_hour < 1 || tm->tm_hour > HOURS_PER_DAY / 2) > ereport(ERROR, > > (errcode(ERRCODE_INVALID_DATETIME_FORMAT), > errmsg("hour \"%d\" is invalid for the > 12-hour clock", > tm->tm_hour), > errhint("Use the 24-hour clock, or > give an hour between 1 and 12."))); > > - if (tmfc.pm && tm->tm_hour < 12) > - tm->tm_hour += 12; > - else if (!tmfc.pm && tm->tm_hour == 12) > + if (tmfc.pm && tm->tm_hour < HOURS_PER_DAY / 2) > + tm->tm_hour += HOURS_PER_DAY / 2; > + else if (!tmfc.pm && tm->tm_hour == HOURS_PER_DAY / 2) > tm->tm_hour = 0; > } > > @@ -3347,7 +3347,7 @@ do_to_timestamp(text *date_txt, text *fmt, > > y = ysum[isleap(tm->tm_year)]; > > - for (i = 1; i <= 12; i++) > + for (i = 1; i <= MONTHS_PER_YEAR; i++) > { > if (tmfc.ddd < y[i]) > break; > diff --git a/src/backend/utils/adt/nabstime.c > b/src/backend/utils/adt/nabstime.c > index 0e25c5f..4b61b21 100644 > --- a/src/backend/utils/adt/nabstime.c > +++ b/src/backend/utils/adt/nabstime.c > @@ -179,13 +179,13 @@ tm2abstime(struct pg_tm * tm, int tz) > > /* validate, before going out of range on some members */ > if (tm->tm_year < 1901 || tm->tm_year > 2038 || > - tm->tm_mon < 1 || tm->tm_mon > 12 || > + tm->tm_mon < 1 || tm->tm_mon > MONTHS_PER_YEAR || > tm->tm_mday < 1 || tm->tm_mday > 31 || > tm->tm_hour < 0 || > - tm->tm_hour > 24 || /* test for > 24:00:00 */ > - (tm->tm_hour == 24 && (tm->tm_min > 0 || tm->tm_sec > 0)) || > - tm->tm_min < 0 || tm->tm_min > 59 || > - tm->tm_sec < 0 || tm->tm_sec > 60) > + 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)) || > + tm->tm_min < 0 || tm->tm_min > MINS_PER_HOUR - 1 || > + tm->tm_sec < 0 || tm->tm_sec > SECS_PER_MINUTE) > return INVALID_ABSTIME; > > day = date2j(tm->tm_year, tm->tm_mon, tm->tm_mday) - UNIX_EPOCH_JDATE; > diff --git a/src/backend/utils/adt/timestamp.c > b/src/backend/utils/adt/timestamp.c > index 1c2c160..45e7002 100644 > --- a/src/backend/utils/adt/timestamp.c > +++ b/src/backend/utils/adt/timestamp.c > @@ -4422,7 +4422,7 @@ timestamp_zone(PG_FUNCTION_ARGS) > > if (type == TZ || type == DTZ) > { > - tz = -(val * 60); > + tz = -(val * MINS_PER_HOUR); > result = dt2local(timestamp, tz); > } > else > @@ -4596,7 +4596,7 @@ timestamptz_zone(PG_FUNCTION_ARGS) > > if (type == TZ || type == DTZ) > { > - tz = val * 60; > + tz = val * MINS_PER_HOUR; > result = dt2local(timestamp, tz); > } > else > diff --git a/src/include/utils/timestamp.h b/src/include/utils/timestamp.h > index 9e51b58..e14285f 100644 > --- a/src/include/utils/timestamp.h > +++ b/src/include/utils/timestamp.h > @@ -81,6 +81,8 @@ typedef struct > */ > #define DAYS_PER_MONTH 30 /* assumes exactly 30 days per > month */ > #define HOURS_PER_DAY 24 /* assume no daylight savings > time changes */ > +#define MINS_PER_HOUR 60 /* assume no daylight savings > time changes */ > +#define SECS_PER_MINUTE 60 /* assume no daylight savings > time changes */ > > /* > * This doesn't adjust for uneven daylight savings time intervals or leap > > -- > Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) > To make changes to your subscription: > http://www.postgresql.org/mailpref/pgsql-hackers -- Bruce Momjian <br...@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
