projects / postgresql.git / commitdiff
summary | shortlog | log | commit | commitdiff | tree
raw | patch | inline | side by side (parent: ed00200)
Rewrite interval_hash() so that the hashcodes are equal for values that
authorTom Lane <tgl@sss.pgh.pa.us>
Sat, 4 Apr 2009 04:54:07 +0000 (04:54 +0000)
committerTom Lane <tgl@sss.pgh.pa.us>
Sat, 4 Apr 2009 04:54:07 +0000 (04:54 +0000)
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".

src/backend/utils/adt/timestamp.c patch | blob | blame | history
src/test/regress/expected/interval.out patch | blob | blame | history
src/test/regress/sql/interval.sql patch | blob | blame | history

diff --git a/src/backend/utils/adt/timestamp.c b/src/backend/utils/adt/timestamp.c
index 4c0de321e039c695785117e621e0ba5b2253bff2..dc09841f12038587d81d06fa467bc61078acb81b 100644 (file)
--- a/src/backend/utils/adt/timestamp.c
+++ b/src/backend/utils/adt/timestamp.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *   $Header: /cvsroot/pgsql/src/backend/utils/adt/timestamp.c,v 1.96.2.3 2005/05/26 02:14:31 neilc Exp $
+ *   $Header: /cvsroot/pgsql/src/backend/utils/adt/timestamp.c,v 1.96.2.4 2009/04/04 04:54:07 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -1399,33 +1399,36 @@ timestamp_cmp(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
-   if (interval1->month != 0)
-       span1 += ((interval1->month * INT64CONST(30) * INT64CONST(86400000000)));
-   if (interval2->month != 0)
-       span2 += ((interval2->month * INT64CONST(30) * INT64CONST(86400000000)));
+   if (interval->month != 0)
+       span += ((interval->month * INT64CONST(30) * INT64CONST(86400000000)));
 #else
-   if (interval1->month != 0)
-       span1 += (interval1->month * (30.0 * 86400));
-   if (interval2->month != 0)
-       span2 += (interval2->month * (30.0 * 86400));
+   if (interval->month != 0)
+       span += (interval->month * (30.0 * 86400));
 #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);
 }
 
@@ -1493,19 +1496,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->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 87cbcfa428893e37b2624cccf1c02a7d6c4f8be7..e59c6da19951d6f1eab5f38be0a0ef26113bf8d0 100644 (file)
--- a/src/test/regress/expected/interval.out
+++ b/src/test/regress/expected/interval.out
@@ -228,3 +228,16 @@ select '4 millenniums 5 centuries 4 decades 1 year 4 months 4 days 17 minutes 31
  @ 4541 years 4 mons 4 days 17 mins 31 secs
 (1 row)
 
+-- 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 2ae8f1f785745dab9cd6f71754b7c72124698ffa..cab4c48676748211c38d1f30b22ddb24bea6c132 100644 (file)
--- a/src/test/regress/sql/interval.sql
+++ b/src/test/regress/sql/interval.sql
@@ -69,3 +69,7 @@ select avg(f1) from interval_tbl;
 
 -- test long interval input
 select '4 millenniums 5 centuries 4 decades 1 year 4 months 4 days 17 minutes 31 seconds'::interval;
+
+-- 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;
This is the main PostgreSQL git repository.