ametel01 commented on code in PR #23038:
URL: https://github.com/apache/datafusion/pull/23038#discussion_r3494036727
##########
datafusion/functions-aggregate-common/src/aggregate/groups_accumulator.rs:
##########
@@ -196,13 +220,24 @@ impl GroupsAccumulatorAdapter {
{
self.make_accumulators_if_needed(total_num_groups)?;
- assert_eq!(values[0].len(), group_indices.len());
+ if values.is_empty() {
Review Comment:
Addressed in f238096c8. I added comments explaining that `values` is empty
for nullary aggregates, that the path still assigns input row indices to
groups, and that it applies `opt_filter` while assigning indices because there
are no value arrays to filter later via `slice_and_maybe_filter`.
##########
datafusion/functions-aggregate-common/src/aggregate/groups_accumulator.rs:
##########
@@ -137,6 +143,24 @@ impl GroupsAccumulatorAdapter {
factory: Box::new(factory),
states: vec![],
allocation_bytes: 0,
+ supports_convert_to_state: true,
+ }
+ }
+
+ /// Create a new adapter with explicit control over whether row-to-state
Review Comment:
Addressed in f238096c8. I replaced `new_with_convert_to_state` with
builder-style `with_supports_convert_to_state(...)` and updated the `row_hash`
call site to use it after `GroupsAccumulatorAdapter::new(factory)`.
##########
datafusion/physical-plan/src/aggregates/no_grouping.rs:
##########
@@ -463,6 +463,13 @@ fn aggregate_batch(
// 1.3
let values = evaluate_expressions_to_arrays(expr, batch.as_ref())?;
+ if values.is_empty()
Review Comment:
Addressed in f238096c8. I added a comment explaining that nullary raw
aggregates have no input expressions, so `values` is empty; when filtering
removes all rows there is no row cardinality to pass to `update_batch`, and the
guard is limited to raw mode because partial mode still needs to merge state
arrays.
--
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]
