alamb commented on a change in pull request #8739:
URL: https://github.com/apache/arrow/pull/8739#discussion_r532032623



##########
File path: rust/arrow/src/array/equal/structure.rs
##########
@@ -30,19 +32,51 @@ fn equal_values(
         .iter()
         .zip(rhs.child_data())
         .all(|(lhs_values, rhs_values)| {
-            equal_range(lhs_values, rhs_values, lhs_start, rhs_start, len)
+            // merge the null data
+            let lhs_merged_nulls = match (lhs_nulls, lhs_values.null_buffer()) 
{
+                (None, None) => None,
+                (None, Some(c)) => Some(c.clone()),
+                (Some(p), None) => Some(p.clone()),
+                (Some(p), Some(c)) => {
+                    let merged = (p & c).unwrap();
+                    Some(merged)
+                }
+            };
+            let rhs_merged_nulls = match (rhs_nulls, rhs_values.null_buffer()) 
{
+                (None, None) => None,
+                (None, Some(c)) => Some(c.clone()),
+                (Some(p), None) => Some(p.clone()),
+                (Some(p), Some(c)) => {
+                    let merged = (p & c).unwrap();
+                    Some(merged)
+                }
+            };
+            equal_range(
+                lhs_values,
+                rhs_values,
+                lhs_merged_nulls.as_ref(),
+                rhs_merged_nulls.as_ref(),
+                lhs_start,
+                rhs_start,
+                len,
+            )
         })
 }
 
 pub(super) fn struct_equal(
     lhs: &ArrayData,
     rhs: &ArrayData,
+    lhs_nulls: Option<&Buffer>,
+    rhs_nulls: Option<&Buffer>,
     lhs_start: usize,
     rhs_start: usize,
     len: usize,
 ) -> bool {
-    if lhs.null_count() == 0 && rhs.null_count() == 0 {
-        equal_values(lhs, rhs, lhs_start, rhs_start, len)
+    // we have to recalculate null counts from the null bitmaps
+    let lhs_null_count = count_nulls(lhs_nulls, lhs_start, len);
+    let rhs_null_count = count_nulls(rhs_nulls, rhs_start, len);
+    if lhs_null_count == 0 && rhs_null_count == 0 {
+        equal_values(lhs, rhs, lhs_nulls, rhs_nulls, lhs_start, rhs_start, len)
     } else {
         // with nulls, we need to compare item by item whenever it is not null
         (0..len).all(|i| {

Review comment:
       @nevi-me  I think I was confused -- I was trying to offer some way to 
avoid @jorgecarleitao 's concern of double merge concerns but now that I 
re-read my comments I am not sure that they make any sense. Sorry for the 
confusion




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

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to