Skip to content

test: rename CometCastSuite to CometNativeCastSuite - #5268

Merged
andygrove merged 1 commit into
apache:mainfrom
andygrove:rename-cast-suite
Aug 5, 2026
Merged

test: rename CometCastSuite to CometNativeCastSuite#5268
andygrove merged 1 commit into
apache:mainfrom
andygrove:rename-cast-suite

Conversation

@andygrove

Copy link
Copy Markdown
Member

Which issue does this PR close?

N/A — test/docs clarity.

Rationale for this change

CometCast mixes in CodegenDispatchFallback, so a cast that CometCast.isSupported rejects does not fall back to Spark — it still runs inside the Comet operator by way of the Arrow-direct codegen dispatcher, evaluating Spark's own Cast against Arrow vectors.

CometCastSuite only covers the first half of that: native cast parity, plus the isSupported matrix that decides which casts reach the native kernel. The name didn't convey the split, and reading a (from, to) pair asserted as Unsupported in 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?

  • Rename CometCastSuiteCometNativeCastSuite.
  • Add a class-level scaladoc stating the scope explicitly: Spark-parity for the native Cast implementation and the isSupported matrix that gates it; codegen dispatch semantics are out of scope and belong to CometCodegenSuite and friends. It also notes that adding a native cast means moving a pair out of the Unsupported assertions and into the parity matrix.
  • Annotate "cast ArrayType(DateType) to unsupported ArrayType routes through codegen dispatch" to say it is a boundary case — it only pins that Unsupported keeps the enclosing operator native — not dispatch coverage.
  • Update every cross-reference to the old name: the expressions suite group in pr_build_linux.yml / pr_build_macos.yml, the -Dsuites= examples in development.md, a note in spark_configs_support.md, two SQL fixture headers under sql-tests/expressions/cast/, the CometCast.scala comment on the shared reason string, and two test comments in native/spark-expr/src/conversion_funcs/string.rs.

The historical entry in docs/source/changelog/0.1.0.md is 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, and prettier are all clean.

`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.
@andygrove

Copy link
Copy Markdown
Member Author

@comphead hopefully this helps re your question in #5185 (comment)

@comphead comphead left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @andygrove makes sense

@andygrove
andygrove merged commit d3b1072 into apache:main Aug 5, 2026
138 of 139 checks passed
@andygrove
andygrove deleted the rename-cast-suite branch August 5, 2026 18:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants