gabotechs commented on code in PR #14413: URL: https://github.com/apache/datafusion/pull/14413#discussion_r1938546489
########## datafusion/functions-aggregate-common/src/merge_arrays.rs: ########## @@ -193,3 +193,149 @@ pub fn merge_ordered_arrays( Ok((merged_values, merged_orderings)) } + +#[cfg(test)] +mod tests { Review Comment: I moved this tests from https://github.com/apache/datafusion/blob/f23681ca3d836748fea6aa79e513e3d9e1367c1b/datafusion/functions-aggregate/src/array_agg.rs#L655 to this file, as they are just testing the `merge_ordered_arrays` function present in this file, and nothing related to `ARRAY_AGG`. As I added there some unit tests that do test `ARRAY_AGG`, I though that it might be a good idea to move these ones out to a more suitable place. ########## datafusion/functions-aggregate/src/array_agg.rs: ########## @@ -339,8 +370,8 @@ impl Accumulator for DistinctArrayAggAccumulator { } fn update_batch(&mut self, values: &[ArrayRef]) -> Result<()> { - if values.len() != 1 { - return internal_err!("expects single batch"); + if values.is_empty() { + return Ok(()); Review Comment: As now the distinct accumulator can accept more than 1 batch because of the ordering, removing this restriction was necessary. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org For additional commands, e-mail: github-h...@datafusion.apache.org