fix: reject casts involving non-default collated strings

[stantheman0128]

Which issue does this PR close?

Closes #4489.

Rationale for this change

Spark 4.0 carries collation metadata on StringType, but serializeDataType maps every StringType to a single proto type id (QueryPlanSerde.scala:615), so the collation is dropped on the way into the native plan with no warning. Nothing in CometCast stopped that from happening.

CometCast.isSupported matches string casts through case (DataTypes.StringType, _) and case (_, DataTypes.StringType). Those only fail to match a collated StringType because DataTypes.StringType is the default-collation singleton and Scala pattern equality compares the whole instance. The right answer fell out of an accident of pattern matching rather than a check anyone wrote, which is what the issue title means by "implicit". The fromType == toType shortcut at CometCast.scala:217 ran ahead of all of it, so identity casts on collated types were reported Compatible() regardless. CAST(ARRAY<STRING COLLATE UTF8_LCASE> AS ARRAY<STRING COLLATE UTF8_LCASE>) is the clearest example. Results are right today because the cast is a byte-level no-op, but the plan reached the native side with the collation stripped and nothing recording that.

Nine other places already use CometTypeShim.hasNonDefaultStringCollation for this exact purpose (aggregates.scala, arrays.scala, collectionOperations.scala, datetime.scala, maps.scala, predicates.scala, strings.scala, CometExprShim4x.scala, CometWindowGroupLimitExec.scala). CometCast mixes in CometTypeShim already and simply never called it.

What changes are included in this PR?

Adds the guard to CometCast.isSupported, above the fromType == toType shortcut so identity casts are checked too. hasNonDefaultStringCollation walks nested element, key, value, and field types, and is stubbed to false on Spark 3.x where collation does not exist, so the guard compiles away to a constant on the 3.x profiles.

The guard returns a reason of its own instead of the generic Cast from $fromType to $toType is not supported template, since that template prints both sides identically for an identity cast and gives no hint that collation is the cause. The string lives on CometCast.nonDefaultCollationReason and the suite reads it from production so the two cannot drift.

Adds CometCastCollatedStringSuite under spark/src/test/spark-4.x, which every 4.x profile compiles. It covers the scalar matrix in both directions and between two collations, the nested cases (array element, struct field, map key, map value, and the array-to-string recursion), and it checks that default-collation casts are still Compatible so the guard cannot quietly over-block.

Three pairs that the guard newly blocks are worth calling out, because they were Compatible before and are not identity casts. A struct whose collated field is unchanged while a sibling field is cast came out Compatible, because the field zip answered per field and the collated field hit the identity shortcut. MapType(lcase, IntegerType) -> MapType(lcase, LongType) did the same through the key. ArrayType(NullType) -> ArrayType(lcase) was Compatible through the elementType == NullType branch, which runs ahead of everything else. Each of those let a collated type reach the native plan on a sibling's cast, and each now has a test.

Four end-to-end tests run a query and assert what the planner does with the answer. Two use CAST(_1 COLLATE utf8_lcase AS INT) under both settings of spark.comet.exec.scalaUDF.codegen.enabled. The other two cast a struct carrying a collated field, which is the shape that actually exercises the new guard end to end, again under both settings. A scalar identity cast cannot be reached from SQL, because Spark's SimplifyCasts drops a cast whose child already has the target type, and there is a comment in the suite recording that.

One correction to an earlier revision of this PR. Unsupported does not mean the query falls back to Spark. CometCast mixes in CodegenDispatchFallback, so exprToProtoInternal offers the expression to the JVM codegen dispatcher before recording any fallback reason (QueryPlanSerde.scala:965-980). spark.comet.exec.scalaUDF.codegen.enabled defaults to true and CometBatchKernelCodegen.isSupportedDataType admits every StringType regardless of its collation, so under default config a collated cast usually stays inside the Comet pipeline running Spark's own doGenCode. The test names and the suite Scaladoc now say the cast has no native path instead.

Registers the suite in pr_build_linux.yml and pr_build_macos.yml.

How are these changes tested?

Run on Linux against a debug libcomet.so built from this branch, after the 2026-09-20 rebase onto current main. Every figure below was measured on that tree, not on the earlier revision. On the spark-4.1 profile (Spark 4.1.3, Scala 2.13.17, JDK 17):

$ ./mvnw -B -Pspark-4.1 test -Dsuites="org.apache.comet.CometCastCollatedStringSuite" -Dtest=none -pl spark

CometCastCollatedStringSuite:
- cast collated string to IntegerType has no native path (14 milliseconds)
- cast IntegerType to collated string has no native path (1 millisecond)
- cast collated string to default-collation StringType has no native path (2 milliseconds)
- cast default-collation StringType to collated string has no native path (1 millisecond)
- cast between two different collations has no native path (1 millisecond)
- cast collated string to the same collation has no native path (1 millisecond)
- cast array of collated strings to another collation has no native path (1 millisecond)
- cast array of collated strings to the same collation has no native path (1 millisecond)
- cast array of collated strings to StringType has no native path (1 millisecond)
- cast struct with a collated field has no native path (6 milliseconds)
- cast map with a collated key has no native path (1 millisecond)
- cast map with a collated value has no native path (0 milliseconds)
- cast struct whose collated field is unchanged while a sibling field is cast (1 millisecond)
- cast map whose collated key is unchanged while the value type is cast (1 millisecond)
- cast array of nulls to array of collated strings has no native path (1 millisecond)
- default-collation string casts are untouched by the collation guard (4 milliseconds)
- nested default-collation string casts are untouched by the collation guard (1 millisecond)
- cast from a collated string falls back to Spark when codegen dispatch is off (27 seconds, 496 milliseconds)
- cast from a collated string routes through the codegen dispatcher when it is on (586 milliseconds)
- cast of a struct carrying a collated field has no native path end to end (570 milliseconds)
- cast of a struct carrying a collated field routes through the codegen dispatcher (499 milliseconds)
Run completed in 34 seconds, 248 milliseconds.
Total number of tests run: 21
Tests: succeeded 21, failed 0, canceled 0, ignored 0, pending 0

The same suite on spark-4.0: 21 succeeded, 0 failed, 0 ignored.

The guard is production code, so the suites in its blast radius were run as well. On spark-4.0 (Spark 4.0.4, JDK 17), which is where the full CometCollationSuite including the #4051 join tests lives:

$ ./mvnw -B -Pspark-4.0 test -Dtest=none -pl spark \
    -Dsuites="org.apache.spark.sql.CometCollationSuite,org.apache.comet.CometNativeCastSuite"

Total number of tests run: 211
Tests: succeeded 211, failed 0, canceled 0, ignored 8, pending 0

The same pair on spark-4.1: 206 succeeded, 0 failed, 8 ignored.

CometSqlFileTestSuite was run in full on spark-4.1, not filtered: 550 fixtures, all passed. That covers all 16 collation fixtures, expressions/string/collation.sql among them.

A negative control, so the suite cannot be mistaken for guard-invariant. With the guard neutralised and nothing else changed, CometCastCollatedStringSuite on spark-4.0 fails 11 of its 21 tests, including the scalar end-to-end test and both struct end-to-end tests:

- cast from a collated string falls back to Spark when codegen dispatch is off *** FAILED ***
  Expected fallback reason 'Cast involving a non-default string collation is not supported
  (https://github.com/apache/datafusion-comet/issues/4489)' not found in
  [Cast from StringType(UTF8_LCASE) to IntegerType is not supported,
   cast: spark.comet.exec.scalaUDF.codegen.enabled=false;
   expression has no native path so the plan falls back to Spark]

./mvnw -B -Pspark-4.1 spotless:check -pl spark,common passes with the spotless index deleted first, so the result is a real check rather than a cache hit (Index file does not exist. Fallback to an empty index): 506 Scala files and 65 Java files, 0 needing changes.

Scope of what was run locally: the spark-4.0 and spark-4.1 profiles. spark-3.4, spark-3.5, and spark-4.2 were not run here, so CI is the first place those execute. On the 3.x profiles the guard is inert by construction, because CometTypeShim.hasNonDefaultStringCollation is a false literal there (spark/src/main/spark-3.x/org/apache/comet/shims/CometTypeShim.scala:41). spark-4.2 is a real profile in the root pom but carries no spark/src/test/spark-4.2 source root, so there is no CometCollationSuite to run against it. CometSqlFileTestSuite was run on 4.1 only, not on 4.0.

spark/src/test/resources/sql-tests/expressions/string/collation.sql is the largest existing test in the blast radius, since it casts a default-collation column to a collated target on nearly every query. Those pairs keep the same answer under the guard. The reason string they carry changes to the dedicated collation message, and the fixture asserts on results rather than on that text, so it passes either way. It was rerun on spark-4.1 after the change.

🤖 Generated with Claude Code

[andygrove]

Thanks for picking this up. The analysis of why StringType equality produces the fallback today is accurate and clearly written, and the writeup in the PR description made this easy to follow. A few things I'd like to work through before merge.

Unsupported on CometCast does not mean "falls back to Spark"

CometCast mixes in CodegenDispatchFallback (CometCast.scala:38). In QueryPlanSerde.scala:869-886, an Unsupported support level first goes to dispatchIfFallback, and the fallback reason is only recorded if the dispatcher declines. spark.comet.exec.scalaUDF.codegen.enabled defaults to true (CometConf.scala:364), and CometBatchKernelCodegen deliberately admits ResolvedCollation (CometBatchKernelCodegen.scala:156-157), so on default config a collated cast most likely stays inside the Comet pipeline running Spark's own doGenCode rather than falling back.

That outcome is still result-correct, so this is not a Comet bug. But every test name in the suite says "falls back", and the PR description says Comet "correctly falls back to Spark", and neither is quite right. Could those be reworded to say the cast has no native path?

Could these live in CometCollationSuite?

CometCollationSuite (spark/src/test/spark-4.0/org/apache/spark/sql/CometCollationSuite.scala) is already the home for collation fallback tests across #1947, #4051, and #4646. Reusing it would also avoid a new registration in two workflow files.

If the reason for a separate suite is that CometCollationSuite lives in spark-4.0 and so does not run on the 4.1 or 4.2 profiles, that is a good catch. In that case, would moving CometCollationSuite itself to spark-4.x be the better change? That gets the whole existing collation suite running on 4.1 and 4.2 as well.

End-to-end coverage is what the issue asked for, and it looks reachable

Issue #4489 asks for tests asserting that the cast falls back and does not run native, and this suite only exercises isSupported in isolation. The datetime tests in CometCollationSuite show that expression-level collation is reachable end to end from a plain-string Parquet column, and that the serde's reason surfaces because getSupportLevel runs before children are serialized. Would something like this work?

withSQLConf(CometConf.COMET_SCALA_UDF_CODEGEN_ENABLED.key -> "false") {
  checkSparkAnswerAndFallbackReason(
    "SELECT CAST(_1 COLLATE utf8_lcase AS INT) FROM tbl",
    "Cast from StringType(UTF8_LCASE) to IntegerType is not supported")
}

That verifies the runtime outcome and the reason string together, which is a lot stronger than asserting on the matrix alone. It would also be worth adding the COMET_SCALA_UDF_CODEGEN_ENABLED=true counterpart with checkSparkAnswerAndOperator so the dispatcher path is pinned down too. If the cast turns out not to be reachable end to end because something upstream short-circuits, could you document why in the same style as the join tests in CometCollationSuite?

The identity-cast baselines conflict with the guard the issue prefers

CometCast already mixes in CometTypeShim (CometCast.scala:37), which gives you hasNonDefaultStringCollation. That helper already walks nested element, key, value, and field types (spark/src/main/spark-4.x/org/apache/comet/shims/CometTypeShim.scala:41-48). So option 1 from the issue is about three lines at the top of isSupported, above the fromType == toType shortcut:

if (hasNonDefaultStringCollation(fromType) || hasNonDefaultStringCollation(toType)) {
  return unsupported(fromType, toType)
}

Would you consider adding that here rather than leaving it for a follow-up? Two reasons. The issue title is "implicit and untested", and this PR only addresses the second half. And assert(CometCast.isSupported(lcase, lcase, None, evalMode) == Compatible()) would fail once that guard exists, so whoever adds it later has to choose between weakening the guard and deleting your test. Adding the guard now and dropping that baseline avoids that.

Nested collated types are the part that is only safe by accident

Because the fromType == toType shortcut at CometCast.scala:189 runs before any pattern matching, CAST(ARRAY<STRING COLLATE UTF8_LCASE> AS ARRAY<STRING COLLATE UTF8_LCASE>) returns Compatible(), and the same holds for a struct with a collated field or a map with a collated key. serializeDataType maps every StringType to proto type id 7 (QueryPlanSerde.scala:565), so the collation is silently dropped from the proto with no warning. Identity is byte-safe so results are right today, but this is the same implicit behavior the issue describes and it is the case hasNonDefaultStringCollation was written for.

Since the suite is specifically about pinning the matrix down, it would be worth covering the nested paths as well: ArrayType(lcase) -> ArrayType(unicode), a StructType with a collated field, a MapType with a collated key, and the array-to-string recursion at CometCast.scala:203. Those go through different code than the scalar catch-all.

Small correction in the suite Scaladoc

The header says the fallback happens "via canCastFromString/canCastToString's own catch-all when only one side is collated". That holds for lcase -> StringType and StringType -> lcase, but IntegerType -> lcase actually exits through canCastFromInt's catch-all at CometCast.scala:410-411, since (_, DataTypes.StringType) does not match a collated target. Worth correcting, since the explanation is the main value of this file.

CI

CI has not run yet on this branch. All three workflow runs are sitting in action_required waiting on approval, so I will get those going. The registration itself looks correct to me. java-test passes the list as -DwildcardSuites, so the suite is simply not matched on the 3.4 and 3.5 profiles rather than failing, which matches how CometWidthBucketSuite is already handled.

[stantheman0128]

Thanks, this was a genuinely useful review. I had the dispatch semantics backwards and that wording is now fixed everywhere it was copied.

Unsupported does not mean "falls back to Spark"

You are right, and I should have caught this from the repo itself. collation.sql already spells out the same mechanism for the predicate serdes, and predicates.scala carries the comment explaining it. I traced exprToProtoInternal again and the Unsupported branch really does reach dispatchIfFallback before any reason is recorded. Every test name, the suite Scaladoc, and the PR description now say the cast has no native path.

The guard

Added, three lines, above the fromType == toType shortcut so identity casts are covered. It calls hasNonDefaultStringCollation the same way the array, collection, datetime, map, predicate, and string serdes already do. The lcase -> lcase Compatible() baseline is gone, since the guard makes that pair Unsupported. The default-collation baseline stays and I added a nested version of it, so the guard cannot over-block without a test noticing.

Nested types

Covered: ArrayType(lcase) -> ArrayType(unicode), the identity array case, a struct with a collated field, a map with a collated key, a map with a collated value, and ArrayType(lcase) -> StringType for the recursion at CometCast.scala:203.

Working through which pairs the guard actually changes turned up three more that were Compatible on main, so I added tests for those too rather than leave the over-block surface undescribed. A struct whose collated field is unchanged while a sibling field is cast came out Compatible, because the field zip answered per field and the collated field hit the identity shortcut. MapType(lcase, IntegerType) -> MapType(lcase, LongType) did the same through the key. And ArrayType(NullType) -> ArrayType(lcase) was Compatible through the elementType == NullType branch at CometCast.scala:199, which runs before any of the rest. So the collated type could ride into the native plan on a sibling's cast in each case.

The remaining nested pairs already returned Unsupported. The guard only moved which type the reason string names.

Scaladoc correction

Fixed. The IntegerType -> lcase explanation now points at canCastFromInt's catch-all, and the test for that pair carries a comment saying why (_, DataTypes.StringType) never matches a collated target.

End-to-end coverage

Added both, your query and the =true counterpart, and the reason surfaces exactly as you predicted.

One thing I would rather flag myself than let read better than it is. Those two are guard-invariant. (StringType(UTF8_LCASE), IntegerType) already fell through to the case _ catch-all on main and produced the same reason string, so both pass with the guard reverted. They pin down the planner's treatment of an Unsupported cast, which is the thing your first point corrects, but they do not exercise the new check.

I tried to add one that does, using the identity pair the guard actually changed, and it turns out that hits the case you asked me to document. CAST(_1 COLLATE utf8_lcase AS STRING COLLATE UTF8_LCASE) never reaches Comet as a cast, because Spark's SimplifyCasts drops a cast whose child already has the target type. The query arrives at the planner as a bare Collate and the only fallback reason on the plan is collate is not supported, from a different serde:

Expected fallback reason 'Cast from StringType(UTF8_LCASE) to StringType(UTF8_LCASE) is not
supported' not found in [collate is not supported]

So the scalar identity pairs stay pinned at the isSupported level, with a comment in the suite explaining why, written in the style of the join tests in CometCollationSuite.

A struct turned out to be the way in. When a sibling field changes type the cast survives SimplifyCasts and the collated field rides along inside it, so there is now a third end-to-end test on CAST(struct(_2 AS a, _1 COLLATE utf8_lcase AS s) AS STRUCT<a: STRING, s: STRING COLLATE UTF8_LCASE>). That one does fail without the guard, because the old field zip answered Compatible and the struct went native with the collation dropped. The reason it produces is:

Cast from StructType(StructField(a,IntegerType,true),StructField(s,StringType(UTF8_LCASE),true))
to StructType(StructField(a,StringType,true),StructField(s,StringType(UTF8_LCASE),true))
is not supported

Local runs are the spark-4.0 and spark-4.1 profiles, against a debug libcomet.so built from this branch. 3.4, 3.5, and 4.2 first execute in CI. On the 3.x profiles the guard is inert anyway, since hasNonDefaultStringCollation is a false literal in the 3.x shim.

Why these are Scala tests and not SQL fixtures

The review-comet-pr skill checked into this repo says expression tests should use CometSqlFileTestSuite where it can express them, so I should say why most of this suite does not.

The bulk of the file asserts on CometCast.isSupported over type pairs that SQL cannot construct. ArrayType(NullType) -> ArrayType(STRING COLLATE UTF8_LCASE), and a struct whose collated field is unchanged while a sibling field is cast, are only reachable by building the types in Scala. That is the carve-out the skill already allows for.

The three end-to-end tests were closer to workable as a fixture, but --Config and --ConfigMatrix are both file scoped, and ConfigMatrix reruns every query in the file under every combination. The query that has to assert a fallback reason with spark.comet.exec.scalaUDF.codegen.enabled=false and the query that has to assert native execution with it set to true therefore cannot share a file. Expressing them as fixtures means a file per query for what is two lines of SQL each. I am happy to split them out that way if you would rather have them there.

I did run the existing fixtures as part of the blast radius. CometSqlFileTestSuite passes on 4.1 with the guard in, including collation.sql, which is the fixture most exposed to this change since it casts a default-collation column to a collated target on nearly every query.

One gap I did not close

getSupportLevel returns Compatible() for any cast whose child is a Literal, before isSupported is consulted, so CAST('abc' AS STRING COLLATE UTF8_LCASE) still reaches the native side with the collation stripped. ConstantFolding normally removes that cast first, but CometSqlFileTestSuite excludes ConstantFolding for every fixture file it runs, so the path is reachable inside our own harness. It is the same class of problem as #4489 but it sits in a different method, and closing it means deciding what CometLiteral should do with a collated literal rather than adding a line to isSupported. Would you rather I pulled it into this PR or filed it separately?

Moving CometCollationSuite to spark-4.x

I looked at this and I do not think it fits inside this PR. Three things came up.

There is already a second copy at spark/src/test/spark-4.1/org/apache/spark/sql/CometCollationSuite.scala, added by #4097. On the 4.1 profile, src/test/spark-4.x and src/test/spark-4.1 are both test source roots (spark/pom.xml:538-540), so moving the 4.0 copy up without deleting that one gives two classes with the same fully qualified name.

The two copies differ by exactly the #4051 join block, and Spark 4.1 looks like the reason. BroadcastHashJoinExec and ShuffledHashJoinExec became case class ... private there, with an explicit companion apply that runs HashJoin.normalizeJoinKeys. That wraps collated keys in CollationKey, whose dataType is BinaryType. So on 4.1 and 4.2 the exec never receives a collated key, Comet's guard has nothing to reject, and the two result.isEmpty assertions would not hold. SortMergeJoinExec is still a plain public case class on both, so that one test would survive a move.

There is no spark/src/test/spark-4.2 directory at all, so 4.2 has no CometCollationSuite today. A move would run the shuffle and datetime tests there for the first time, which is the real payoff in your suggestion and also the part most likely to surface something new.

So the move is worth doing, but it means reconciling two divergent copies and deciding what happens to the #4051 join tests on 4.1 and later. Happy to file an issue and take it as a follow-up if you agree that is the right shape.

One thing that fell out of the above and may deserve its own issue. If Spark 4.1 normalizes collated join keys to binary before the exec is constructed, is Comet's collated-join guard from #4051 still reachable on 4.1 and later, and could Comet legitimately accept those joins natively there? I did not chase it far enough to be sure, but it did not look like something the current tests would tell us.

For this PR I kept the cast tests in their own spark-4.x suite. With two CometCollationSuite copies in the tree, folding them in would mean writing the same tests twice.

[andygrove]

Note on this review: this was generated by an LLM (Claude Code) at my request while I worked through a review backlog. I have not verified the individual findings myself. Please treat everything below as suggestions to evaluate rather than as authoritative review feedback, and push back on anything that is wrong or already handled.

The analysis in the description is excellent. "The right answer fell out of an accident of pattern matching rather than a check anyone wrote" is exactly the situation, and moving the check above the fromType == toType shortcut so identity casts on collated types are caught too is the detail that makes this actually complete. Adding the new suite to both the Linux and macOS workflows is the right thing to do.

Two things I would like to understand.

Where does the cast actually go now?

Unsupported routes through the JVM codegen dispatcher rather than straight to Spark row execution. The dispatcher builds Arrow vectors for its inputs and outputs, and Arrow has no notion of Spark collation either. If a collated STRING reaches the dispatcher, does it hit the same silent metadata loss this PR is fixing, just one layer down?

CometBatchKernelCodegen.canHandle would be the place to check. If collated strings are already rejected there, a sentence in the comment saying so would close the loop. If they are not, this fix moves the problem rather than removing it, and the dispatcher needs the same guard.

The fallback reason is generic

unsupported(fromType, toType) produces the standard "Cast from X to Y is not supported" message. For a user whose query slowed down because one column has COLLATE UTF8_LCASE, that message gives no hint that collation is the cause, and both types will print as string in the message, making it look like a nonsensical rejection of a string-to-string cast.

Could this return Unsupported(Some("Cast involving a non-default string collation is not supported (https://github.com/apache/datafusion-comet/issues/4489)")) instead? That is the string users will see in EXPLAIN output, and it is the difference between a confusing fallback and an actionable one.

One smaller note

hasNonDefaultStringCollation now walks nested element, key, value, and field types on every cast support check, for every cast in every plan, on every Spark version including 3.x where it always returns false. Is the 3.x shim short-circuiting before the walk, or does it recurse and then return false? Planning-time cost is usually irrelevant, but isSupported is called a lot and deeply nested schemas are not rare.

[stantheman0128]

@andygrove I checked the three points on this branch.

Where the cast goes: canHandle does not reject collated strings. isSupportedDataType treats every StringType as supported (_: StringType => true at CometBatchKernelCodegen.scala:89), and there is no hasNonDefaultStringCollation check in canHandle. So a collated cast that this PR marks Unsupported does reach the dispatcher, via dispatchIfFallback -> emitJvmCodegenDispatch.

That is not the same silent drop as the native path. serializeDataType is what flattens every StringType to proto id 7. The dispatcher never calls it. It runs Spark's own Cast.doGenCode against the expression, which still has the collationId. Arrow is only the byte store. The COMET_SCALA_UDF_CODEGEN_ENABLED=true end-to-end cases in this PR are there to pin that the dispatcher answers match Spark.

I can add a sentence next to canHandle saying collated strings are admitted on purpose because the kernel uses Spark codegen, not native serde. I would not copy the isSupported guard into canHandle. That would refuse the dispatcher path this mixin is for.

The fallback reason: $fromType on Spark 4 already prints as StringType(UTF8_LCASE), not a bare string, so EXPLAIN is not as opaque as it first looks. A dedicated "non-default string collation" sentence would still be clearer for identity casts. I can switch unsupported() for this guard if you want that wording. I left the generic template for now because the suite asserts on it.

The 3.x shim: hasNonDefaultStringCollation is false with no walk (spark-3.x CometTypeShim.scala:31). The 4.x helper is the one that recurses.

[stantheman0128]

Thanks. I chased the first point further than my September comment did, and it turned up a correction to something I told you plus three things I did not expect to find.

Where the cast actually goes

The first half has not changed. CometBatchKernelCodegen.isSupportedDataType matches case _: StringType | _: BinaryType => true, and on Spark 4 a collated StringType is still a StringType, so canHandle admits it. A collated cast that this PR marks Unsupported does reach the dispatcher.

The correction is the second half. I told you the dispatcher never calls serializeDataType. That was wrong. CometScalaUDF.emitJvmCodegenDispatch declares the return type through it at CometScalaUDF.scala:152, and serializeDataType flattens every StringType to proto id 7 at QueryPlanSerde.scala:615. So the dispatcher's output column does arrive on the native side with the collation gone, which is what you suspected.

It still does not change an answer on this route, for three reasons.

The evaluation never reads the proto type. The kernel compiles the closure-serialized bound expression (CometScalaUDFCodegen.scala:156-169), whose collationId survives serialization, and runs Spark's own doGenCode. Arrow is the byte store for the UTF8Strings that code produces.

The Arrow output field agrees with the proto rather than contradicting it. lookupOrCompile builds the field from boundExpr.dataType (CometScalaUDFCodegen.scala:172-175), which goes through Utils.toArrowType's case _: StringType => ArrowType.Utf8.INSTANCE (Utils.scala:157). Both sides flatten the same way, so there is no FFI type mismatch hiding in here.

And nothing reads the flattened type back. The values are bytes, the kernel produced them, and whether a downstream operator may treat the column collation-blind is decided per operator against the Catalyst DataType, which keeps its collation. That decision does not depend on what the dispatcher admitted.

So rejecting collated strings in canHandle would refuse a route that is already correct and put every collated cast on a full Spark fallback instead. I have written that up on isSupportedDataType, with a pointer to it from the guard in CometCast. I put it there and not on canHandle because isSupportedDataType is the line doing the admitting, and canHandle's own doc already points at it. Happy to move it to where you pointed if you would rather have it there.

Three serde-level omissions your question turned up, none of them this PR's

My first draft of that comment claimed the downstream guards were exhaustive. They are not. What I can show you is three places carrying no collation test in the serde. What I cannot show you is a plan that reaches any of them, so please read the list with that limit attached.

The limit matters because this repo guards collation systematically, and two of those guards sit upstream of everything below. CometScanRule refuses any collated column outright, with the comment that it is a convenient place to force the whole query back to Spark (CometScanRule.scala:1073-1076). Both partitionings are guarded as well, range at CometShuffleExchangeExec.scala:537 and hash at :432. Grouping keys (operators.scala:1931), join keys in both the broadcast-hash and sort-merge paths (:2410 and :2968) and the sort-merge equal-key type check (:3029) each have their own. So a collated column read from a table does not reach a native operator at all, and the shape that does get through is the one this PR's suite already uses: a plain column with COLLATE applied above it.

Against that background, the three with no test.

supportedSortType only type-checks single-key sorts. It opens with if (sortOrder.length == 1) (QueryPlanSerde.scala:1287) and its else returns true unconditionally (:1301-1302), while the single-key branch rejects collation through supportedScalarSortElementType at :1276. The two branches disagree with each other, which reads like an oversight rather than a decision. A multi-key global ORDER BY needs a range shuffle and that is guarded, so the shapes where this could bite are sortWithinPartitions and TakeOrderedAndProject. I have built neither.

hash() and xxhash64() accept collated children. CometMurmur3Hash (hash.scala:53) and CometXxHash64 (:31) route getSupportLevel to HashUtils.supportLevelForChildren, and unsupportedReasonFor (:136-147) has no collation case. This is the one I would call a plain omission rather than an unproven reachability claim, because the repo has already written down why it is wrong for the neighbouring case: CometApproxCountDistinct excludes collated strings at aggregates.scala:1233-1244, because Spark hashes them via the collation sort key, and says so in getUnsupportedReasons at :1250. Nothing in that reasoning is specific to approx_count_distinct.

CometWindowExec.convert serializes partitionSpec and orderSpec straight through exprToProto (CometWindowExec.scala:65-67), which applies the per-expression serde gates but adds no collation gate of its own for a bare attribute key. A window partition needs a hash shuffle, guarded at CometShuffleExchangeExec.scala:432, so this carries the same reachability caveat as the sort. The neighbouring operator does have one: CometWindowGroupLimitExec filters its ordering through hasNonDefaultStringCollation at :92. That is what makes the window case look like an omission rather than a decision.

None of the three is a confirmed divergence, and none is a confirmed reachable plan. I can file them as serde-level omissions, together or separately, and take the hash one myself if you want it fixed rather than only tracked.

The fallback reason

Changed, with your wording verbatim. CometCast.nonDefaultCollationReason holds the string and the guard returns it instead of unsupported(fromType, toType). It is private[comet] and the suite reads it from production instead of retyping it, the way negativeScaleDecimalToStringReason is already shared with CometNativeCastSuite.

Two notes on it. The first is a limit. The reason only reaches EXPLAIN when the dispatcher declines the expression, because exprToProtoInternal offers an Unsupported case to dispatchIfFallback first and calls withFallbackReason only if that returns None (QueryPlanSerde.scala:965-980). On default config a collated cast is accepted by the dispatcher and nothing is recorded. The user who sees this string is the one who turned spark.comet.exec.scalaUDF.codegen.enabled off or hit a canHandle refusal, which is also the user whose query really did fall back. So it lands where it matters, just not as widely as it first looks.

The second is that it closed a weakness I flagged last round. I said the scalar end-to-end tests were guard-invariant, because StringType(UTF8_LCASE) -> IntegerType already exited through the case _ catch-all with the same reason string. With a dedicated reason that stops being true. I checked by neutralising the guard and rerunning, and the assertion fails:

Expected fallback reason 'Cast involving a non-default string collation is not supported
(https://github.com/apache/datafusion-comet/issues/4489)' not found in
[Cast from StringType(UTF8_LCASE) to IntegerType is not supported,
 cast: spark.comet.exec.scalaUDF.codegen.enabled=false;
 expression has no native path so the plan falls back to Spark]

cast.md is unaffected. supportedTypes has no collated entry so the generator never asks for this pair, and GenerateDocs renders an Unsupported cell as U without the note anyway.

The 3.x walk

Already handled. hasNonDefaultStringCollation in the 3.x shim is a false literal with no match and no recursion (spark/src/main/spark-3.x/org/apache/comet/shims/CometTypeShim.scala:41). Nothing recurses on 3.4 or 3.5.

On 4.x I do not think the walk is worth short-circuiting. The helper's own first case is the scalar one (spark/src/main/spark-4.x/org/apache/comet/shims/CometTypeShim.scala:48-54), so a plain StringType answers on that first arm and an IntegerType drops straight to the case _ at :54. Neither one recurses. The walk only happens for array, map and struct types, and for exactly those the statement directly below the guard is if (fromType == toType), where DataType equality is structural and walks both trees anyway. canHandle further along the same path then runs numOfNestedFields over the output type and a collect over the whole expression tree. The guard adds a constant factor to something already linear in the same quantity, once per cast, at plan time. A short-circuit would only make the guard harder to read for no measurable gain, so I left it out. I will add one if you disagree.

Also in this push

One more end-to-end test. The struct case is the only pair where the guard changes the answer and not just the reason string, and it was covered only with the dispatcher off. It now has the dispatcher-on half too, so both settings are exercised on both the scalar and the struct case.

The suite header now carries the reason these are Scala tests rather than CometSqlFileTestSuite fixtures. That explanation was in the thread but not in the file, where the next reader would look.

The branch is also rebased onto current main. The only collision was the isSupportedDataType Scaladoc, where #5766 added a paragraph on duplicate struct field names in the same place; both paragraphs are kept. The code change is unchanged at 315 added lines across five files with nothing removed.

Verification

Local, against a debug libcomet.so built from this branch after the rebase.

• CometCastCollatedStringSuite, spark-4.0: 21 succeeded, 0 failed.
• CometCastCollatedStringSuite, spark-4.1: 21 succeeded, 0 failed.
• CometCollationSuite plus CometNativeCastSuite, spark-4.0: 211 succeeded, 0 failed, 8 ignored.
• CometCollationSuite plus CometNativeCastSuite, spark-4.1: 206 succeeded, 0 failed, 8 ignored.
• CometSqlFileTestSuite in full, spark-4.1: 550 fixtures, all passed. That covers all 16 collation fixtures, expressions/string/collation.sql among them.
• Negative control: guard neutralised, spark-4.0, 11 of 21 fail, including the scalar end-to-end test and both struct end-to-end tests.

Not run locally: 3.4, 3.5 and 4.2. The guard is inert on the 3.x profiles since the shim is a false literal. 4.2 has no CometCollationSuite today, which is part of the follow-up we discussed.

CI has never executed on this PR. Every workflow run it has produced so far finished without starting a single job: some are still sitting at action_required, and the older ones were marked failed once the approval window lapsed. The red mark is that gate expiring rather than a test failure. If you can approve the workflows, that is the fastest way to get a real signal here.

One question still open from last round

getSupportLevel returns Compatible() for any cast whose child is a Literal, before isSupported runs, so CAST('abc' AS STRING COLLATE UTF8_LCASE) still reaches the native side with the collation stripped. ConstantFolding normally removes that cast first, but CometSqlFileTestSuite excludes ConstantFolding for every fixture it runs, so our own harness can reach it. Closing it means deciding what CometLiteral should do with a collated literal, not adding a line to isSupported. Do you want it in this PR or in its own issue? I will open the issue if I do not hear otherwise, so it does not sit as an untracked remark.

[andygrove]

Thanks for chasing the dispatcher question all the way down. The guard in isSupported is what #4489 asked for, the dedicated reason reads well in EXPLAIN, and the negative control settles that the suite isn't guard-invariant.

To answer your open questions, a separate issue is right for the collated literal, since folding produces the same bytes Spark would, and the same goes for moving CometCollationSuite to spark-4.x, given the two copies and the 4.1 join-key normalization you found. Please go ahead and open both as you offered. Of the three serde omissions, the multi-key supportedSortType gap is real, and #6110 ran into the same one from the shuffle side, so I filed #6158 to cover both. I don't think the hash one is a divergence at default config. Since 4.0.1, Spark's user-facing Murmur3Hash and XxHash64 set isCollationAware = false and only hash by the collation key under the internal spark.sql.legacy.collationAwareHashFunctions flag, so raw-byte hashing matches Spark unless that flag is on.

The comment on the struct test describes the old behaviour differently from what I get from the code. It says the struct went native with the collation dropped, but Collate has no serde, so on main that query falls back with collate is not supported. The guard still changes the outcome, it just moves the query from a Spark fallback into the dispatcher. Could the comment and the PR description say that?