Re: BUG #1671: Long interval string representation rejected - Mailing list pgsql-bugs

Tom Lane wrote:
> Considering that you're incrementing bufptr inside the macro, it hardly
> seems that double-evaluation is a problem: the argument pretty much has
> to be a variable.  OTOH there is no reason for the input argument to be
> treated that way.  I'd suggest just one macro [...]

Ah, yeah, that works. Attached is a revised patch -- I'll apply it
tomorrow barring any objections. I also fixed the semi-colon -- thanks
for the review.

-Neil
Index: src/backend/utils/adt/date.c
===================================================================
RCS file: /var/lib/cvs/pgsql/src/backend/utils/adt/date.c,v
retrieving revision 1.108
diff -c -r1.108 date.c
*** src/backend/utils/adt/date.c    24 May 2005 02:09:45 -0000    1.108
--- src/backend/utils/adt/date.c    25 May 2005 01:06:27 -0000
***************
*** 65,76 ****
      int            dterr;
      char       *field[MAXDATEFIELDS];
      int            ftype[MAXDATEFIELDS];
!     char        lowstr[MAXDATELEN + 1];

!     if (strlen(str) >= sizeof(lowstr))
!         dterr = DTERR_BAD_FORMAT;
!     else
!         dterr = ParseDateTime(str, lowstr, field, ftype, MAXDATEFIELDS, &nf);
      if (dterr == 0)
          dterr = DecodeDateTime(field, ftype, nf, &dtype, tm, &fsec, &tzp);
      if (dterr != 0)
--- 65,74 ----
      int            dterr;
      char       *field[MAXDATEFIELDS];
      int            ftype[MAXDATEFIELDS];
!     char        workbuf[MAXDATELEN + 1];

!     dterr = ParseDateTime(str, workbuf, sizeof(workbuf),
!                           field, ftype, MAXDATEFIELDS, &nf);
      if (dterr == 0)
          dterr = DecodeDateTime(field, ftype, nf, &dtype, tm, &fsec, &tzp);
      if (dterr != 0)
***************
*** 894,908 ****
      int            tz;
      int            nf;
      int            dterr;
!     char        lowstr[MAXDATELEN + 1];
      char       *field[MAXDATEFIELDS];
      int            dtype;
      int            ftype[MAXDATEFIELDS];

!     if (strlen(str) >= sizeof(lowstr))
!         dterr = DTERR_BAD_FORMAT;
!     else
!         dterr = ParseDateTime(str, lowstr, field, ftype, MAXDATEFIELDS, &nf);
      if (dterr == 0)
          dterr = DecodeTimeOnly(field, ftype, nf, &dtype, tm, &fsec, &tz);
      if (dterr != 0)
--- 892,904 ----
      int            tz;
      int            nf;
      int            dterr;
!     char        workbuf[MAXDATELEN + 1];
      char       *field[MAXDATEFIELDS];
      int            dtype;
      int            ftype[MAXDATEFIELDS];

!     dterr = ParseDateTime(str, workbuf, sizeof(workbuf),
!                           field, ftype, MAXDATEFIELDS, &nf);
      if (dterr == 0)
          dterr = DecodeTimeOnly(field, ftype, nf, &dtype, tm, &fsec, &tz);
      if (dterr != 0)
***************
*** 1733,1747 ****
      int            tz;
      int            nf;
      int            dterr;
!     char        lowstr[MAXDATELEN + 1];
      char       *field[MAXDATEFIELDS];
      int            dtype;
      int            ftype[MAXDATEFIELDS];

!     if (strlen(str) >= sizeof(lowstr))
!         dterr = DTERR_BAD_FORMAT;
!     else
!         dterr = ParseDateTime(str, lowstr, field, ftype, MAXDATEFIELDS, &nf);
      if (dterr == 0)
          dterr = DecodeTimeOnly(field, ftype, nf, &dtype, tm, &fsec, &tz);
      if (dterr != 0)
--- 1729,1741 ----
      int            tz;
      int            nf;
      int            dterr;
!     char        workbuf[MAXDATELEN + 1];
      char       *field[MAXDATEFIELDS];
      int            dtype;
      int            ftype[MAXDATEFIELDS];

!     dterr = ParseDateTime(str, workbuf, sizeof(workbuf),
!                           field, ftype, MAXDATEFIELDS, &nf);
      if (dterr == 0)
          dterr = DecodeTimeOnly(field, ftype, nf, &dtype, tm, &fsec, &tz);
      if (dterr != 0)
Index: src/backend/utils/adt/datetime.c
===================================================================
RCS file: /var/lib/cvs/pgsql/src/backend/utils/adt/datetime.c,v
retrieving revision 1.144
diff -c -r1.144 datetime.c
*** src/backend/utils/adt/datetime.c    24 May 2005 02:09:45 -0000    1.144
--- src/backend/utils/adt/datetime.c    25 May 2005 01:43:50 -0000
***************
*** 699,719 ****
      }
  }

-
  /* ParseDateTime()
   *    Break string into tokens based on a date/time context.
   *    Returns 0 if successful, DTERR code if bogus input detected.
   *
   * timestr - the input string
!  * lowstr - workspace for field string storage (must be large enough for
!  *        a copy of the input string, including trailing null)
   * field[] - pointers to field strings are returned in this array
   * ftype[] - field type indicators are returned in this array
   * maxfields - dimensions of the above two arrays
   * *numfields - set to the actual number of fields detected
   *
!  * The fields extracted from the input are stored as separate, null-terminated
!  * strings in the workspace at lowstr.    Any text is converted to lower case.
   *
   * Several field types are assigned:
   *    DTK_NUMBER - digits and (possibly) a decimal point
--- 699,721 ----
      }
  }

  /* ParseDateTime()
   *    Break string into tokens based on a date/time context.
   *    Returns 0 if successful, DTERR code if bogus input detected.
   *
   * timestr - the input string
!  * workbuf - workspace for field string storage. This must be
!  *   larger than the largest legal input for this datetime type --
!  *   some additional space will be needed to NUL terminate fields.
!  * buflen - the size of workbuf
   * field[] - pointers to field strings are returned in this array
   * ftype[] - field type indicators are returned in this array
   * maxfields - dimensions of the above two arrays
   * *numfields - set to the actual number of fields detected
   *
!  * The fields extracted from the input are stored as separate,
!  * null-terminated strings in the workspace at workbuf. Any text is
!  * converted to lower case.
   *
   * Several field types are assigned:
   *    DTK_NUMBER - digits and (possibly) a decimal point
***************
*** 729,740 ****
   *    DTK_DATE can hold Posix time zones (GMT-8)
   */
  int
! ParseDateTime(const char *timestr, char *lowstr,
                char **field, int *ftype, int maxfields, int *numfields)
  {
      int            nf = 0;
      const char *cp = timestr;
!     char       *lp = lowstr;

      /* outer loop through fields */
      while (*cp != '\0')
--- 731,757 ----
   *    DTK_DATE can hold Posix time zones (GMT-8)
   */
  int
! ParseDateTime(const char *timestr, char *workbuf, size_t buflen,
                char **field, int *ftype, int maxfields, int *numfields)
  {
      int            nf = 0;
      const char *cp = timestr;
!     char       *bufp = workbuf;
!     const char *bufend = workbuf + buflen;
!
!     /*
!      * Set the character pointed-to by "bufptr" to "newchar", and
!      * increment "bufptr". "end" gives the end of the buffer --
!      * returns an error if there is no space left to append a
!      * character.
!      */
! #define APPEND_CHAR(bufptr, end, newchar)        \
!     do                                            \
!     {                                            \
!         if (((bufptr) + 1) >= (end))            \
!             return DTERR_BAD_FORMAT;            \
!         *(bufptr)++ = newchar;                    \
!     } while (0)

      /* outer loop through fields */
      while (*cp != '\0')
***************
*** 749,771 ****
          /* Record start of current field */
          if (nf >= maxfields)
              return DTERR_BAD_FORMAT;
!         field[nf] = lp;

          /* leading digit? then date or time */
          if (isdigit((unsigned char) *cp))
          {
!             *lp++ = *cp++;
              while (isdigit((unsigned char) *cp))
!                 *lp++ = *cp++;

              /* time field? */
              if (*cp == ':')
              {
                  ftype[nf] = DTK_TIME;
!                 *lp++ = *cp++;
                  while (isdigit((unsigned char) *cp) ||
                         (*cp == ':') || (*cp == '.'))
!                     *lp++ = *cp++;
              }
              /* date field? allow embedded text month */
              else if (*cp == '-' || *cp == '/' || *cp == '.')
--- 766,788 ----
          /* Record start of current field */
          if (nf >= maxfields)
              return DTERR_BAD_FORMAT;
!         field[nf] = bufp;

          /* leading digit? then date or time */
          if (isdigit((unsigned char) *cp))
          {
!             APPEND_CHAR(bufp, bufend, *cp++);
              while (isdigit((unsigned char) *cp))
!                 APPEND_CHAR(bufp, bufend, *cp++);

              /* time field? */
              if (*cp == ':')
              {
                  ftype[nf] = DTK_TIME;
!                 APPEND_CHAR(bufp, bufend, *cp++);
                  while (isdigit((unsigned char) *cp) ||
                         (*cp == ':') || (*cp == '.'))
!                     APPEND_CHAR(bufp, bufend, *cp++);
              }
              /* date field? allow embedded text month */
              else if (*cp == '-' || *cp == '/' || *cp == '.')
***************
*** 773,785 ****
                  /* save delimiting character to use later */
                  char        delim = *cp;

!                 *lp++ = *cp++;
                  /* second field is all digits? then no embedded text month */
                  if (isdigit((unsigned char) *cp))
                  {
                      ftype[nf] = ((delim == '.') ? DTK_NUMBER : DTK_DATE);
                      while (isdigit((unsigned char) *cp))
!                         *lp++ = *cp++;

                      /*
                       * insist that the delimiters match to get a
--- 790,802 ----
                  /* save delimiting character to use later */
                  char        delim = *cp;

!                 APPEND_CHAR(bufp, bufend, *cp++);
                  /* second field is all digits? then no embedded text month */
                  if (isdigit((unsigned char) *cp))
                  {
                      ftype[nf] = ((delim == '.') ? DTK_NUMBER : DTK_DATE);
                      while (isdigit((unsigned char) *cp))
!                         APPEND_CHAR(bufp, bufend, *cp++);

                      /*
                       * insist that the delimiters match to get a
***************
*** 788,803 ****
                      if (*cp == delim)
                      {
                          ftype[nf] = DTK_DATE;
!                         *lp++ = *cp++;
                          while (isdigit((unsigned char) *cp) || *cp == delim)
!                             *lp++ = *cp++;
                      }
                  }
                  else
                  {
                      ftype[nf] = DTK_DATE;
                      while (isalnum((unsigned char) *cp) || *cp == delim)
!                         *lp++ = pg_tolower((unsigned char) *cp++);
                  }
              }

--- 805,820 ----
                      if (*cp == delim)
                      {
                          ftype[nf] = DTK_DATE;
!                         APPEND_CHAR(bufp, bufend, *cp++);
                          while (isdigit((unsigned char) *cp) || *cp == delim)
!                             APPEND_CHAR(bufp, bufend, pg_tolower((unsigned char) *cp++));
                      }
                  }
                  else
                  {
                      ftype[nf] = DTK_DATE;
                      while (isalnum((unsigned char) *cp) || *cp == delim)
!                         APPEND_CHAR(bufp, bufend, pg_tolower((unsigned char) *cp++));
                  }
              }

***************
*** 811,819 ****
          /* Leading decimal point? Then fractional seconds... */
          else if (*cp == '.')
          {
!             *lp++ = *cp++;
              while (isdigit((unsigned char) *cp))
!                 *lp++ = *cp++;

              ftype[nf] = DTK_NUMBER;
          }
--- 828,836 ----
          /* Leading decimal point? Then fractional seconds... */
          else if (*cp == '.')
          {
!             APPEND_CHAR(bufp, bufend, *cp++);
              while (isdigit((unsigned char) *cp))
!                 APPEND_CHAR(bufp, bufend, *cp++);

              ftype[nf] = DTK_NUMBER;
          }
***************
*** 825,833 ****
          else if (isalpha((unsigned char) *cp))
          {
              ftype[nf] = DTK_STRING;
!             *lp++ = pg_tolower((unsigned char) *cp++);
              while (isalpha((unsigned char) *cp))
!                 *lp++ = pg_tolower((unsigned char) *cp++);

              /*
               * Full date string with leading text month? Could also be a
--- 842,850 ----
          else if (isalpha((unsigned char) *cp))
          {
              ftype[nf] = DTK_STRING;
!             APPEND_CHAR(bufp, bufend, pg_tolower((unsigned char) *cp++));
              while (isalpha((unsigned char) *cp))
!                 APPEND_CHAR(bufp, bufend, pg_tolower((unsigned char) *cp++));

              /*
               * Full date string with leading text month? Could also be a
***************
*** 838,852 ****
                  char        delim = *cp;

                  ftype[nf] = DTK_DATE;
!                 *lp++ = *cp++;
                  while (isdigit((unsigned char) *cp) || *cp == delim)
!                     *lp++ = *cp++;
              }
          }
          /* sign? then special or numeric timezone */
          else if (*cp == '+' || *cp == '-')
          {
!             *lp++ = *cp++;
              /* soak up leading whitespace */
              while (isspace((unsigned char) *cp))
                  cp++;
--- 855,869 ----
                  char        delim = *cp;

                  ftype[nf] = DTK_DATE;
!                 APPEND_CHAR(bufp, bufend, *cp++);
                  while (isdigit((unsigned char) *cp) || *cp == delim)
!                     APPEND_CHAR(bufp, bufend, *cp++);
              }
          }
          /* sign? then special or numeric timezone */
          else if (*cp == '+' || *cp == '-')
          {
!             APPEND_CHAR(bufp, bufend, *cp++);
              /* soak up leading whitespace */
              while (isspace((unsigned char) *cp))
                  cp++;
***************
*** 854,871 ****
              if (isdigit((unsigned char) *cp))
              {
                  ftype[nf] = DTK_TZ;
!                 *lp++ = *cp++;
                  while (isdigit((unsigned char) *cp) ||
                         *cp == ':' || *cp == '.')
!                     *lp++ = *cp++;
              }
              /* special? */
              else if (isalpha((unsigned char) *cp))
              {
                  ftype[nf] = DTK_SPECIAL;
!                 *lp++ = pg_tolower((unsigned char) *cp++);
                  while (isalpha((unsigned char) *cp))
!                     *lp++ = pg_tolower((unsigned char) *cp++);
              }
              /* otherwise something wrong... */
              else
--- 871,888 ----
              if (isdigit((unsigned char) *cp))
              {
                  ftype[nf] = DTK_TZ;
!                 APPEND_CHAR(bufp, bufend, *cp++);
                  while (isdigit((unsigned char) *cp) ||
                         *cp == ':' || *cp == '.')
!                     APPEND_CHAR(bufp, bufend, *cp++);
              }
              /* special? */
              else if (isalpha((unsigned char) *cp))
              {
                  ftype[nf] = DTK_SPECIAL;
!                 APPEND_CHAR(bufp, bufend, pg_tolower((unsigned char) *cp++));
                  while (isalpha((unsigned char) *cp))
!                     APPEND_CHAR(bufp, bufend, pg_tolower((unsigned char) *cp++));
              }
              /* otherwise something wrong... */
              else
***************
*** 882,888 ****
              return DTERR_BAD_FORMAT;

          /* force in a delimiter after each field */
!         *lp++ = '\0';
          nf++;
      }

--- 899,905 ----
              return DTERR_BAD_FORMAT;

          /* force in a delimiter after each field */
!         *bufp++ = '\0';
          nf++;
      }

Index: src/backend/utils/adt/nabstime.c
===================================================================
RCS file: /var/lib/cvs/pgsql/src/backend/utils/adt/nabstime.c,v
retrieving revision 1.131
diff -c -r1.131 nabstime.c
*** src/backend/utils/adt/nabstime.c    24 May 2005 02:09:45 -0000    1.131
--- src/backend/utils/adt/nabstime.c    25 May 2005 01:06:27 -0000
***************
*** 306,320 ****
                 *tm = &date;
      int            dterr;
      char       *field[MAXDATEFIELDS];
!     char        lowstr[MAXDATELEN + 1];
      int            dtype;
      int            nf,
                  ftype[MAXDATEFIELDS];

!     if (strlen(str) >= sizeof(lowstr))
!         dterr = DTERR_BAD_FORMAT;
!     else
!         dterr = ParseDateTime(str, lowstr, field, ftype, MAXDATEFIELDS, &nf);
      if (dterr == 0)
          dterr = DecodeDateTime(field, ftype, nf, &dtype, tm, &fsec, &tz);
      if (dterr != 0)
--- 306,318 ----
                 *tm = &date;
      int            dterr;
      char       *field[MAXDATEFIELDS];
!     char        workbuf[MAXDATELEN + 1];
      int            dtype;
      int            nf,
                  ftype[MAXDATEFIELDS];

!     dterr = ParseDateTime(str, workbuf, sizeof(workbuf),
!                           field, ftype, MAXDATEFIELDS, &nf);
      if (dterr == 0)
          dterr = DecodeDateTime(field, ftype, nf, &dtype, tm, &fsec, &tz);
      if (dterr != 0)
***************
*** 711,722 ****
      char       *field[MAXDATEFIELDS];
      int            nf,
                  ftype[MAXDATEFIELDS];
!     char        lowstr[MAXDATELEN + 1];

!     if (strlen(str) >= sizeof(lowstr))
!         dterr = DTERR_BAD_FORMAT;
!     else
!         dterr = ParseDateTime(str, lowstr, field, ftype, MAXDATEFIELDS, &nf);
      if (dterr == 0)
          dterr = DecodeInterval(field, ftype, nf, &dtype, tm, &fsec);
      if (dterr != 0)
--- 709,718 ----
      char       *field[MAXDATEFIELDS];
      int            nf,
                  ftype[MAXDATEFIELDS];
!     char        workbuf[MAXDATELEN + 1];

!     dterr = ParseDateTime(str, workbuf, sizeof(workbuf),
!                           field, ftype, MAXDATEFIELDS, &nf);
      if (dterr == 0)
          dterr = DecodeInterval(field, ftype, nf, &dtype, tm, &fsec);
      if (dterr != 0)
Index: src/backend/utils/adt/timestamp.c
===================================================================
RCS file: /var/lib/cvs/pgsql/src/backend/utils/adt/timestamp.c,v
retrieving revision 1.123
diff -c -r1.123 timestamp.c
*** src/backend/utils/adt/timestamp.c    24 May 2005 02:09:45 -0000    1.123
--- src/backend/utils/adt/timestamp.c    25 May 2005 01:06:27 -0000
***************
*** 77,88 ****
      int            dterr;
      char       *field[MAXDATEFIELDS];
      int            ftype[MAXDATEFIELDS];
!     char        lowstr[MAXDATELEN + MAXDATEFIELDS];

!     if (strlen(str) >= sizeof(lowstr))
!         dterr = DTERR_BAD_FORMAT;
!     else
!         dterr = ParseDateTime(str, lowstr, field, ftype, MAXDATEFIELDS, &nf);
      if (dterr == 0)
          dterr = DecodeDateTime(field, ftype, nf, &dtype, tm, &fsec, &tz);
      if (dterr != 0)
--- 77,86 ----
      int            dterr;
      char       *field[MAXDATEFIELDS];
      int            ftype[MAXDATEFIELDS];
!     char        workbuf[MAXDATELEN + MAXDATEFIELDS];

!     dterr = ParseDateTime(str, workbuf, sizeof(workbuf),
!                           field, ftype, MAXDATEFIELDS, &nf);
      if (dterr == 0)
          dterr = DecodeDateTime(field, ftype, nf, &dtype, tm, &fsec, &tz);
      if (dterr != 0)
***************
*** 317,328 ****
      int            dterr;
      char       *field[MAXDATEFIELDS];
      int            ftype[MAXDATEFIELDS];
!     char        lowstr[MAXDATELEN + MAXDATEFIELDS];

!     if (strlen(str) >= sizeof(lowstr))
!         dterr = DTERR_BAD_FORMAT;
!     else
!         dterr = ParseDateTime(str, lowstr, field, ftype, MAXDATEFIELDS, &nf);
      if (dterr == 0)
          dterr = DecodeDateTime(field, ftype, nf, &dtype, tm, &fsec, &tz);
      if (dterr != 0)
--- 315,324 ----
      int            dterr;
      char       *field[MAXDATEFIELDS];
      int            ftype[MAXDATEFIELDS];
!     char        workbuf[MAXDATELEN + MAXDATEFIELDS];

!     dterr = ParseDateTime(str, workbuf, sizeof(workbuf),
!                           field, ftype, MAXDATEFIELDS, &nf);
      if (dterr == 0)
          dterr = DecodeDateTime(field, ftype, nf, &dtype, tm, &fsec, &tz);
      if (dterr != 0)
***************
*** 493,499 ****
      int            dterr;
      char       *field[MAXDATEFIELDS];
      int            ftype[MAXDATEFIELDS];
!     char        lowstr[MAXDATELEN + MAXDATEFIELDS];

      tm->tm_year = 0;
      tm->tm_mon = 0;
--- 489,495 ----
      int            dterr;
      char       *field[MAXDATEFIELDS];
      int            ftype[MAXDATEFIELDS];
!     char        workbuf[256];

      tm->tm_year = 0;
      tm->tm_mon = 0;
***************
*** 503,512 ****
      tm->tm_sec = 0;
      fsec = 0;

!     if (strlen(str) >= sizeof(lowstr))
!         dterr = DTERR_BAD_FORMAT;
!     else
!         dterr = ParseDateTime(str, lowstr, field, ftype, MAXDATEFIELDS, &nf);
      if (dterr == 0)
          dterr = DecodeInterval(field, ftype, nf, &dtype, tm, &fsec);
      if (dterr != 0)
--- 499,506 ----
      tm->tm_sec = 0;
      fsec = 0;

!     dterr = ParseDateTime(str, workbuf, sizeof(workbuf), field,
!                           ftype, MAXDATEFIELDS, &nf);
      if (dterr == 0)
          dterr = DecodeInterval(field, ftype, nf, &dtype, tm, &fsec);
      if (dterr != 0)
Index: src/include/utils/datetime.h
===================================================================
RCS file: /var/lib/cvs/pgsql/src/include/utils/datetime.h,v
retrieving revision 1.53
diff -c -r1.53 datetime.h
*** src/include/utils/datetime.h    24 May 2005 04:03:01 -0000    1.53
--- src/include/utils/datetime.h    25 May 2005 01:06:27 -0000
***************
*** 276,282 ****
  extern void j2date(int jd, int *year, int *month, int *day);
  extern int    date2j(int year, int month, int day);

! extern int ParseDateTime(const char *timestr, char *lowstr,
                char **field, int *ftype,
                int maxfields, int *numfields);
  extern int DecodeDateTime(char **field, int *ftype,
--- 276,282 ----
  extern void j2date(int jd, int *year, int *month, int *day);
  extern int    date2j(int year, int month, int day);

! extern int ParseDateTime(const char *timestr, char *workbuf, size_t buflen,
                char **field, int *ftype,
                int maxfields, int *numfields);
  extern int DecodeDateTime(char **field, int *ftype,
Index: src/test/regress/expected/interval.out
===================================================================
RCS file: /var/lib/cvs/pgsql/src/test/regress/expected/interval.out,v
retrieving revision 1.10
diff -c -r1.10 interval.out
*** src/test/regress/expected/interval.out    7 Apr 2005 01:51:40 -0000    1.10
--- src/test/regress/expected/interval.out    25 May 2005 01:06:27 -0000
***************
*** 221,223 ****
--- 221,230 ----
   @ 4 years 1 mon 10 days 4 hours 18 mins 23 secs
  (1 row)

+ -- test long interval input
+ select '4 millenniums 5 centuries 4 decades 1 year 4 months 4 days 17 minutes 31 seconds'::interval;
+                   interval
+ --------------------------------------------
+  @ 4541 years 4 mons 4 days 17 mins 31 secs
+ (1 row)
+
Index: src/test/regress/sql/interval.sql
===================================================================
RCS file: /var/lib/cvs/pgsql/src/test/regress/sql/interval.sql,v
retrieving revision 1.6
diff -c -r1.6 interval.sql
*** src/test/regress/sql/interval.sql    7 Apr 2005 01:51:41 -0000    1.6
--- src/test/regress/sql/interval.sql    25 May 2005 01:06:27 -0000
***************
*** 66,68 ****
--- 66,71 ----
  -- updating pg_aggregate.agginitval

  select avg(f1) from interval_tbl;
+
+ -- test long interval input
+ select '4 millenniums 5 centuries 4 decades 1 year 4 months 4 days 17 minutes 31 seconds'::interval;