This is an automated email from the ASF dual-hosted git repository.
dongjoon-hyun pushed a commit to branch branch-4.x
in repository https://gitbox.apache.org/repos/asf/spark.git
The following commit(s) were added to refs/heads/branch-4.x by this push:
new a693daa45c33 [SPARK-58236][SQL] Fix hash codegen dropping the
CalendarInterval days field
a693daa45c33 is described below
commit a693daa45c33143d36cf5ef44a5661c3cf728de7
Author: YangJie <[email protected]>
AuthorDate: Wed Jul 22 12:45:54 2026 -0700
[SPARK-58236][SQL] Fix hash codegen dropping the CalendarInterval days field
### What changes were proposed in this pull request?
The codegen path of `genHashCalendarInterval` (in the `HashExpression` base
class, used by `Murmur3Hash`, `XxHash64` and their collation-aware variants)
hashes only the `months` and `microseconds` fields of a `CalendarInterval`,
while the interpreted path in `InterpretedHashFunction.hash` folds all three
fields (`months`, `days`, `microseconds`).
`CalendarInterval` gained its third field, `days`, in SPARK-29486. That
change updated the interpreted path and
`HiveHashFunction.hashCalendarInterval`, but missed this codegen helper, which
has been unchanged since SPARK-18287. This PR adds the missing `days` hashing
round to the codegen path so it matches the interpreted path, updates the class
doc to describe the three-field folding order, and adds a SQL migration guide
note since the hash output changes.
### Why are the changes needed?
The codegen and interpreted paths produce different hashes for the same
`CalendarInterval` value. Because `Murmur3_x86_32.hashInt(0, seed) != seed`,
the extra hashing round for `days` shifts the seed chain, so the two paths
disagree for every interval value, not only those with a non-zero `days`.
`CalendarIntervalType` is a valid input to `hash()` / `xxhash64()`, and
since SPARK-46536 it is also a supported GROUP BY key whose shuffle uses
`Murmur3Hash` for partitioning. A value hashed via codegen in one place and via
the interpreted path in another (for example a foldable literal versus a column
reference, or when whole-stage codegen falls back to interpretation) can
produce different hashes, which is unsafe for hash-based grouping and joins.
### Does this PR introduce _any_ user-facing change?
Yes. The `hash()` / `xxhash64()` value of a `CalendarInterval` computed
through the codegen path changes: it now folds the `days` field, matching the
interpreted path.
On current releases the two paths disagree, which is directly observable.
For example `hash(INTERVAL '5' DAYS)` (a foldable literal, evaluated by the
interpreted path) and `hash(interval_col)` over the same value (evaluated by
codegen) return different numbers. Likewise `SELECT count(distinct
hash(make_interval(0, 0, 0, id, 0, 0, 0))) FROM range(100)` returns 1 instead
of 100, because the codegen hash ignores `days`. After this fix both paths
agree. Only `CalendarIntervalType` is affe [...]
### How was this patch tested?
Added a regression test in `HashExpressionsSuite` (`SPARK-58236`). It
asserts that `Murmur3Hash` / `XxHash64` change their hash when any single field
(`months`, `days`, `microseconds`) changes, and uses `checkEvaluation` (which
runs both the interpreted and codegen paths and asserts equality) on intervals
whose only non-zero field is `days`. The test fails on the current codegen path
and passes with the fix.
### Was this patch authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Opus 4.8)
Closes #57398 from LuciferYang/hash-calendarinterval-days.
Authored-by: YangJie <[email protected]>
Signed-off-by: Dongjoon Hyun <[email protected]>
(cherry picked from commit a2616be0f3b303ad7189f7bd7b2c0bc49c834a27)
Signed-off-by: Dongjoon Hyun <[email protected]>
---
docs/sql-migration-guide.md | 1 +
.../spark/sql/catalyst/expressions/hash.scala | 7 ++---
.../expressions/HashExpressionsSuite.scala | 30 +++++++++++++++++++++-
3 files changed, 34 insertions(+), 4 deletions(-)
diff --git a/docs/sql-migration-guide.md b/docs/sql-migration-guide.md
index 010e903eb3bb..9cd5b664e8eb 100644
--- a/docs/sql-migration-guide.md
+++ b/docs/sql-migration-guide.md
@@ -31,6 +31,7 @@ license: |
- Since Spark 4.3, the adaptive execution rule
`org.apache.spark.sql.execution.adaptive.DynamicJoinSelection` has been renamed
to `DemoteBroadcastHashJoin`, which now only demotes broadcast hash joins
(emitting `NO_BROADCAST_HASH`). Its selection of shuffled hash join over sort
merge join has moved to a new physical rule gated by
`spark.sql.adaptive.convertSortMergeJoinToShuffledHashJoin.enabled` (default
`true`). If you previously disabled the shuffled-hash-join preference by
listing `o [...]
- Since Spark 4.3, the exact `percentile`, `percentile_cont`, and `median`
aggregate functions (including their `WITHIN GROUP (ORDER BY ...)` forms)
compute the linear interpolation between two neighboring values as `lower +
fraction * (higher - lower)` instead of `(1 - fraction) * lower + fraction *
higher`. The two are equal in exact arithmetic, but the new form is
monotonically non-decreasing in the requested percentage and avoids a rounding
error the old form could introduce. As a re [...]
- Since Spark 4.3, the new `COMMENT ON COLUMN ... IS NULL` syntax removes a
column comment by passing a `null` comment to
`TableChange.updateColumnComment(String[], String)`, so a `UpdateColumnComment`
table change may now carry a `null` `newComment()`. Previously `newComment()`
was always non-null. DataSource V2 catalogs that handle `UpdateColumnComment`
should null-check `newComment()` and treat `null` as "remove the column
comment" (mirroring how `UpdateColumnDefaultValue` already car [...]
+- Since Spark 4.3, `hash()` and `xxhash64()` include the `days` field of
`CalendarInterval` when computing the hash, so their output for interval values
differs from earlier releases. Previously the codegen path dropped `days`,
disagreeing with interpreted evaluation.
## Upgrading from Spark SQL 4.1 to 4.2
diff --git
a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/hash.scala
b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/hash.scala
index 2a9a140ef2a9..085586041b07 100644
---
a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/hash.scala
+++
b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/hash.scala
@@ -264,8 +264,8 @@ case class Crc32(child: Expression)
* and hash it.
* - decimal: if it's a small decimal, i.e. precision <= 18,
turn it into long
* and hash it. Else, turn it into bytes and hash
it.
- * - calendar interval: hash `microseconds` first, and use the result
as seed
- * to hash `months`.
+ * - calendar interval: hash `microseconds` first, use the result as
seed to hash `days`,
+ * then use that result as seed to hash `months`.
* - interval day to second: it store long value of `microseconds`, use
murmur3 to hash the long
* input with seed.
* - interval year to month: it store int value of `months`, use murmur3 to
hash the int
@@ -441,7 +441,8 @@ abstract class HashExpression[E] extends Expression {
protected def genHashCalendarInterval(input: String, result: String): String
= {
val microsecondsHash = s"$hasherClassName.hashLong($input.microseconds,
$result)"
- s"$result = $hasherClassName.hashInt($input.months, $microsecondsHash);"
+ val daysHash = s"$hasherClassName.hashInt($input.days, $microsecondsHash)"
+ s"$result = $hasherClassName.hashInt($input.months, $daysHash);"
}
protected def genHashTimestampNanos(input: String, result: String): String =
{
diff --git
a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/HashExpressionsSuite.scala
b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/HashExpressionsSuite.scala
index b26c48d65796..02846b560ad0 100644
---
a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/HashExpressionsSuite.scala
+++
b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/HashExpressionsSuite.scala
@@ -34,7 +34,7 @@ import org.apache.spark.sql.catalyst.util.{ArrayBasedMapData,
CollationFactory,
import org.apache.spark.sql.internal.SQLConf
import org.apache.spark.sql.types.{ArrayType, StructType, _}
import org.apache.spark.unsafe.hash.Murmur3_x86_32
-import org.apache.spark.unsafe.types.{TimestampNanosVal, UTF8String}
+import org.apache.spark.unsafe.types.{CalendarInterval, TimestampNanosVal,
UTF8String}
import org.apache.spark.util.ArrayImplicits._
class HashExpressionsSuite extends SparkFunSuite with ExpressionEvalHelper {
@@ -942,6 +942,34 @@ class HashExpressionsSuite extends SparkFunSuite with
ExpressionEvalHelper {
}
}
+ test("SPARK-58236: CalendarInterval hash folds all fields and agrees across
codegen") {
+ def lit(months: Int, days: Int, micros: Long): Literal =
+ Literal.create(new CalendarInterval(months, days, micros),
CalendarIntervalType)
+
+ // Murmur3Hash and XxHash64 treat months, days and microseconds as
independent fields, so
+ // changing any one of them changes the hash. HiveHash mirrors Hive's
day-time interval
+ // instead, dropping months and folding days into microseconds, so it
joins only the
+ // interpreted-vs-codegen check below.
+ val base = lit(1, 5, 7L)
+ Seq[Expression => Any](
+ e => Murmur3Hash(Seq(e), 42).eval(),
+ e => XxHash64(Seq(e), 42L).eval()).foreach { hash =>
+ assert(hash(base) === hash(lit(1, 5, 7L)))
+ assert(hash(base) !== hash(lit(2, 5, 7L))) // months
+ assert(hash(base) !== hash(lit(1, 6, 7L))) // days
+ assert(hash(base) !== hash(lit(1, 5, 8L))) // microseconds
+ }
+
+ // The interpreted and codegen paths must agree, above all when days is
the only non-zero
+ // field. checkEvaluation runs both and asserts they match, catching a
codegen path that
+ // omits days.
+ Seq(lit(0, 5, 0L), lit(0, -5, 0L), lit(3, 5, 0L), lit(1, 5, 7L)).foreach {
interval =>
+ checkEvaluation(Murmur3Hash(Seq(interval), 42),
Murmur3Hash(Seq(interval), 42).eval())
+ checkEvaluation(XxHash64(Seq(interval), 42L), XxHash64(Seq(interval),
42L).eval())
+ checkEvaluation(HiveHash(Seq(interval)), HiveHash(Seq(interval)).eval())
+ }
+ }
+
test("nanosecond timestamp hash matches expected golden values") {
// The expected values are composed independently of the expression under
test -- directly
// from the primitive hashers (and the separate hashTimestamp for Hive)
with an explicit
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
