cloud-fan commented on code in PR #44942:
URL: https://github.com/apache/spark/pull/44942#discussion_r1542184876
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/arithmetic.scala:
##########
@@ -60,23 +60,15 @@ case class UnaryMinus(
override def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode =
dataType match {
case _: DecimalType => defineCodeGen(ctx, ev, c => s"$c.unary_$$minus()")
- case ByteType | ShortType if failOnError =>
+ case ByteType | ShortType | IntegerType | LongType if failOnError =>
+ val typeUtils = TypeUtils.getClass.getCanonicalName.stripSuffix("$")
+ val refDataType = ctx.addReferenceObj("refDataType", dataType,
dataType.getClass.getName)
nullSafeCodeGen(ctx, ev, eval => {
val javaBoxedType = CodeGenerator.boxedType(dataType)
- val javaType = CodeGenerator.javaType(dataType)
- val originValue = ctx.freshName("origin")
s"""
- |$javaType $originValue = ($javaType)($eval);
- |if ($originValue == $javaBoxedType.MIN_VALUE) {
- | throw
QueryExecutionErrors.unaryMinusCauseOverflowError($originValue);
- |}
- |${ev.value} = ($javaType)(-($originValue));
- """.stripMargin
- })
- case IntegerType | LongType if failOnError =>
- val mathUtils = MathUtils.getClass.getCanonicalName.stripSuffix("$")
- nullSafeCodeGen(ctx, ev, eval => {
- s"${ev.value} = $mathUtils.negateExact($eval);"
Review Comment:
The previous code is more efficient as it leverages the static data type
information, while the new code simply passes the data type as a parameter,
which means we will do data type case match per row. It will cause significant
perf regression.
I suggest that we add more overloads of `MathUtils.negateExact` to take byte
and short, so that the generated code can still call it directly.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
