From 067b788b05846e659615feef8613f8965573f150 Mon Sep 17 00:00:00 2001 From: Sandeep Singh Date: Sun, 3 Jul 2016 14:30:11 +0530 Subject: [PATCH 01/20] [SPARK-16323] [SQL] Add IntegerDivide to avoid unnecessary cast Before: ``` scala> spark.sql("select 6 div 3").explain(true) ... == Analyzed Logical Plan == CAST((6 / 3) AS BIGINT): bigint Project [cast((cast(6 as double) / cast(3 as double)) as bigint) AS CAST((6 / 3) AS BIGINT)#5L] +- OneRowRelation$ ... ``` After: ``` scala> spark.sql("select 6 div 3").explain(true) ... == Analyzed Logical Plan == (6 / 3): int Project [(6 / 3) AS (6 / 3)#11] +- OneRowRelation$ ... ``` --- .../catalyst/analysis/FunctionRegistry.scala | 1 + .../spark/sql/catalyst/dsl/package.scala | 1 + .../sql/catalyst/expressions/arithmetic.scala | 70 ++++++++++++++++++- .../sql/catalyst/parser/AstBuilder.scala | 2 +- .../ArithmeticExpressionSuite.scala | 16 ++--- .../parser/ExpressionParserSuite.scala | 4 +- .../scala/org/apache/spark/sql/Column.scala | 15 ++++ 7 files changed, 97 insertions(+), 12 deletions(-) diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/FunctionRegistry.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/FunctionRegistry.scala index 26b0c30db4e11..7ba4afb0977dd 100644 --- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/FunctionRegistry.scala +++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/FunctionRegistry.scala @@ -232,6 +232,7 @@ object FunctionRegistry { expression[Subtract]("-"), expression[Multiply]("*"), expression[Divide]("/"), + expression[IntegerDivide]("div"), expression[Remainder]("%"), // aggregate functions diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/dsl/package.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/dsl/package.scala index 84c9cc8c8e7f7..d1c7b29bd8513 100644 --- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/dsl/package.scala +++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/dsl/package.scala @@ -71,6 +71,7 @@ package object dsl { def - (other: Expression): Expression = Subtract(expr, other) def * (other: Expression): Expression = Multiply(expr, other) def / (other: Expression): Expression = Divide(expr, other) + def div (other: Expression): Expression = IntegerDivide(expr, other) def % (other: Expression): Expression = Remainder(expr, other) def & (other: Expression): Expression = BitwiseAnd(expr, other) def | (other: Expression): Expression = BitwiseOr(expr, other) diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/arithmetic.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/arithmetic.scala index 4db1352291e0b..0a385507f90b2 100644 --- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/arithmetic.scala +++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/arithmetic.scala @@ -216,7 +216,6 @@ case class Divide(left: Expression, right: Expression) override def inputType: AbstractDataType = TypeCollection(DoubleType, DecimalType) override def symbol: String = "/" - override def decimalMethod: String = "$div" override def nullable: Boolean = true private lazy val div: (Any, Any) => Any = dataType match { @@ -284,6 +283,75 @@ case class Divide(left: Expression, right: Expression) } } +@ExpressionDescription( + usage = "a _FUNC_ b - Divides a by b.", + extended = "> SELECT 3 _FUNC_ 2;\n 1") +case class IntegerDivide(left: Expression, right: Expression) + extends BinaryArithmetic with NullIntolerant { + + override def inputType: AbstractDataType = IntegralType + + override def symbol: String = "/" + override def decimalMethod: String = "$div" + override def nullable: Boolean = true + + private lazy val div: (Any, Any) => Any = dataType match { + case i: IntegralType => i.integral.asInstanceOf[Integral[Any]].quot + } + + override def eval(input: InternalRow): Any = { + val input2 = right.eval(input) + if (input2 == null || input2 == 0) { + null + } else { + val input1 = left.eval(input) + if (input1 == null) { + null + } else { + div(input1, input2) + } + } + } + + /** + * Special case handling due to division by 0 => null. + */ + override def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = { + val eval1 = left.genCode(ctx) + val eval2 = right.genCode(ctx) + val isZero = s"${eval2.value} == 0" + val javaType = ctx.javaType(dataType) + val divide = s"($javaType)(${eval1.value} $symbol ${eval2.value})" + if (!left.nullable && !right.nullable) { + ev.copy(code = s""" + ${eval2.code} + boolean ${ev.isNull} = false; + $javaType ${ev.value} = ${ctx.defaultValue(javaType)}; + if ($isZero) { + ${ev.isNull} = true; + } else { + ${eval1.code} + ${ev.value} = $divide; + }""") + } else { + ev.copy(code = s""" + ${eval2.code} + boolean ${ev.isNull} = false; + $javaType ${ev.value} = ${ctx.defaultValue(javaType)}; + if (${eval2.isNull} || $isZero) { + ${ev.isNull} = true; + } else { + ${eval1.code} + if (${eval1.isNull}) { + ${ev.isNull} = true; + } else { + ${ev.value} = $divide; + } + }""") + } + } +} + @ExpressionDescription( usage = "a _FUNC_ b - Returns the remainder when dividing a by b.") case class Remainder(left: Expression, right: Expression) diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala index f2cc8d362478a..03944c5cd0da1 100644 --- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala +++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala @@ -957,7 +957,7 @@ class AstBuilder extends SqlBaseBaseVisitor[AnyRef] with Logging { case SqlBaseParser.PERCENT => Remainder(left, right) case SqlBaseParser.DIV => - Cast(Divide(left, right), LongType) + IntegerDivide(left, right) case SqlBaseParser.PLUS => Add(left, right) case SqlBaseParser.MINUS => diff --git a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/ArithmeticExpressionSuite.scala b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/ArithmeticExpressionSuite.scala index 2e37887fbc822..dd9e6f6d96661 100644 --- a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/ArithmeticExpressionSuite.scala +++ b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/ArithmeticExpressionSuite.scala @@ -140,14 +140,14 @@ class ArithmeticExpressionSuite extends SparkFunSuite with ExpressionEvalHelper // By fixing SPARK-15776, Divide's inputType is required to be DoubleType of DecimalType. // TODO: in future release, we should add a IntegerDivide to support integral types. - ignore("/ (Divide) for integral type") { - checkEvaluation(Divide(Literal(1.toByte), Literal(2.toByte)), 0.toByte) - checkEvaluation(Divide(Literal(1.toShort), Literal(2.toShort)), 0.toShort) - checkEvaluation(Divide(Literal(1), Literal(2)), 0) - checkEvaluation(Divide(Literal(1.toLong), Literal(2.toLong)), 0.toLong) - checkEvaluation(Divide(positiveShortLit, negativeShortLit), 0.toShort) - checkEvaluation(Divide(positiveIntLit, negativeIntLit), 0) - checkEvaluation(Divide(positiveLongLit, negativeLongLit), 0L) + test("/ (Divide) for integral type") { + checkEvaluation(IntegerDivide(Literal(1.toByte), Literal(2.toByte)), 0.toByte) + checkEvaluation(IntegerDivide(Literal(1.toShort), Literal(2.toShort)), 0.toShort) + checkEvaluation(IntegerDivide(Literal(1), Literal(2)), 0) + checkEvaluation(IntegerDivide(Literal(1.toLong), Literal(2.toLong)), 0.toLong) + checkEvaluation(IntegerDivide(positiveShortLit, negativeShortLit), 0.toShort) + checkEvaluation(IntegerDivide(positiveIntLit, negativeIntLit), 0) + checkEvaluation(IntegerDivide(positiveLongLit, negativeLongLit), 0L) } test("% (Remainder)") { diff --git a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/parser/ExpressionParserSuite.scala b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/parser/ExpressionParserSuite.scala index e73592c7afa28..a207c1c1d5a95 100644 --- a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/parser/ExpressionParserSuite.scala +++ b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/parser/ExpressionParserSuite.scala @@ -169,7 +169,7 @@ class ExpressionParserSuite extends PlanTest { // Simple operations assertEqual("a * b", 'a * 'b) assertEqual("a / b", 'a / 'b) - assertEqual("a DIV b", ('a / 'b).cast(LongType)) + assertEqual("a DIV b", ('a div 'b)) assertEqual("a % b", 'a % 'b) assertEqual("a + b", 'a + 'b) assertEqual("a - b", 'a - 'b) @@ -180,7 +180,7 @@ class ExpressionParserSuite extends PlanTest { // Check precedences assertEqual( "a * t | b ^ c & d - e + f % g DIV h / i * k", - 'a * 't | ('b ^ ('c & ('d - 'e + (('f % 'g / 'h).cast(LongType) / 'i * 'k))))) + 'a * 't | ('b ^ ('c & ('d - 'e + (('f % 'g div 'h) / 'i * 'k))))) } test("unary arithmetic expressions") { diff --git a/sql/core/src/main/scala/org/apache/spark/sql/Column.scala b/sql/core/src/main/scala/org/apache/spark/sql/Column.scala index a46d1949e94ae..2e94c840b3436 100644 --- a/sql/core/src/main/scala/org/apache/spark/sql/Column.scala +++ b/sql/core/src/main/scala/org/apache/spark/sql/Column.scala @@ -731,6 +731,21 @@ class Column(protected[sql] val expr: Expression) extends Logging { */ def / (other: Any): Column = withExpr { Divide(expr, lit(other).expr) } + /** + * Integer Division this expression by another expression. + * {{{ + * // Scala: The following divides a person's height by their weight. + * people.select( people("height") div people("weight") ) + * + * // Java: + * people.select( people("height").div(people("weight")) ); + * }}} + * + * @group expr_ops + * @since 2.1.0 + */ + def div (other: Any): Column = withExpr { IntegerDivide(expr, lit(other).expr) } + /** * Division this expression by another expression. * {{{ From 5db17fd2e05578bd152d9f2654d49aa4e07abe5b Mon Sep 17 00:00:00 2001 From: Sandeep Singh Date: Sun, 3 Jul 2016 15:54:36 +0530 Subject: [PATCH 02/20] fix docs # Conflicts: # sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Funct ionRegistry.scala # sql/core/src/main/scala/org/apache/spark/sql/functions.scala # sql/core/src/test/scala/org/apache/spark/sql/DataFrameFunctionsSuite.sca la # sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveSessionCatalog.sca la --- .../scala/org/apache/spark/sql/Column.scala | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/sql/core/src/main/scala/org/apache/spark/sql/Column.scala b/sql/core/src/main/scala/org/apache/spark/sql/Column.scala index 2e94c840b3436..2dc09ec3f12c0 100644 --- a/sql/core/src/main/scala/org/apache/spark/sql/Column.scala +++ b/sql/core/src/main/scala/org/apache/spark/sql/Column.scala @@ -732,18 +732,18 @@ class Column(protected[sql] val expr: Expression) extends Logging { def / (other: Any): Column = withExpr { Divide(expr, lit(other).expr) } /** - * Integer Division this expression by another expression. - * {{{ - * // Scala: The following divides a person's height by their weight. - * people.select( people("height") div people("weight") ) - * - * // Java: - * people.select( people("height").div(people("weight")) ); - * }}} - * - * @group expr_ops - * @since 2.1.0 - */ + * Integer Division this expression by another expression. + * {{{ + * // Scala: The following divides a person's height by their weight. + * people.select( people("height") div people("weight") ) + * + * // Java: + * people.select( people("height").div(people("weight")) ); + * }}} + * + * @group expr_ops + * @since 2.1.0 + */ def div (other: Any): Column = withExpr { IntegerDivide(expr, lit(other).expr) } /** From faa2fd3b3d8a91f46bac6260e3ddd9745cbce758 Mon Sep 17 00:00:00 2001 From: Sandeep Singh Date: Sun, 3 Jul 2016 19:38:12 +0530 Subject: [PATCH 03/20] replace symbol / with div for integer division --- .../org/apache/spark/sql/catalyst/expressions/arithmetic.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/arithmetic.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/arithmetic.scala index 0a385507f90b2..66f0e15ac27c4 100644 --- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/arithmetic.scala +++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/arithmetic.scala @@ -291,7 +291,7 @@ case class IntegerDivide(left: Expression, right: Expression) override def inputType: AbstractDataType = IntegralType - override def symbol: String = "/" + override def symbol: String = "div" override def decimalMethod: String = "$div" override def nullable: Boolean = true From e30747a9126e88e04fa4adc36c4b42d70245cb85 Mon Sep 17 00:00:00 2001 From: Sandeep Singh Date: Sun, 3 Jul 2016 21:20:16 +0530 Subject: [PATCH 04/20] add decimalMethod --- .../org/apache/spark/sql/catalyst/expressions/arithmetic.scala | 1 + 1 file changed, 1 insertion(+) diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/arithmetic.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/arithmetic.scala index 66f0e15ac27c4..2c629a66a5eb4 100644 --- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/arithmetic.scala +++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/arithmetic.scala @@ -216,6 +216,7 @@ case class Divide(left: Expression, right: Expression) override def inputType: AbstractDataType = TypeCollection(DoubleType, DecimalType) override def symbol: String = "/" + override def decimalMethod: String = "$div" override def nullable: Boolean = true private lazy val div: (Any, Any) => Any = dataType match { From 699ff8b95d74bb192d4303ab84d7d73ebfa76ccf Mon Sep 17 00:00:00 2001 From: Sandeep Singh Date: Sun, 3 Jul 2016 23:47:11 +0530 Subject: [PATCH 05/20] use / in java for integer division --- .../org/apache/spark/sql/catalyst/expressions/arithmetic.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/arithmetic.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/arithmetic.scala index 2c629a66a5eb4..241de257864cd 100644 --- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/arithmetic.scala +++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/arithmetic.scala @@ -322,7 +322,7 @@ case class IntegerDivide(left: Expression, right: Expression) val eval2 = right.genCode(ctx) val isZero = s"${eval2.value} == 0" val javaType = ctx.javaType(dataType) - val divide = s"($javaType)(${eval1.value} $symbol ${eval2.value})" + val divide = s"($javaType)(${eval1.value}/(${eval2.value}))" if (!left.nullable && !right.nullable) { ev.copy(code = s""" ${eval2.code} From 054d54e1dae7c372fb1c7ef5ee52f7d9ff6a619d Mon Sep 17 00:00:00 2001 From: Sandeep Singh Date: Sun, 3 Jul 2016 23:51:44 +0530 Subject: [PATCH 06/20] / --- .../apache/spark/sql/catalyst/expressions/arithmetic.scala | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/arithmetic.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/arithmetic.scala index 241de257864cd..1838374715d9f 100644 --- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/arithmetic.scala +++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/arithmetic.scala @@ -293,7 +293,7 @@ case class IntegerDivide(left: Expression, right: Expression) override def inputType: AbstractDataType = IntegralType override def symbol: String = "div" - override def decimalMethod: String = "$div" + override def decimalMethod: String = "/" override def nullable: Boolean = true private lazy val div: (Any, Any) => Any = dataType match { @@ -322,7 +322,7 @@ case class IntegerDivide(left: Expression, right: Expression) val eval2 = right.genCode(ctx) val isZero = s"${eval2.value} == 0" val javaType = ctx.javaType(dataType) - val divide = s"($javaType)(${eval1.value}/(${eval2.value}))" + val divide = s"($javaType)(${eval1.value} $decimalMethod (${eval2.value}))" if (!left.nullable && !right.nullable) { ev.copy(code = s""" ${eval2.code} From ff9745776fcf97ff063dec0811762a3e0c4b1840 Mon Sep 17 00:00:00 2001 From: Sandeep Singh Date: Mon, 4 Jul 2016 10:04:43 +0530 Subject: [PATCH 07/20] Abstract out into DivisionArithmetic --- .../sql/catalyst/expressions/arithmetic.scala | 125 +++++++----------- 1 file changed, 47 insertions(+), 78 deletions(-) diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/arithmetic.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/arithmetic.scala index 1838374715d9f..db78f4ec65ef5 100644 --- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/arithmetic.scala +++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/arithmetic.scala @@ -207,20 +207,12 @@ case class Multiply(left: Expression, right: Expression) protected override def nullSafeEval(input1: Any, input2: Any): Any = numeric.times(input1, input2) } -@ExpressionDescription( - usage = "a _FUNC_ b - Divides a by b.", - extended = "> SELECT 3 _FUNC_ 2;\n 1.5") -case class Divide(left: Expression, right: Expression) - extends BinaryArithmetic with NullIntolerant { - - override def inputType: AbstractDataType = TypeCollection(DoubleType, DecimalType) - - override def symbol: String = "/" - override def decimalMethod: String = "$div" +abstract class DivisionArithmetic extends BinaryArithmetic with NullIntolerant { override def nullable: Boolean = true private lazy val div: (Any, Any) => Any = dataType match { case ft: FractionalType => ft.fractional.asInstanceOf[Fractional[Any]].div + case i: IntegralType => i.integral.asInstanceOf[Integral[Any]].quot } override def eval(input: InternalRow): Any = { @@ -237,119 +229,96 @@ case class Divide(left: Expression, right: Expression) } } + // Used by doGenCode + def divide(eval1: ExprCode, eval2: ExprCode, javaType: String): String + def isZero(eval2: ExprCode): String + /** * Special case handling due to division by 0 => null. */ override def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = { val eval1 = left.genCode(ctx) val eval2 = right.genCode(ctx) - val isZero = if (dataType.isInstanceOf[DecimalType]) { - s"${eval2.value}.isZero()" - } else { - s"${eval2.value} == 0" - } + val isZeroCheck = isZero(eval2) val javaType = ctx.javaType(dataType) - val divide = if (dataType.isInstanceOf[DecimalType]) { - s"${eval1.value}.$decimalMethod(${eval2.value})" - } else { - s"($javaType)(${eval1.value} $symbol ${eval2.value})" - } + val division = divide(eval1, eval2, javaType) if (!left.nullable && !right.nullable) { ev.copy(code = s""" ${eval2.code} boolean ${ev.isNull} = false; $javaType ${ev.value} = ${ctx.defaultValue(javaType)}; - if ($isZero) { + if ($isZeroCheck) { ${ev.isNull} = true; } else { ${eval1.code} - ${ev.value} = $divide; + ${ev.value} = $division; }""") } else { ev.copy(code = s""" ${eval2.code} boolean ${ev.isNull} = false; $javaType ${ev.value} = ${ctx.defaultValue(javaType)}; - if (${eval2.isNull} || $isZero) { + if (${eval2.isNull} || $isZeroCheck) { ${ev.isNull} = true; } else { ${eval1.code} if (${eval1.isNull}) { ${ev.isNull} = true; } else { - ${ev.value} = $divide; + ${ev.value} = $division; } }""") } } } +@ExpressionDescription( + usage = "a _FUNC_ b - Divides a by b.", + extended = "> SELECT 3 _FUNC_ 2;\n 1.5") +case class Divide(left: Expression, right: Expression) + extends DivisionArithmetic { + + override def inputType: AbstractDataType = TypeCollection(DoubleType, DecimalType) + + override def symbol: String = "/" + override def decimalMethod: String = "$div" + + // Used by doGenCode + override def divide(eval1: ExprCode, eval2: ExprCode, javaType: String): String = { + if (dataType.isInstanceOf[DecimalType]) { + s"${eval1.value}.$decimalMethod(${eval2.value})" + } else { + s"($javaType)(${eval1.value} $symbol ${eval2.value})" + } + } + + override def isZero(eval2: ExprCode): String = { + if (dataType.isInstanceOf[DecimalType]) { + s"${eval2.value}.isZero()" + } else { + s"${eval2.value} == 0" + } + } +} + @ExpressionDescription( usage = "a _FUNC_ b - Divides a by b.", extended = "> SELECT 3 _FUNC_ 2;\n 1") case class IntegerDivide(left: Expression, right: Expression) - extends BinaryArithmetic with NullIntolerant { + extends DivisionArithmetic { override def inputType: AbstractDataType = IntegralType override def symbol: String = "div" override def decimalMethod: String = "/" - override def nullable: Boolean = true - private lazy val div: (Any, Any) => Any = dataType match { - case i: IntegralType => i.integral.asInstanceOf[Integral[Any]].quot - } - - override def eval(input: InternalRow): Any = { - val input2 = right.eval(input) - if (input2 == null || input2 == 0) { - null - } else { - val input1 = left.eval(input) - if (input1 == null) { - null - } else { - div(input1, input2) - } - } + // Used by doGenCode + override def divide(eval1: ExprCode, eval2: ExprCode, javaType: String): String = { + s"($javaType)(${eval1.value} $decimalMethod (${eval2.value}))" } - /** - * Special case handling due to division by 0 => null. - */ - override def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = { - val eval1 = left.genCode(ctx) - val eval2 = right.genCode(ctx) - val isZero = s"${eval2.value} == 0" - val javaType = ctx.javaType(dataType) - val divide = s"($javaType)(${eval1.value} $decimalMethod (${eval2.value}))" - if (!left.nullable && !right.nullable) { - ev.copy(code = s""" - ${eval2.code} - boolean ${ev.isNull} = false; - $javaType ${ev.value} = ${ctx.defaultValue(javaType)}; - if ($isZero) { - ${ev.isNull} = true; - } else { - ${eval1.code} - ${ev.value} = $divide; - }""") - } else { - ev.copy(code = s""" - ${eval2.code} - boolean ${ev.isNull} = false; - $javaType ${ev.value} = ${ctx.defaultValue(javaType)}; - if (${eval2.isNull} || $isZero) { - ${ev.isNull} = true; - } else { - ${eval1.code} - if (${eval1.isNull}) { - ${ev.isNull} = true; - } else { - ${ev.value} = $divide; - } - }""") - } + override def isZero(eval2: ExprCode): String = { + s"${eval2.value} == 0" } } From f85433b6f2cb66f7524c792e8fcc6f6583ce8d50 Mon Sep 17 00:00:00 2001 From: Sandeep Singh Date: Tue, 5 Jul 2016 22:55:26 +0530 Subject: [PATCH 08/20] make a single isZero --- .../sql/catalyst/expressions/arithmetic.scala | 25 +++++++------------ 1 file changed, 9 insertions(+), 16 deletions(-) diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/arithmetic.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/arithmetic.scala index db78f4ec65ef5..d990b5d4d9fba 100644 --- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/arithmetic.scala +++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/arithmetic.scala @@ -231,7 +231,6 @@ abstract class DivisionArithmetic extends BinaryArithmetic with NullIntolerant { // Used by doGenCode def divide(eval1: ExprCode, eval2: ExprCode, javaType: String): String - def isZero(eval2: ExprCode): String /** * Special case handling due to division by 0 => null. @@ -239,7 +238,13 @@ abstract class DivisionArithmetic extends BinaryArithmetic with NullIntolerant { override def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = { val eval1 = left.genCode(ctx) val eval2 = right.genCode(ctx) - val isZeroCheck = isZero(eval2) + val isZero: String = { + if (dataType.isInstanceOf[DecimalType]) { + s"${eval2.value}.isZero()" + } else { + s"${eval2.value} == 0" + } + } val javaType = ctx.javaType(dataType) val division = divide(eval1, eval2, javaType) if (!left.nullable && !right.nullable) { @@ -247,7 +252,7 @@ abstract class DivisionArithmetic extends BinaryArithmetic with NullIntolerant { ${eval2.code} boolean ${ev.isNull} = false; $javaType ${ev.value} = ${ctx.defaultValue(javaType)}; - if ($isZeroCheck) { + if ($isZero) { ${ev.isNull} = true; } else { ${eval1.code} @@ -258,7 +263,7 @@ abstract class DivisionArithmetic extends BinaryArithmetic with NullIntolerant { ${eval2.code} boolean ${ev.isNull} = false; $javaType ${ev.value} = ${ctx.defaultValue(javaType)}; - if (${eval2.isNull} || $isZeroCheck) { + if (${eval2.isNull} || $isZero) { ${ev.isNull} = true; } else { ${eval1.code} @@ -291,14 +296,6 @@ case class Divide(left: Expression, right: Expression) s"($javaType)(${eval1.value} $symbol ${eval2.value})" } } - - override def isZero(eval2: ExprCode): String = { - if (dataType.isInstanceOf[DecimalType]) { - s"${eval2.value}.isZero()" - } else { - s"${eval2.value} == 0" - } - } } @ExpressionDescription( @@ -316,10 +313,6 @@ case class IntegerDivide(left: Expression, right: Expression) override def divide(eval1: ExprCode, eval2: ExprCode, javaType: String): String = { s"($javaType)(${eval1.value} $decimalMethod (${eval2.value}))" } - - override def isZero(eval2: ExprCode): String = { - s"${eval2.value} == 0" - } } @ExpressionDescription( From f35e605a14db17dbf359c3704ac03e1fe755fb5d Mon Sep 17 00:00:00 2001 From: Sandeep Singh Date: Tue, 5 Jul 2016 22:57:40 +0530 Subject: [PATCH 09/20] make divide protected --- .../apache/spark/sql/catalyst/expressions/arithmetic.scala | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/arithmetic.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/arithmetic.scala index d990b5d4d9fba..704e7fb0b6b84 100644 --- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/arithmetic.scala +++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/arithmetic.scala @@ -230,7 +230,7 @@ abstract class DivisionArithmetic extends BinaryArithmetic with NullIntolerant { } // Used by doGenCode - def divide(eval1: ExprCode, eval2: ExprCode, javaType: String): String + protected def divide(eval1: ExprCode, eval2: ExprCode, javaType: String): String /** * Special case handling due to division by 0 => null. @@ -289,7 +289,7 @@ case class Divide(left: Expression, right: Expression) override def decimalMethod: String = "$div" // Used by doGenCode - override def divide(eval1: ExprCode, eval2: ExprCode, javaType: String): String = { + protected override def divide(eval1: ExprCode, eval2: ExprCode, javaType: String): String = { if (dataType.isInstanceOf[DecimalType]) { s"${eval1.value}.$decimalMethod(${eval2.value})" } else { @@ -310,7 +310,7 @@ case class IntegerDivide(left: Expression, right: Expression) override def decimalMethod: String = "/" // Used by doGenCode - override def divide(eval1: ExprCode, eval2: ExprCode, javaType: String): String = { + protected override def divide(eval1: ExprCode, eval2: ExprCode, javaType: String): String = { s"($javaType)(${eval1.value} $decimalMethod (${eval2.value}))" } } From e89ffc06691d0a1cda037aa63e20cdae3df77793 Mon Sep 17 00:00:00 2001 From: Sandeep Singh Date: Tue, 5 Jul 2016 23:00:18 +0530 Subject: [PATCH 10/20] small fix --- .../spark/sql/catalyst/expressions/arithmetic.scala | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/arithmetic.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/arithmetic.scala index 704e7fb0b6b84..076ed1867dd8c 100644 --- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/arithmetic.scala +++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/arithmetic.scala @@ -238,12 +238,10 @@ abstract class DivisionArithmetic extends BinaryArithmetic with NullIntolerant { override def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = { val eval1 = left.genCode(ctx) val eval2 = right.genCode(ctx) - val isZero: String = { - if (dataType.isInstanceOf[DecimalType]) { - s"${eval2.value}.isZero()" - } else { - s"${eval2.value} == 0" - } + val isZero = if (dataType.isInstanceOf[DecimalType]) { + s"${eval2.value}.isZero()" + } else { + s"${eval2.value} == 0" } val javaType = ctx.javaType(dataType) val division = divide(eval1, eval2, javaType) From 2c023704a4573b20946158410897213a5617b24f Mon Sep 17 00:00:00 2001 From: Sandeep Singh Date: Wed, 6 Jul 2016 22:13:56 +0530 Subject: [PATCH 11/20] remove div --- .../main/scala/org/apache/spark/sql/Column.scala | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/sql/core/src/main/scala/org/apache/spark/sql/Column.scala b/sql/core/src/main/scala/org/apache/spark/sql/Column.scala index 2dc09ec3f12c0..a46d1949e94ae 100644 --- a/sql/core/src/main/scala/org/apache/spark/sql/Column.scala +++ b/sql/core/src/main/scala/org/apache/spark/sql/Column.scala @@ -731,21 +731,6 @@ class Column(protected[sql] val expr: Expression) extends Logging { */ def / (other: Any): Column = withExpr { Divide(expr, lit(other).expr) } - /** - * Integer Division this expression by another expression. - * {{{ - * // Scala: The following divides a person's height by their weight. - * people.select( people("height") div people("weight") ) - * - * // Java: - * people.select( people("height").div(people("weight")) ); - * }}} - * - * @group expr_ops - * @since 2.1.0 - */ - def div (other: Any): Column = withExpr { IntegerDivide(expr, lit(other).expr) } - /** * Division this expression by another expression. * {{{ From 38f8bd4588f4eb09822eeb071c60e9f981101b3a Mon Sep 17 00:00:00 2001 From: Sandeep Singh Date: Mon, 11 Jul 2016 18:52:22 +0530 Subject: [PATCH 12/20] add better message --- .../org/apache/spark/sql/catalyst/expressions/arithmetic.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/arithmetic.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/arithmetic.scala index 076ed1867dd8c..ac481b3fbbe83 100644 --- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/arithmetic.scala +++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/arithmetic.scala @@ -276,7 +276,7 @@ abstract class DivisionArithmetic extends BinaryArithmetic with NullIntolerant { } @ExpressionDescription( - usage = "a _FUNC_ b - Divides a by b.", + usage = "a _FUNC_ b - Fraction Division a by b.", extended = "> SELECT 3 _FUNC_ 2;\n 1.5") case class Divide(left: Expression, right: Expression) extends DivisionArithmetic { From a461e35af7be59fa555b4bbe176ac3e9ef4d08ef Mon Sep 17 00:00:00 2001 From: Sandeep Singh Date: Mon, 11 Jul 2016 18:53:42 +0530 Subject: [PATCH 13/20] remove fn --- .../apache/spark/sql/catalyst/analysis/FunctionRegistry.scala | 1 - 1 file changed, 1 deletion(-) diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/FunctionRegistry.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/FunctionRegistry.scala index 4e1ea9d5e6594..f6ebcaeded484 100644 --- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/FunctionRegistry.scala +++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/FunctionRegistry.scala @@ -236,7 +236,6 @@ object FunctionRegistry { expression[Subtract]("-"), expression[Multiply]("*"), expression[Divide]("/"), - expression[IntegerDivide]("div"), expression[Remainder]("%"), // aggregate functions From b2f44897eff5b256b9ba5d16118d4dc068d55b7a Mon Sep 17 00:00:00 2001 From: Sandeep Singh Date: Tue, 12 Jul 2016 21:42:23 +0530 Subject: [PATCH 14/20] use default divide --- .../sql/catalyst/expressions/arithmetic.scala | 28 ++++++------------- 1 file changed, 8 insertions(+), 20 deletions(-) diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/arithmetic.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/arithmetic.scala index ac481b3fbbe83..7ce74d4cc9bdf 100644 --- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/arithmetic.scala +++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/arithmetic.scala @@ -229,9 +229,6 @@ abstract class DivisionArithmetic extends BinaryArithmetic with NullIntolerant { } } - // Used by doGenCode - protected def divide(eval1: ExprCode, eval2: ExprCode, javaType: String): String - /** * Special case handling due to division by 0 => null. */ @@ -244,7 +241,12 @@ abstract class DivisionArithmetic extends BinaryArithmetic with NullIntolerant { s"${eval2.value} == 0" } val javaType = ctx.javaType(dataType) - val division = divide(eval1, eval2, javaType) + val divide = if (dataType.isInstanceOf[DecimalType] || dataType.isInstanceOf[DoubleType]) { + s"${eval1.value}.$decimalMethod(${eval2.value})" + } else { + s"($javaType)(${eval1.value} $decimalMethod ${eval2.value})" + } + if (!left.nullable && !right.nullable) { ev.copy(code = s""" ${eval2.code} @@ -254,7 +256,7 @@ abstract class DivisionArithmetic extends BinaryArithmetic with NullIntolerant { ${ev.isNull} = true; } else { ${eval1.code} - ${ev.value} = $division; + ${ev.value} = $divide; }""") } else { ev.copy(code = s""" @@ -268,7 +270,7 @@ abstract class DivisionArithmetic extends BinaryArithmetic with NullIntolerant { if (${eval1.isNull}) { ${ev.isNull} = true; } else { - ${ev.value} = $division; + ${ev.value} = $divide; } }""") } @@ -285,15 +287,6 @@ case class Divide(left: Expression, right: Expression) override def symbol: String = "/" override def decimalMethod: String = "$div" - - // Used by doGenCode - protected override def divide(eval1: ExprCode, eval2: ExprCode, javaType: String): String = { - if (dataType.isInstanceOf[DecimalType]) { - s"${eval1.value}.$decimalMethod(${eval2.value})" - } else { - s"($javaType)(${eval1.value} $symbol ${eval2.value})" - } - } } @ExpressionDescription( @@ -306,11 +299,6 @@ case class IntegerDivide(left: Expression, right: Expression) override def symbol: String = "div" override def decimalMethod: String = "/" - - // Used by doGenCode - protected override def divide(eval1: ExprCode, eval2: ExprCode, javaType: String): String = { - s"($javaType)(${eval1.value} $decimalMethod (${eval2.value}))" - } } @ExpressionDescription( From feee3cdca56b529d55c60a185517fca2d54ee5d5 Mon Sep 17 00:00:00 2001 From: Sandeep Singh Date: Wed, 13 Jul 2016 11:08:28 +0530 Subject: [PATCH 15/20] fix --- .../spark/sql/catalyst/expressions/arithmetic.scala | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/arithmetic.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/arithmetic.scala index 783408138c1f7..8b6df375f7104 100644 --- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/arithmetic.scala +++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/arithmetic.scala @@ -209,8 +209,9 @@ case class Multiply(left: Expression, right: Expression) abstract class DivisionArithmetic extends BinaryArithmetic with NullIntolerant { override def nullable: Boolean = true + override def decimalMethod: String = "$div" - private lazy val div: (Any, Any) => Any = dataType match { + val div: (Any, Any) => Any = dataType match { case ft: FractionalType => ft.fractional.asInstanceOf[Fractional[Any]].div case i: IntegralType => i.integral.asInstanceOf[Integral[Any]].quot } @@ -241,10 +242,10 @@ abstract class DivisionArithmetic extends BinaryArithmetic with NullIntolerant { s"${eval2.value} == 0" } val javaType = ctx.javaType(dataType) - val divide = if (dataType.isInstanceOf[DecimalType] || dataType.isInstanceOf[DoubleType]) { - s"${eval1.value}.$decimalMethod(${eval2.value})" + val divide = if (dataType.isInstanceOf[DecimalType]) { + s"${eval1.value}." + "$div" + s"(${eval2.value})" } else { - s"($javaType)(${eval1.value} $decimalMethod ${eval2.value})" + s"($javaType)(${eval1.value} / ${eval2.value})" } if (!left.nullable && !right.nullable) { @@ -286,7 +287,6 @@ case class Divide(left: Expression, right: Expression) override def inputType: AbstractDataType = TypeCollection(DoubleType, DecimalType) override def symbol: String = "/" - override def decimalMethod: String = "$div" } @ExpressionDescription( @@ -298,7 +298,6 @@ case class IntegerDivide(left: Expression, right: Expression) override def inputType: AbstractDataType = IntegralType override def symbol: String = "div" - override def decimalMethod: String = "/" } @ExpressionDescription( From ab6858cac3f8f53a3437038b7cd767e73d170eaa Mon Sep 17 00:00:00 2001 From: Sandeep Singh Date: Wed, 13 Jul 2016 11:08:46 +0530 Subject: [PATCH 16/20] private lazy --- .../org/apache/spark/sql/catalyst/expressions/arithmetic.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/arithmetic.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/arithmetic.scala index 8b6df375f7104..b3444de342281 100644 --- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/arithmetic.scala +++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/arithmetic.scala @@ -211,7 +211,7 @@ abstract class DivisionArithmetic extends BinaryArithmetic with NullIntolerant { override def nullable: Boolean = true override def decimalMethod: String = "$div" - val div: (Any, Any) => Any = dataType match { + private lazy val div: (Any, Any) => Any = dataType match { case ft: FractionalType => ft.fractional.asInstanceOf[Fractional[Any]].div case i: IntegralType => i.integral.asInstanceOf[Integral[Any]].quot } From 8d9a04d61a155f5bc131cc7a06a1f9378ceb1cbe Mon Sep 17 00:00:00 2001 From: Sandeep Singh Date: Wed, 13 Jul 2016 11:13:44 +0530 Subject: [PATCH 17/20] remove decimalMethod and do $$div --- .../org/apache/spark/sql/catalyst/expressions/arithmetic.scala | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/arithmetic.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/arithmetic.scala index b3444de342281..106f4a2610f32 100644 --- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/arithmetic.scala +++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/arithmetic.scala @@ -209,7 +209,6 @@ case class Multiply(left: Expression, right: Expression) abstract class DivisionArithmetic extends BinaryArithmetic with NullIntolerant { override def nullable: Boolean = true - override def decimalMethod: String = "$div" private lazy val div: (Any, Any) => Any = dataType match { case ft: FractionalType => ft.fractional.asInstanceOf[Fractional[Any]].div @@ -243,7 +242,7 @@ abstract class DivisionArithmetic extends BinaryArithmetic with NullIntolerant { } val javaType = ctx.javaType(dataType) val divide = if (dataType.isInstanceOf[DecimalType]) { - s"${eval1.value}." + "$div" + s"(${eval2.value})" + s"${eval1.value}.$$div(${eval2.value})" } else { s"($javaType)(${eval1.value} / ${eval2.value})" } From 7bb3e430c9e0595bee5306c0bd0df0d7a9a97cc7 Mon Sep 17 00:00:00 2001 From: Sandeep Singh Date: Wed, 13 Jul 2016 13:31:08 +0530 Subject: [PATCH 18/20] Rename --- .../spark/sql/catalyst/dsl/package.scala | 2 +- .../sql/catalyst/expressions/arithmetic.scala | 8 ++++---- .../spark/sql/catalyst/parser/AstBuilder.scala | 2 +- .../ArithmeticExpressionSuite.scala | 18 ++++++++---------- 4 files changed, 14 insertions(+), 16 deletions(-) diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/dsl/package.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/dsl/package.scala index 92f4296cbb4bf..81c9580d3f28c 100644 --- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/dsl/package.scala +++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/dsl/package.scala @@ -71,7 +71,7 @@ package object dsl { def - (other: Expression): Expression = Subtract(expr, other) def * (other: Expression): Expression = Multiply(expr, other) def / (other: Expression): Expression = Divide(expr, other) - def div (other: Expression): Expression = IntegerDivide(expr, other) + def div (other: Expression): Expression = IntegralDivide(expr, other) def % (other: Expression): Expression = Remainder(expr, other) def & (other: Expression): Expression = BitwiseAnd(expr, other) def | (other: Expression): Expression = BitwiseOr(expr, other) diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/arithmetic.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/arithmetic.scala index 106f4a2610f32..56e475853e876 100644 --- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/arithmetic.scala +++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/arithmetic.scala @@ -207,7 +207,7 @@ case class Multiply(left: Expression, right: Expression) protected override def nullSafeEval(input1: Any, input2: Any): Any = numeric.times(input1, input2) } -abstract class DivisionArithmetic extends BinaryArithmetic with NullIntolerant { +abstract class DivideBase extends BinaryArithmetic with NullIntolerant { override def nullable: Boolean = true private lazy val div: (Any, Any) => Any = dataType match { @@ -281,7 +281,7 @@ abstract class DivisionArithmetic extends BinaryArithmetic with NullIntolerant { usage = "a _FUNC_ b - Fraction Division a by b.", extended = "> SELECT 3 _FUNC_ 2;\n 1.5") case class Divide(left: Expression, right: Expression) - extends DivisionArithmetic { + extends DivideBase { override def inputType: AbstractDataType = TypeCollection(DoubleType, DecimalType) @@ -291,8 +291,8 @@ case class Divide(left: Expression, right: Expression) @ExpressionDescription( usage = "a _FUNC_ b - Divides a by b.", extended = "> SELECT 3 _FUNC_ 2;\n 1") -case class IntegerDivide(left: Expression, right: Expression) - extends DivisionArithmetic { +case class IntegralDivide(left: Expression, right: Expression) + extends DivideBase { override def inputType: AbstractDataType = IntegralType diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala index 03944c5cd0da1..3234327349a70 100644 --- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala +++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala @@ -957,7 +957,7 @@ class AstBuilder extends SqlBaseBaseVisitor[AnyRef] with Logging { case SqlBaseParser.PERCENT => Remainder(left, right) case SqlBaseParser.DIV => - IntegerDivide(left, right) + IntegralDivide(left, right) case SqlBaseParser.PLUS => Add(left, right) case SqlBaseParser.MINUS => diff --git a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/ArithmeticExpressionSuite.scala b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/ArithmeticExpressionSuite.scala index dd9e6f6d96661..8652bc91ad185 100644 --- a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/ArithmeticExpressionSuite.scala +++ b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/ArithmeticExpressionSuite.scala @@ -137,17 +137,15 @@ class ArithmeticExpressionSuite extends SparkFunSuite with ExpressionEvalHelper checkConsistencyBetweenInterpretedAndCodegen(Divide, tpe, tpe) } } - - // By fixing SPARK-15776, Divide's inputType is required to be DoubleType of DecimalType. - // TODO: in future release, we should add a IntegerDivide to support integral types. + test("/ (Divide) for integral type") { - checkEvaluation(IntegerDivide(Literal(1.toByte), Literal(2.toByte)), 0.toByte) - checkEvaluation(IntegerDivide(Literal(1.toShort), Literal(2.toShort)), 0.toShort) - checkEvaluation(IntegerDivide(Literal(1), Literal(2)), 0) - checkEvaluation(IntegerDivide(Literal(1.toLong), Literal(2.toLong)), 0.toLong) - checkEvaluation(IntegerDivide(positiveShortLit, negativeShortLit), 0.toShort) - checkEvaluation(IntegerDivide(positiveIntLit, negativeIntLit), 0) - checkEvaluation(IntegerDivide(positiveLongLit, negativeLongLit), 0L) + checkEvaluation(IntegralDivide(Literal(1.toByte), Literal(2.toByte)), 0.toByte) + checkEvaluation(IntegralDivide(Literal(1.toShort), Literal(2.toShort)), 0.toShort) + checkEvaluation(IntegralDivide(Literal(1), Literal(2)), 0) + checkEvaluation(IntegralDivide(Literal(1.toLong), Literal(2.toLong)), 0.toLong) + checkEvaluation(IntegralDivide(positiveShortLit, negativeShortLit), 0.toShort) + checkEvaluation(IntegralDivide(positiveIntLit, negativeIntLit), 0) + checkEvaluation(IntegralDivide(positiveLongLit, negativeLongLit), 0L) } test("% (Remainder)") { From 1ba502f5fb273eda7b80aa250fee49ceba1719a6 Mon Sep 17 00:00:00 2001 From: Sandeep Singh Date: Wed, 13 Jul 2016 13:31:37 +0530 Subject: [PATCH 19/20] remove whitespace --- .../sql/catalyst/expressions/ArithmeticExpressionSuite.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/ArithmeticExpressionSuite.scala b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/ArithmeticExpressionSuite.scala index 8652bc91ad185..fceefa4540a28 100644 --- a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/ArithmeticExpressionSuite.scala +++ b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/ArithmeticExpressionSuite.scala @@ -137,7 +137,7 @@ class ArithmeticExpressionSuite extends SparkFunSuite with ExpressionEvalHelper checkConsistencyBetweenInterpretedAndCodegen(Divide, tpe, tpe) } } - + test("/ (Divide) for integral type") { checkEvaluation(IntegralDivide(Literal(1.toByte), Literal(2.toByte)), 0.toByte) checkEvaluation(IntegralDivide(Literal(1.toShort), Literal(2.toShort)), 0.toShort) From 16eff2071a1ce2f532000e61f6990eb9d77c173f Mon Sep 17 00:00:00 2001 From: Sandeep Singh Date: Wed, 13 Jul 2016 13:33:04 +0530 Subject: [PATCH 20/20] style fix --- .../apache/spark/sql/catalyst/expressions/arithmetic.scala | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/arithmetic.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/arithmetic.scala index 56e475853e876..ac58da9e90a95 100644 --- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/arithmetic.scala +++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/arithmetic.scala @@ -280,8 +280,7 @@ abstract class DivideBase extends BinaryArithmetic with NullIntolerant { @ExpressionDescription( usage = "a _FUNC_ b - Fraction Division a by b.", extended = "> SELECT 3 _FUNC_ 2;\n 1.5") -case class Divide(left: Expression, right: Expression) - extends DivideBase { +case class Divide(left: Expression, right: Expression) extends DivideBase { override def inputType: AbstractDataType = TypeCollection(DoubleType, DecimalType) @@ -291,8 +290,7 @@ case class Divide(left: Expression, right: Expression) @ExpressionDescription( usage = "a _FUNC_ b - Divides a by b.", extended = "> SELECT 3 _FUNC_ 2;\n 1") -case class IntegralDivide(left: Expression, right: Expression) - extends DivideBase { +case class IntegralDivide(left: Expression, right: Expression) extends DivideBase { override def inputType: AbstractDataType = IntegralType