Repository: jorgecarleitao/arrow2 (rust)

Issue:
Regression parquet read Utf8 column.
See issue downstream: https://github.com/pola-rs/polars/issues/5795

I did a bisect and it was introduced in #1316. Decoding we hit is `OptionalDictionary`.

Pull request:
fix invalid parquet read
fixes #1329 

The length of the `offsets` container return `n - 1`. This is unintuitive and lead to a bug where the `extend_constant` added a single value too little. I made the distinction between `len` (the real number of indexes) and `len_proxy` the number of values an array with this many offsets would have.

Patch:
diff --git a/src/array/binary/mutable_values.rs b/src/array/binary/mutable_values.rs
index e52516877d..5617945611 100644
--- a/src/array/binary/mutable_values.rs
+++ b/src/array/binary/mutable_values.rs
@@ -128,7 +128,7 @@ impl<O: Offset> MutableBinaryValuesArray<O> {
     /// Returns the length of this array
     #[inline]
     pub fn len(&self) -> usize {
-        self.offsets.len()
+        self.offsets.len_proxy()
     }
 
     /// Pushes a new item to the array.
diff --git a/src/array/list/mutable.rs b/src/array/list/mutable.rs
index 98280dc89f..881cb620a0 100644
--- a/src/array/list/mutable.rs
+++ b/src/array/list/mutable.rs
@@ -191,7 +191,7 @@ impl<O: Offset, M: MutableArray> MutableListArray<O, M> {
     /// Returns the length of this array
     #[inline]
     pub fn len(&self) -> usize {
-        self.offsets.len()
+        self.offsets.len_proxy()
     }
 
     /// The values
@@ -210,7 +210,7 @@ impl<O: Offset, M: MutableArray> MutableListArray<O, M> {
     }
 
     fn init_validity(&mut self) {
-        let len = self.offsets.len();
+        let len = self.offsets.len_proxy();
 
         let mut validity = MutableBitmap::with_capacity(self.offsets.capacity());
         validity.extend_constant(len, true);
diff --git a/src/array/physical_binary.rs b/src/array/physical_binary.rs
index adbf62d6c2..161371603e 100644
--- a/src/array/physical_binary.rs
+++ b/src/array/physical_binary.rs
@@ -142,14 +142,14 @@ where
 
     offsets.reserve(size_hint);
 
-    let start_index = offsets.len();
+    let start_index = offsets.len_proxy();
 
     for item in iterator {
         let bytes = item.as_ref();
         values.extend_from_slice(bytes);
         offsets.try_push_usize(bytes.len()).unwrap();
     }
-    offsets.len() - start_index
+    offsets.len_proxy() - start_index
 }
 
 // Populates `offsets`, `values`, and `validity` [`Vec`]s with
diff --git a/src/array/utf8/mutable_values.rs b/src/array/utf8/mutable_values.rs
index 47c6b9e3ee..edb5dcfd89 100644
--- a/src/array/utf8/mutable_values.rs
+++ b/src/array/utf8/mutable_values.rs
@@ -169,7 +169,7 @@ impl<O: Offset> MutableUtf8ValuesArray<O> {
     /// Returns the length of this array
     #[inline]
     pub fn len(&self) -> usize {
-        self.offsets.len()
+        self.offsets.len_proxy()
     }
 
     /// Pushes a new item to the array.
diff --git a/src/io/avro/read/nested.rs b/src/io/avro/read/nested.rs
index 7886bd0b81..056d9a8f83 100644
--- a/src/io/avro/read/nested.rs
+++ b/src/io/avro/read/nested.rs
@@ -53,7 +53,7 @@ impl<O: Offset> DynMutableListArray<O> {
     }
 
     fn init_validity(&mut self) {
-        let len = self.offsets.len();
+        let len = self.offsets.len_proxy();
 
         let mut validity = MutableBitmap::new();
         validity.extend_constant(len, true);
@@ -64,7 +64,7 @@ impl<O: Offset> DynMutableListArray<O> {
 
 impl<O: Offset> MutableArray for DynMutableListArray<O> {
     fn len(&self) -> usize {
-        self.offsets.len()
+        self.offsets.len_proxy()
     }
 
     fn validity(&self) -> Option<&MutableBitmap> {
diff --git a/src/io/parquet/read/deserialize/binary/utils.rs b/src/io/parquet/read/deserialize/binary/utils.rs
index 9c28c8785a..ec514766fa 100644
--- a/src/io/parquet/read/deserialize/binary/utils.rs
+++ b/src/io/parquet/read/deserialize/binary/utils.rs
@@ -15,7 +15,7 @@ impl<O: Offset> Pushable<usize> for Offsets<O> {
     }
     #[inline]
     fn len(&self) -> usize {
-        self.len()
+        self.len_proxy()
     }
 
     #[inline]
@@ -45,7 +45,7 @@ impl<O: Offset> Binary<O> {
 
     #[inline]
     pub fn push(&mut self, v: &[u8]) {
-        if self.offsets.len() == 100 && self.offsets.capacity() > 100 {
+        if self.offsets.len_proxy() == 100 && self.offsets.capacity() > 100 {
             let bytes_per_row = self.values.len() / 100 + 1;
             let bytes_estimate = bytes_per_row * self.offsets.capacity();
             if bytes_estimate > self.values.capacity() {
@@ -64,7 +64,7 @@ impl<O: Offset> Binary<O> {
 
     #[inline]
     pub fn len(&self) -> usize {
-        self.offsets.len()
+        self.offsets.len_proxy()
     }
 
     #[inline]
diff --git a/src/offset.rs b/src/offset.rs
index 2337f08221..113358ca54 100644
--- a/src/offset.rs
+++ b/src/offset.rs
@@ -150,7 +150,7 @@ impl<O: Offset> Offsets<O> {
     #[inline]
     pub fn start_end(&self, index: usize) -> (usize, usize) {
         // soundness: the invariant of the function
-        assert!(index < self.len());
+        assert!(index < self.len_proxy());
         unsafe { self.start_end_unchecked(index) }
     }
 
@@ -165,12 +165,18 @@ impl<O: Offset> Offsets<O> {
         (start, end)
     }
 
-    /// Returns the length of this container
+    /// Returns the length an array with these offsets would be.
     #[inline]
-    pub fn len(&self) -> usize {
+    pub fn len_proxy(&self) -> usize {
         self.0.len() - 1
     }
 
+    #[inline]
+    /// Returns the number of offsets in this container.
+    pub fn len(&self) -> usize {
+        self.0.len()
+    }
+
     /// Returns the byte slice stored in this buffer
     #[inline]
     pub fn as_slice(&self) -> &[O] {
@@ -180,7 +186,7 @@ impl<O: Offset> Offsets<O> {
     /// Pops the last element
     #[inline]
     pub fn pop(&mut self) -> Option<O> {
-        if self.len() == 0 {
+        if self.len_proxy() == 0 {
             None
         } else {
             self.0.pop()


Test patch:
diff --git a/tests/it/array/list/mutable.rs b/tests/it/array/list/mutable.rs
index 4d7483fe31..a5732315ea 100644
--- a/tests/it/array/list/mutable.rs
+++ b/tests/it/array/list/mutable.rs
@@ -32,7 +32,7 @@ fn basics() {
 fn with_capacity() {
     let array = MutableListArray::<i32, MutablePrimitiveArray<i32>>::with_capacity(10);
     assert!(array.offsets().capacity() >= 10);
-    assert_eq!(array.offsets().len(), 0);
+    assert_eq!(array.offsets().len_proxy(), 0);
     assert_eq!(array.values().values().capacity(), 0);
     assert_eq!(array.validity(), None);
 }