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


##########
datafusion/common/src/float_semantics.rs:
##########
@@ -153,6 +153,20 @@ pub fn normalize_floats(array: &ArrayRef) -> ArrayRef {
     }
 }
 
+/// Like [`normalize_floats`], but takes ownership of the array.
+///
+/// This avoids any copying (including arc ref counts) if there is nothing, and
+/// will reuse the underlying allocation if possible
+pub fn normalize_floats_owned(array: ArrayRef) -> ArrayRef {

Review Comment:
   Given how much of @geoffreyclaude 's obsession has gone into performance, I 
wanted to minimize any impact of this fix, so this is a version that avoids a 
clone and reuses the allocation when needed
   
   I could have just changed `normalize_floats` to take an owned array but that 
makes for a larger diff. I am happy to do so if reviewers prefer



##########
datafusion/physical-expr/src/expressions/in_list.rs:
##########
@@ -373,7 +374,8 @@ impl PhysicalExpr for InListExpr {
 
                 // Helper: compare value against a single list expression
                 let compare_one = |expr: &Arc<dyn PhysicalExpr>| -> 
Result<BooleanArray> {
-                    match expr.evaluate(batch)? {
+                    let list_value = expr.evaluate(batch)?.normalize(); // 0.0 
== -0.0

Review Comment:
   here is the other call 



##########
datafusion/physical-expr/src/expressions/in_list.rs:
##########
@@ -324,6 +324,7 @@ impl PhysicalExpr for InListExpr {
     fn evaluate(&self, batch: &RecordBatch) -> Result<ColumnarValue> {
         let num_rows = batch.num_rows();
         let value = self.expr.evaluate(batch)?;
+        let value = value.normalize(); // so 0.0 == -0.0

Review Comment:
   the fix is basically to sprinkle calls to `normalize` around



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