Thread: BUG #1671: Long interval string representation rejected
The following bug has been logged online: Bug reference: 1671 Logged by: Mark Dilger Email address: markdilger@yahoo.com PostgreSQL version: 8.0.2 Operating system: Mandrake Linux 9.1 (2.4.21-0.13mdkenterprise #1 SMP) Description: Long interval string representation rejected Details: <QUOTE> bash-2.05b$ psql Welcome to psql 8.0.2, the PostgreSQL interactive terminal. Type: \copyright for distribution terms \h for help with SQL commands \? for help with psql commands \g or terminate with semicolon to execute query \q to quit mark=# select '4 millenniums 5 centuries 4 decades 1 year 4 months 4 days 17 minutes 31 seconds'::interval; ERROR: invalid input syntax for type interval: "4 millenniums 5 centuries 4 decades 1 year 4 months 4 days 17 minutes 31 seconds" mark=# select '4 millenniums 5 centuries 4 decades 1 year 4 months 4 days 17 minutes'::interval; interval ----------------------------------- 4541 years 4 mons 4 days 00:17:00 (1 row) </QUOTE> It appears that any string representation of an interval of length greater than 76 is rejected. (76 = 51 + 25 = MAXDATELEN + MAXDATEFIELDS). This appears to be a limitation enforced within function interval_in() in the file src/backend/utils/adt/timestamp.c
Mark Dilger wrote: > It appears that any string representation of an interval of length greater > than 76 is rejected. (76 = 51 + 25 = MAXDATELEN + MAXDATEFIELDS). This > appears to be a limitation enforced within function interval_in() in the > file src/backend/utils/adt/timestamp.c Yeah, this seems bogus. It's not even clear to me why MAXDATELEN + MAXDATEFIELDS is used as the size of that buffer in the first place. I don't know the datetime code particularly well; perhaps someone who does can shed some light on this? -Neil
Neil Conway <neilc@samurai.com> writes: > Yeah, this seems bogus. It's not even clear to me why MAXDATELEN + > MAXDATEFIELDS is used as the size of that buffer in the first place. I > don't know the datetime code particularly well; perhaps someone who does > can shed some light on this? My rule of thumb with the datetime code is that if it looks bogus, it probably is :-( There are a lot of fixed-size local buffers in that code. The ones used in output routines seem defensible since the string to be generated is predictable. The ones that are used for processing input are likely wrong. OTOH I'm not eager to throw a palloc into each of those code paths ... can we avoid that? regards, tom lane
Tom Lane wrote: > There are a lot of fixed-size local buffers in that code. The ones > used in output routines seem defensible since the string to be generated > is predictable. The ones that are used for processing input are likely > wrong. OTOH I'm not eager to throw a palloc into each of those code > paths ... can we avoid that? I'm not sure offhand what the upper bounds on legal input for each of the datetime types is. Why not just allocate a larger but still fixed-size buffer -- say, 256 bytes? (While we're on the subject, it seems rather silly for ParseDateTime() not to do its own bounds checking -- all of its call sites do a strlen() on the input buffer before calling it, which could be avoided if ParseDateTime() we passed the size of `lowstr') -Neil
Neil Conway <neilc@samurai.com> writes: > Tom Lane wrote: >> There are a lot of fixed-size local buffers in that code. The ones >> used in output routines seem defensible since the string to be generated >> is predictable. The ones that are used for processing input are likely >> wrong. > I'm not sure offhand what the upper bounds on legal input for each of > the datetime types is. Well, if you allow for whitespace between tokens then it's immediately clear that there is no fixed upper bound. Perhaps it would work to downcase just one token at a time, so that the max buffer length equals the max acceptable token? regards, tom lane
Tom Lane wrote: > Well, if you allow for whitespace between tokens then it's immediately > clear that there is no fixed upper bound. Good point -- there is no upper bound on the input string, but since we skip whitespace, AFAICS this shouldn't affect the requirements for the size of the working buffer (lowstr). So if we passed the size of the working buffer to ParseDateTime() (per earlier gripe), we could only bail out when we actually need to use more working space than was allocated, not simply when strlen(input) >= sizeof(buffer). The implementation might be a bit ugly, though. > Perhaps it would work to downcase just one token at a time, so that > the max buffer length equals the max acceptable token? Not sure I follow you. -Neil
Neil Conway <neilc@samurai.com> writes: >> Perhaps it would work to downcase just one token at a time, so that >> the max buffer length equals the max acceptable token? > Not sure I follow you. I believe that the reason for the local buffer is to hold a downcased version of the input, which we can compare to the all-lower-case tables of relevant keywords. So plan A for fixing it is to downcase and compare one token at a time, instead of downcasing the whole string at once. Plan B is to make the token comparison routines case-insensitive themselves, so that there's no need for a temporary copy of the input. Probably someone can think of plans C and D too ... I don't have any strong allegiance to any one way to fix it. regards, tom lane
Tom Lane wrote: > I believe that the reason for the local buffer is to hold a downcased > version of the input, which we can compare to the all-lower-case tables > of relevant keywords. Well, that's one of the reasons, but not the only one. For example, how do you parse '17 minutes31 seconds'::interval without either a working buffer or the ability to resize the input buffer? (We need to split the input into two fields, each of which must be a NUL-terminated string. These fields are currently implemented as into the working buffer. If we changed that to be pointers into the input string, we have no where to put the NUL terminator. So you either need a separate buffer, repalloc() plus shuffling everything down, or palloc'd allocation for the fields themselves.) So I don't see that fixing the casing issue is sufficient. On closer inspection, the current code seems to get this wrong as well :-( For example, given the query: select '4millenniums5centuries4decades1year4months4days17minutes31 seconds'::interval; attaching via gdb, with a breakpoint at datetime.c:890 -- (gdb) p lp $1 = 0xbfffdde1 "ÿÞ@" (gdb) p lowstr + 25 + 51 $4 = 0xbfffdddc "onds" (i.e. lp points past the end of lowstr) -Neil
Neil Conway <neilc@samurai.com> writes: > Tom Lane wrote: >> I believe that the reason for the local buffer is to hold a downcased >> version of the input, which we can compare to the all-lower-case tables >> of relevant keywords. > Well, that's one of the reasons, but not the only one. For example, how > do you parse '17 minutes31 seconds'::interval without either a working > buffer or the ability to resize the input buffer? Sorry, s/downcased/downcased and null-terminated/. I have not read the parsing code in question lately, but offhand it seems like transferring one token at a time into a work buffer isn't a completely broken idea... > On closer inspection, the current code seems to get this wrong as well > :-( Wouldn't be surprised :-( regards, tom lane
Tom Lane wrote: > Sorry, s/downcased/downcased and null-terminated/. I have not read the > parsing code in question lately, but offhand it seems like transferring > one token at a time into a work buffer isn't a completely broken idea... I wouldn't call it "broken", but I don't see how it could be done without a significant refactoring of how datetime parsing is done, and your handwaving has yet to convince me :) The gist of the current code is: 1. parse the input string into fields, which are arrays of pointers into a working buffer, via ParseDateTime() 2. decode the fields as appropriate for the datatype via DecodeInterval(), DecodeDatetime(), DecodeTimeOnly(), etc. i.e. I don't see an easy way to do field decoding one field at a time. -Neil
Neil Conway <neilc@samurai.com> writes: > your handwaving has yet to convince me :) Zing ;-) I'm too tired to think about this more tonight, but it seems like the basic point is that we can't just construct a list of pointers into (a copy of) the original input string. A certain amount of data copying is going to have to occur; or else we change the token matching code to not assume null-termination. regards, tom lane
Tom Lane wrote: > I'm too tired to think about this more tonight, but it seems like > the basic point is that we can't just construct a list of pointers > into (a copy of) the original input string. I think we _can_ do it that way, it's just a question of whether that is the best approach. I think the solution I outlined before would work fine: pass the length of the working buffer to ParseDateTime(), and reject the input only if the parsing process actually requires more working space than was provided. ParseDateTime() can easily skip whitespace without storing it in the temporary buffer -- and once we've done that, AFAIK we _can_ place an upper bound on the amount of working space that is needed (each field has a maximum length and there are a maximum number of fields). Or we could rewrite how we do parsing, which seems to be what you're suggesting. I agree the code could do with cleanup, although we will also need some kind of minimally-invasive fix for back branches. -Neil
Neil Conway wrote: > I think we _can_ do it that way, it's just a question of whether that is > the best approach. I think the solution I outlined before would work > fine: pass the length of the working buffer to ParseDateTime(), and > reject the input only if the parsing process actually requires more > working space than was provided. [...] > Or we could rewrite how we do parsing, which seems to be what you're > suggesting. Any thoughts on this, Tom? If you're planning on doing the rewrite you described, that's fine; alternatively I'm happy to implement the fix that I described above. -Neil
Neil Conway <neilc@samurai.com> writes: > Any thoughts on this, Tom? If you're planning on doing the rewrite you > described, that's fine; alternatively I'm happy to implement the fix > that I described above. I'm not set on it --- fix it as you suggested. regards, tom lane
Tom Lane wrote: > I'm not set on it --- fix it as you suggested. Attached is a patch that implements this. I'm not especially happy about the implementation: defining _two_ local macros (that both doubly-evaluate one of their arguments) is pretty ugly, but I didn't see a cleaner alternative -- suggestions welcome. This patch fixes the stack overrun. Since we now pass the length of the working space into ParseDateTime(), it is also easy to fix Mark's bug: I just increased the size of the interval_in() working buffer to 256 bytes. I didn't touch any of the other input buffer sizes, although that shouldn't be taken as a vote of confidence that they are correct :) At least the consequence of an insufficiently-large buffer is limited to rejecting otherwise valid input, rather than potentially scribbling on the stack. -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 24 May 2005 06:24:54 -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 24 May 2005 06:40:01 -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,765 ---- * 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; ! ! /* ! * Append the character at "inptr" to the current position ! * "bufptr", and increment both pointers. Returns an error if ! * there is no space left in bufptr ("end" marks the last element ! * of the buffer). Beware of the double evaluation of "bufptr". ! */ ! #define APPEND_CHAR(bufptr, inptr, end) \ ! do \ ! { \ ! if (((bufptr) + 1) >= (end)) \ ! return DTERR_BAD_FORMAT; \ ! *(bufptr)++ = *(inptr)++; \ ! } while (0); ! ! #define APPEND_LOWER_CHAR(bufptr, inptr, end) \ ! do \ ! { \ ! if (((bufptr) + 1) >= (end)) \ ! return DTERR_BAD_FORMAT; \ ! *(bufptr)++ = pg_tolower((unsigned char) *(inptr)++); \ ! } 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 == '.') --- 774,796 ---- /* 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, cp, bufend); while (isdigit((unsigned char) *cp)) ! APPEND_CHAR(bufp, cp, bufend); /* time field? */ if (*cp == ':') { ftype[nf] = DTK_TIME; ! APPEND_CHAR(bufp, cp, bufend); while (isdigit((unsigned char) *cp) || (*cp == ':') || (*cp == '.')) ! APPEND_CHAR(bufp, cp, bufend); } /* 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 --- 798,810 ---- /* save delimiting character to use later */ char delim = *cp; ! APPEND_CHAR(bufp, cp, bufend); /* 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, cp, bufend); /* * 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++); } } --- 813,828 ---- if (*cp == delim) { ftype[nf] = DTK_DATE; ! APPEND_CHAR(bufp, cp, bufend); while (isdigit((unsigned char) *cp) || *cp == delim) ! APPEND_LOWER_CHAR(bufp, cp, bufend); } } else { ftype[nf] = DTK_DATE; while (isalnum((unsigned char) *cp) || *cp == delim) ! APPEND_LOWER_CHAR(bufp, cp, bufend); } } *************** *** 811,819 **** /* Leading decimal point? Then fractional seconds... */ else if (*cp == '.') { ! *lp++ = *cp++; while (isdigit((unsigned char) *cp)) ! *lp++ = *cp++; ftype[nf] = DTK_NUMBER; } --- 836,844 ---- /* Leading decimal point? Then fractional seconds... */ else if (*cp == '.') { ! APPEND_CHAR(bufp, cp, bufend); while (isdigit((unsigned char) *cp)) ! APPEND_CHAR(bufp, cp, bufend); 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 --- 850,858 ---- else if (isalpha((unsigned char) *cp)) { ftype[nf] = DTK_STRING; ! APPEND_LOWER_CHAR(bufp, cp, bufend); while (isalpha((unsigned char) *cp)) ! APPEND_LOWER_CHAR(bufp, cp, bufend); /* * 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++; --- 863,877 ---- char delim = *cp; ftype[nf] = DTK_DATE; ! APPEND_CHAR(bufp, cp, bufend); while (isdigit((unsigned char) *cp) || *cp == delim) ! APPEND_CHAR(bufp, cp, bufend); } } /* sign? then special or numeric timezone */ else if (*cp == '+' || *cp == '-') { ! APPEND_CHAR(bufp, cp, bufend); /* 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 --- 879,896 ---- if (isdigit((unsigned char) *cp)) { ftype[nf] = DTK_TZ; ! APPEND_CHAR(bufp, cp, bufend); while (isdigit((unsigned char) *cp) || *cp == ':' || *cp == '.') ! APPEND_CHAR(bufp, cp, bufend); } /* special? */ else if (isalpha((unsigned char) *cp)) { ftype[nf] = DTK_SPECIAL; ! APPEND_LOWER_CHAR(bufp, cp, bufend); while (isalpha((unsigned char) *cp)) ! APPEND_LOWER_CHAR(bufp, cp, bufend); } /* otherwise something wrong... */ else *************** *** 882,888 **** return DTERR_BAD_FORMAT; /* force in a delimiter after each field */ ! *lp++ = '\0'; nf++; } --- 907,913 ---- 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 24 May 2005 06:24:54 -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 24 May 2005 06:24:54 -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 24 May 2005 06:24:54 -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 24 May 2005 06:24:54 -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 24 May 2005 06:24:54 -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;
Neil Conway <neilc@samurai.com> writes: > Attached is a patch that implements this. I'm not especially happy about > the implementation: defining _two_ local macros (that both > doubly-evaluate one of their arguments) is pretty ugly, but I didn't see > a cleaner alternative -- suggestions welcome. 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 #define APPEND_CHAR(bufptr, end, newchar) \ do \ { \ if (((bufptr) + 1) >= (end)) \ return DTERR_BAD_FORMAT; \ *(bufptr)++ = (newchar); \ } while (0); used as APPEND_CHAR(bufp, bufend, *cp++); APPEND_CHAR(bufp, bufend, pg_tolower((unsigned char) *cp++)); respectively. regards, tom lane
I wrote: > #define APPEND_CHAR(bufptr, end, newchar) \ > do \ > { \ > if (((bufptr) + 1) >= (end)) \ > return DTERR_BAD_FORMAT; \ > *(bufptr)++ = (newchar); \ > } while (0); Oh BTW, there definitely should not be a semicolon at the end of that macro ... regards, tom lane
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;
Neil Conway wrote: > 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. Patch applied to HEAD, REL8_0_STABLE, and REL7_4_STABLE. Thanks for the report, Mark. -Neil