You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2020/12/12 21:24:55 UTC

[GitHub] [arrow] nevi-me commented on a change in pull request #8829: ARROW-10804: Removed undefined behavior from parquet crate, causing tests to fail

nevi-me commented on a change in pull request #8829:
URL: https://github.com/apache/arrow/pull/8829#discussion_r541782467



##########
File path: rust/parquet/src/arrow/record_reader.rs
##########
@@ -58,40 +55,6 @@ struct FatPtr<'a, T> {
     ptr: &'a mut [T],

Review comment:
       We can also remove this

##########
File path: rust/parquet/src/arrow/record_reader.rs
##########
@@ -424,13 +389,16 @@ impl<T: DataType> RecordReader<T> {
     /// Split values into records according repetition definition and returns number of
     /// records read.
     fn split_records(&mut self, records_to_read: usize) -> Result<usize> {
-        let rep_levels_buf = self
+        let rep_levels = self
             .rep_levels
             .as_mut()
-            .map(|buf| FatPtr::<i16>::with_offset(buf, 0));
-        let rep_levels_buf = rep_levels_buf.as_ref().map(|x| x.to_slice());
+            .map(|buf| {
+                let (prefix, rep_levels, suffix) = unsafe { buf.data_mut().align_to_mut::<i16>() };

Review comment:
       It looks like this doesn't need to be mutable. Compiles fine if I change to the below (and changing the `as_mut` to `as_ref`
   ```suggestion
                   let (prefix, rep_levels, suffix) = unsafe { buf.data().align_to::<i16>() };
   ```




----------------------------------------------------------------
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:
users@infra.apache.org