stantheman0128 commented on PR #5302:
URL: 
https://github.com/apache/datafusion-comet/pull/5302#issuecomment-5575002421

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


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to