[GitHub] [spark] MaxGekk commented on a diff in pull request #41156: [SPARK-40129][SQL] Fix Decimal multiply can produce the wrong answer

["MaxGekk (via GitHub)" <gi...@apache.org>]

MaxGekk commented on code in PR #41156:
URL: https://github.com/apache/spark/pull/41156#discussion_r1252244221


##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/arithmetic.scala:
##########
@@ -516,7 +530,8 @@ case class Subtract(
 
   override def symbol: String = "-"
 
-  override def decimalMethod: String = "$minus"
+  override def decimalMethod(mathContextValue: GlobalValue, value1: String, value2: String):
+  String = s"$value1.$$minus($value2)"

Review Comment:
   ditto: indentation



##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/arithmetic.scala:
##########
@@ -430,7 +443,8 @@ case class Add(
 
   override def symbol: String = "+"
 
-  override def decimalMethod: String = "$plus"
+  override def decimalMethod(mathContextValue: GlobalValue, value1: String, value2: String):
+  String = s"$value1.$$plus($value2)"

Review Comment:
   Please, fix indentation.



##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/arithmetic.scala:
##########
@@ -279,7 +282,7 @@ abstract class BinaryArithmetic extends BinaryOperator
   }
 
   /** Name of the function for this expression on a [[Decimal]] type. */
-  def decimalMethod: String =
+  def decimalMethod(mathContextValue: GlobalValue, value1: String, value2: String): String =

Review Comment:
   Where are the parameters `mathContextValue`, `value1`, `value2` used for?



##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/arithmetic.scala:
##########
@@ -592,7 +607,10 @@ case class Multiply(
   override def inputType: AbstractDataType = NumericType
 
   override def symbol: String = "*"
-  override def decimalMethod: String = "$times"
+
+  override def decimalMethod(mathContextValue: GlobalValue, value1: String, value2: String):
+  String = s"Decimal.apply($value1.toJavaBigDecimal()" +
+    s".multiply($value2.toJavaBigDecimal(), $mathContextValue))"

Review Comment:
   Adjust indentation according to https://github.com/databricks/scala-style-guide#indent



-- 
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-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org