ErikBPF commented on code in PR #6945:
URL: https://github.com/apache/datafusion-comet/pull/6945#discussion_r4266150792


##########
native/core/src/execution/delta_dv.rs:
##########
@@ -1120,12 +1120,30 @@ mod tests {
 
     #[test]
     fn native_format_decodes() {
-        let values = [0u64, 2, 3, 100, (1u64 << 32) + 7];
+        let values = [0u64, 2, 3, 100, (2u64 << 32) + 7];

Review Comment:
   Added the assertion as suggested: `native_format_decodes` now checks 
`decoded.bitmaps()` keys are `[0, 2]` and names the empty partition in a 
comment, so the skip is asserted where it is guarded. Pushed as `263ad46c8`.



##########
native/core/src/execution/delta_dv.rs:
##########
@@ -1120,12 +1120,30 @@ mod tests {
 
     #[test]
     fn native_format_decodes() {
-        let values = [0u64, 2, 3, 100, (1u64 << 32) + 7];
+        let values = [0u64, 2, 3, 100, (2u64 << 32) + 7];
         let decoded = deserialize_dv_bitmap(&native_bytes(&values)).unwrap();
         let expected: RoaringTreemap = values.into_iter().collect();
         assert_eq!(decoded, expected);
     }
 
+    #[test]
+    fn native_decode_preserves_dense_bitmap_compression() {
+        let mut bitmap = RoaringBitmap::new();
+        bitmap.insert_range(0..1_000_000);
+        let compressed_size = bitmap.serialized_size();
+        let mut bitmap_bytes = Vec::new();
+        bitmap.serialize_into(&mut bitmap_bytes).unwrap();
+
+        let mut data = NATIVE_MAGIC.to_le_bytes().to_vec();
+        data.extend(1i32.to_le_bytes());
+        data.extend((bitmap_bytes.len() as i32).to_le_bytes());
+        data.extend(bitmap_bytes);
+
+        let decoded = deserialize_dv_bitmap(&data).unwrap();
+        let (_, decoded_bitmap) = decoded.bitmaps().next().unwrap();
+        assert_eq!(decoded_bitmap.serialized_size(), compressed_size);

Review Comment:
   Extended the test with the plan-builder push: 
`full_file_access_plan(&[600_000, 600_000], &decoded)` asserts group 0 is 
`Skip`, group 1 is `skip 400_000` then `select 200_000`, and `decoded.len() == 
1_000_000`. That is the first case in the file that drives a run container 
through `build_access_plan`. Pushed as `263ad46c8`.



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