maropu commented on a change in pull request #29792:
URL: https://github.com/apache/spark/pull/29792#discussion_r502400334
##########
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:
Looks it does not have any test for this code path, so could you add
some tests for it. (NOTE: I think `byte`, `int`, and `long` are not tested in
`UnwrapCastInBinaryComparisonSuite`, too)
----------------------------------------------------------------
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]