2010YOUY01 commented on code in PR #23309:
URL: https://github.com/apache/datafusion/pull/23309#discussion_r3522628660
##########
datafusion/physical-plan/src/aggregates/aggregate_hash_table/common.rs:
##########
@@ -85,29 +114,63 @@ pub(in crate::aggregates) struct
AggregateHashTable<AggrMode> {
/// Lifecycle-specific state: building stage / outputting stage.
pub(super) state: AggregateHashTableState,
-
- pub(super) _mode: PhantomData<AggrMode>,
}
/// Methods shared by all aggregate hash table modes.
-impl<AggrMode> AggregateHashTable<AggrMode> {
- pub(super) fn new_with_filters(
+impl AggregateHashTable {
+ pub(in crate::aggregates) fn new(
agg: &AggregateExec,
partition: usize,
output_schema: SchemaRef,
batch_size: usize,
- filters: Vec<Option<Arc<dyn PhysicalExpr>>>,
) -> Result<Self> {
if batch_size == 0 {
return internal_err!("AggregateHashTable requires config
batch_size >= 1");
}
+ // Infer the internal `AggregateTableMode` based on `AggregateExec`'s
mode
+ //
+ // TODO(simplification): `AggregateMode` seems bloated for aggregate
hash
+ // table semantics. Consider remove `AggregateMode` and only use
`AggregateTableMode`
+ // after the refactor has finished.
+ //
+ // Issue: <https://github.com/apache/datafusion/pull/22729>
Review Comment:
Yes, that's a wrong link. Fixed
--
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]
