alamb commented on code in PR #25186:
URL: https://github.com/apache/datafusion/pull/25186#discussion_r4501319899


##########
datafusion/common/src/scalar/mod.rs:
##########
@@ -2762,24 +2762,43 @@ impl ScalarValue {
         )
     }
 
-    /// Applies [`normalize_float`] to a `Float16`, `Float32` or `Float64`
-    /// value, so `-0.0` becomes `0.0`. Other values are returned as is.
+    /// Applies DataFusion's floating-point equality semantics to this value,
+    /// including floats inside nested and encoded values. Negative zero 
becomes
+    /// positive zero; nulls, NaN payloads and other values are preserved.
     ///
-    /// [`normalize_float`]: crate::float_semantics::normalize_float
-    pub fn normalize(self) -> Self {
-        use crate::float_semantics::normalize_float;
-        match self {
-            ScalarValue::Float16(Some(v)) => {
-                ScalarValue::Float16(Some(normalize_float(v)))
-            }
-            ScalarValue::Float32(Some(v)) => {
-                ScalarValue::Float32(Some(normalize_float(v)))
+    /// See [`crate::float_semantics`] for the shared normalization rules.
+    pub fn normalize(mut self) -> Self {
+        use crate::float_semantics::{normalize_float, normalize_floats};
+
+        fn normalize_array<A: Array + From<ArrayData> + 'static>(array: &mut 
Arc<A>) {

Review Comment:
   I found this overly complicated -- I think it is working around the fact you 
can't pass an owned array to normalize_floats. Here is a proposed follow on PR 
to fix it
   - https://github.com/apache/datafusion/pull/27202



##########
datafusion/physical-expr/src/expressions/in_list.rs:
##########
@@ -71,6 +72,13 @@ fn supports_arrow_eq(dt: &DataType) -> bool {
     }
 }
 
+fn normalize_in_list_value(value: ColumnarValue) -> ColumnarValue {

Review Comment:
   this I also think should be a method on ColumnarValue. I will try and 
refactor it



##########
datafusion/physical-expr/src/expressions/in_list/strategy.rs:
##########
@@ -43,6 +53,32 @@ pub(super) fn instantiate_static_filter(
     }
 }
 
+/// SQL membership equates the zero signs, while the primitive filters compare 
bits.
+fn add_missing_signed_zero<T: ArrowPrimitiveType>(in_array: ArrayRef) -> 
Result<ArrayRef>

Review Comment:
   nice



-- 
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]

Reply via email to