LuciferYang opened a new pull request, #57398: URL: https://github.com/apache/spark/pull/57398
### 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, and updates the class doc to describe the three-field folding order. ### 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 affected; no other type's hash changes. ### 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) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
