From 52fa1b8e0319b643fffb3ced14e7dc1fbebf4efd Mon Sep 17 00:00:00 2001 From: Andrew Lamb Date: Tue, 17 Sep 2024 08:57:34 -0400 Subject: [PATCH] Minor: improve GroupsAccumulator docs --- .../expr-common/src/groups_accumulator.rs | 25 ++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/datafusion/expr-common/src/groups_accumulator.rs b/datafusion/expr-common/src/groups_accumulator.rs index 156e21d9ae20..8e81c51d8460 100644 --- a/datafusion/expr-common/src/groups_accumulator.rs +++ b/datafusion/expr-common/src/groups_accumulator.rs @@ -56,9 +56,32 @@ impl EmitTo { } } -/// `GroupAccumulator` implements a single aggregate (e.g. AVG) and +/// `GroupsAccumulator` implements a single aggregate (e.g. AVG) and /// stores the state for *all* groups internally. /// +/// Logically, a [`GroupsAccumulator`] stores a mapping from each group index to +/// the state of the aggregate for that group. For example an implementation for +/// `min` might look like +/// +/// ```text +/// ┌─────┐ +/// │ 0 │───────────▶ 100 +/// ├─────┤ +/// │ 1 │───────────▶ 200 +/// └─────┘ +/// ... ... +/// ┌─────┐ +/// │ N-2 │───────────▶ 50 +/// ├─────┤ +/// │ N-1 │───────────▶ 200 +/// └─────┘ +/// +/// +/// Logical group Current Min +/// number value for that +/// group +/// ``` +/// /// # Notes on Implementing `GroupAccumulator` /// /// All aggregates must first implement the simpler [`Accumulator`] trait, which