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

Reply via email to