cloud-fan commented on a change in pull request #32488:
URL: https://github.com/apache/spark/pull/32488#discussion_r643062062
##########
File path:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/UnwrapCastInBinaryComparison.scala
##########
@@ -121,6 +131,77 @@ object UnwrapCastInBinaryComparison extends
Rule[LogicalPlan] {
if canImplicitlyCast(fromExp, toType, literalType) =>
simplifyNumericComparison(be, fromExp, toType, value)
+ // As the analyzer makes sure that the list of In is already of the same
data type, then the
+ // rule can simply check the first literal in `in.list` can implicitly
cast to `toType` or not,
+ // and note that:
Review comment:
The null literal handling is a bit tricky and I think it's better to put
all the steps in the comment. There are 3 kinds of literals in the list:
1. null literals
2. The literals that can cast to `fromExp.dataType`
3. The literals that cannot cast to `fromExp.dataType`
null literals is special, because if we call `unwrapCast` directly, null
means cannot cast, which is misleading as we can cast null literals to any data
type.
The ideal steps in my mind:
1. Call `unwrapCast` with non-null literals in the list
2. If there is no literal that can cast to `fromExp.dataType`, return the
original expression
3. Otherwise, use the literals that can cast to `fromExp.dataType` and the
null literals to create a new `In` expression, with additional `falseIfNotNull`
check.
--
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.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
