This is an automated email from the ASF dual-hosted git repository.

Jefffrey pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow-rs.git


The following commit(s) were added to refs/heads/main by this push:
     new bab2817d31 Support map/struct capacities in MutableArrayData (#10801)
bab2817d31 is described below

commit bab2817d31706b6164c148434efd49cf98a06f3e
Author: Emily Matheys <[email protected]>
AuthorDate: Mon Aug 24 08:03:44 2026 +0300

    Support map/struct capacities in MutableArrayData (#10801)
    
    The tests seems to completely skip over this and not even use a
    MutableArrayData, causing them to pass, but all 3 added tests would
    panic before adding the Map and Struct cases to the match statement.
    With this change we can now use MutableArrayData on maps properly with
    preallocation(meaning using Capacities::List and Capacities::Struct).
    
    Not actually a breaking change, although it does not panic anymore,
    which is a change I guess.
---
 arrow-data/src/transform/mod.rs | 52 +++++++++++++++++++++++++++++++++--
 arrow/tests/array_transform.rs  | 60 ++++++++++++++++++++++++++++-------------
 2 files changed, 91 insertions(+), 21 deletions(-)

diff --git a/arrow-data/src/transform/mod.rs b/arrow-data/src/transform/mod.rs
index 7558d1e887..989e502e09 100644
--- a/arrow-data/src/transform/mod.rs
+++ b/arrow-data/src/transform/mod.rs
@@ -371,11 +371,14 @@ pub enum Capacities {
     /// * the capacity of the array offsets
     /// * the capacity of the binary/ str buffer
     Binary(usize, Option<usize>),
-    /// List and LargeList data types
+    /// List, LargeList and Map data types
     ///
     /// Defines
     /// * the capacity of the array offsets
     /// * the capacity of the child data
+    ///
+    /// For Map the child data is the entries [`DataType::Struct`], so the 
child
+    /// capacity is a [`Capacities::Struct`] holding the key and value 
capacities.
     List(usize, Option<Box<Capacities>>),
     /// Struct type
     ///
@@ -492,12 +495,17 @@ impl<'a> MutableArrayData<'a> {
                 | DataType::LargeList(_)
                 | DataType::ListView(_)
                 | DataType::LargeListView(_)
-                | DataType::FixedSizeList(_, _),
+                | DataType::FixedSizeList(_, _)
+                | DataType::Map(_, _),
                 Capacities::List(capacity, _),
             ) => {
                 array_capacity = *capacity;
                 new_buffers(data_type, *capacity)
             }
+            (DataType::Struct(_), Capacities::Struct(capacity, _)) => {
+                array_capacity = *capacity;
+                new_buffers(data_type, *capacity)
+            }
             _ => panic!("Capacities: {capacities:?} not yet supported"),
         };
 
@@ -1029,4 +1037,44 @@ mod test {
         assert_eq!(mutable.data.buffer1.capacity(), 64);
         assert_eq!(mutable.data.child_data[0].data.buffer1.capacity(), 192);
     }
+
+    #[test]
+    fn test_map_append_with_capacities() {
+        let entries = Arc::new(Field::new(
+            "entries",
+            DataType::Struct(
+                vec![
+                    Field::new("keys", DataType::Int64, false),
+                    Field::new("values", DataType::Int64, true),
+                ]
+                .into(),
+            ),
+            false,
+        ));
+        let array = ArrayData::new_empty(&DataType::Map(entries, false));
+
+        let mutable = MutableArrayData::with_capacities(
+            vec![&array],
+            false,
+            Capacities::List(
+                6,
+                Some(Box::new(Capacities::Struct(
+                    17,
+                    Some(vec![Capacities::Array(17), Capacities::Array(17)]),
+                ))),
+            ),
+        );
+
+        // capacities are rounded up to multiples of 64 by MutableBuffer
+        // the map offsets buffer holds `1 + 6` i32s
+        assert_eq!(mutable.data.buffer1.capacity(), 64);
+
+        // the entries struct itself has no buffers of its own
+        let entries = &mutable.data.child_data[0];
+        assert_eq!(entries.data.buffer1.capacity(), 0);
+
+        // both key and value buffers hold 17 i64s
+        assert_eq!(entries.data.child_data[0].data.buffer1.capacity(), 192);
+        assert_eq!(entries.data.child_data[1].data.buffer1.capacity(), 192);
+    }
 }
diff --git a/arrow/tests/array_transform.rs b/arrow/tests/array_transform.rs
index 1f82ca4927..ff005b8630 100644
--- a/arrow/tests/array_transform.rs
+++ b/arrow/tests/array_transform.rs
@@ -25,7 +25,7 @@ use arrow::datatypes::{Int16Type, IntervalMonthDayNanoType};
 use arrow_array::StringViewArray;
 use arrow_buffer::{Buffer, ScalarBuffer};
 use arrow_data::ArrayData;
-use arrow_data::transform::MutableArrayData;
+use arrow_data::transform::{Capacities, MutableArrayData};
 use arrow_schema::{DataType, Field, Fields, UnionFields};
 use std::sync::Arc;
 
@@ -449,14 +449,6 @@ fn test_struct_many() {
     let array = StructArray::try_from(vec![("f1", strings.clone()), ("f2", 
ints.clone())])
         .unwrap()
         .into_data();
-    let arrays = vec![&array, &array];
-    let mut mutable = MutableArrayData::new(arrays, false, 0);
-
-    mutable.try_extend(0, 1, 3).unwrap();
-    mutable.try_extend(1, 0, 2).unwrap();
-    let data = mutable.freeze();
-    let array = StructArray::from(data);
-
     let expected_string =
         Arc::new(StringArray::from(vec![None, None, Some("joe"), None])) as 
ArrayRef;
     let expected_int =
@@ -464,7 +456,24 @@ fn test_struct_many() {
 
     let expected =
         StructArray::try_from(vec![("f1", expected_string), ("f2", 
expected_int)]).unwrap();
-    assert_eq!(array, expected)
+
+    // exact capacities must produce the same result as letting the buffers 
grow
+    for capacities in [
+        Capacities::Array(0),
+        Capacities::Struct(
+            4,
+            // 4 slots per field, and "joe" is the only string kept
+            Some(vec![Capacities::Binary(4, Some(3)), Capacities::Array(4)]),
+        ),
+    ] {
+        let mut mutable =
+            MutableArrayData::with_capacities(vec![&array, &array], false, 
capacities);
+
+        mutable.try_extend(0, 1, 3).unwrap();
+        mutable.try_extend(1, 0, 2).unwrap();
+
+        assert_eq!(StructArray::from(mutable.freeze()), expected);
+    }
 }
 
 #[test]
@@ -732,14 +741,6 @@ fn test_map_nulls_append() {
     let c = b.slice(1, 2);
     let d = b.slice(2, 2);
 
-    let mut mutable = MutableArrayData::new(vec![&a, &b, &c, &d], false, 10);
-
-    mutable.try_extend(0, 0, a.len()).unwrap();
-    mutable.try_extend(1, 0, b.len()).unwrap();
-    mutable.try_extend(2, 0, c.len()).unwrap();
-    mutable.try_extend(3, 0, d.len()).unwrap();
-    let result = mutable.freeze();
-
     let expected_key_array = Int64Array::from(vec![
         Some(1),
         Some(2),
@@ -836,7 +837,28 @@ fn test_map_nulls_append() {
         vec![expected_entry_array.into_data()],
     )
     .unwrap();
-    assert_eq!(result, expected_list_data);
+
+    // exact capacities must produce the same result as letting the buffers 
grow
+    for capacities in [
+        Capacities::Array(10),
+        Capacities::List(
+            12,
+            Some(Box::new(Capacities::Struct(
+                23,
+                Some(vec![Capacities::Array(23), Capacities::Array(23)]),
+            ))),
+        ),
+    ] {
+        let mut mutable =
+            MutableArrayData::with_capacities(vec![&a, &b, &c, &d], false, 
capacities);
+
+        mutable.try_extend(0, 0, a.len()).unwrap();
+        mutable.try_extend(1, 0, b.len()).unwrap();
+        mutable.try_extend(2, 0, c.len()).unwrap();
+        mutable.try_extend(3, 0, d.len()).unwrap();
+
+        assert_eq!(mutable.freeze(), expected_list_data);
+    }
 }
 
 #[test]

Reply via email to