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 a128b3b47c5046c08bff09d001525b85d463ba0e..332fe94c460b8330b6f455e1a094f83eaab4f8d6 100644 (file)
--- a/src/backend/utils/adt/timestamp.c
+++ b/src/backend/utils/adt/timestamp.c
@@ -1654,32 +1654,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);
 }
 
@@ -1747,20 +1751,28 @@ interval_cmp(PG_FUNCTION_ARGS)
 }
 
 /*
- * interval, being an unusual size, needs a specialized hash function.
+ * 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;
 
-       /*
-        * Specify hash length as sizeof(double) + sizeof(int4), not as
-        * sizeof(Interval), so that any garbage pad bytes in the structure won't
-        * be included in the hash!
-        */
-       return hash_any((unsigned char *) key,
-                                 sizeof(key->time) + sizeof(key->day) + sizeof(key->month));
+#ifdef HAVE_INT64_TIMESTAMP
+       thash = DatumGetUInt32(DirectFunctionCall1(hashint8,
+                                                                                          Int64GetDatumFast(span)));
+#else
+       thash = DatumGetUInt32(DirectFunctionCall1(hashfloat8,
+                                                                                          Float8GetDatumFast(span)));
+#endif
+
+       PG_RETURN_UINT32(thash);
 }
 
 /* overlaps_timestamp() --- implements the SQL92 OVERLAPS operator.

diff --git a/src/test/regress/expected/interval.out b/src/test/regress/expected/interval.out
index 17b18a1a0f9f7cde07e42ee6467beaa480411a58..0a54b06cda1d9e593f3f2629df5fb6c095b738bb 100644 (file)
--- a/src/test/regress/expected/interval.out
+++ b/src/test/regress/expected/interval.out
@@ -264,3 +264,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 920ce8a09a49b716b0d801a5ccf2048ac2c8f4c6..540b887992d7d8e498a0b4037daf1f5da646ffd5 100644 (file)
--- a/src/test/regress/sql/interval.sql
+++ b/src/test/regress/sql/interval.sql
@@ -85,4 +85,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;