Macros for time magic values - Mailing list pgsql-hackers

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  <bruce@momjian.us>        http://momjian.us
  EnterpriseDB                             http://enterprisedb.com

  + It's impossible for everything to be true. +
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