Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 1c2d408

Browse files
committed
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".
1 parent 1c855f0 commit 1c2d408

File tree

3 files changed

+46
-34
lines changed

3 files changed

+46
-34
lines changed

src/backend/utils/adt/timestamp.c

+29-34
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/utils/adt/timestamp.c,v 1.197 2009/03/15 20:31:19 tgl Exp $
11+
* $PostgreSQL: pgsql/src/backend/utils/adt/timestamp.c,v 1.198 2009/04/04 04:53:25 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -2041,27 +2041,30 @@ timestamptz_cmp_timestamp(PG_FUNCTION_ARGS)
20412041
*
20422042
* collate invalid interval at the end
20432043
*/
2044-
static int
2045-
interval_cmp_internal(Interval *interval1, Interval *interval2)
2044+
static inline TimeOffset
2045+
interval_cmp_value(const Interval *interval)
20462046
{
2047-
TimeOffset span1,
2048-
span2;
2047+
TimeOffset span;
20492048

2050-
span1 = interval1->time;
2051-
span2 = interval2->time;
2049+
span = interval->time;
20522050

20532051
#ifdef HAVE_INT64_TIMESTAMP
2054-
span1 += interval1->month * INT64CONST(30) * USECS_PER_DAY;
2055-
span1 += interval1->day * INT64CONST(24) * USECS_PER_HOUR;
2056-
span2 += interval2->month * INT64CONST(30) * USECS_PER_DAY;
2057-
span2 += interval2->day * INT64CONST(24) * USECS_PER_HOUR;
2052+
span += interval->month * INT64CONST(30) * USECS_PER_DAY;
2053+
span += interval->day * INT64CONST(24) * USECS_PER_HOUR;
20582054
#else
2059-
span1 += interval1->month * ((double) DAYS_PER_MONTH * SECS_PER_DAY);
2060-
span1 += interval1->day * ((double) HOURS_PER_DAY * SECS_PER_HOUR);
2061-
span2 += interval2->month * ((double) DAYS_PER_MONTH * SECS_PER_DAY);
2062-
span2 += interval2->day * ((double) HOURS_PER_DAY * SECS_PER_HOUR);
2055+
span += interval->month * ((double) DAYS_PER_MONTH * SECS_PER_DAY);
2056+
span += interval->day * ((double) HOURS_PER_DAY * SECS_PER_HOUR);
20632057
#endif
20642058

2059+
return span;
2060+
}
2061+
2062+
static int
2063+
interval_cmp_internal(Interval *interval1, Interval *interval2)
2064+
{
2065+
TimeOffset span1 = interval_cmp_value(interval1);
2066+
TimeOffset span2 = interval_cmp_value(interval2);
2067+
20652068
return ((span1 < span2) ? -1 : (span1 > span2) ? 1 : 0);
20662069
}
20672070

@@ -2128,32 +2131,24 @@ interval_cmp(PG_FUNCTION_ARGS)
21282131
PG_RETURN_INT32(interval_cmp_internal(interval1, interval2));
21292132
}
21302133

2134+
/*
2135+
* Hashing for intervals
2136+
*
2137+
* We must produce equal hashvals for values that interval_cmp_internal()
2138+
* considers equal. So, compute the net span the same way it does,
2139+
* and then hash that, using either int64 or float8 hashing.
2140+
*/
21312141
Datum
21322142
interval_hash(PG_FUNCTION_ARGS)
21332143
{
2134-
Interval *key = PG_GETARG_INTERVAL_P(0);
2135-
uint32 thash;
2136-
uint32 mhash;
2144+
Interval *interval = PG_GETARG_INTERVAL_P(0);
2145+
TimeOffset span = interval_cmp_value(interval);
21372146

2138-
/*
2139-
* To avoid any problems with padding bytes in the struct, we figure the
2140-
* field hashes separately and XOR them. This also provides a convenient
2141-
* framework for dealing with the fact that the time field might be either
2142-
* double or int64.
2143-
*/
21442147
#ifdef HAVE_INT64_TIMESTAMP
2145-
thash = DatumGetUInt32(DirectFunctionCall1(hashint8,
2146-
Int64GetDatumFast(key->time)));
2148+
return DirectFunctionCall1(hashint8, Int64GetDatumFast(span));
21472149
#else
2148-
thash = DatumGetUInt32(DirectFunctionCall1(hashfloat8,
2149-
Float8GetDatumFast(key->time)));
2150+
return DirectFunctionCall1(hashfloat8, Float8GetDatumFast(span));
21502151
#endif
2151-
thash ^= DatumGetUInt32(hash_uint32(key->day));
2152-
/* Shift so "k days" and "k months" don't hash to the same thing */
2153-
mhash = DatumGetUInt32(hash_uint32(key->month));
2154-
thash ^= mhash << 24;
2155-
thash ^= mhash >> 8;
2156-
PG_RETURN_UINT32(thash);
21572152
}
21582153

21592154
/* overlaps_timestamp() --- implements the SQL92 OVERLAPS operator.

src/test/regress/expected/interval.out

+13
Original file line numberDiff line numberDiff line change
@@ -717,3 +717,16 @@ select interval '0:0:0.7', interval '@ 0.70 secs', interval '0.7 seconds';
717717
@ 0.7 secs | @ 0.7 secs | @ 0.7 secs
718718
(1 row)
719719

720+
-- check that '30 days' equals '1 month' according to the hash function
721+
select '30 days'::interval = '1 month'::interval as t;
722+
t
723+
---
724+
t
725+
(1 row)
726+
727+
select interval_hash('30 days'::interval) = interval_hash('1 month'::interval) as t;
728+
t
729+
---
730+
t
731+
(1 row)
732+

src/test/regress/sql/interval.sql

+4
Original file line numberDiff line numberDiff line change
@@ -241,3 +241,7 @@ SET IntervalStyle to postgres_verbose;
241241
select interval '-10 mons -3 days +03:55:06.70';
242242
select interval '1 year 2 mons 3 days 04:05:06.699999';
243243
select interval '0:0:0.7', interval '@ 0.70 secs', interval '0.7 seconds';
244+
245+
-- check that '30 days' equals '1 month' according to the hash function
246+
select '30 days'::interval = '1 month'::interval as t;
247+
select interval_hash('30 days'::interval) = interval_hash('1 month'::interval) as t;

0 commit comments

Comments
 (0)