Add some comments to tinterval_cmp_internal pointing out its severe

implementation deficiencies.  Per discussion of bug #5592, we're not
going to change it, but these things should be documented so that if
anyone ever reimplements type tinterval, they will be more careful.

diff --git a/src/backend/utils/adt/nabstime.c b/src/backend/utils/adt/nabstime.c
index 3c744ad8bbd01712f57b34d647fffec3e0195887..30730f7b0963c96f70c58af4e392eb2272958d6d 100644 (file)
--- a/src/backend/utils/adt/nabstime.c
+++ b/src/backend/utils/adt/nabstime.c
@@ -10,7 +10,7 @@
  *
  *
  * IDENTIFICATION
- *       $PostgreSQL: pgsql/src/backend/utils/adt/nabstime.c,v 1.164 2010/02/26 02:01:09 momjian Exp $
+ *       $PostgreSQL: pgsql/src/backend/utils/adt/nabstime.c,v 1.165 2010/08/03 16:31:02 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -1155,9 +1155,22 @@ tintervalsame(PG_FUNCTION_ARGS)
 /*
  * tinterval comparison routines
  *
- * Note: comparison is based on the lengths of the tintervals, not on
- * endpoint value.     This is pretty bogus, but since it's only a legacy
- * datatype I'm not going to propose changing it.
+ * Note: comparison is based only on the lengths of the tintervals, not on
+ * endpoint values (as long as they're not INVALID).  This is pretty bogus,
+ * but since it's only a legacy datatype, we're not going to change it.
+ *
+ * Some other bogus things that won't be changed for compatibility reasons:
+ * 1. The interval length computations overflow at 2^31 seconds, causing
+ * intervals longer than that to sort oddly compared to those shorter.
+ * 2. infinity and minus infinity (NOEND_ABSTIME and NOSTART_ABSTIME) are
+ * just ordinary integers.  Since this code doesn't handle them specially,
+ * it's possible for [a b] to be considered longer than [c infinity] for
+ * finite abstimes a, b, c.  In combination with the previous point, the
+ * interval [-infinity infinity] is treated as being shorter than many finite
+ * intervals :-(
+ *
+ * If tinterval is ever reimplemented atop timestamp, it'd be good to give
+ * some consideration to avoiding these problems.
  */
 static int
 tinterval_cmp_internal(TimeInterval a, TimeInterval b)