adriangb commented on code in PR #11235:
URL: https://github.com/apache/arrow-rs/pull/11235#discussion_r4221502739


##########
parquet/src/util/push_buffers.rs:
##########
@@ -180,15 +185,156 @@ impl PushBuffers {
                 new_buffers.push(buffer.clone());
             }
         }
+        self.buffered_bytes = new_ranges.iter().map(|r| r.end - r.start).sum();
         self.ranges = new_ranges;
         self.buffers = new_buffers;
     }
 
+    /// Remove all buffered bytes in `ranges`, whatever the shape of the
+    /// pushed buffers.
+    ///
+    /// A buffer that overlaps a range is trimmed or split, and the parts

Review Comment:
   Addressed in https://github.com/apache/arrow-rs/pull/11235/commits/207057d5b6



##########
parquet/src/util/push_buffers.rs:
##########
@@ -180,15 +185,156 @@ impl PushBuffers {
                 new_buffers.push(buffer.clone());
             }
         }
+        self.buffered_bytes = new_ranges.iter().map(|r| r.end - r.start).sum();
         self.ranges = new_ranges;
         self.buffers = new_buffers;
     }
 
+    /// Remove all buffered bytes in `ranges`, whatever the shape of the
+    /// pushed buffers.
+    ///
+    /// A buffer that overlaps a range is trimmed or split, and the parts
+    /// outside the range are kept. The kept parts are zero-copy slices. Thus,
+    /// the allocator frees the memory of a pushed [`Bytes`] only after all of
+    /// its parts are removed.
+    ///
+    /// If the buffers are sorted by start, they stay sorted. The order of

Review Comment:
   Addressed in https://github.com/apache/arrow-rs/pull/11235/commits/207057d5b6



##########
parquet/src/util/push_buffers.rs:
##########
@@ -253,6 +399,126 @@ impl ChunkReader for PushBuffers {
 mod tests {
     use super::*;
 
+    /// Release one range.
+    #[cfg(feature = "arrow")]
+    fn release(buffers: &mut PushBuffers, range: Range<u64>) {
+        buffers.release_ranges(std::slice::from_ref(&range));
+    }
+
+    #[test]
+    #[cfg(feature = "arrow")]
+    fn release_ranges_without_overlap_keeps_buffers() {
+        let mut buffers = PushBuffers::new(100);
+        buffers
+            .push_range(20..24, Bytes::from_static(b"abcd"))
+            .unwrap();
+        buffers
+            .push_range(0..10, Bytes::from_static(b"0123456789"))
+            .unwrap();
+
+        release(&mut buffers, 90..95);
+        release(&mut buffers, 10..20);
+        assert_eq!(buffers.ranges, vec![20..24, 0..10]);
+        assert_eq!(buffers.buffered_bytes(), 14);
+
+        // A release that overlaps a buffer sorts the buffers.
+        release(&mut buffers, 22..24);

Review Comment:
   Addressed in https://github.com/apache/arrow-rs/pull/11235/commits/207057d5b6



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