theirix commented on code in PR #24934:
URL: https://github.com/apache/datafusion/pull/24934#discussion_r3940518911
##########
datafusion/sqllogictest/src/engines/conversion.rs:
##########
@@ -40,158 +41,83 @@ pub(crate) fn varchar_to_str(value: &str) -> String {
}
}
-pub(crate) fn f16_to_str(value: f16) -> String {
+pub(crate) fn float_to_str<T: Float + ToString>(value: T, round_digits: i64)
-> String {
if value.is_nan() {
// The sign of NaN can be different depending on platform.
// So the string representation of NaN ignores the sign.
"NaN".to_string()
- } else if value == f16::INFINITY {
+ } else if value == T::infinity() {
"Infinity".to_string()
- } else if value == f16::NEG_INFINITY {
+ } else if value == T::neg_infinity() {
"-Infinity".to_string()
} else {
- big_decimal_to_str(BigDecimal::from_str(&value.to_string()).unwrap(),
None)
+ float_decimal_to_str(value, round_digits)
}
}
+pub(crate) fn f16_to_str(value: f16) -> String {
+ float_to_str(value, 12)
+}
+
pub(crate) fn f32_to_str(value: f32) -> String {
- if value.is_nan() {
- // The sign of NaN can be different depending on platform.
- // So the string representation of NaN ignores the sign.
- "NaN".to_string()
- } else if value == f32::INFINITY {
- "Infinity".to_string()
- } else if value == f32::NEG_INFINITY {
- "-Infinity".to_string()
- } else {
- big_decimal_to_str(BigDecimal::from_str(&value.to_string()).unwrap(),
None)
- }
+ float_to_str(value, 12)
Review Comment:
Thank you! I added constants and some explanations to the code and README.
12 was a rational choice when introducing Postgres tests to handle rounding
the smallest 16-bit type in
https://github.com/apache/datafusion/pull/4834#discussion_r1080585778
> 12 is chosen to pass the existing set of tests. I think it could produce
errors, for example, when rounding f16 to 12 digits. I would probably use 3 (or
4) decimal digits if high precision is not required for Postgres compatibility
tests.
And 15 relates to expm1 Spark behaviour in the mentioned ticket 15168
--
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]
