yinli-systems commented on code in PR #25300:
URL: https://github.com/apache/datafusion/pull/25300#discussion_r4039647809


##########
datafusion/functions-nested/src/set_ops.rs:
##########
@@ -354,26 +366,18 @@ fn generic_set_lists<OffsetSize: OffsetSizeTrait>(
     // Normalize -0.0 → +0.0 so RowConverter (which uses IEEE 754 totalOrder
     // and treats ±0 as distinct) groups them together. Use the normalized
     // arrays for both row conversion and the final output values.
-    let l_values_norm = normalize_float_zero(l.values());
-    let r_values_norm = normalize_float_zero(r.values());
-
-    // Only convert the visible portion of the values array. For sliced
-    // ListArrays, values() returns the full underlying array but only
-    // elements between the first and last offset are referenced.
     let l_first = l.offsets()[0].as_usize();
     let l_len = l.offsets()[l.len()].as_usize() - l_first;
-    let l_values = l_values_norm.slice(l_first, l_len);
-    let rows_l = converter.convert_columns(&[Arc::clone(&l_values)])?;
+    let l_values_norm = normalize_visible_values(l);
+    let rows_l = converter.convert_columns(&[Arc::clone(&l_values_norm)])?;
 
-    let r_first = r.offsets()[0].as_usize();
-    let r_len = r.offsets()[r.len()].as_usize() - r_first;
-    let r_values = r_values_norm.slice(r_first, r_len);
-    let rows_r = converter.convert_columns(&[Arc::clone(&r_values)])?;
+    let r_values_norm = normalize_visible_values(r);
+    let rows_r = converter.convert_columns(&[Arc::clone(&r_values_norm)])?;
 
     // Indices from the row converter are 0-based in the per-side slice;
     // concatenating those same slices lets indices map directly into the
     // combined values array.
-    let combined_values = concat(&[l_values.as_ref(), r_values.as_ref()])?;
+    let combined_values = concat(&[l_values_norm.as_ref(), 
r_values_norm.as_ref()])?;
     let r_offset = l_len;

Review Comment:
   Applied as suggested in 800a042cd — the offset now comes directly from 
`l_values_norm.len()`, and the redundant offset calculations are gone. All 128 
nested-function tests and full all-target/all-feature Clippy pass.



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