comphead commented on code in PR #6179:
URL: https://github.com/apache/datafusion-comet/pull/6179#discussion_r4361444300
##########
spark/src/main/scala/org/apache/comet/expressions/CometCast.scala:
##########
@@ -308,6 +308,16 @@ object CometCast
}
}
Compatible()
+ case (from_map: MapType, to_map: MapType)
+ if evalMode == CometEvalMode.TRY && !keyCastCannotFail(
+ from_map.keyType,
+ to_map.keyType) =>
+ // Under TRY a key cast that throws becomes a null key, and Spark
keeps the row. Arrow's map
+ // format requires a non-nullable key field, so neither the native
cast nor the codegen
+ // dispatcher can hold that result, and `canHandle` refuses the
dispatch. Key casts that
+ // cannot throw stay on the native path. Maps nested in the value are
checked when the
+ // value cast recurses back into this arm.
+ Unsupported(Some(tryCastNullMapKeyReason))
Review Comment:
`canProduceNullMapKey(cast)` from #6178 already walks map, array and struct
types and tests `keyCastCannotFail` at every depth, so this arm adds a second
traversal that decides the same question through `isSupported`'s recursion.
That recursion is also why #6200 exists: the struct arm returns `other` as soon
as a field is not `Compatible`, and the general map arm returns the key's
`Incompatible` before it looks at the value, so an earlier `Incompatible` can
hide this `Unsupported`. Would it be simpler to check the helper once at the
top of `getSupportLevel`, next to the literal and timezone checks, for example
`else if (canProduceNullMapKey(cast))
Unsupported(Some(tryCastNullMapKeyReason))` before the `isSupported` branch?
That would drop the new arm, keep one definition of "can put a null key in a
map", and might remove the masking limitation in #6200. I have not run it and
have not checked that every caller of `isSupported` goes through
`getSupportLevel`.
--
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]
