revans2 commented on code in PR #38739:
URL: https://github.com/apache/spark/pull/38739#discussion_r1029400640
##########
sql/core/src/test/scala/org/apache/spark/sql/DataFrameSuite.scala:
##########
@@ -3532,6 +3532,49 @@ class DataFrameSuite extends QueryTest
}.isEmpty)
}
}
+
+ test("SPARK-41207: Fix BinaryArithmetic with negative scale") {
+ withSQLConf(SQLConf.LEGACY_ALLOW_NEGATIVE_SCALE_OF_DECIMAL_ENABLED.key ->
"true") {
Review Comment:
To be clear we don't have it on in production. We just noticed it when
trying to match the functionality changes after #36698 in the RAPIDs
Accelerator for Apache Spark and filed SPARK-41207. Most of the changes were
small and logical, like when the output precision would be larger than 38 Spark
will now round after doing an add instead of adjusting scale and rounding LHS
and RHS before doing the add. This is one place that is arguably a regression
and we wanted to be sure it was documented.
Putting on my Spark contributor hat now, negative scale support is off by
default as a legacy feature, but not deprecated. I am of the opinion that we
should not support something half way. If add only worked for positive numbers
we would fix it right away. But from what I have seen negative scale decimal is
not widely used and "Officially supporting negative scale is really a hard
problem". As such I would like to see us deprecate support for it, and not fix
bugs that come up.
If on the other hand if there are enough contributors that want or need
support for negative scale decimal, then they should come up with a plan to get
it to a good place.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
