Skip to content

Clarify docs about Accumulator::update and Accumulator::update_batch #1542

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
alamb merged 3 commits into from
Jan 11, 2022
Merged

Clarify docs about Accumulator::update and Accumulator::update_batch #1542

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
432ba43
Clarify docs about `Accumulator::update` and `Accumulator::update_batch`
alamb Jan 10, 2022
a0dd38f
Update datafusion/src/physical_plan/mod.rs
alamb Jan 11, 2022
e37679f
Improve docs, add comments on merge_batch
alamb Jan 11, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 13 additions & 1 deletion datafusion/src/physical_plan/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -565,7 +565,17 @@ pub trait Accumulator: Send + Sync + Debug {
// of two values, sum and n.
fn state(&self) -> Result<Vec<ScalarValue>>;

/// updates the accumulator's state from a vector of scalars.
/// updates the accumulator's state from a vector of scalars
/// (called by default implementation of [`update_batch`])
///
/// Note: this method is the often the simplest to implement and
Comment thread
alamb marked this conversation as resolved.
Outdated
/// is backwards compatible to help to lower the barrier to entry for
/// new users to write `Accumulators`
///
/// You should always implement `update_batch` instead of this
/// method for production aggregators or if you find yourself
/// wanting mathematical kernels for `ScalarValue`s such as `add`,
/// `mul`, etc
fn update(&mut self, values: &[ScalarValue]) -> Result<()>;

/// updates the accumulator's state from a vector of arrays.
Expand All @@ -583,6 +593,8 @@ pub trait Accumulator: Send + Sync + Debug {
}

/// updates the accumulator's state from a vector of scalars.
///
/// (called by default implementation of [`merge`])
fn merge(&mut self, states: &[ScalarValue]) -> Result<()>;

/// updates the accumulator's state from a vector of states.
Expand Down