[GitHub] [arrow-rs] alamb commented on a diff in pull request #1683: Use bytes in parquet rather than custom Buffer implementation (#1474)

[GitBox <gi...@apache.org>]

alamb commented on code in PR #1683:
URL: https://github.com/apache/arrow-rs/pull/1683#discussion_r869635621


##########
parquet/src/column/reader.rs:
##########
@@ -460,20 +460,20 @@ fn parse_v1_level(
     num_buffered_values: u32,
     encoding: Encoding,
     buf: ByteBufferPtr,
-) -> Result<ByteBufferPtr> {
+) -> Result<(usize, ByteBufferPtr)> {

Review Comment:
   I recommend documenting in comments what this `usize` is -- namely the number of bytes that was read?



##########
parquet/src/util/memory.rs:
##########
@@ -17,532 +17,119 @@
 
 //! Utility methods and structs for working with memory.
 
+use bytes::Bytes;
 use std::{
     fmt::{Debug, Display, Formatter, Result as FmtResult},
-    io::{Result as IoResult, Write},
-    mem,
-    ops::{Index, IndexMut},
-    sync::{
-        atomic::{AtomicI64, Ordering},
-        Arc, Weak,
-    },
+    ops::Index,
 };
 
-// ----------------------------------------------------------------------

Review Comment:
   I think highlighting the loss of `MemTracker` in this change is probably important for anyone who is using it currently



-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org