Github user ssimeonov commented on a diff in the pull request:
https://github.com/apache/spark/pull/8893#discussion_r40337580
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/mathExpressions.scala
---
@@ -52,11 +52,12 @@ abstract class LeafMathExpression(c: Double, name:
String)
* @param f The math function.
* @param name The short name of the function
*/
-abstract class UnaryMathExpression(f: Double => Double, name: String)
+abstract class UnaryMathExpression[T <: DataType](
--- End diff --
I am no Scala expert either but I think what you want is something along
the lines of:
```scala
// Add to imports
import scala.reflect.runtime.universe.TypeTag
abstract class UnaryMathExpression[T <: NumericType](
f: Double => Any, name: String)(implicit ttag: TypeTag[T], longttag:
TypeTag[LongType])
extends UnaryExpression with Serializable with ImplicitCastInputTypes {
override def inputTypes: Seq[DataType] = Seq(DoubleType)
override def dataType: NumericType = if (ttag == longttag) LongType else
DoubleType
override def nullable: Boolean = true
override def toString: String = s"$name($child)"
protected override def nullSafeEval(input: Any): Any = {
f(input.asInstanceOf[Double])
}
// name of function in java.lang.Math
def funcName: String = name.toLowerCase
override final def genCode(ctx: CodeGenContext, ev:
GeneratedExpressionCode): String = {
defineCodeGen(ctx, ev, codeBuilder(ctx, ev))
}
protected def codeBuilder(ctx: CodeGenContext, ev:
GeneratedExpressionCode): (String) => String =
c => s"(${dataType.typeName})java.lang.Math.$funcName($c)"
}
```
Notes:
- Eliminates the need for creating a subclass just for the purpose of
`LongType` return specialization.
- Uses tighter constraint: `NumericType` instead of `DataType`, which makes
sense intuitively for math functions.
- Sets up type to handle the universe of cases: `LongType` vs. `DoubleType`
returns. Since we no longer subclass, that's an OK approach. It is better to
use automatic companion object discovery but that would require modifying all
data type case objects, which may not be a bad idea but belongs in a separate
PR. See [this](http://stackoverflow.com/a/9173117/622495) for more information.
- Uses `dataType.typeName` instead of hard-coding Java casts.
This would require changing all references of `UnaryMathExpression` to
`UnaryMathExpression[DoubleType]` and then using
`UnaryMathExpression[LongType]` for `ceil` and `floor`.
The code compiles and should behave as intended.
Hope this helps.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]