snmvaughan commented on code in PR #7522:
URL: https://github.com/apache/datafusion-comet/pull/7522#discussion_r4485978338
##########
native/spark-expr/src/conversion_funcs/string.rs:
##########
@@ -729,8 +761,15 @@ fn parse_string_to_decimal(
// fit in an int (e.g. "1e-2147483647" to a negative target scale), so do
not round to zero.
// Spark raises a raw `ArithmeticException` there in every eval mode.
Spark 4.0.5, 4.1.4 and
// 4.2.1 skip `setScale` when the value is far below the target scale's
ulp (SPARK-60119) and
- // return 0 for that example; see
https://github.com/apache/datafusion-comet/issues/6966.
+ // return 0 for that example; see
https://github.com/apache/datafusion-comet/issues/6966 and
+ // https://github.com/apache/datafusion-comet/issues/7187. A difference
below the int range
+ // puts a value of at most 38 digits more than 2^31 places below the
target scale, so it
+ // rounds to 0 whatever its sign. A difference above the int range is a
value too large for
+ // any precision, which stays an error.
if i32::try_from(scale_adjustment).is_err() {
Review Comment:
Agreed, and thanks for checking it with a Java program. It predates this PR
and it is the other half of #6966, which already notes that the threshold comes
from the JDK's `BigInteger` limits (zero up to about 1e8, an exception from
about 6e8), so the native cast can't match it exactly, for positive target
scales as well. Two details from your check aren't there yet, the 12-second
case and the `i32::MIN` boundary (`'1e-2147483647'` to scale -1), so I'll add
them to #6966 and leave this PR as is.
--
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]
