sunchao commented on a change in pull request #29792:
URL: https://github.com/apache/spark/pull/29792#discussion_r502558580



##########
File path: 
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/UnwrapCastInBinaryComparison.scala
##########
@@ -200,25 +248,27 @@ object UnwrapCastInBinaryComparison extends 
Rule[LogicalPlan] {
   /**
    * Check if the input `fromExp` can be safely cast to `toType` without any 
loss of precision,
    * i.e., the conversion is injective. Note this only handles the case when 
both sides are of
-   * integral type.
+   * numeric type.
    */
   private def canImplicitlyCast(
       fromExp: Expression,
       toType: DataType,
       literalType: DataType): Boolean = {
     toType.sameType(literalType) &&
       !fromExp.foldable &&
-      fromExp.dataType.isInstanceOf[IntegralType] &&
-      toType.isInstanceOf[IntegralType] &&
+      fromExp.dataType.isInstanceOf[NumericType] &&
+      toType.isInstanceOf[NumericType] &&
       Cast.canUpCast(fromExp.dataType, toType)
   }
 
-  private def getRange(dt: DataType): (Any, Any) = dt match {
-    case ByteType => (Byte.MinValue, Byte.MaxValue)
-    case ShortType => (Short.MinValue, Short.MaxValue)
-    case IntegerType => (Int.MinValue, Int.MaxValue)
-    case LongType => (Long.MinValue, Long.MaxValue)
-    case other => throw new IllegalArgumentException(s"Unsupported type: 
${other.catalogString}")
+  private def getRange(dt: DataType): Option[(Any, Any)] = dt match {
+    case ByteType => Some((Byte.MinValue, Byte.MaxValue))
+    case ShortType => Some((Short.MinValue, Short.MaxValue))
+    case IntegerType => Some((Int.MinValue, Int.MaxValue))
+    case LongType => Some((Long.MinValue, Long.MaxValue))
+    case FloatType => Some((Float.NegativeInfinity, Float.NaN))
+    case DoubleType => Some((Double.NegativeInfinity, Double.NaN))

Review comment:
       Will add a test case (although I think it will be pretty trivial). I 
only added tests for `short` in the previous PR because the handling for other 
integral types is exactly the same.




----------------------------------------------------------------
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]

Reply via email to