-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Move array ArrayAgg
to a UserDefinedAggregate
#11448
Changes from 11 commits
6f76390
d1634bc
dc712f4
41b9c8a
a7712e7
d5d21b0
7a1056c
5ddaddb
9d17c1c
ac640fb
f67d8f7
a9b925b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -0,0 +1,249 @@ | ||||||
// Licensed to the Apache Software Foundation (ASF) under one | ||||||
// or more contributor license agreements. See the NOTICE file | ||||||
// distributed with this work for additional information | ||||||
// regarding copyright ownership. The ASF licenses this file | ||||||
// to you under the Apache License, Version 2.0 (the | ||||||
// "License"); you may not use this file except in compliance | ||||||
// with the License. You may obtain a copy of the License at | ||||||
// | ||||||
// http://www.apache.org/licenses/LICENSE-2.0 | ||||||
// | ||||||
// Unless required by applicable law or agreed to in writing, | ||||||
// software distributed under the License is distributed on an | ||||||
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY | ||||||
// KIND, either express or implied. See the License for the | ||||||
// specific language governing permissions and limitations | ||||||
// under the License. | ||||||
|
||||||
//! Defines physical expressions that can evaluated at runtime during query execution | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
|
||||||
use arrow::array::{Array, ArrayRef, AsArray}; | ||||||
use arrow::datatypes::DataType; | ||||||
use arrow_schema::Field; | ||||||
|
||||||
use datafusion_common::cast::as_list_array; | ||||||
use datafusion_common::utils::array_into_list_array_nullable; | ||||||
use datafusion_common::Result; | ||||||
use datafusion_common::ScalarValue; | ||||||
use datafusion_expr::function::{AccumulatorArgs, StateFieldsArgs}; | ||||||
use datafusion_expr::utils::format_state_name; | ||||||
use datafusion_expr::AggregateUDFImpl; | ||||||
use datafusion_expr::{Accumulator, Signature, Volatility}; | ||||||
use std::collections::HashSet; | ||||||
use std::sync::Arc; | ||||||
|
||||||
make_udaf_expr_and_func!( | ||||||
ArrayAgg, | ||||||
array_agg, | ||||||
expression, | ||||||
"input values, including nulls, concatenated into an array", | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this is not clear & can we improve for clarity just the other functions ? |
||||||
array_agg_udaf | ||||||
); | ||||||
|
||||||
#[derive(Debug)] | ||||||
/// ARRAY_AGG aggregate expression | ||||||
pub struct ArrayAgg { | ||||||
signature: Signature, | ||||||
alias: Vec<String>, | ||||||
} | ||||||
|
||||||
impl Default for ArrayAgg { | ||||||
fn default() -> Self { | ||||||
Self { | ||||||
signature: Signature::any(1, Volatility::Immutable), | ||||||
alias: vec!["array_agg".to_string()], | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why are we using the smallcase as alias ? Shouldn't this be the function name itself ? |
||||||
} | ||||||
} | ||||||
} | ||||||
|
||||||
impl AggregateUDFImpl for ArrayAgg { | ||||||
fn as_any(&self) -> &dyn std::any::Any { | ||||||
self | ||||||
} | ||||||
|
||||||
// TODO: change name to lowercase | ||||||
fn name(&self) -> &str { | ||||||
"ARRAY_AGG" | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Any specific reason for having this in uppercase ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Interestingly it is inconsistent with datafusion/datafusion/functions-aggregate/src/count.rs Lines 112 to 113 in b685e2d
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure why they are upper case initially, however we plan to convert them all to lowercase |
||||||
} | ||||||
|
||||||
fn aliases(&self) -> &[String] { | ||||||
&self.alias | ||||||
} | ||||||
|
||||||
fn signature(&self) -> &Signature { | ||||||
&self.signature | ||||||
} | ||||||
|
||||||
fn return_type(&self, arg_types: &[DataType]) -> Result<DataType> { | ||||||
Ok(DataType::List(Arc::new(Field::new( | ||||||
"item", | ||||||
arg_types[0].clone(), | ||||||
true, | ||||||
)))) | ||||||
} | ||||||
|
||||||
fn state_fields(&self, args: StateFieldsArgs) -> Result<Vec<Field>> { | ||||||
if args.is_distinct { | ||||||
return Ok(vec![Field::new_list( | ||||||
format_state_name(args.name, "distinct_array_agg"), | ||||||
Field::new("item", args.input_type.clone(), true), | ||||||
true, | ||||||
)]); | ||||||
} | ||||||
|
||||||
Ok(vec![Field::new_list( | ||||||
format_state_name(args.name, "array_agg"), | ||||||
Field::new("item", args.input_type.clone(), true), | ||||||
true, | ||||||
)]) | ||||||
} | ||||||
|
||||||
fn accumulator(&self, acc_args: AccumulatorArgs) -> Result<Box<dyn Accumulator>> { | ||||||
if acc_args.is_distinct { | ||||||
return Ok(Box::new(DistinctArrayAggAccumulator::try_new( | ||||||
acc_args.input_type, | ||||||
)?)); | ||||||
} | ||||||
|
||||||
Ok(Box::new(ArrayAggAccumulator::try_new(acc_args.input_type)?)) | ||||||
} | ||||||
} | ||||||
|
||||||
#[derive(Debug)] | ||||||
pub struct ArrayAggAccumulator { | ||||||
values: Vec<ArrayRef>, | ||||||
datatype: DataType, | ||||||
} | ||||||
|
||||||
impl ArrayAggAccumulator { | ||||||
/// new array_agg accumulator based on given item data type | ||||||
pub fn try_new(datatype: &DataType) -> Result<Self> { | ||||||
Ok(Self { | ||||||
values: vec![], | ||||||
datatype: datatype.clone(), | ||||||
}) | ||||||
} | ||||||
} | ||||||
|
||||||
impl Accumulator for ArrayAggAccumulator { | ||||||
// Append value like Int64Array(1,2,3) | ||||||
fn update_batch(&mut self, values: &[ArrayRef]) -> Result<()> { | ||||||
if values.is_empty() { | ||||||
return Ok(()); | ||||||
} | ||||||
assert!(values.len() == 1, "array_agg can only take 1 param!"); | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This will invoke the |
||||||
|
||||||
let val = Arc::clone(&values[0]); | ||||||
if val.len() > 0 { | ||||||
self.values.push(val); | ||||||
} | ||||||
Ok(()) | ||||||
} | ||||||
|
||||||
// Append value like ListArray(Int64Array(1,2,3), Int64Array(4,5,6)) | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: If this is code document, then we can move this inside the function. |
||||||
fn merge_batch(&mut self, states: &[ArrayRef]) -> Result<()> { | ||||||
if states.is_empty() { | ||||||
return Ok(()); | ||||||
} | ||||||
assert!(states.len() == 1, "array_agg states must be singleton!"); | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same as above. Can we remove all |
||||||
|
||||||
let list_arr = as_list_array(&states[0])?; | ||||||
for arr in list_arr.iter().flatten() { | ||||||
self.values.push(arr); | ||||||
} | ||||||
Ok(()) | ||||||
} | ||||||
|
||||||
fn state(&mut self) -> Result<Vec<ScalarValue>> { | ||||||
Ok(vec![self.evaluate()?]) | ||||||
} | ||||||
|
||||||
fn evaluate(&mut self) -> Result<ScalarValue> { | ||||||
// Transform Vec<ListArr> to ListArr | ||||||
let element_arrays: Vec<&dyn Array> = | ||||||
self.values.iter().map(|a| a.as_ref()).collect(); | ||||||
|
||||||
if element_arrays.is_empty() { | ||||||
return Ok(ScalarValue::new_null_list(self.datatype.clone(), true, 1)); | ||||||
} | ||||||
|
||||||
let concated_array = arrow::compute::concat(&element_arrays)?; | ||||||
let list_array = array_into_list_array_nullable(concated_array); | ||||||
|
||||||
Ok(ScalarValue::List(Arc::new(list_array))) | ||||||
} | ||||||
|
||||||
fn size(&self) -> usize { | ||||||
std::mem::size_of_val(self) | ||||||
+ (std::mem::size_of::<ArrayRef>() * self.values.capacity()) | ||||||
+ self | ||||||
.values | ||||||
.iter() | ||||||
.map(|arr| arr.get_array_memory_size()) | ||||||
.sum::<usize>() | ||||||
+ self.datatype.size() | ||||||
- std::mem::size_of_val(&self.datatype) | ||||||
} | ||||||
} | ||||||
|
||||||
#[derive(Debug)] | ||||||
struct DistinctArrayAggAccumulator { | ||||||
values: HashSet<ScalarValue>, | ||||||
datatype: DataType, | ||||||
} | ||||||
|
||||||
impl DistinctArrayAggAccumulator { | ||||||
pub fn try_new(datatype: &DataType) -> Result<Self> { | ||||||
Ok(Self { | ||||||
values: HashSet::new(), | ||||||
datatype: datatype.clone(), | ||||||
}) | ||||||
} | ||||||
} | ||||||
|
||||||
impl Accumulator for DistinctArrayAggAccumulator { | ||||||
fn state(&mut self) -> Result<Vec<ScalarValue>> { | ||||||
Ok(vec![self.evaluate()?]) | ||||||
} | ||||||
|
||||||
fn update_batch(&mut self, values: &[ArrayRef]) -> Result<()> { | ||||||
assert_eq!(values.len(), 1, "batch input should only include 1 column!"); | ||||||
|
||||||
let array = &values[0]; | ||||||
|
||||||
for i in 0..array.len() { | ||||||
let scalar = ScalarValue::try_from_array(&array, i)?; | ||||||
self.values.insert(scalar); | ||||||
} | ||||||
|
||||||
Ok(()) | ||||||
} | ||||||
|
||||||
fn merge_batch(&mut self, states: &[ArrayRef]) -> Result<()> { | ||||||
if states.is_empty() { | ||||||
return Ok(()); | ||||||
} | ||||||
|
||||||
states[0] | ||||||
.as_list::<i32>() | ||||||
.iter() | ||||||
.flatten() | ||||||
.try_for_each(|val| self.update_batch(&[val])) | ||||||
} | ||||||
|
||||||
fn evaluate(&mut self) -> Result<ScalarValue> { | ||||||
let values: Vec<ScalarValue> = self.values.iter().cloned().collect(); | ||||||
if values.is_empty() { | ||||||
return Ok(ScalarValue::new_null_list(self.datatype.clone(), true, 1)); | ||||||
} | ||||||
let arr = ScalarValue::new_list(&values, &self.datatype, true); | ||||||
Ok(ScalarValue::List(arr)) | ||||||
} | ||||||
|
||||||
fn size(&self) -> usize { | ||||||
std::mem::size_of_val(self) + ScalarValue::size_of_hashset(&self.values) | ||||||
- std::mem::size_of_val(&self.values) | ||||||
+ self.datatype.size() | ||||||
- std::mem::size_of_val(&self.datatype) | ||||||
} | ||||||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: I think we can remove this