Skip to content

Commit

Permalink
[SPARK-49864][SQL] Improve message of BINARY_ARITHMETIC_OVERFLOW
Browse files Browse the repository at this point in the history
### What changes were proposed in this pull request?
BINARY_ARITHMETIC_OVERFLOW did not have a suggestion on bypassing the error. This PR improves on that.

### Why are the changes needed?
All errors should suggest a way to overcome an issue, so that customers can fix problems easier.

### Does this PR introduce _any_ user-facing change?
Yes, change in error message.

### How was this patch tested?
Tests added for all paths for bytes.

### Was this patch authored or co-authored using generative AI tooling?
No.

Closes apache#48335 from mihailom-db/binary_arithmetic_overflow.

Authored-by: Mihailo Milosevic <mihailo.milosevic@databricks.com>
Signed-off-by: Max Gekk <max.gekk@gmail.com>
  • Loading branch information
mihailom-db authored and MaxGekk committed Oct 4, 2024
1 parent 4cf9d14 commit fcda935
Show file tree
Hide file tree
Showing 5 changed files with 58 additions and 15 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@
},
"BINARY_ARITHMETIC_OVERFLOW" : {
"message" : [
"<value1> <symbol> <value2> caused overflow."
"<value1> <symbol> <value2> caused overflow. Use <functionName> to ignore overflow problem and return NULL."
],
"sqlState" : "22003"
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -294,12 +294,18 @@ abstract class BinaryArithmetic extends BinaryOperator
case ByteType | ShortType =>
nullSafeCodeGen(ctx, ev, (eval1, eval2) => {
val tmpResult = ctx.freshName("tmpResult")
val try_suggestion = symbol match {
case "+" => "try_add"
case "-" => "try_subtract"
case "*" => "try_multiply"
case _ => ""
}
val overflowCheck = if (failOnError) {
val javaType = CodeGenerator.boxedType(dataType)
s"""
|if ($tmpResult < $javaType.MIN_VALUE || $tmpResult > $javaType.MAX_VALUE) {
| throw QueryExecutionErrors.binaryArithmeticCauseOverflowError(
| $eval1, "$symbol", $eval2);
| $eval1, "$symbol", $eval2, "$try_suggestion");
|}
""".stripMargin
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -610,13 +610,17 @@ private[sql] object QueryExecutionErrors extends QueryErrorsBase with ExecutionE
}

def binaryArithmeticCauseOverflowError(
eval1: Short, symbol: String, eval2: Short): SparkArithmeticException = {
eval1: Short,
symbol: String,
eval2: Short,
suggestedFunc: String): SparkArithmeticException = {
new SparkArithmeticException(
errorClass = "BINARY_ARITHMETIC_OVERFLOW",
messageParameters = Map(
"value1" -> toSQLValue(eval1, ShortType),
"symbol" -> symbol,
"value2" -> toSQLValue(eval2, ShortType)),
"value2" -> toSQLValue(eval2, ShortType),
"functionName" -> toSQLId(suggestedFunc)),
context = Array.empty,
summary = "")
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,27 +24,27 @@ import org.apache.spark.sql.errors.QueryExecutionErrors
import org.apache.spark.sql.types.Decimal.DecimalIsConflicted

private[sql] object ByteExactNumeric extends ByteIsIntegral with Ordering.ByteOrdering {
private def checkOverflow(res: Int, x: Byte, y: Byte, op: String): Unit = {
private def checkOverflow(res: Int, x: Byte, y: Byte, op: String, hint: String): Unit = {
if (res > Byte.MaxValue || res < Byte.MinValue) {
throw QueryExecutionErrors.binaryArithmeticCauseOverflowError(x, op, y)
throw QueryExecutionErrors.binaryArithmeticCauseOverflowError(x, op, y, hint)
}
}

override def plus(x: Byte, y: Byte): Byte = {
val tmp = x + y
checkOverflow(tmp, x, y, "+")
checkOverflow(tmp, x, y, "+", "try_add")
tmp.toByte
}

override def minus(x: Byte, y: Byte): Byte = {
val tmp = x - y
checkOverflow(tmp, x, y, "-")
checkOverflow(tmp, x, y, "-", "try_subtract")
tmp.toByte
}

override def times(x: Byte, y: Byte): Byte = {
val tmp = x * y
checkOverflow(tmp, x, y, "*")
checkOverflow(tmp, x, y, "*", "try_multiply")
tmp.toByte
}

Expand All @@ -55,27 +55,27 @@ private[sql] object ByteExactNumeric extends ByteIsIntegral with Ordering.ByteOr


private[sql] object ShortExactNumeric extends ShortIsIntegral with Ordering.ShortOrdering {
private def checkOverflow(res: Int, x: Short, y: Short, op: String): Unit = {
private def checkOverflow(res: Int, x: Short, y: Short, op: String, hint: String): Unit = {
if (res > Short.MaxValue || res < Short.MinValue) {
throw QueryExecutionErrors.binaryArithmeticCauseOverflowError(x, op, y)
throw QueryExecutionErrors.binaryArithmeticCauseOverflowError(x, op, y, hint)
}
}

override def plus(x: Short, y: Short): Short = {
val tmp = x + y
checkOverflow(tmp, x, y, "+")
checkOverflow(tmp, x, y, "+", "try_add")
tmp.toShort
}

override def minus(x: Short, y: Short): Short = {
val tmp = x - y
checkOverflow(tmp, x, y, "-")
checkOverflow(tmp, x, y, "-", "try_subtract")
tmp.toShort
}

override def times(x: Short, y: Short): Short = {
val tmp = x * y
checkOverflow(tmp, x, y, "*")
checkOverflow(tmp, x, y, "*", "try_multiply")
tmp.toShort
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -767,7 +767,40 @@ class QueryExecutionErrorsSuite
parameters = Map(
"value1" -> "127S",
"symbol" -> "+",
"value2" -> "5S"),
"value2" -> "5S",
"functionName" -> "`try_add`"),
sqlState = "22003")
}
}

test("BINARY_ARITHMETIC_OVERFLOW: byte minus byte result overflow") {
withSQLConf(SQLConf.ANSI_ENABLED.key -> "true") {
checkError(
exception = intercept[SparkArithmeticException] {
sql(s"select -2Y - 127Y").collect()
},
condition = "BINARY_ARITHMETIC_OVERFLOW",
parameters = Map(
"value1" -> "-2S",
"symbol" -> "-",
"value2" -> "127S",
"functionName" -> "`try_subtract`"),
sqlState = "22003")
}
}

test("BINARY_ARITHMETIC_OVERFLOW: byte multiply byte result overflow") {
withSQLConf(SQLConf.ANSI_ENABLED.key -> "true") {
checkError(
exception = intercept[SparkArithmeticException] {
sql(s"select 127Y * 5Y").collect()
},
condition = "BINARY_ARITHMETIC_OVERFLOW",
parameters = Map(
"value1" -> "127S",
"symbol" -> "*",
"value2" -> "5S",
"functionName" -> "`try_multiply`"),
sqlState = "22003")
}
}
Expand Down

0 comments on commit fcda935

Please sign in to comment.