From 72405b066b8ea7ba323f773a751e5dec779c4ec9 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Tue, 3 Aug 2010 16:31:02 +0000 Subject: [PATCH] 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. --- src/backend/utils/adt/nabstime.c | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/src/backend/utils/adt/nabstime.c b/src/backend/utils/adt/nabstime.c index 3c744ad8bb..30730f7b09 100644 --- 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) -- 2.40.0