Skip to content

[SPARK-25457][SQL] IntegralDivide returns data type of the operands #22465

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
mgaido91 wants to merge 2 commits into from
Closed

[SPARK-25457][SQL] IntegralDivide returns data type of the operands #22465

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
19ff368
[SPARK-25457][SQL] IntegralDivide returns data type of the operands
mgaido91 Sep 19, 2018
3f045c0
address comment
mgaido91 Sep 19, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 13 additions & 4 deletions sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/arithmetic.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import org.apache.spark.sql.catalyst.analysis.{TypeCheckResult, TypeCoercion}
import org.apache.spark.sql.catalyst.expressions.codegen._
import org.apache.spark.sql.catalyst.expressions.codegen.Block._
import org.apache.spark.sql.catalyst.util.TypeUtils
import org.apache.spark.sql.internal.SQLConf
import org.apache.spark.sql.types._
import org.apache.spark.unsafe.types.CalendarInterval

Expand Down Expand Up @@ -327,16 +328,24 @@ case class Divide(left: Expression, right: Expression) extends DivModLike {
case class IntegralDivide(left: Expression, right: Expression) extends DivModLike {

override def inputType: AbstractDataType = IntegralType
override def dataType: DataType = LongType
override def dataType: DataType = if (SQLConf.get.integralDivideReturnLong) {
LongType
} else {
left.dataType
}

override def symbol: String = "/"
override def sqlOperator: String = "div"

private lazy val div: (Any, Any) => Long = left.dataType match {
private lazy val div: (Any, Any) => Any = left.dataType match {
case i: IntegralType =>
val divide = i.integral.asInstanceOf[Integral[Any]].quot _
val toLong = i.integral.asInstanceOf[Integral[Any]].toLong _
(x, y) => toLong(divide(x, y))
if (SQLConf.get.integralDivideReturnLong) {
val toLong = i.integral.asInstanceOf[Integral[Any]].toLong _
(x, y) => toLong(divide(x, y))
} else {
divide
}
}

override def evalOperation(left: Any, right: Any): Any = div(left, right)
Expand Down
9 changes: 9 additions & 0 deletions sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1561,6 +1561,13 @@ object SQLConf {
"are performed before any UNION, EXCEPT and MINUS operations.")
.booleanConf
.createWithDefault(false)

val LEGACY_INTEGRALDIVIDE_RETURN_LONG = buildConf("spark.sql.legacy.integralDivide.returnBigint")
Copy link
Member

@viirya viirya Sep 19, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

returnBigint or returnLong? I think we use "long" instead of "big int" as type name?

Copy link
Contributor

@cloud-fan cloud-fan Sep 20, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

big int is more commonly used in the SQL world

.doc("If it is set to true, the div operator returns always a bigint. This behavior was " +
"inherited from Hive. Otherwise, the return type is the data type of the operands.")
.internal()
.booleanConf
.createWithDefault(false)
}

/**
Expand Down Expand Up @@ -1973,6 +1980,8 @@ class SQLConf extends Serializable with Logging {

def setOpsPrecedenceEnforced: Boolean = getConf(SQLConf.LEGACY_SETOPS_PRECEDENCE_ENABLED)

def integralDivideReturnLong: Boolean = getConf(SQLConf.LEGACY_INTEGRALDIVIDE_RETURN_LONG)

/** ********************** SQLConf functionality methods ************ */

/** Set Spark SQL configuration properties. */
Expand Down
26 changes: 19 additions & 7 deletions .../src/test/scala/org/apache/spark/sql/catalyst/expressions/ArithmeticExpressionSuite.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import org.apache.spark.sql.catalyst.InternalRow
import org.apache.spark.sql.catalyst.analysis.TypeCheckResult.TypeCheckFailure
import org.apache.spark.sql.catalyst.dsl.expressions._
import org.apache.spark.sql.catalyst.expressions.codegen.CodegenContext
import org.apache.spark.sql.internal.SQLConf
import org.apache.spark.sql.types._

class ArithmeticExpressionSuite extends SparkFunSuite with ExpressionEvalHelper {
Expand Down Expand Up @@ -144,13 +145,24 @@ class ArithmeticExpressionSuite extends SparkFunSuite with ExpressionEvalHelper
}

test("/ (Divide) for integral type") {
checkEvaluation(IntegralDivide(Literal(1.toByte), Literal(2.toByte)), 0L)
checkEvaluation(IntegralDivide(Literal(1.toShort), Literal(2.toShort)), 0L)
checkEvaluation(IntegralDivide(Literal(1), Literal(2)), 0L)
checkEvaluation(IntegralDivide(Literal(1.toLong), Literal(2.toLong)), 0L)
checkEvaluation(IntegralDivide(positiveShortLit, negativeShortLit), 0L)
checkEvaluation(IntegralDivide(positiveIntLit, negativeIntLit), 0L)
checkEvaluation(IntegralDivide(positiveLongLit, negativeLongLit), 0L)
withSQLConf(SQLConf.LEGACY_INTEGRALDIVIDE_RETURN_LONG.key -> "false") {
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)
}
withSQLConf(SQLConf.LEGACY_INTEGRALDIVIDE_RETURN_LONG.key -> "true") {
checkEvaluation(IntegralDivide(Literal(1.toByte), Literal(2.toByte)), 0L)
checkEvaluation(IntegralDivide(Literal(1.toShort), Literal(2.toShort)), 0L)
checkEvaluation(IntegralDivide(Literal(1), Literal(2)), 0L)
checkEvaluation(IntegralDivide(Literal(1.toLong), Literal(2.toLong)), 0L)
checkEvaluation(IntegralDivide(positiveShortLit, negativeShortLit), 0L)
checkEvaluation(IntegralDivide(positiveIntLit, negativeIntLit), 0L)
checkEvaluation(IntegralDivide(positiveLongLit, negativeLongLit), 0L)
}
}

test("% (Remainder)") {
Expand Down
14 changes: 14 additions & 0 deletions sql/core/src/test/resources/sql-tests/inputs/operator-div.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
set spark.sql.legacy.integralDivide.returnBigint=true;

select 5 div 2;
select 5 div 0;
select 5 div null;
select null div 5;

set spark.sql.legacy.integralDivide.returnBigint=false;

select 5 div 2;
select 5 div 0;
select 5 div null;
select null div 5;

6 changes: 1 addition & 5 deletions sql/core/src/test/resources/sql-tests/inputs/operators.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,15 +16,11 @@ select + + 100;
select - - max(key) from testdata;
select + - key from testdata where key = 33;

-- div
-- division
select 5 / 2;
select 5 / 0;
select 5 / null;
select null / 5;
select 5 div 2;
select 5 div 0;
select 5 div null;
select null div 5;

-- other arithmetics
select 1 + 2;
Expand Down
82 changes: 82 additions & 0 deletions sql/core/src/test/resources/sql-tests/results/operator-div.sql.out
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
-- Automatically generated by SQLQueryTestSuite
-- Number of queries: 10


-- !query 0
set spark.sql.legacy.integralDivide.returnBigint=true
-- !query 0 schema
struct<key:string,value:string>
-- !query 0 output
spark.sql.legacy.integralDivide.returnBigint true


-- !query 1
select 5 div 2
-- !query 1 schema
struct<(5 div 2):bigint>
-- !query 1 output
2


-- !query 2
select 5 div 0
-- !query 2 schema
struct<(5 div 0):bigint>
-- !query 2 output
NULL


-- !query 3
select 5 div null
-- !query 3 schema
struct<(5 div CAST(NULL AS INT)):bigint>
-- !query 3 output
NULL


-- !query 4
select null div 5
-- !query 4 schema
struct<(CAST(NULL AS INT) div 5):bigint>
-- !query 4 output
NULL


-- !query 5
set spark.sql.legacy.integralDivide.returnBigint=false
-- !query 5 schema
struct<key:string,value:string>
-- !query 5 output
spark.sql.legacy.integralDivide.returnBigint false


-- !query 6
select 5 div 2
-- !query 6 schema
struct<(5 div 2):int>
-- !query 6 output
2


-- !query 7
select 5 div 0
-- !query 7 schema
struct<(5 div 0):int>
-- !query 7 output
NULL


-- !query 8
select 5 div null
-- !query 8 schema
struct<(5 div CAST(NULL AS INT)):int>
-- !query 8 output
NULL


-- !query 9
select null div 5
-- !query 9 schema
struct<(CAST(NULL AS INT) div 5):int>
-- !query 9 output
NULL
Loading