Mark1626 commented on code in PR #18999:
URL: https://github.com/apache/datafusion/pull/18999#discussion_r2621733417
##########
datafusion/functions/src/utils.rs:
##########
@@ -219,6 +221,52 @@ pub fn decimal128_to_i128(value: i128, scale: i8) ->
Result<i128, ArrowError> {
}
}
+pub fn decimal32_to_i32(value: i32, precision: u8, scale: i8) -> Result<i32,
ArrowError> {
+ if scale < 0 {
+ Err(ArrowError::ComputeError(
+ "Negative scale is not supported".into(),
+ ))
+ } else if scale as u8 > precision {
+ Err(ArrowError::ComputeError(format!(
+ "scale {scale} is greater than precision {precision}"
+ )))
+ } else if scale == 0 {
+ Ok(value)
+ } else {
+ validate_decimal32_precision(value, precision, scale)?;
Review Comment:
This was added to validate cases where `precision > 9` and also check the
max value (Decimal(32, 0) max value 999999999).
This conversion in the decimal128 testcase is actually incorrect as it's
greater than the max decimal128 value
https://github.com/Mark1626/datafusion/blob/05eea8b1144487a6a698d3be8815c93b689d15a3/datafusion/functions/src/utils.rs#L411
Do you think it's redundant, in which case I'll remove it? ScalarValue does
a validation but it doesn't validate on the max value
https://github.com/Mark1626/datafusion/blob/05eea8b1144487a6a698d3be8815c93b689d15a3/datafusion/common/src/scalar/mod.rs#L4446
--
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]
