Update documentation for creating User Defined Aggregates (AggregateUDF) #6729
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
github-actions
bot
added
core
alamb
commented
Jun 20, 2023
| //! uses [Apache Arrow] as its in-memory format. DataFusion's [use | ||
| //! cases] include building very fast database and analytic systems, | ||
| //! customized to particular workloads. | ||
| //! uses [Apache Arrow] as its in-memory format. DataFusion's many [use |
There was a problem hiding this comment.
this was a drive by cleanup as I pretended to be a new user navigating to the AggregateUDF page
| /// other partial states from different instances of this | ||
| /// accumulator (that ran on different partitions, for | ||
| /// example). | ||
| /// Updates the accumulator's state from its input. |
There was a problem hiding this comment.
The trait is the same -- I did reorderd the methods to be better grouped together by use, but the actual methods are the same
| @@ -42,7 +42,7 @@ pub type ReturnTypeFunction = | |||
|
|
|||
| /// Factory that returns an accumulator for the given aggregate, given | |||
| /// its return datatype. | |||
| pub type AccumulatorFunctionImplementation = | |||
| pub type AccumulatorFactoryFunction = | |||
There was a problem hiding this comment.
This was just misleading, so I changed the name
There was a problem hiding this comment.
Totally agree – this name is much clearer
stuartcarnie
approved these changes
Jun 20, 2023
Comment on lines
+153
to
+178
| /// | ||
| /// # Example | ||
| /// | ||
| /// For example, given the following input partition | ||
| /// | ||
| /// ```text | ||
| /// │ current │ | ||
| /// window | ||
| /// │ │ | ||
| /// ┌────┬────┬────┬────┬────┬────┬────┬────┬────┐ | ||
| /// Input │ A │ B │ C │ D │ E │ F │ G │ H │ I │ | ||
| /// partition └────┴────┴────┴────┼────┴────┴────┴────┼────┘ | ||
| /// | ||
| /// │ next │ | ||
| /// window | ||
| /// ``` | ||
| /// | ||
| /// First, [`Self::evaluate`] will be called to produce the output | ||
| /// for the current window. | ||
| /// | ||
| /// Then, to advance to the next window: | ||
| /// | ||
| /// First, [`Self::retract_batch`] will be called with the values | ||
| /// that are leaving the window, `[B, C, D]` and then | ||
| /// [`Self::update_batch`] will be called with the values that are | ||
| /// entering the window, `[F, G, H]`. |
There was a problem hiding this comment.
Very clear explanation 💯
| /// time (e.g. median) | ||
| /// Note that [`ScalarValue::List`] can be used to pass multiple | ||
| /// values if the number of intermediate values is not known at | ||
| /// planning time (e.g. for `MEDIAN`) | ||
| fn state(&self) -> Result<Vec<ScalarValue>>; |
There was a problem hiding this comment.
This updated documentation is fantastic, thank you @alamb
| @@ -42,7 +42,7 @@ pub type ReturnTypeFunction = | |||
|
|
|||
| /// Factory that returns an accumulator for the given aggregate, given | |||
| /// its return datatype. | |||
| pub type AccumulatorFunctionImplementation = | |||
| pub type AccumulatorFactoryFunction = | |||
There was a problem hiding this comment.
Totally agree – this name is much clearer
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Which issue does this PR close?
related to #6611
Rationale for this change
@stuartcarnie had some questions about how this API should work, and so I wanted to encode the answers into documentation for others as well
What changes are included in this PR?
AggregateUDFAccumulatorAccumulatorFunctionImplementationtoAccumulatorFactoryFunctionto better describe what it doesAre these changes tested?
Yes (existing tests + doc tests)
Are there any user-facing changes?
AccumulatorFunctionImplementationtoAccumulatorFactoryFunction)