ClSlaid commented on code in PR #5707:
URL: https://github.com/apache/arrow-rs/pull/5707#discussion_r1589975826


##########
arrow-array/src/array/byte_view_array.rs:
##########
@@ -645,4 +816,175 @@ mod tests {
 
         StringViewArray::new(views, buffers, None);
     }
+
+    #[test]
+    #[should_panic(expected = "Invalid interval: offset 0 length 13 is out of 
bound of length 12")]
+    fn test_compact_checker() {
+        use super::CompactChecker;
+
+        // single coverage, full
+        let mut checker = CompactChecker::new(10);
+        checker.accumulate(0, 10);
+        assert!(checker.finish());
+
+        // single coverage, partial
+        let mut checker = CompactChecker::new(10);
+        checker.accumulate(0, 5);
+        assert!(!checker.finish());
+
+        // multiple coverage, no overlapping, partial
+        let mut checker = CompactChecker::new(10);
+        checker.accumulate(0, 5);
+        checker.accumulate(5, 4);
+        assert!(!checker.finish());
+
+        //multiple coverage, no overlapping, full
+        let mut checker = CompactChecker::new(10);
+        checker.accumulate(0, 5);
+        checker.accumulate(5, 5);
+        assert!(checker.finish());
+
+        //multiple coverage, overlapping, partial
+        let mut checker = CompactChecker::new(10);
+        checker.accumulate(0, 5);
+        checker.accumulate(4, 5);
+        assert!(!checker.finish());
+
+        //multiple coverage, overlapping, full
+        let mut checker = CompactChecker::new(10);
+        checker.accumulate(0, 5);
+        checker.accumulate(4, 6);
+        assert!(checker.finish());
+
+        //mutiple coverage, no overlapping, full, out of order
+        let mut checker = CompactChecker::new(10);
+        checker.accumulate(4, 6);
+        checker.accumulate(0, 4);
+        assert!(checker.finish());
+
+        // multiple coverage, overlapping, full, out of order
+        let mut checker = CompactChecker::new(10);
+        checker.accumulate(4, 6);
+        checker.accumulate(0, 4);
+        assert!(checker.finish());
+
+        // multiple coverage, overlapping, full, containing null
+        let mut checker = CompactChecker::new(10);
+        checker.accumulate(0, 5);
+        checker.accumulate(5, 0);
+        checker.accumulate(5, 5);
+        assert!(checker.finish());
+
+        // multiple coverage, overlapping, full, containing null
+        let mut checker = CompactChecker::new(10);
+        checker.accumulate(0, 5);
+        checker.accumulate(5, 0);
+        checker.accumulate(4, 6);
+        checker.accumulate(5, 5);
+        assert!(checker.finish());
+
+        // multiple coverage, overlapping, full, containing null
+        //
+        // this case is for attacking those implementation that only check
+        // the lower-bound of the interval
+        let mut checker = CompactChecker::new(10);
+        checker.accumulate(0, 5);
+        checker.accumulate(5, 0);
+        checker.accumulate(1, 9);
+        checker.accumulate(2, 3);
+        checker.accumulate(3, 1);
+        checker.accumulate(9, 1);
+        assert!(checker.finish());
+
+        // panic case, out of bound
+        let mut checker = CompactChecker::new(12);
+        checker.accumulate(0, 13);
+        checker.finish();
+    }
+
+    #[test]
+    fn test_gc() {
+        // 
---------------------------------------------------------------------
+        // test compact on compacted data
+
+        let array = {
+            let mut builder = StringViewBuilder::new();
+            builder.append_value("I look at you all");
+            builder.append_option(Some("see the love there that's sleeping"));
+            builder.finish()
+        };
+        let compacted = array.gc();
+        // verify it is a shallow copy
+        assert_eq!(array.buffers[0].as_ptr(), compacted.buffers[0].as_ptr());
+
+        // 
---------------------------------------------------------------------
+        // test compact on non-compacted data
+
+        let mut array = {
+            let mut builder = StringViewBuilder::new();
+            builder.append_value("while my guitar gently weeps");
+            builder.finish()
+        };
+        // shrink the view
+        let mut view = ByteView::from(array.views[0]);
+        view.length = 15;
+        let new_views = ScalarBuffer::from(vec![view.into()]);
+        array.views = new_views;
+        let compacted = array.gc();
+        // verify it is a deep copy
+        assert_ne!(array.buffers[0].as_ptr(), compacted.buffers[0].as_ptr());
+        // verify content
+        assert_eq!(array.value(0), compacted.value(0));
+        // verify compacted
+        assert!(compacted.compact_check().iter().all(|x| *x));
+
+        // 
---------------------------------------------------------------------
+        // test compact on array containing null
+
+        let mut array = {
+            let mut builder = StringViewBuilder::new();
+            builder.append_null();
+            builder.append_option(Some("I don't know why nobody told you"));
+            builder.finish()
+        };
+
+        let mut view = ByteView::from(array.views[1]);
+        view.length = 15;
+        let new_views = ScalarBuffer::from(vec![array.views[0], view.into()]);

Review Comment:
   > that StringViewArrays will mostly often be compacted (I would actually 
expect the opposite)
   
   We assume the same idea, it's likely to be *not* compacted.



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

Reply via email to