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]

Reply via email to