Rewrite interval_hash() so that the hashcodes are equal for values that

interval_eq() considers equal.  I'm not sure how that fundamental requirement
escaped us through multiple revisions of this hash function, but there it is;
it's been wrong since interval_hash was first written for PG 7.1.
Per bug #4748 from Roman Kononov.

Backpatch to all supported releases.

This patch changes the contents of hash indexes for interval columns.  That's
no particular problem for PG 8.4, since we've broken on-disk compatibility
of hash indexes already; but it will require a migration warning note in
the next minor releases of all existing branches: "if you have any hash
indexes on columns of type interval, REINDEX them after updating".

diff --git a/src/backend/utils/adt/timestamp.c b/src/backend/utils/adt/timestamp.c
index 5a08e584d95b75fe887f83f55b97b7c979973b35..74b56631aed3b53d845256e471e90bb7697f1a79 100644 (file)
--- a/src/backend/utils/adt/timestamp.c
+++ b/src/backend/utils/adt/timestamp.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *       $PostgreSQL: pgsql/src/backend/utils/adt/timestamp.c,v 1.184.2.1 2008/07/07 18:09:53 tgl Exp $
+ *       $PostgreSQL: pgsql/src/backend/utils/adt/timestamp.c,v 1.184.2.2 2009/04/04 04:53:34 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -2029,32 +2029,36 @@ timestamptz_cmp_timestamp(PG_FUNCTION_ARGS)
  *
  *             collate invalid interval at the end
  */
-static int
-interval_cmp_internal(Interval *interval1, Interval *interval2)
-{
 #ifdef HAVE_INT64_TIMESTAMP
-       int64           span1,
-                               span2;
+typedef int64 TimeOffset;
 #else
-       double          span1,
-                               span2;
+typedef double TimeOffset;
 #endif
 
-       span1 = interval1->time;
-       span2 = interval2->time;
+static inline TimeOffset
+interval_cmp_value(const Interval *interval)
+{
+       TimeOffset      span;
+
+       span = interval->time;
 
 #ifdef HAVE_INT64_TIMESTAMP
-       span1 += interval1->month * INT64CONST(30) * USECS_PER_DAY;
-       span1 += interval1->day * INT64CONST(24) * USECS_PER_HOUR;
-       span2 += interval2->month * INT64CONST(30) * USECS_PER_DAY;
-       span2 += interval2->day * INT64CONST(24) * USECS_PER_HOUR;
+       span += interval->month * INT64CONST(30) * USECS_PER_DAY;
+       span += interval->day * INT64CONST(24) * USECS_PER_HOUR;
 #else
-       span1 += interval1->month * ((double) DAYS_PER_MONTH * SECS_PER_DAY);
-       span1 += interval1->day * ((double) HOURS_PER_DAY * SECS_PER_HOUR);
-       span2 += interval2->month * ((double) DAYS_PER_MONTH * SECS_PER_DAY);
-       span2 += interval2->day * ((double) HOURS_PER_DAY * SECS_PER_HOUR);
+       span += interval->month * ((double) DAYS_PER_MONTH * SECS_PER_DAY);
+       span += interval->day * ((double) HOURS_PER_DAY * SECS_PER_HOUR);
 #endif
 
+       return span;
+}
+
+static int
+interval_cmp_internal(Interval *interval1, Interval *interval2)
+{
+       TimeOffset      span1 = interval_cmp_value(interval1);
+       TimeOffset      span2 = interval_cmp_value(interval2);
+
        return ((span1 < span2) ? -1 : (span1 > span2) ? 1 : 0);
 }
 
@@ -2121,31 +2125,28 @@ interval_cmp(PG_FUNCTION_ARGS)
        PG_RETURN_INT32(interval_cmp_internal(interval1, interval2));
 }
 
+/*
+ * Hashing for intervals
+ *
+ * We must produce equal hashvals for values that interval_cmp_internal()
+ * considers equal.  So, compute the net span the same way it does,
+ * and then hash that, using either int64 or float8 hashing.
+ */
 Datum
 interval_hash(PG_FUNCTION_ARGS)
 {
-       Interval   *key = PG_GETARG_INTERVAL_P(0);
+       Interval   *interval = PG_GETARG_INTERVAL_P(0);
+       TimeOffset      span = interval_cmp_value(interval);
        uint32          thash;
-       uint32          mhash;
 
-       /*
-        * To avoid any problems with padding bytes in the struct, we figure the
-        * field hashes separately and XOR them.  This also provides a convenient
-        * framework for dealing with the fact that the time field might be either
-        * double or int64.
-        */
 #ifdef HAVE_INT64_TIMESTAMP
        thash = DatumGetUInt32(DirectFunctionCall1(hashint8,
-                                                                                          Int64GetDatumFast(key->time)));
+                                                                                          Int64GetDatumFast(span)));
 #else
        thash = DatumGetUInt32(DirectFunctionCall1(hashfloat8,
-                                                                                        Float8GetDatumFast(key->time)));
+                                                                                          Float8GetDatumFast(span)));
 #endif
-       thash ^= DatumGetUInt32(hash_uint32(key->day));
-       /* Shift so "k days" and "k months" don't hash to the same thing */
-       mhash = DatumGetUInt32(hash_uint32(key->month));
-       thash ^= mhash << 24;
-       thash ^= mhash >> 8;
+
        PG_RETURN_UINT32(thash);
 }
 

diff --git a/src/test/regress/expected/interval.out b/src/test/regress/expected/interval.out
index 72a031df5f1d9f1d08e1b186fa0749690e6ad544..5f3531430bba314eecce1110f64330712f0b438d 100644 (file)
--- a/src/test/regress/expected/interval.out
+++ b/src/test/regress/expected/interval.out
@@ -349,3 +349,16 @@ 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"
+-- check that '30 days' equals '1 month' according to the hash function
+select '30 days'::interval = '1 month'::interval as t;
+ t 
+---
+ t
+(1 row)
+
+select interval_hash('30 days'::interval) = interval_hash('1 month'::interval) as t;
+ t 
+---
+ t
+(1 row)
+

diff --git a/src/test/regress/sql/interval.sql b/src/test/regress/sql/interval.sql
index d081bf1ffedf2d504f2c422e49abd9a2a6a48045..3541d68cc8b54044e223085bedd4485c79e09e9c 100644 (file)
--- a/src/test/regress/sql/interval.sql
+++ b/src/test/regress/sql/interval.sql
@@ -126,4 +126,8 @@ 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
+SELECT '1:20:05 5 microseconds'::interval;          -- error
+
+-- check that '30 days' equals '1 month' according to the hash function
+select '30 days'::interval = '1 month'::interval as t;
+select interval_hash('30 days'::interval) = interval_hash('1 month'::interval) as t;