peterxcli commented on PR #4901: URL: https://github.com/apache/datafusion-comet/pull/4901#issuecomment-5447394053
Thanks for the review! Point by point: 1. **Map ordering** — fixed; moved `DivideDTInterval` after `Days`. 2. **Wide decimals** — the dispatcher handles all decimal precisions: `CometBatchKernelCodegen.isSupportedDataType` accepts any `DecimalType`, and inputs with precision > 18 read through a `BigDecimal` path on `DecimalVector` (precision ≤ 18 uses an unscaled-long fast path). No fallback, so no `expressions.md` caveat needed. I've added `DECIMAL(38, 18)` and `DECIMAL(38, 0)` divisors to the fixture — both as columns (to exercise the wide-decimal Arrow read path) and as literals — and it passes under `checkSparkAnswerAndOperator`, which fails on any fallback. (`multiply_dt_interval.sql` has the same `DECIMAL(10, 2)` cap; happy to extend it in a follow-up.) 3. **Spark versions** — `CometSqlFileTestSuite` runs in the `[expressions]` CI job on all five profiles (Spark 3.4/JDK 11/Scala 2.12, 3.5/JDK 17/Scala 2.13, 4.0/JDK 21, 4.1/JDK 17, 4.2/JDK 17), so this fixture is exercised per Spark version before merge. Also, the dispatcher's `DurationVector` support isn't introduced here — it landed with `multiply_dt_interval` (#4900) and #4976 and is already covered per-version on main; this PR's diff is just the serde registration, fixture, and docs. I additionally ran the fixture locally against the default Spark 4.1 profile. 4. **Performance** — no standalone speedup claim: the dispatcher runs Spark's own generated code per batch, so the expression itself is at parity. The win is that the enclosing operator stays native instead of forcing a columnar→row transition and operator fallback. I've updated the PR description to say so explicitly. -- 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]
