andygrove opened a new issue, #5059:
URL: https://github.com/apache/datafusion-comet/issues/5059

   ### Describe the bug
   
   Hashing a `CalendarIntervalType` value fails the query with a native error 
instead of either working or falling back to Spark:
   
   ```
   org.apache.comet.CometNativeException: Unsupported data type in hasher: 
Interval(MonthDayNano).
   This issue was likely caused by a bug in DataFusion's code. Please help us 
to resolve this by filing a bug report in our issue tracker: 
https://github.com/apache/datafusion/issues
        at org.apache.comet.Native.executePlan(Native Method)
        at 
org.apache.comet.CometExecIterator.$anonfun$getNextBatch$2(CometExecIterator.scala:155)
        ...
   ```
   
   #4898 added `CalendarIntervalType` to `QueryPlanSerde.supportedDataType`:
   
   
https://github.com/apache/datafusion-comet/blob/1d3044f35bda457a20f1ccf4324a4699b35f5158/spark/src/main/scala/org/apache/comet/serde/QueryPlanSerde.scala#L530-L534
   
   That is the gate `hash.scala` consults when deciding whether a hash 
expression's child type is supported:
   
   
https://github.com/apache/datafusion-comet/blob/1d3044f35bda457a20f1ccf4324a4699b35f5158/spark/src/main/scala/org/apache/comet/serde/hash.scala#L136-L147
   
   So `Murmur3Hash` / `XxHash64` over a CalendarInterval child now reports 
`Compatible` and is serialized to native, but 
`native/spark-expr/src/hash_funcs/utils.rs` has no branch for 
`Interval(MonthDayNano)` and falls through to the catch-all 
`DataFusionError::Internal("Unsupported data type in hasher: ...")`.
   
   The ANSI interval types are not in `supportedDataType`, so 
`hash(dt_interval)` and `hash(ym_interval)` correctly fall back to Spark. Only 
`CalendarIntervalType` reaches native and fails.
   
   ### Steps to reproduce
   
   Against a Parquet-backed table so the expression is not constant-folded away:
   
   ```sql
   CREATE TABLE t(d int, h int, y int, m int) USING parquet;
   INSERT INTO t VALUES (1, 2, 1, 2), (0, 0, 0, 3);
   
   SELECT hash(make_interval(y, m, 0, d, h, 0, 0)) FROM t;
   ```
   
   Equivalently, as a sql-file test under 
`spark/src/test/resources/sql-tests/expressions/datetime/`.
   
   A `OneRowRelation` form reproduces it too:
   
   ```sql
   SELECT hash(make_interval(1, 2, 3, 4, 5, 6, 7.008009));
   ```
   
   Physical plan at failure time (the whole projection is native):
   
   ```
   CometNativeColumnarToRow
   +- CometProject [hash(make_interval(1, 2, 3, 4, 5, 6, cast(7.008009 as 
decimal(18,6)), false), 42) ...]
      +- CometSparkRowToColumnar
         +- *(1) Scan OneRowRelation[]
   ```
   
   ### Expected behavior
   
   The query should return the same values as Spark. Failing that, it should 
fall back to Spark rather than aborting the stage.
   
   Two possible fixes:
   
   1. Implement Spark-compatible hashing for `Interval(MonthDayNano)` (and 
`Interval(YearMonth)` / `Duration(Microsecond)` while we are there) in 
`native/spark-expr/src/hash_funcs/utils.rs`. Spark's `HashExpression` hashes a 
`CalendarInterval` as its `months`, `days` and `microseconds` fields, so the 
native side needs to match that field order and width exactly.
   2. Or narrow the Scala gate so `CalendarIntervalType` is rejected for hash 
expressions, matching how the ANSI interval types behave today, and leave 
native hashing for the follow-up that implements it properly.
   
   Either way this needs a regression test covering `hash` and `xxhash64` over 
all three interval types.
   
   ### Additional context
   
   - Found while auditing interval support after #4898 and #4976 landed. Part 
of #4540.
   - Reproduced on `1d3044f35` with the Spark 4.1 profile.
   - `QueryPlanSerde.supportedDataType` is also consulted by 
`CometScalarSubquery` and `CometSink`, so it is worth checking whether a 
CalendarInterval-typed scalar subquery hits a similar unhandled-type path in 
the planner.
   - Related gap worth noting separately: native shuffle accepts all three 
interval types after #4976, but `supportedHashPartitioningDataType` in 
`CometShuffleExchangeExec` rejects them, so hash partitioning on an interval 
key still falls back. That is a fallback rather than a failure, so it is not 
part of this issue.
   


-- 
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]

Reply via email to