On Mon, 2007-28-05 at 10:50 -0400, Tom Lane wrote:
> I'd argue that it's an oversight. I don't have a problem with adding up
> the values of units that really translate to the same thing (eg,
> '1 week 1 day' -> '8 days'), but I think '1 second 2 second' should
> be rejected because it's almost certainly user error.
I don't see why "1 week 1 week" is any less likely to be user error than
"1 second 1 second".
> Does your patch allow '1 millisec 2 microsec', which should be allowed
> by this argument?
Yes: in order to allow the above input, the straightforward coding also
allows "1 second 2 second".
> I suspect that to make it work unsurprisingly, we'd
> need to allocate a distinct tmask bit to each logically distinct unit.
Okay, attached is a patch that does this. To summarize, the changes are:
* add tmask bits for msec, usec (I reordered the #defines to keep
them logically contiguous, but AFAICS this isn't necessary)
* if the user specifies multiple instances of usec, msec, or sec,
reject as invalid input
* if the user specifies a fractional second ("5.5 seconds"), also
consider that to be millisecond and microsecond input from the
POV of rejecting duplicate units. So "5.5 seconds 1 millisecond"
will be rejected.
Docs and regression tests updated. If people are happy with the above
behavior, I'll commit this to HEAD shortly (today/tomorrow), and perhaps
backport it: not accepting "1 msec" and similar inputs is a clear bug,
IMHO.
BTW, does anyone know why a few of the regression tests (tinterval,
point, geometry, etc.) explicitly disable and then re-enable GEQO?
AFAICS the regression tests are just doing fairly routine two-table
joins, so GEQO will not be invoked with a sane configuration anyway.
-Neil
Index: doc/src/sgml/datatype.sgml
===================================================================
RCS file: /home/neilc/postgres/cvs_root/pgsql/doc/src/sgml/datatype.sgml,v
retrieving revision 1.201
diff -p -c -r1.201 datatype.sgml
*** doc/src/sgml/datatype.sgml 21 May 2007 17:10:28 -0000 1.201
--- doc/src/sgml/datatype.sgml 28 May 2007 17:11:14 -0000
*************** January 8 04:05:06 1999 PST
*** 1880,1886 ****
</programlisting>
Where: <replaceable>quantity</> is a number (possibly signed);
! <replaceable>unit</> is <literal>second</literal>,
<literal>minute</literal>, <literal>hour</literal>, <literal>day</literal>,
<literal>week</literal>, <literal>month</literal>, <literal>year</literal>,
<literal>decade</literal>, <literal>century</literal>, <literal>millennium</literal>,
--- 1880,1887 ----
</programlisting>
Where: <replaceable>quantity</> is a number (possibly signed);
! <replaceable>unit</> is <literal>microsecond</literal>,
! <literal>millisecond</literal>, <literal>second</literal>,
<literal>minute</literal>, <literal>hour</literal>, <literal>day</literal>,
<literal>week</literal>, <literal>month</literal>, <literal>year</literal>,
<literal>decade</literal>, <literal>century</literal>, <literal>millennium</literal>,
Index: src/backend/utils/adt/datetime.c
===================================================================
RCS file: /home/neilc/postgres/cvs_root/pgsql/src/backend/utils/adt/datetime.c,v
retrieving revision 1.179
diff -p -c -r1.179 datetime.c
*** src/backend/utils/adt/datetime.c 27 May 2007 20:32:16 -0000 1.179
--- src/backend/utils/adt/datetime.c 28 May 2007 17:25:09 -0000
*************** DecodeDateTime(char **field, int *ftype,
*** 924,929 ****
--- 924,930 ----
#else
*fsec = frac;
#endif
+ tmask = DTK_ALL_SECS_M;
}
break;
*************** DecodeTimeOnly(char **field, int *ftype,
*** 1699,1704 ****
--- 1700,1706 ----
#else
*fsec = frac;
#endif
+ tmask = DTK_ALL_SECS_M;
}
break;
*************** DecodeInterval(char **field, int *ftype,
*** 2805,2810 ****
--- 2807,2813 ----
#else
*fsec += (val + fval) * 1e-6;
#endif
+ tmask = DTK_M(MICROSECOND);
break;
case DTK_MILLISEC:
*************** DecodeInterval(char **field, int *ftype,
*** 2813,2818 ****
--- 2816,2822 ----
#else
*fsec += (val + fval) * 1e-3;
#endif
+ tmask = DTK_M(MILLISECOND);
break;
case DTK_SECOND:
*************** DecodeInterval(char **field, int *ftype,
*** 2822,2828 ****
#else
*fsec += fval;
#endif
! tmask = DTK_M(SECOND);
break;
case DTK_MINUTE:
--- 2826,2840 ----
#else
*fsec += fval;
#endif
! /*
! * If any subseconds were specified, consider
! * this microsecond and millisecond input as
! * well.
! */
! if (fval == 0)
! tmask = DTK_M(SECOND);
! else
! tmask = DTK_ALL_SECS_M;
break;
case DTK_MINUTE:
Index: src/include/utils/datetime.h
===================================================================
RCS file: /home/neilc/postgres/cvs_root/pgsql/src/include/utils/datetime.h,v
retrieving revision 1.65
diff -p -c -r1.65 datetime.h
*** src/include/utils/datetime.h 19 Feb 2007 17:41:39 -0000 1.65
--- src/include/utils/datetime.h 28 May 2007 17:18:47 -0000
***************
*** 100,116 ****
#define HOUR 10
#define MINUTE 11
#define SECOND 12
! #define DOY 13
! #define DOW 14
! #define UNITS 15
! #define ADBC 16
/* these are only for relative dates */
! #define AGO 17
! #define ABS_BEFORE 18
! #define ABS_AFTER 19
/* generic fields to help with parsing */
! #define ISODATE 20
! #define ISOTIME 21
/* reserved for unrecognized string values */
#define UNKNOWN_FIELD 31
--- 100,118 ----
#define HOUR 10
#define MINUTE 11
#define SECOND 12
! #define MILLISECOND 13
! #define MICROSECOND 14
! #define DOY 15
! #define DOW 16
! #define UNITS 17
! #define ADBC 18
/* these are only for relative dates */
! #define AGO 19
! #define ABS_BEFORE 20
! #define ABS_AFTER 21
/* generic fields to help with parsing */
! #define ISODATE 22
! #define ISOTIME 23
/* reserved for unrecognized string values */
#define UNKNOWN_FIELD 31
***************
*** 175,182 ****
#define DTK_M(t) (0x01 << (t))
#define DTK_DATE_M (DTK_M(YEAR) | DTK_M(MONTH) | DTK_M(DAY))
! #define DTK_TIME_M (DTK_M(HOUR) | DTK_M(MINUTE) | DTK_M(SECOND))
#define MAXDATELEN 63 /* maximum possible length of an input date
* string (not counting tr. null) */
--- 177,186 ----
#define DTK_M(t) (0x01 << (t))
+ /* Convenvience: a second, plus any fractional component */
+ #define DTK_ALL_SECS_M (DTK_M(SECOND) | DTK_M(MILLISECOND) | DTK_M(MICROSECOND))
#define DTK_DATE_M (DTK_M(YEAR) | DTK_M(MONTH) | DTK_M(DAY))
! #define DTK_TIME_M (DTK_M(HOUR) | DTK_M(MINUTE) | DTK_ALL_SECS_M)
#define MAXDATELEN 63 /* maximum possible length of an input date
* string (not counting tr. null) */
Index: src/test/regress/expected/interval.out
===================================================================
RCS file: /home/neilc/postgres/cvs_root/pgsql/src/test/regress/expected/interval.out,v
retrieving revision 1.18
diff -p -c -r1.18 interval.out
*** src/test/regress/expected/interval.out 6 Sep 2006 02:05:41 -0000 1.18
--- src/test/regress/expected/interval.out 28 May 2007 17:41:26 -0000
*************** SELECT justify_interval(interval '1 mont
*** 326,328 ****
--- 326,351 ----
@ 29 days 23 hours
(1 row)
+ -- test fractional second input, and detection of duplicate units
+ SET DATESTYLE = 'ISO';
+ SELECT '1 millisecond'::interval, '1 microsecond'::interval,
+ '500 seconds 99 milliseconds 51 microseconds'::interval;
+ interval | interval | interval
+ --------------+-----------------+-----------------
+ 00:00:00.001 | 00:00:00.000001 | 00:08:20.099051
+ (1 row)
+
+ SELECT '3 days 5 milliseconds'::interval;
+ interval
+ ---------------------
+ 3 days 00:00:00.005
+ (1 row)
+
+ SELECT '1 second 2 seconds'::interval; -- error
+ ERROR: invalid input syntax for type interval: "1 second 2 seconds"
+ SELECT '10 milliseconds 20 milliseconds'::interval; -- error
+ ERROR: invalid input syntax for type interval: "10 milliseconds 20 milliseconds"
+ SELECT '5.5 seconds 3 milliseconds'::interval; -- error
+ ERROR: invalid input syntax for type interval: "5.5 seconds 3 milliseconds"
+ SELECT '1:20:05 5 microseconds'::interval; -- error
+ ERROR: invalid input syntax for type interval: "1:20:05 5 microseconds"
Index: src/test/regress/sql/interval.sql
===================================================================
RCS file: /home/neilc/postgres/cvs_root/pgsql/src/test/regress/sql/interval.sql,v
retrieving revision 1.11
diff -p -c -r1.11 interval.sql
*** src/test/regress/sql/interval.sql 6 Sep 2006 02:05:41 -0000 1.11
--- src/test/regress/sql/interval.sql 28 May 2007 17:39:00 -0000
*************** SELECT justify_days(interval '6 months 3
*** 116,118 ****
--- 116,129 ----
-- test justify_interval()
SELECT justify_interval(interval '1 month -1 hour') as "1 month -1 hour";
+
+ -- test fractional second input, and detection of duplicate units
+ SET DATESTYLE = 'ISO';
+ SELECT '1 millisecond'::interval, '1 microsecond'::interval,
+ '500 seconds 99 milliseconds 51 microseconds'::interval;
+ SELECT '3 days 5 milliseconds'::interval;
+
+ SELECT '1 second 2 seconds'::interval; -- error
+ SELECT '10 milliseconds 20 milliseconds'::interval; -- error
+ SELECT '5.5 seconds 3 milliseconds'::interval; -- error
+ SELECT '1:20:05 5 microseconds'::interval; -- error
\ No newline at end of file
---------------------------(end of broadcast)---------------------------
TIP 7: You can help support the PostgreSQL project by donating at
http://www.postgresql.org/about/donate
