tustvold commented on code in PR #4400:
URL: https://github.com/apache/arrow-datafusion/pull/4400#discussion_r1035734414
##########
datafusion/expr/src/type_coercion/binary.rs:
##########
@@ -287,8 +287,8 @@ fn get_wider_decimal_type(
(DataType::Decimal128(p1, s1), DataType::Decimal128(p2, s2)) => {
// max(s1, s2) + max(p1-s1, p2-s2), max(s1, s2)
let s = *s1.max(s2);
- let range = (p1 - s1).max(p2 - s2);
- Some(create_decimal_type(range + s, s))
+ let range = (*p1 as i8 - s1).max(*p2 as i8 - s2);
Review Comment:
Spark definitely supports negative scales, it might not expose them in its
SQL frontend though. I think this PR is consistent with that, the SQL frontend
still doesn't support negative scales, but DataFusion the execution engine does
##########
datafusion/expr/src/type_coercion/binary.rs:
##########
@@ -287,8 +287,8 @@ fn get_wider_decimal_type(
(DataType::Decimal128(p1, s1), DataType::Decimal128(p2, s2)) => {
// max(s1, s2) + max(p1-s1, p2-s2), max(s1, s2)
let s = *s1.max(s2);
- let range = (p1 - s1).max(p2 - s2);
- Some(create_decimal_type(range + s, s))
+ let range = (*p1 as i8 - s1).max(*p2 as i8 - s2);
Review Comment:
Spark definitely supports negative scales, it might not expose them in its
SQL frontend though. I think this PR is consistent with that, the SQL frontend
still doesn't support negative scales, but DataFusion the query engine does
--
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]
