test: rename CometCastSuite to CometNativeCastSuite - #5268
Merged
Conversation
`CometCast` mixes in `CodegenDispatchFallback`, so a cast that `CometCast.isSupported` rejects still runs inside the Comet operator via the Arrow-direct codegen dispatcher. The suite name did not convey that split, and reading a cast pair marked `Unsupported` there is easy to mistake for "not tested at all" rather than "tested through the dispatcher instead". Rename the suite to `CometNativeCastSuite` and add a class-level scaladoc stating the scope explicitly: Spark-parity for the native `Cast` kernel plus the `isSupported` matrix that gates it, with codegen dispatch semantics out of scope and covered by `CometCodegenSuite` and friends. Also annotate the one dispatch-adjacent test to say it only pins that `Unsupported` keeps the operator native. Updates the CI suite lists, contributor-guide examples, SQL fixture headers, and the native-crate comments that cross-reference the suite by name.
Member
Author
|
@comphead hopefully this helps re your question in #5185 (comment) |
comphead
approved these changes
Aug 5, 2026
comphead
left a comment
Contributor
There was a problem hiding this comment.
Thanks @andygrove makes sense
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Which issue does this PR close?
N/A — test/docs clarity.
Rationale for this change
CometCastmixes inCodegenDispatchFallback, so a cast thatCometCast.isSupportedrejects does not fall back to Spark — it still runs inside the Comet operator by way of the Arrow-direct codegen dispatcher, evaluating Spark's ownCastagainst Arrow vectors.CometCastSuiteonly covers the first half of that: native cast parity, plus theisSupportedmatrix that decides which casts reach the native kernel. The name didn't convey the split, and reading a(from, to)pair asserted asUnsupportedin that suite is easy to mistake for "this cast isn't tested" rather than "this cast is tested through the dispatcher instead".What changes are included in this PR?
CometCastSuite→CometNativeCastSuite.Castimplementation and theisSupportedmatrix that gates it; codegen dispatch semantics are out of scope and belong toCometCodegenSuiteand friends. It also notes that adding a native cast means moving a pair out of theUnsupportedassertions and into the parity matrix."cast ArrayType(DateType) to unsupported ArrayType routes through codegen dispatch"to say it is a boundary case — it only pins thatUnsupportedkeeps the enclosing operator native — not dispatch coverage.expressionssuite group inpr_build_linux.yml/pr_build_macos.yml, the-Dsuites=examples indevelopment.md, a note inspark_configs_support.md, two SQL fixture headers undersql-tests/expressions/cast/, theCometCast.scalacomment on the shared reason string, and two test comments innative/spark-expr/src/conversion_funcs/string.rs.The historical entry in
docs/source/changelog/0.1.0.mdis left alone.No test logic changed — this is a rename plus comments.
How are these changes tested?
./mvnw test -Dtest=none -Dsuites="org.apache.comet.CometNativeCastSuite valid"— suite is discovered under the new name, 3 tests run, all pass../mvnw test-compile,spotless:apply,scalastyle,cargo fmt --check, andprettierare all clean.