Skip to content

Commit cb2d03c

Browse files
jayzhan211alamb
andauthored
Change ScalarValue::List to store ArrayRef (#7629)
* first draft Signed-off-by: jayzhan211 <[email protected]> * revert subquery Signed-off-by: jayzhan211 <[email protected]> * Decimal128 in list_to_array Signed-off-by: jayzhan211 <[email protected]> * refactor null in list_to_array Signed-off-by: jayzhan211 <[email protected]> * cleanup iter to array v3 Signed-off-by: jayzhan211 <[email protected]> * remove comment Signed-off-by: jayzhan211 <[email protected]> * cleanup Signed-off-by: jayzhan211 <[email protected]> * cleanup struct array Signed-off-by: jayzhan211 <[email protected]> * remove new_list in array Signed-off-by: jayzhan211 <[email protected]> * cleanup Signed-off-by: jayzhan211 <[email protected]> * cleanup Signed-off-by: jayzhan211 <[email protected]> * cleanup list to array Signed-off-by: jayzhan211 <[email protected]> * cleanup list to array Signed-off-by: jayzhan211 <[email protected]> * remove try from array v2 Signed-off-by: jayzhan211 <[email protected]> * remove try from array v4 Signed-off-by: jayzhan211 <[email protected]> * cleanup expr_simply Signed-off-by: jayzhan211 <[email protected]> * cleanup iter to array v3 Signed-off-by: jayzhan211 <[email protected]> * checkpoint Signed-off-by: jayzhan211 <[email protected]> * remove try from array v3 Signed-off-by: jayzhan211 <[email protected]> * remove iter to array v4 Signed-off-by: jayzhan211 <[email protected]> * rewrite iter to array list v2 Signed-off-by: jayzhan211 <[email protected]> * cleanup iter to array list v3 Signed-off-by: jayzhan211 <[email protected]> * remove test Signed-off-by: jayzhan211 <[email protected]> * refactor wrap_into_list_array Signed-off-by: jayzhan211 <[email protected]> * cleanup Signed-off-by: jayzhan211 <[email protected]> * cleanup Signed-off-by: jayzhan211 <[email protected]> * revert math expressions Signed-off-by: jayzhan211 <[email protected]> * remove iter to array v3 Signed-off-by: jayzhan211 <[email protected]> * fmt Signed-off-by: jayzhan211 <[email protected]> * cleanup Signed-off-by: jayzhan211 <[email protected]> * cleanup Signed-off-by: jayzhan211 <[email protected]> * cleanup Signed-off-by: jayzhan211 <[email protected]> * cleanup Signed-off-by: jayzhan211 <[email protected]> * fix check Signed-off-by: jayzhan211 <[email protected]> * fix build_a_list_array_from_scalars Signed-off-by: jayzhan211 <[email protected]> * fmt Signed-off-by: jayzhan211 <[email protected]> * clippy fix Signed-off-by: jayzhan211 <[email protected]> * fix scalars test Signed-off-by: jayzhan211 <[email protected]> * fix array agg distinct Signed-off-by: jayzhan211 <[email protected]> * fix agg test Signed-off-by: jayzhan211 <[email protected]> * cleanup Signed-off-by: jayzhan211 <[email protected]> * add hash for scalar::Listarr Signed-off-by: jayzhan211 <[email protected]> * fix count distinct Signed-off-by: jayzhan211 <[email protected]> * fmt Signed-off-by: jayzhan211 <[email protected]> * comment out scalarvalue list inproto Signed-off-by: jayzhan211 <[email protected]> * rename Signed-off-by: jayzhan211 <[email protected]> * rename and add comment Signed-off-by: jayzhan211 <[email protected]> * reduce tryFromArray for List Signed-off-by: jayzhan211 <[email protected]> * reduce tryFromArray for FixSizeList Signed-off-by: jayzhan211 <[email protected]> * fmt Signed-off-by: jayzhan211 <[email protected]> * proto for list Signed-off-by: jayzhan211 <[email protected]> * cleanup Signed-off-by: jayzhan211 <[email protected]> * replace List with ListArr Signed-off-by: jayzhan211 <[email protected]> * remove build a list ... func Signed-off-by: jayzhan211 <[email protected]> * refactor merge_batch in DistinctBitXorAccumulator Signed-off-by: jayzhan211 <[email protected]> * rewrite merge batch in ArrayAggAccumulator Signed-off-by: jayzhan211 <[email protected]> * clippy Signed-off-by: jayzhan211 <[email protected]> * cleanup Signed-off-by: jayzhan211 <[email protected]> * return array directly in array() Signed-off-by: jayzhan211 <[email protected]> * add expected message Signed-off-by: jayzhan211 <[email protected]> * fmt Signed-off-by: jayzhan211 <[email protected]> * rename add add doc test for core function Signed-off-by: jayzhan211 <[email protected]> * fix ci Signed-off-by: jayzhan211 <[email protected]> * fix type Signed-off-by: jayzhan211 <[email protected]> * rename and address comment Signed-off-by: jayzhan211 <[email protected]> * address comment Signed-off-by: jayzhan211 <[email protected]> * rewrite roundtrip scalars Signed-off-by: jayzhan211 <[email protected]> * fix flatten Signed-off-by: jayzhan211 <[email protected]> * cleanup Signed-off-by: jayzhan211 <[email protected]> * add partial cmp Signed-off-by: jayzhan211 <[email protected]> * fmt Signed-off-by: jayzhan211 <[email protected]> * add comment Signed-off-by: jayzhan211 <[email protected]> * cleanup Signed-off-by: jayzhan211 <[email protected]> --------- Signed-off-by: jayzhan211 <[email protected]> Co-authored-by: Andrew Lamb <[email protected]>
1 parent 90bb5dd commit cb2d03c

File tree

22 files changed

+1219
-994
lines changed

22 files changed

+1219
-994
lines changed

datafusion/common/src/scalar.rs

Lines changed: 585 additions & 312 deletions
Large diffs are not rendered by default.

datafusion/common/src/utils.rs

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,10 +19,12 @@
1919
2020
use crate::{DataFusionError, Result, ScalarValue};
2121
use arrow::array::{ArrayRef, PrimitiveArray};
22+
use arrow::buffer::OffsetBuffer;
2223
use arrow::compute;
2324
use arrow::compute::{partition, SortColumn, SortOptions};
24-
use arrow::datatypes::{SchemaRef, UInt32Type};
25+
use arrow::datatypes::{Field, SchemaRef, UInt32Type};
2526
use arrow::record_batch::RecordBatch;
27+
use arrow_array::ListArray;
2628
use sqlparser::ast::Ident;
2729
use sqlparser::dialect::GenericDialect;
2830
use sqlparser::parser::Parser;
@@ -334,6 +336,18 @@ pub fn longest_consecutive_prefix<T: Borrow<usize>>(
334336
count
335337
}
336338

339+
/// Wrap an array into a single element `ListArray`.
340+
/// For example `[1, 2, 3]` would be converted into `[[1, 2, 3]]`
341+
pub fn wrap_into_list_array(arr: ArrayRef) -> ListArray {
342+
let offsets = OffsetBuffer::from_lengths([arr.len()]);
343+
ListArray::new(
344+
Arc::new(Field::new("item", arr.data_type().to_owned(), true)),
345+
offsets,
346+
arr,
347+
None,
348+
)
349+
}
350+
337351
/// An extension trait for smart pointers. Provides an interface to get a
338352
/// raw pointer to the data (with metadata stripped away).
339353
///

datafusion/core/tests/sql/aggregates.rs

Lines changed: 17 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -47,25 +47,23 @@ async fn csv_query_array_agg_distinct() -> Result<()> {
4747
let column = actual[0].column(0);
4848
assert_eq!(column.len(), 1);
4949

50-
if let ScalarValue::List(Some(mut v), _) = ScalarValue::try_from_array(column, 0)? {
51-
// workaround lack of Ord of ScalarValue
52-
let cmp = |a: &ScalarValue, b: &ScalarValue| {
53-
a.partial_cmp(b).expect("Can compare ScalarValues")
54-
};
55-
v.sort_by(cmp);
56-
assert_eq!(
57-
*v,
58-
vec![
59-
ScalarValue::UInt32(Some(1)),
60-
ScalarValue::UInt32(Some(2)),
61-
ScalarValue::UInt32(Some(3)),
62-
ScalarValue::UInt32(Some(4)),
63-
ScalarValue::UInt32(Some(5))
64-
]
65-
);
66-
} else {
67-
unreachable!();
68-
}
50+
let scalar_vec = ScalarValue::convert_array_to_scalar_vec(&column)?;
51+
let mut scalars = scalar_vec[0].clone();
52+
// workaround lack of Ord of ScalarValue
53+
let cmp = |a: &ScalarValue, b: &ScalarValue| {
54+
a.partial_cmp(b).expect("Can compare ScalarValues")
55+
};
56+
scalars.sort_by(cmp);
57+
assert_eq!(
58+
scalars,
59+
vec![
60+
ScalarValue::UInt32(Some(1)),
61+
ScalarValue::UInt32(Some(2)),
62+
ScalarValue::UInt32(Some(3)),
63+
ScalarValue::UInt32(Some(4)),
64+
ScalarValue::UInt32(Some(5))
65+
]
66+
);
6967

7068
Ok(())
7169
}

datafusion/optimizer/src/simplify_expressions/expr_simplifier.rs

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,10 @@ use arrow::{
3030
error::ArrowError,
3131
record_batch::RecordBatch,
3232
};
33-
use datafusion_common::tree_node::{RewriteRecursion, TreeNode, TreeNodeRewriter};
33+
use datafusion_common::{
34+
cast::{as_large_list_array, as_list_array},
35+
tree_node::{RewriteRecursion, TreeNode, TreeNodeRewriter},
36+
};
3437
use datafusion_common::{
3538
exec_err, internal_err, DFSchema, DFSchemaRef, DataFusionError, Result, ScalarValue,
3639
};
@@ -392,8 +395,11 @@ impl<'a> ConstEvaluator<'a> {
392395
"Could not evaluate the expression, found a result of length {}",
393396
a.len()
394397
)
398+
} else if as_list_array(&a).is_ok() || as_large_list_array(&a).is_ok() {
399+
Ok(ScalarValue::List(a))
395400
} else {
396-
Ok(ScalarValue::try_from_array(&a, 0)?)
401+
// Non-ListArray
402+
ScalarValue::try_from_array(&a, 0)
397403
}
398404
}
399405
ColumnarValue::Scalar(s) => Ok(s),

datafusion/physical-expr/src/aggregate/array_agg.rs

Lines changed: 90 additions & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,11 @@ use crate::expressions::format_state_name;
2222
use crate::{AggregateExpr, PhysicalExpr};
2323
use arrow::array::ArrayRef;
2424
use arrow::datatypes::{DataType, Field};
25+
use arrow_array::Array;
26+
use datafusion_common::cast::as_list_array;
27+
use datafusion_common::utils::wrap_into_list_array;
28+
use datafusion_common::Result;
2529
use datafusion_common::ScalarValue;
26-
use datafusion_common::{internal_err, DataFusionError, Result};
2730
use datafusion_expr::Accumulator;
2831
use std::any::Any;
2932
use std::sync::Arc;
@@ -102,7 +105,7 @@ impl PartialEq<dyn Any> for ArrayAgg {
102105

103106
#[derive(Debug)]
104107
pub(crate) struct ArrayAggAccumulator {
105-
values: Vec<ScalarValue>,
108+
values: Vec<ArrayRef>,
106109
datatype: DataType,
107110
}
108111

@@ -117,50 +120,60 @@ impl ArrayAggAccumulator {
117120
}
118121

119122
impl Accumulator for ArrayAggAccumulator {
123+
// Append value like Int64Array(1,2,3)
120124
fn update_batch(&mut self, values: &[ArrayRef]) -> Result<()> {
121125
if values.is_empty() {
122126
return Ok(());
123127
}
124128
assert!(values.len() == 1, "array_agg can only take 1 param!");
125-
let arr = &values[0];
126-
(0..arr.len()).try_for_each(|index| {
127-
let scalar = ScalarValue::try_from_array(arr, index)?;
128-
self.values.push(scalar);
129-
Ok(())
130-
})
129+
let val = values[0].clone();
130+
self.values.push(val);
131+
Ok(())
131132
}
132133

134+
// Append value like ListArray(Int64Array(1,2,3), Int64Array(4,5,6))
133135
fn merge_batch(&mut self, states: &[ArrayRef]) -> Result<()> {
134136
if states.is_empty() {
135137
return Ok(());
136138
}
137139
assert!(states.len() == 1, "array_agg states must be singleton!");
138-
let arr = &states[0];
139-
(0..arr.len()).try_for_each(|index| {
140-
let scalar = ScalarValue::try_from_array(arr, index)?;
141-
if let ScalarValue::List(Some(values), _) = scalar {
142-
self.values.extend(values);
143-
Ok(())
144-
} else {
145-
internal_err!("array_agg state must be list!")
146-
}
147-
})
140+
141+
let list_arr = as_list_array(&states[0])?;
142+
for arr in list_arr.iter().flatten() {
143+
self.values.push(arr);
144+
}
145+
Ok(())
148146
}
149147

150148
fn state(&self) -> Result<Vec<ScalarValue>> {
151149
Ok(vec![self.evaluate()?])
152150
}
153151

154152
fn evaluate(&self) -> Result<ScalarValue> {
155-
Ok(ScalarValue::new_list(
156-
Some(self.values.clone()),
157-
self.datatype.clone(),
158-
))
153+
// Transform Vec<ListArr> to ListArr
154+
155+
let element_arrays: Vec<&dyn Array> =
156+
self.values.iter().map(|a| a.as_ref()).collect();
157+
158+
if element_arrays.is_empty() {
159+
let arr = ScalarValue::new_list(&[], &self.datatype);
160+
return Ok(ScalarValue::List(arr));
161+
}
162+
163+
let concated_array = arrow::compute::concat(&element_arrays)?;
164+
let list_array = wrap_into_list_array(concated_array);
165+
166+
Ok(ScalarValue::List(Arc::new(list_array)))
159167
}
160168

161169
fn size(&self) -> usize {
162-
std::mem::size_of_val(self) + ScalarValue::size_of_vec(&self.values)
163-
- std::mem::size_of_val(&self.values)
170+
std::mem::size_of_val(self)
171+
+ (std::mem::size_of::<ArrayRef>() * self.values.capacity())
172+
+ self
173+
.values
174+
.iter()
175+
.map(|arr| arr.get_array_memory_size())
176+
.sum::<usize>()
164177
+ self.datatype.size()
165178
- std::mem::size_of_val(&self.datatype)
166179
}
@@ -176,72 +189,78 @@ mod tests {
176189
use arrow::array::Int32Array;
177190
use arrow::datatypes::*;
178191
use arrow::record_batch::RecordBatch;
192+
use arrow_array::Array;
193+
use arrow_array::ListArray;
194+
use arrow_buffer::OffsetBuffer;
195+
use datafusion_common::DataFusionError;
179196
use datafusion_common::Result;
180197

181198
#[test]
182199
fn array_agg_i32() -> Result<()> {
183200
let a: ArrayRef = Arc::new(Int32Array::from(vec![1, 2, 3, 4, 5]));
184201

185-
let list = ScalarValue::new_list(
186-
Some(vec![
187-
ScalarValue::Int32(Some(1)),
188-
ScalarValue::Int32(Some(2)),
189-
ScalarValue::Int32(Some(3)),
190-
ScalarValue::Int32(Some(4)),
191-
ScalarValue::Int32(Some(5)),
192-
]),
193-
DataType::Int32,
194-
);
202+
let list = ListArray::from_iter_primitive::<Int32Type, _, _>(vec![Some(vec![
203+
Some(1),
204+
Some(2),
205+
Some(3),
206+
Some(4),
207+
Some(5),
208+
])]);
209+
let list = ScalarValue::List(Arc::new(list));
195210

196211
generic_test_op!(a, DataType::Int32, ArrayAgg, list, DataType::Int32)
197212
}
198213

199214
#[test]
200215
fn array_agg_nested() -> Result<()> {
201-
let l1 = ScalarValue::new_list(
202-
Some(vec![
203-
ScalarValue::new_list(
204-
Some(vec![
205-
ScalarValue::from(1i32),
206-
ScalarValue::from(2i32),
207-
ScalarValue::from(3i32),
208-
]),
209-
DataType::Int32,
210-
),
211-
ScalarValue::new_list(
212-
Some(vec![ScalarValue::from(4i32), ScalarValue::from(5i32)]),
213-
DataType::Int32,
214-
),
215-
]),
216-
DataType::List(Arc::new(Field::new("item", DataType::Int32, true))),
216+
let a1 = ListArray::from_iter_primitive::<Int32Type, _, _>(vec![Some(vec![
217+
Some(1),
218+
Some(2),
219+
Some(3),
220+
])]);
221+
let a2 = ListArray::from_iter_primitive::<Int32Type, _, _>(vec![Some(vec![
222+
Some(4),
223+
Some(5),
224+
])]);
225+
let l1 = ListArray::new(
226+
Arc::new(Field::new("item", a1.data_type().to_owned(), true)),
227+
OffsetBuffer::from_lengths([a1.len() + a2.len()]),
228+
arrow::compute::concat(&[&a1, &a2])?,
229+
None,
217230
);
218231

219-
let l2 = ScalarValue::new_list(
220-
Some(vec![
221-
ScalarValue::new_list(
222-
Some(vec![ScalarValue::from(6i32)]),
223-
DataType::Int32,
224-
),
225-
ScalarValue::new_list(
226-
Some(vec![ScalarValue::from(7i32), ScalarValue::from(8i32)]),
227-
DataType::Int32,
228-
),
229-
]),
230-
DataType::List(Arc::new(Field::new("item", DataType::Int32, true))),
232+
let a1 =
233+
ListArray::from_iter_primitive::<Int32Type, _, _>(vec![Some(vec![Some(6)])]);
234+
let a2 = ListArray::from_iter_primitive::<Int32Type, _, _>(vec![Some(vec![
235+
Some(7),
236+
Some(8),
237+
])]);
238+
let l2 = ListArray::new(
239+
Arc::new(Field::new("item", a1.data_type().to_owned(), true)),
240+
OffsetBuffer::from_lengths([a1.len() + a2.len()]),
241+
arrow::compute::concat(&[&a1, &a2])?,
242+
None,
231243
);
232244

233-
let l3 = ScalarValue::new_list(
234-
Some(vec![ScalarValue::new_list(
235-
Some(vec![ScalarValue::from(9i32)]),
236-
DataType::Int32,
237-
)]),
238-
DataType::List(Arc::new(Field::new("item", DataType::Int32, true))),
245+
let a1 =
246+
ListArray::from_iter_primitive::<Int32Type, _, _>(vec![Some(vec![Some(9)])]);
247+
let l3 = ListArray::new(
248+
Arc::new(Field::new("item", a1.data_type().to_owned(), true)),
249+
OffsetBuffer::from_lengths([a1.len()]),
250+
arrow::compute::concat(&[&a1])?,
251+
None,
239252
);
240253

241-
let list = ScalarValue::new_list(
242-
Some(vec![l1.clone(), l2.clone(), l3.clone()]),
243-
DataType::List(Arc::new(Field::new("item", DataType::Int32, true))),
254+
let list = ListArray::new(
255+
Arc::new(Field::new("item", l1.data_type().to_owned(), true)),
256+
OffsetBuffer::from_lengths([l1.len() + l2.len() + l3.len()]),
257+
arrow::compute::concat(&[&l1, &l2, &l3])?,
258+
None,
244259
);
260+
let list = ScalarValue::List(Arc::new(list));
261+
let l1 = ScalarValue::List(Arc::new(l1));
262+
let l2 = ScalarValue::List(Arc::new(l2));
263+
let l3 = ScalarValue::List(Arc::new(l3));
245264

246265
let array = ScalarValue::iter_to_array(vec![l1, l2, l3]).unwrap();
247266

0 commit comments

Comments
 (0)