cloud-fan commented on a change in pull request #29792:
URL: https://github.com/apache/spark/pull/29792#discussion_r496523301



##########
File path: 
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/UnwrapCastInBinaryComparison.scala
##########
@@ -35,18 +35,34 @@ import org.apache.spark.sql.types._
  * to be optimized away later and pushed down to data sources.
  *
  * Currently this only handles cases where:
- *   1). `fromType` (of `fromExp`) and `toType` are of integral types (i.e., 
byte, short, int and
- *     long)
+ *   1). `fromType` (of `fromExp`) and `toType` are of numeric types (i.e., 
short, int, float,
+ *     decimal, etc)
  *   2). `fromType` can be safely coerced to `toType` without precision loss 
(e.g., short to int,
  *     int to long, but not long to int)
  *
  * If the above conditions are satisfied, the rule checks to see if the 
literal `value` is within
  * range `(min, max)`, where `min` and `max` are the minimum and maximum value 
of `fromType`,
- * respectively. If this is true then it means we can safely cast `value` to 
`fromType` and thus
+ * respectively. If this is true then it means we may safely cast `value` to 
`fromType` and thus
  * able to move the cast to the literal side. That is:
  *
  *   `cast(fromExp, toType) op value` ==> `fromExp op cast(value, fromType)`
  *
+ * Note there are some exceptions to the above: if casting from `value` to 
`fromType` causes
+ * rounding up or down, the above conversion will no longer be valid. Instead, 
the rule does the
+ * following:
+ *
+ * if casting `value` to `fromType` causes rounding up:
+ *  - `cast(fromExp, toType) > value` ==> `fromExp >= cast(value, fromType)`
+ *  - `cast(fromExp, toType) >= value` ==> `fromExp >= cast(value, fromType)`
+ *  - `cast(fromExp, toType) === value` ==> if(isnull(fromExp), null, false)
+ *  - `cast(fromExp, toType) <=> value` ==> false (if `fromExp` is 
deterministic)
+ *  - `cast(fromExp, toType) <=> value` ==> `cast(fromExp, toType) <=> value` 
(if `fromExp` is

Review comment:
       We can remove this because the rule does nothing for it.




----------------------------------------------------------------
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:
us...@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

Reply via email to