feat: support multiply_ym_interval with YearMonth interval codegen dispatch#4715
feat: support multiply_ym_interval with YearMonth interval codegen dispatch#4715peterxcli wants to merge 9 commits into
Conversation
|
Thanks for this. Using the codegen dispatcher for A few things worth looking at: Rebase on Is the Overflow test. One small heads-up: the On benchmarks: since this runs in the JVM via dispatch rather than as a native Rust kernel, I do not think the usual native microbenchmark expectation applies here. |
|
@andygrove Thanks for the review! I've move the YearMonthIntervalType branch from |
|
Thanks for the rebase, this is much cleaner to review now, and the three points from last round are all addressed well. Moving the YM interval support into One thing before this is good to go: the Minor, non-blocking: a one-line comment in This review was prepared with the assistance of an LLM (Claude). |
Which issue does this PR close?
Close #3102.
Rationale for this change
This adds support for Spark's
multiply_ym_intervalexpression, allowing YearMonth interval multiplication to run through Comet instead of falling back toSpark.
What changes are included in this PR?
YearMonthIntervalTypeserialization to the Comet proto/native type mapping.MakeYMIntervalandMultiplyYMIntervalthrough the JVM codegen dispatcher.IntervalYearVectorinput/output support in the codegen kernel path.How are these changes tested?
cargo fmt./mvnw -Pspark-3.5 -Pscala-2.12 -DskipTests compilecargo +1.94.0 check -p datafusion-cometcargo +1.94.0 build./mvnw test -Pspark-3.5 -Pscala-2.12 -Dtest=none -Dsuites="org.apache.comet.CometSqlFileTestSuite multiply_ym_interval"