SEO

I have written the attached patch which I think does what you suggested.
I found all the places where we disallowed 24:00:00, and make it valid,
including nabstime.c.

test=> select '24:00:00'::time(0);
time
----------
24:00:00
(1 row)

test=> select '24:00:01'::time(0);
ERROR: date/time field value out of range: "24:00:01"

---------------------------------------------------------------------------

Tom Lane wrote:
> Inserting into a time field with limited precision rounds off, which
> is good except for this case:
>
> regression=# select '23:59:59.9'::time(0);
> time
> ----------
> 24:00:00
> (1 row)
>
> This is bad because:
>
> regression=# select '24:00:00'::time(0);
> ERROR: date/time field value out of range: "24:00:00"
>
> which means that data originally accepted will fail to dump and reload.
>
> I see this behavior in all versions back to 7.3. 7.2 was even more
> broken:
>
> regression=# select '23:59:59.9'::time(0);
> time
> ----------
> 00:00:00
> (1 row)
>
> I think the correct behavior has to be to check for overflow again
> after rounding off. Alternatively: why are we forbidding the value
> 24:00:00 anyway? Is there a reason not to allow the hours field
> to exceed 23?
>
> regards, tom lane
>
> ---------------------------(end of broadcast)---------------------------
> TIP 6: explain analyze is your friend
>

--
Bruce Momjian | http://candle.pha.pa.us
pgman@candle.pha.pa.us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073

Index: src/backend/utils/adt/datetime.c
================================================== =================
RCS file: /cvsroot/pgsql/src/backend/utils/adt/datetime.c,v
retrieving revision 1.158
diff -c -c -r1.158 datetime.c
*** src/backend/utils/adt/datetime.c 9 Oct 2005 17:21:46 -0000 1.158
--- src/backend/utils/adt/datetime.c 14 Oct 2005 02:52:39 -0000
***************
*** 1114,1120 ****
* Check upper limit on hours; other limits checked in
* DecodeTime()
*/
! if (tm->tm_hour > 23)
return DTERR_FIELD_OVERFLOW;
break;

--- 1114,1122 ----
* Check upper limit on hours; other limits checked in
* DecodeTime()
*/
! /* test for > 24:00:00 */
! if (tm->tm_hour > 24 ||
! (tm->tm_hour == 24 && (tm->tm_min > 0 || tm->tm_sec > 0)))
return DTERR_FIELD_OVERFLOW;
break;

***************
*** 2243,2256 ****
else if (mer == PM && tm->tm_hour != 12)
tm->tm_hour += 12;

#ifdef HAVE_INT64_TIMESTAMP
! if (tm->tm_hour < 0 || tm->tm_hour > 23 || tm->tm_min < 0 ||
! tm->tm_min > 59 || tm->tm_sec < 0 || tm->tm_sec > 60 ||
*fsec < INT64CONST(0) || *fsec >= USECS_PER_SEC)
return DTERR_FIELD_OVERFLOW;
#else
! if (tm->tm_hour < 0 || tm->tm_hour > 23 || tm->tm_min < 0 ||
! tm->tm_min > 59 || tm->tm_sec < 0 || tm->tm_sec > 60 ||
*fsec < 0 || *fsec >= 1)
return DTERR_FIELD_OVERFLOW;
#endif
--- 2245,2260 ----
else if (mer == PM && tm->tm_hour != 12)
tm->tm_hour += 12;

+ 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 ||
+ /* Allow 24:00:00 */
+ (tm->tm_hour == 24 && (tm->tm_min > 0 || tm->tm_sec > 0 ||
#ifdef HAVE_INT64_TIMESTAMP
! *fsec > INT64CONST(0))) ||
*fsec < INT64CONST(0) || *fsec >= USECS_PER_SEC)
return DTERR_FIELD_OVERFLOW;
#else
! *fsec > 0)) ||
*fsec < 0 || *fsec >= 1)
return DTERR_FIELD_OVERFLOW;
#endif
Index: src/backend/utils/adt/nabstime.c
================================================== =================
RCS file: /cvsroot/pgsql/src/backend/utils/adt/nabstime.c,v
retrieving revision 1.143
diff -c -c -r1.143 nabstime.c
*** src/backend/utils/adt/nabstime.c 24 Sep 2005 22:54:38 -0000 1.143
--- src/backend/utils/adt/nabstime.c 14 Oct 2005 02:52:40 -0000
***************
*** 187,193 ****
if (tm->tm_year < 1901 || tm->tm_year > 2038
|| tm->tm_mon < 1 || tm->tm_mon > 12
|| tm->tm_mday < 1 || tm->tm_mday > 31
! || tm->tm_hour < 0 || tm->tm_hour > 23
|| tm->tm_min < 0 || tm->tm_min > 59
|| tm->tm_sec < 0 || tm->tm_sec > 60)
return INVALID_ABSTIME;
--- 187,195 ----
if (tm->tm_year < 1901 || tm->tm_year > 2038
|| tm->tm_mon < 1 || tm->tm_mon > 12
|| tm->tm_mday < 1 || tm->tm_mday > 31
! || tm->tm_hour < 0
! || tm->tm_hour > 24 /* Allow 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)
return INVALID_ABSTIME;
Index: src/interfaces/ecpg/pgtypeslib/dt_common.c
================================================== =================
RCS file: /cvsroot/pgsql/src/interfaces/ecpg/pgtypeslib/dt_common.c,v
retrieving revision 1.30
diff -c -c -r1.30 dt_common.c
*** src/interfaces/ecpg/pgtypeslib/dt_common.c 9 Oct 2005 17:21:45 -0000 1.30
--- src/interfaces/ecpg/pgtypeslib/dt_common.c 14 Oct 2005 02:52:41 -0000
***************
*** 2095,2101 ****
* Check upper limit on hours; other limits checked in
* DecodeTime()
*/
! if (tm->tm_hour > 23)
return -1;
break;

--- 2095,2103 ----
* Check upper limit on hours; other limits checked in
* DecodeTime()
*/
! /* test for > 24:00:00 */
! if (tm->tm_hour > 24 ||
! (tm->tm_hour == 24 && (tm->tm_min > 0 || tm->tm_sec > 0)))
return -1;
break;

***************
*** 3161,3167 ****
err = 1;
*minute = 0;
}
! if (*hour > 23)
{
err = 1;
*hour = 0;
--- 3163,3170 ----
err = 1;
*minute = 0;
}
! if (*hour > 24 ||
! (*hour == 24 && (*minute > 0 || *second > 0)))
{
err = 1;
*hour = 0;


---------------------------(end of broadcast)---------------------------
TIP 3: Have you checked our extensive FAQ?

http://www.postgresql.org/docs/faq

Bruce Momjian <pgman@candle.pha.pa.us> writes:
> I have written the attached patch which I think does what you suggested.
> I found all the places where we disallowed 24:00:00, and make it valid,
> including nabstime.c.

Looks reasonable right offhand ... don't forget to update the docs too.

regards, tom lane

---------------------------(end of broadcast)---------------------------
TIP 5: don't forget to increase your free space map settings

On Thu, 2005-13-10 at 22:54 -0400, Bruce Momjian wrote:
> I have written the attached patch which I think does what you suggested.
> I found all the places where we disallowed 24:00:00, and make it valid,
> including nabstime.c.

Should this be added to the regression tests?

-Neil



---------------------------(end of broadcast)---------------------------
TIP 9: In versions below 8.0, the planner will ignore your desire to
choose an index scan if your joining column's datatypes do not
match

Neil Conway wrote:
> On Thu, 2005-13-10 at 22:54 -0400, Bruce Momjian wrote:
> > I have written the attached patch which I think does what you suggested.
> > I found all the places where we disallowed 24:00:00, and make it valid,
> > including nabstime.c.
>
> Should this be added to the regression tests?

Yes.

--
Bruce Momjian | http://candle.pha.pa.us
pgman@candle.pha.pa.us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073

---------------------------(end of broadcast)---------------------------
TIP 1: if posting/reading through Usenet, please send an appropriate
subscribe-nomail command to majordomo@postgresql.org so that your
message can get through to the mailing list cleanly