Skip to content

Commit

Permalink
Cleanup more uses of Array::data (#3880) (#4002)
Browse files Browse the repository at this point in the history 
* Cleanup more uses of Array::data (#3880)

* Fix failing test

* Fix parquet map array

* Further cleanup

* Further cleanup

* More cleanup

* Fix test

* Clippy
  • Loading branch information
@tustvold
tustvold authored Apr 3, 2023
1 parent 5a63a63 commit d7bba0a
Show file tree
Hide file tree
Showing 42 changed files with 352 additions and 415 deletions.
4 changes: 2 additions & 2 deletions arrow-array/src/array/binary_array.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -683,7 +683,7 @@ mod tests {
let data = vec![None];
let array = BinaryArray::from(data);
array
.data()
.into_data()
.validate_full()
.expect("All null array has valid array data");
}
Expand All @@ -693,7 +693,7 @@ mod tests {
let data = vec![None];
let array = LargeBinaryArray::from(data);
array
.data()
.into_data()
.validate_full()
.expect("All null array has valid array data");
}
Expand Down
16 changes: 8 additions & 8 deletions arrow-array/src/array/dictionary_array.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -252,11 +252,11 @@ impl<K: ArrowDictionaryKeyType> DictionaryArray<K> {

// Note: This use the ArrayDataBuilder::build_unchecked and afterwards
// call the new function which only validates that the keys are in bounds.
let data = keys.data().clone();
let data = keys.to_data();
let builder = data
.into_builder()
.data_type(dict_data_type)
.add_child_data(values.data().clone());
.add_child_data(values.to_data());

// Safety: `validate` ensures key type is correct, and
// `validate_values` ensures all offsets are within range
Expand Down Expand Up @@ -397,7 +397,7 @@ impl<K: ArrowDictionaryKeyType> DictionaryArray<K> {
Box::new(K::DATA_TYPE),
Box::new(values.data_type().clone()),
))
.child_data(vec![values.data().clone()]);
.child_data(vec![values.to_data()]);

// SAFETY:
// Offsets were valid before and verified length is greater than or equal
Expand Down Expand Up @@ -789,7 +789,7 @@ mod tests {
let dict_array = Int16DictionaryArray::from(dict_data);

let values = dict_array.values();
assert_eq!(&value_data, values.data());
assert_eq!(value_data, values.to_data());
assert_eq!(DataType::Int8, dict_array.value_type());
assert_eq!(3, dict_array.len());

Expand All @@ -809,7 +809,7 @@ mod tests {
let dict_array = Int16DictionaryArray::from(dict_data);

let values = dict_array.values();
assert_eq!(&value_data, values.data());
assert_eq!(value_data, values.to_data());
assert_eq!(DataType::Int8, dict_array.value_type());
assert_eq!(2, dict_array.len());
assert_eq!(dict_array.keys(), &Int16Array::from(vec![3_i16, 4]));
Expand Down Expand Up @@ -911,7 +911,7 @@ mod tests {
let test = vec![None, None, None];
let array: DictionaryArray<Int32Type> = test.into_iter().collect();
array
.data()
.into_data()
.validate_full()
.expect("All null array has valid array data");
}
Expand Down Expand Up @@ -987,7 +987,7 @@ mod tests {
assert_eq!(array.keys().data_type(), &DataType::Int32);
assert_eq!(array.values().data_type(), &DataType::Utf8);

assert_eq!(array.data().null_count(), 1);
assert_eq!(array.null_count(), 1);

assert!(array.keys().is_valid(0));
assert!(array.keys().is_valid(1));
Expand Down Expand Up @@ -1076,7 +1076,7 @@ mod tests {
let boxed: ArrayRef = Arc::new(dict_array);

let col: DictionaryArray<Int8Type> =
DictionaryArray::<Int8Type>::from(boxed.data().clone());
DictionaryArray::<Int8Type>::from(boxed.to_data());
let err = col.into_primitive_dict_builder::<Int32Type>();

let returned = err.unwrap_err();
Expand Down
2 changes: 1 addition & 1 deletion arrow-array/src/array/fixed_size_binary_array.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -792,7 +792,7 @@ mod tests {
FixedSizeBinaryArray::try_from_sparse_iter_with_size(data.into_iter(), 0)
.unwrap();
array
.data()
.into_data()
.validate_full()
.expect("All null array has valid array data");
}
Expand Down
18 changes: 6 additions & 12 deletions arrow-array/src/array/fixed_size_list_array.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -256,6 +256,8 @@ impl std::fmt::Debug for FixedSizeListArray {
#[cfg(test)]
mod tests {
use super::*;
use crate::cast::AsArray;
use crate::types::Int32Type;
use crate::Int32Array;
use arrow_buffer::{bit_util, Buffer};
use arrow_schema::Field;
Expand All @@ -281,7 +283,7 @@ mod tests {
.unwrap();
let list_array = FixedSizeListArray::from(list_data);

assert_eq!(&value_data, list_array.values().data());
assert_eq!(value_data, list_array.values().to_data());
assert_eq!(DataType::Int32, list_array.value_type());
assert_eq!(3, list_array.len());
assert_eq!(0, list_array.null_count());
Expand Down Expand Up @@ -310,19 +312,11 @@ mod tests {
.unwrap();
let list_array = FixedSizeListArray::from(list_data);

assert_eq!(&value_data, list_array.values().data());
assert_eq!(value_data, list_array.values().to_data());
assert_eq!(DataType::Int32, list_array.value_type());
assert_eq!(3, list_array.len());
assert_eq!(0, list_array.null_count());
assert_eq!(
3,
list_array
.value(0)
.as_any()
.downcast_ref::<Int32Array>()
.unwrap()
.value(0)
);
assert_eq!(3, list_array.value(0).as_primitive::<Int32Type>().value(0));
assert_eq!(6, list_array.value_offset(1));
assert_eq!(3, list_array.value_length());
}
Expand Down Expand Up @@ -386,7 +380,7 @@ mod tests {
.unwrap();
let list_array = FixedSizeListArray::from(list_data);

assert_eq!(&value_data, list_array.values().data());
assert_eq!(value_data, list_array.values().to_data());
assert_eq!(DataType::Int32, list_array.value_type());
assert_eq!(5, list_array.len());
assert_eq!(2, list_array.null_count());
Expand Down
12 changes: 6 additions & 6 deletions arrow-array/src/array/list_array.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -442,7 +442,7 @@ mod tests {
let list_array = ListArray::from(list_data);

let values = list_array.values();
assert_eq!(&value_data, values.data());
assert_eq!(value_data, values.to_data());
assert_eq!(DataType::Int32, list_array.value_type());
assert_eq!(3, list_array.len());
assert_eq!(0, list_array.null_count());
Expand Down Expand Up @@ -482,7 +482,7 @@ mod tests {
let list_array = ListArray::from(list_data);

let values = list_array.values();
assert_eq!(&value_data, values.data());
assert_eq!(value_data, values.to_data());
assert_eq!(DataType::Int32, list_array.value_type());
assert_eq!(2, list_array.len());
assert_eq!(0, list_array.null_count());
Expand Down Expand Up @@ -532,7 +532,7 @@ mod tests {
let list_array = LargeListArray::from(list_data);

let values = list_array.values();
assert_eq!(&value_data, values.data());
assert_eq!(value_data, values.to_data());
assert_eq!(DataType::Int32, list_array.value_type());
assert_eq!(3, list_array.len());
assert_eq!(0, list_array.null_count());
Expand Down Expand Up @@ -572,7 +572,7 @@ mod tests {
let list_array = LargeListArray::from(list_data);

let values = list_array.values();
assert_eq!(&value_data, values.data());
assert_eq!(value_data, values.to_data());
assert_eq!(DataType::Int32, list_array.value_type());
assert_eq!(2, list_array.len());
assert_eq!(0, list_array.null_count());
Expand Down Expand Up @@ -630,7 +630,7 @@ mod tests {
let list_array = ListArray::from(list_data);

let values = list_array.values();
assert_eq!(&value_data, values.data());
assert_eq!(value_data, values.to_data());
assert_eq!(DataType::Int32, list_array.value_type());
assert_eq!(9, list_array.len());
assert_eq!(4, list_array.null_count());
Expand Down Expand Up @@ -694,7 +694,7 @@ mod tests {
let list_array = LargeListArray::from(list_data);

let values = list_array.values();
assert_eq!(&value_data, values.data());
assert_eq!(value_data, values.to_data());
assert_eq!(DataType::Int32, list_array.value_type());
assert_eq!(9, list_array.len());
assert_eq!(4, list_array.null_count());
Expand Down
22 changes: 10 additions & 12 deletions arrow-array/src/array/map_array.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,11 @@ impl MapArray {
&self.values
}

/// Returns a reference to the [`StructArray`] entries of this map
pub fn entries(&self) -> &ArrayRef {
&self.entries
}

/// Returns the data type of the map's keys.
pub fn key_type(&self) -> &DataType {
self.keys.data_type()
Expand Down Expand Up @@ -189,7 +194,7 @@ impl MapArray {

let entry_struct = StructArray::from(vec![
(keys_field, Arc::new(keys_data) as ArrayRef),
(values_field, make_array(values.data().clone())),
(values_field, make_array(values.to_data())),
]);

let map_data_type = DataType::Map(
Expand Down Expand Up @@ -369,7 +374,7 @@ mod tests {
.unwrap();
let map_array = MapArray::from(map_data);

assert_eq!(&value_data, map_array.values().data());
assert_eq!(value_data, map_array.values().to_data());
assert_eq!(&DataType::UInt32, map_array.value_type());
assert_eq!(3, map_array.len());
assert_eq!(0, map_array.null_count());
Expand Down Expand Up @@ -400,16 +405,9 @@ mod tests {
}

// Now test with a non-zero offset
let map_data = ArrayData::builder(map_array.data_type().clone())
.len(2)
.offset(1)
.add_buffer(map_array.data().buffers()[0].clone())
.add_child_data(map_array.data().child_data()[0].clone())
.build()
.unwrap();
let map_array = MapArray::from(map_data);
let map_array = map_array.slice(1, 2);

assert_eq!(&value_data, map_array.values().data());
assert_eq!(value_data, map_array.values().to_data());
assert_eq!(&DataType::UInt32, map_array.value_type());
assert_eq!(2, map_array.len());
assert_eq!(0, map_array.null_count());
Expand Down Expand Up @@ -446,7 +444,7 @@ mod tests {
let sliced_array = map_array.slice(1, 2);
assert_eq!(2, sliced_array.len());
assert_eq!(1, sliced_array.offset());
let sliced_array_data = sliced_array.data();
let sliced_array_data = sliced_array.to_data();
for array_data in sliced_array_data.child_data() {
assert_eq!(array_data.offset(), 1);
}
Expand Down
4 changes: 2 additions & 2 deletions arrow-array/src/array/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -290,7 +290,7 @@ impl Array for ArrayRef {
}

fn into_data(self) -> ArrayData {
self.data().clone()
self.to_data()
}

#[allow(deprecated)]
Expand Down Expand Up @@ -357,7 +357,7 @@ impl<'a, T: Array> Array for &'a T {
}

fn into_data(self) -> ArrayData {
self.data().clone()
self.to_data()
}

#[allow(deprecated)]
Expand Down
23 changes: 10 additions & 13 deletions arrow-array/src/array/primitive_array.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -466,9 +466,7 @@ impl<T: ArrowPrimitiveType> PrimitiveArray<T> {
O: ArrowPrimitiveType,
F: Fn(T::Native) -> O::Native,
{
let data = self.data();

let nulls = data.nulls().cloned();
let nulls = self.nulls().cloned();
let values = self.values().iter().map(|v| op(*v));
// JUSTIFICATION
// Benefit
Expand Down Expand Up @@ -593,9 +591,8 @@ impl<T: ArrowPrimitiveType> PrimitiveArray<T> {
O: ArrowPrimitiveType,
F: Fn(T::Native) -> Option<O::Native>,
{
let data = self.data();
let len = data.len();
let (nulls, null_count, offset) = match data.nulls() {
let len = self.len();
let (nulls, null_count, offset) = match self.nulls() {
Some(n) => (Some(n.validity()), n.null_count(), n.offset()),
None => (None, 0, 0),
};
Expand Down Expand Up @@ -1185,7 +1182,7 @@ impl<T: DecimalType + ArrowPrimitiveType> PrimitiveArray<T> {
pub fn precision(&self) -> u8 {
match T::BYTE_LENGTH {
16 => {
if let DataType::Decimal128(p, _) = self.data().data_type() {
if let DataType::Decimal128(p, _) = self.data_type() {
*p
} else {
unreachable!(
Expand All @@ -1195,7 +1192,7 @@ impl<T: DecimalType + ArrowPrimitiveType> PrimitiveArray<T> {
}
}
32 => {
if let DataType::Decimal256(p, _) = self.data().data_type() {
if let DataType::Decimal256(p, _) = self.data_type() {
*p
} else {
unreachable!(
Expand All @@ -1212,7 +1209,7 @@ impl<T: DecimalType + ArrowPrimitiveType> PrimitiveArray<T> {
pub fn scale(&self) -> i8 {
match T::BYTE_LENGTH {
16 => {
if let DataType::Decimal128(_, s) = self.data().data_type() {
if let DataType::Decimal128(_, s) = self.data_type() {
*s
} else {
unreachable!(
Expand All @@ -1222,7 +1219,7 @@ impl<T: DecimalType + ArrowPrimitiveType> PrimitiveArray<T> {
}
}
32 => {
if let DataType::Decimal256(_, s) = self.data().data_type() {
if let DataType::Decimal256(_, s) = self.data_type() {
*s
} else {
unreachable!(
Expand Down Expand Up @@ -1874,7 +1871,7 @@ mod tests {
let array = PrimitiveArray::<Decimal128Type>::from(values.clone());
assert_eq!(array.values(), &values);

let array = PrimitiveArray::<Decimal128Type>::from(array.data().clone());
let array = PrimitiveArray::<Decimal128Type>::from(array.to_data());
assert_eq!(array.values(), &values);
}

Expand All @@ -1894,7 +1891,7 @@ mod tests {
let array = PrimitiveArray::<Decimal256Type>::from(values.clone());
assert_eq!(array.values(), &values);

let array = PrimitiveArray::<Decimal256Type>::from(array.data().clone());
let array = PrimitiveArray::<Decimal256Type>::from(array.to_data());
assert_eq!(array.values(), &values);
}

Expand Down Expand Up @@ -2190,7 +2187,7 @@ mod tests {

let boxed: ArrayRef = Arc::new(array);

let col: Int32Array = PrimitiveArray::<Int32Type>::from(boxed.data().clone());
let col: Int32Array = PrimitiveArray::<Int32Type>::from(boxed.to_data());
let err = col.into_builder();

match err {
Expand Down
6 changes: 3 additions & 3 deletions arrow-array/src/array/run_array.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -104,8 +104,8 @@ impl<R: RunEndIndexType> RunArray<R> {
let len = RunArray::logical_len(run_ends);
let builder = ArrayDataBuilder::new(ree_array_type)
.len(len)
.add_child_data(run_ends.data().clone())
.add_child_data(values.data().clone());
.add_child_data(run_ends.to_data())
.add_child_data(values.to_data());

// `build_unchecked` is used to avoid recursive validation of child arrays.
let array_data = unsafe { builder.build_unchecked() };
Expand Down Expand Up @@ -665,7 +665,7 @@ mod tests {
assert_eq!(ree_array.null_count(), 0);

let values = ree_array.values();
assert_eq!(&value_data.into_data(), values.data());
assert_eq!(value_data.into_data(), values.to_data());
assert_eq!(&DataType::Int8, values.data_type());

let run_ends = ree_array.run_ends();
Expand Down
4 changes: 2 additions & 2 deletions arrow-array/src/array/string_array.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -456,7 +456,7 @@ mod tests {
let data: Vec<Option<&str>> = vec![None];
let array = StringArray::from(data);
array
.data()
.into_data()
.validate_full()
.expect("All null array has valid array data");
}
Expand All @@ -466,7 +466,7 @@ mod tests {
let data: Vec<Option<&str>> = vec![None];
let array = LargeStringArray::from(data);
array
.data()
.into_data()
.validate_full()
.expect("All null array has valid array data");
}
Expand Down
Loading

0 comments on commit d7bba0a

Please sign in to comment.