From edd2910d9e98473a038a5ac75af16789ba808cb5 Mon Sep 17 00:00:00 2001
From: Andrew Lamb <andrew@nerdnetworks.org>
Date: Sun, 24 Oct 2021 08:47:13 -0400
Subject: [PATCH] comments

---
 arrow/src/array/array_binary.rs |  4 +++-
 arrow/src/array/data.rs         | 22 +++++++++++-----------
 2 files changed, 14 insertions(+), 12 deletions(-)

diff --git a/arrow/src/array/array_binary.rs b/arrow/src/array/array_binary.rs
index 51743d8668a3..45d28776dbb2 100644
--- a/arrow/src/array/array_binary.rs
+++ b/arrow/src/array/array_binary.rs
@@ -1212,13 +1212,15 @@ mod tests {
 
     #[test]
     #[should_panic(
-        expected = "FixedSizeBinaryArray can only be created from FixedSizeList<u8> arrays"
+        expected = "FixedSizeBinaryArray can only be created from list array of u8 values \
+                    (i.e. FixedSizeList<PrimitiveArray<u8>>)."
     )]
     fn test_fixed_size_binary_array_from_incorrect_list_array() {
         let values: [u32; 12] = [0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11];
         let values_data = ArrayData::builder(DataType::UInt32)
             .len(12)
             .add_buffer(Buffer::from_slice_ref(&values))
+            .add_child_data(ArrayData::builder(DataType::Boolean).build().unwrap())
             .build()
             .unwrap();
 
diff --git a/arrow/src/array/data.rs b/arrow/src/array/data.rs
index 6b6e483c53ff..180983cb4270 100644
--- a/arrow/src/array/data.rs
+++ b/arrow/src/array/data.rs
@@ -190,6 +190,11 @@ pub(crate) fn new_buffers(data_type: &DataType, capacity: usize) -> [MutableBuff
     }
 }
 
+/// Ensures that at least `min_size` elements of type `data_type` can
+/// be stored in a buffer of `buffer_size`.
+///
+/// `buffer_index` is used in error messages to identify which buffer
+/// had the invalid index
 fn ensure_size(
     data_type: &DataType,
     min_size: usize,
@@ -566,13 +571,13 @@ impl ArrayData {
         }
     }
 
-    /// Validates that buffers in this ArrayData are sufficiently
-    /// sized, to store `len` + `offset` total elements of
-    /// `data_type`.
+    /// "cheap" validation of an `ArrayData`. Ensures buffers are
+    /// sufficiently sized to store `len` + `offset` total elements of
+    /// `data_type` and performs other inexpensive consistency checks.
     ///
     /// This check is "cheap" in the sense that it does not validate the
-    /// contents of the buffers (e.g. that string offsets for UTF8 arrays
-    /// are within the length of the buffer).
+    /// contents of the buffers (e.g. that all offsets for UTF8 arrays
+    /// are within the bounds of the values buffer).
     ///
     /// TODO: add a validate_full that validates the offsets
     pub fn validate(&self) -> Result<()> {
@@ -587,12 +592,7 @@ impl ArrayData {
         //
         // Tracking tickets:
         //
-        // https://github.com/apache/arrow-rs/issues/814 where the number
-        // of buffers is not known apriori
-        //
-        // TODO File another ticket to track adding validation to
-        // UnionArray and null handling, after / as a part of updating
-        // arrow-rs as described in
+        // https://github.com/apache/arrow-rs/issues/814
         // https://github.com/apache/arrow-rs/issues/85
         if matches!(&self.data_type, DataType::Union(..)) {
             return Ok(());