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

Reply via email to