This is an automated email from the ASF dual-hosted git repository.
andygrove pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/datafusion-comet.git
The following commit(s) were added to refs/heads/main by this push:
new d3b1072dbe test: rename CometCastSuite to CometNativeCastSuite (#5268)
d3b1072dbe is described below
commit d3b1072dbe6aaaf349d502d347ead36041ca7bc0
Author: Andy Grove <[email protected]>
AuthorDate: Wed Aug 5 11:59:23 2026 -0600
test: rename CometCastSuite to CometNativeCastSuite (#5268)
`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.
---
.github/workflows/pr_build_linux.yml | 2 +-
.github/workflows/pr_build_macos.yml | 2 +-
docs/source/contributor-guide/development.md | 8 +++----
.../contributor-guide/spark_configs_support.md | 2 +-
native/spark-expr/src/conversion_funcs/string.rs | 4 ++--
.../org/apache/comet/expressions/CometCast.scala | 2 +-
.../expressions/cast/cast_string_to_date_ansi.sql | 2 +-
.../expressions/cast/cast_string_trim.sql | 2 +-
...tCastSuite.scala => CometNativeCastSuite.scala} | 26 +++++++++++++++++++++-
9 files changed, 37 insertions(+), 13 deletions(-)
diff --git a/.github/workflows/pr_build_linux.yml
b/.github/workflows/pr_build_linux.yml
index 0a71607ead..8fbf82d3f9 100644
--- a/.github/workflows/pr_build_linux.yml
+++ b/.github/workflows/pr_build_linux.yml
@@ -381,7 +381,7 @@ jobs:
org.apache.comet.CometHashExpressionSuite
org.apache.comet.CometTemporalExpressionSuite
org.apache.comet.CometArrayExpressionSuite
- org.apache.comet.CometCastSuite
+ org.apache.comet.CometNativeCastSuite
org.apache.comet.CometDateTimeUtilsSuite
org.apache.comet.CometMathExpressionSuite
org.apache.comet.CometStringExpressionSuite
diff --git a/.github/workflows/pr_build_macos.yml
b/.github/workflows/pr_build_macos.yml
index 2335588b70..5b92c53c4a 100644
--- a/.github/workflows/pr_build_macos.yml
+++ b/.github/workflows/pr_build_macos.yml
@@ -197,7 +197,7 @@ jobs:
org.apache.comet.CometHashExpressionSuite
org.apache.comet.CometTemporalExpressionSuite
org.apache.comet.CometArrayExpressionSuite
- org.apache.comet.CometCastSuite
+ org.apache.comet.CometNativeCastSuite
org.apache.comet.CometDateTimeUtilsSuite
org.apache.comet.CometMathExpressionSuite
org.apache.comet.CometStringExpressionSuite
diff --git a/docs/source/contributor-guide/development.md
b/docs/source/contributor-guide/development.md
index 7e5c20c7ec..26fdbbb404 100644
--- a/docs/source/contributor-guide/development.md
+++ b/docs/source/contributor-guide/development.md
@@ -269,10 +269,10 @@ and `-Dtest=none`:
./mvnw test -Dtest=none -Dsuites="org.apache.comet.CometArrayExpressionSuite"
# Run multiple suites (comma-separated, fully qualified)
-./mvnw test -Dtest=none
-Dsuites="org.apache.comet.CometCastSuite,org.apache.comet.CometArrayExpressionSuite"
+./mvnw test -Dtest=none
-Dsuites="org.apache.comet.CometNativeCastSuite,org.apache.comet.CometArrayExpressionSuite"
# Run only tests whose name contains "valid" inside one suite
-./mvnw test -Dtest=none -Dsuites="org.apache.comet.CometCastSuite valid"
+./mvnw test -Dtest=none -Dsuites="org.apache.comet.CometNativeCastSuite valid"
```
`-Dtest=none` tells the Surefire (JUnit) plugin to skip its tests; without it,
Surefire still
@@ -474,10 +474,10 @@ However if the tests is related to the native side.
Please make sure to run `mak
Specify which ScalaTest suites to run with the `suites` argument and disable
Surefire's JUnit
discovery with `-Dtest=none`. For example, to run only the test cases
containing _valid_ in
-their name from `org.apache.comet.CometCastSuite`:
+their name from `org.apache.comet.CometNativeCastSuite`:
```sh
-./mvnw test -Dtest=none -Dsuites="org.apache.comet.CometCastSuite valid"
+./mvnw test -Dtest=none -Dsuites="org.apache.comet.CometNativeCastSuite valid"
```
See [Running a Specific ScalaTest Suite](#running-a-specific-scalatest-suite)
above for the full
diff --git a/docs/source/contributor-guide/spark_configs_support.md
b/docs/source/contributor-guide/spark_configs_support.md
index 4ec8a99edf..b2c55bc146 100644
--- a/docs/source/contributor-guide/spark_configs_support.md
+++ b/docs/source/contributor-guide/spark_configs_support.md
@@ -104,7 +104,7 @@ across Spark 3.4, 3.5, 4.0, and 4.1. Three expression
classes mix in
`to_date`, and Spark 4's `try_to_timestamp`)
`Cast` between strings and date / timestamp also reads the policy via the
default
-formatters but is tested separately by `CometCastSuite` and is out of scope
here.
+formatters but is tested separately by `CometNativeCastSuite` and is out of
scope here.
**Comet status.** None of the native implementations consult
`legacyTimeParserPolicy` directly. Comet instead uses native implementations
only for
diff --git a/native/spark-expr/src/conversion_funcs/string.rs
b/native/spark-expr/src/conversion_funcs/string.rs
index 46495ed981..040c5737f7 100644
--- a/native/spark-expr/src/conversion_funcs/string.rs
+++ b/native/spark-expr/src/conversion_funcs/string.rs
@@ -2201,7 +2201,7 @@ mod tests {
/// The codepoint matrix from
/// <https://github.com/apache/datafusion-comet/issues/5149>: the ASCII
control bytes and
/// DELETE, plus the non-ASCII codepoints that are whitespace to Unicode
but that no Spark
- /// cast trims. `CometCastSuite` runs the same matrix with Spark itself as
the oracle.
+ /// cast trims. `CometNativeCastSuite` runs the same matrix with Spark
itself as the oracle.
fn trim_pads() -> Vec<String> {
let mut pads: Vec<String> = (0x00u8..=0x20).map(|b| String::from(b as
char)).collect();
pads.push("\u{7f}".to_string());
@@ -2305,7 +2305,7 @@ mod tests {
/// than change behaviour silently. `timestamp_parser` and
`timestamp_ntz_parser` still use
/// `str::trim`, so they accept the non-ASCII whitespace that Spark's
/// `SparkDateTimeUtils.getTrimmedStart` / `getTrimmedEnd` leave in place,
where Spark returns
- /// NULL. `CometCastSuite` cannot cover this, because Spark is the oracle
there and Comet does
+ /// NULL. `CometNativeCastSuite` cannot cover this, because Spark is the
oracle there and Comet does
/// not fall back -- it silently returns a value.
#[test]
fn test_cast_string_to_timestamp_unicode_whitespace_divergence() {
diff --git a/spark/src/main/scala/org/apache/comet/expressions/CometCast.scala
b/spark/src/main/scala/org/apache/comet/expressions/CometCast.scala
index df4140078e..1aced4a96e 100644
--- a/spark/src/main/scala/org/apache/comet/expressions/CometCast.scala
+++ b/spark/src/main/scala/org/apache/comet/expressions/CometCast.scala
@@ -37,7 +37,7 @@ object CometCast
with CometTypeShim
with CodegenDispatchFallback {
- // Shared with CometCastSuite so the asserted reason cannot drift from
production.
+ // Shared with CometNativeCastSuite so the asserted reason cannot drift from
production.
private[comet] val negativeScaleDecimalToStringReason: String =
"Negative-scale decimal requires
spark.sql.legacy.allowNegativeScaleOfDecimal=true"
diff --git
a/spark/src/test/resources/sql-tests/expressions/cast/cast_string_to_date_ansi.sql
b/spark/src/test/resources/sql-tests/expressions/cast/cast_string_to_date_ansi.sql
index 763ec9705a..4b2a2405dd 100644
---
a/spark/src/test/resources/sql-tests/expressions/cast/cast_string_to_date_ansi.sql
+++
b/spark/src/test/resources/sql-tests/expressions/cast/cast_string_to_date_ansi.sql
@@ -19,7 +19,7 @@
-- shape of a date but names no real calendar date, rather than returning NULL.
-- See https://github.com/apache/datafusion-comet/issues/5012.
--
--- Per-string parity with Spark is covered by CometCastSuite "cast StringType
to DateType";
+-- Per-string parity with Spark is covered by CometNativeCastSuite "cast
StringType to DateType";
-- this fixture covers the plan shapes that suite does not exercise.
-- Config: spark.sql.ansi.enabled=true
diff --git
a/spark/src/test/resources/sql-tests/expressions/cast/cast_string_trim.sql
b/spark/src/test/resources/sql-tests/expressions/cast/cast_string_trim.sql
index f7edaf315d..c14b893b0c 100644
--- a/spark/src/test/resources/sql-tests/expressions/cast/cast_string_trim.sql
+++ b/spark/src/test/resources/sql-tests/expressions/cast/cast_string_trim.sql
@@ -22,7 +22,7 @@
-- and NBSP / U+3000 are trimmed for nothing. See conversion_funcs::trim in
the native crate and
-- https://github.com/apache/datafusion-comet/issues/5149.
--
--- Per-codepoint parity across all three eval modes is covered by the
CometCastSuite
+-- Per-codepoint parity across all three eval modes is covered by the
CometNativeCastSuite
-- "whitespace trim parity" tests; this fixture covers the same regimes over a
Parquet column.
statement
diff --git a/spark/src/test/scala/org/apache/comet/CometCastSuite.scala
b/spark/src/test/scala/org/apache/comet/CometNativeCastSuite.scala
similarity index 98%
rename from spark/src/test/scala/org/apache/comet/CometCastSuite.scala
rename to spark/src/test/scala/org/apache/comet/CometNativeCastSuite.scala
index 6e5b03ee35..3ac221c107 100644
--- a/spark/src/test/scala/org/apache/comet/CometCastSuite.scala
+++ b/spark/src/test/scala/org/apache/comet/CometNativeCastSuite.scala
@@ -40,7 +40,28 @@ import org.apache.comet.expressions.{CometCast,
CometEvalMode}
import org.apache.comet.rules.CometScanTypeChecker
import org.apache.comet.serde.{Compatible, Incompatible, Unsupported}
-class CometCastSuite extends CometTestBase with AdaptiveSparkPlanHelper {
+/**
+ * Spark-parity coverage for Comet's **native** `Cast` implementation, and for
the
+ * `CometCast.isSupported` matrix that decides which casts reach it.
+ *
+ * Scope: for every `(from, to)` type pair that `CometCast` reports as
`Compatible` or
+ * `Incompatible`, run the cast against Spark as the oracle and assert both
that the results match
+ * and — via `checkSparkAnswerAndOperator` — that the native kernel actually
executed. Support
+ * decisions themselves (`Compatible` / `Incompatible` / `Unsupported`, and
the reason strings)
+ * are asserted directly against `CometCast.isSupported`.
+ *
+ * Out of scope: the JVM codegen dispatch path. `CometCast` mixes in
`CodegenDispatchFallback`, so
+ * a cast that `isSupported` rejects still runs inside the Comet operator by
way of the
+ * Arrow-direct codegen dispatcher, evaluating Spark's own `Cast` against
Arrow vectors. That path
+ * is covered by `CometCodegenSuite` and friends. Where this suite touches it
at all, it does so
+ * only to pin that an `Unsupported` cast keeps the enclosing operator native
rather than falling
+ * back to Spark; it does not attempt to cover dispatch semantics.
+ *
+ * Because of that split, a cast marked `Unsupported` here is not untested
overall — it is tested
+ * through the dispatcher instead. Adding a native cast implementation
therefore means moving a
+ * pair out of the `Unsupported` assertions and into the parity matrix below.
+ */
+class CometNativeCastSuite extends CometTestBase with AdaptiveSparkPlanHelper {
import testImplicits._
@@ -1853,6 +1874,9 @@ class CometCastSuite extends CometTestBase with
AdaptiveSparkPlanHelper {
}
test("cast ArrayType(DateType) to unsupported ArrayType routes through
codegen dispatch") {
+ // Boundary case rather than dispatch coverage: these pairs have no native
cast, so all this
+ // asserts is that `Unsupported` keeps the operator native via
`CodegenDispatchFallback`
+ // instead of falling back to Spark. Dispatch semantics belong in the
codegen suites.
val fromType = ArrayType(DateType)
val unsupportedElementTypes =
Seq(BooleanType, ByteType, ShortType, LongType, FloatType, DoubleType,
DecimalType(10, 2))
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
