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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
