sam-1112 commented on PR #5835: URL: https://github.com/apache/datafusion-comet/pull/5835#issuecomment-5635867244
Thanks for the review. I addressed the routing, fixture, and benchmark points in the latest commit. **TimeType.** It is now explicitly excluded from the dispatcher enrollment. `HashUtils.supportLevelForChildren` returns `Compatible` when a child contains `TimeType`, so `CodegenDispatchFallback` does not intercept it. The native conversion is then guarded by `convertNativeOrSparkFallback`, which records `TimeType` is not supported and returns `None`, preserving the enclosing Spark fallback. `CometSha2` performs the same TimeType check before considering the non-foldable `numBits` dispatcher path. I also updated the hash audit note and added Spark 4.1+ routing tests for both `hash(to_time(...))` and `xxhash64(to_time(...))`. This keeps the TimeType arm out of scope for #5581, consistent with #5218. **Wide-decimal fixtures.** I added values whose unscaled representations exceed 64 bits for both `DECIMAL(20,2)` and `DECIMAL(38,10)`, including positive/negative near-limit values, top-level nulls, and null decimal elements inside array/struct/map inputs. `hash` and `xxhash64` are asserted separately to take the dispatcher path, while the existing native-compatible controls still require zero dispatcher activity. **Microbenchmark.** I added `CometHashCodegenDispatchBenchmark`, covering wide-decimal `hash` / `xxhash64`, column-valued `sha2(payload, numBits)`, and a mixed projection containing native expressions plus a dispatched wide-decimal hash. It compares three matched-result arms: dispatcher enabled, dispatcher disabled, and Spark, with explicit routing/plan checks and separate first-use and steady-state measurements. On my Apple M4 Mac (`local[1]`, Spark 4.1.3), the isolated dispatched expressions are roughly neutral/slower than dispatcher-off because both Comet paths ultimately run Spark `doGenCode` and the dispatcher adds JNI/Arrow and first-use compilation overhead. The useful case is the mixed projection, where keeping the enclosing projection in Comet measured about 67 ms with dispatch versus 97 ms with dispatcher disabled (~1.4 X)and 138 ms in Spark over 1M rows. I am treating these as directional local measurements, not a CI performance gate, so I did not add a timing job. I also exercised the `GROUP BY hash(wide decimal)` bucketing/dedup shape from the issue. The dispatcher keeps that plan in Comet, but on this small local corpus the `BigDecimal` hashing cost dominates the aggregate, so I am not claiming a grouped throughput improvement. -- 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]
