alamb commented on code in PR #23038:
URL: https://github.com/apache/datafusion/pull/23038#discussion_r3493090369
##########
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:
can you please add some comments explaining how values.empty can happen and
what should happen if it does (basically it assigns group indicies
Aso, why does this loop have to check opt_filter but the loop when there is
more than 1 value does not?
##########
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:
can you please add a comment explaining ho this can happen (and why is it
checking the input_mode too?)
##########
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:
can we simplify this by just adding a `with_supports_convert_to_state`
method? and then setting
so instead of
```rust
Ok(Box::new(
GroupsAccumulatorAdapter::new_with_convert_to_state(
factory,
supports_convert_to_state,
),
))
```
Soething like
```rust
let adapter = GroupsAccumulatorAdapter::new(factory)
.with_supports_conert_to_state(supports_convert_to_state);
Ok(Box::new(adapter))
```
--
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]
