andygrove commented on PR #1380:
URL:
https://github.com/apache/datafusion-comet/pull/1380#issuecomment-2654127513
> It's hard to spot the two functional changes made in this PR because of
the large amount of code moved. Can you tag the places where the changes were
made?
Sure, functional change #1 moved some of the pre-condition checks. For
example, for `min`, we originally had:
```scala
case min @ Min(child) if minMaxDataTypeSupported(min.dataType) =>
```
And now we have:
```scala
case _: Min => CometMin
```
The pre-condition check is now moved to:
```scala
object CometMin extends CometAggregateExpressionSerde {
override def convert(...) {
if (!AggSerde.minMaxDataTypeSupported(expr.dataType)) {
withInfo(aggExpr, s"Unsupported data type: ${expr.dataType}")
return None
}
}
}
```
Functional change #2 tightened up the checks for supported types:
Example for `Sum`:
Before:
```scala
private def sumDataTypeSupported(dt: DataType): Boolean = {
dt match {
case _: NumericType => true
case _ => false
}
}
```
After:
```scala
def sumDataTypeSupported(dt: DataType): Boolean = {
dt match {
case ByteType | ShortType | IntegerType | LongType => true
case FloatType | DoubleType => true
case _: DecimalType => true
case _ => false
}
}
```
--
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: github-unsubscr...@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org
For additional commands, e-mail: github-h...@datafusion.apache.org
