-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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.
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
Minor: Add additional docstrings to Window function implementations #6592
Merged
Merged
Changes from all commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
5899adf
Add additional docstrings to Window function implementations
alamb e1d6663
Merge remote-tracking branch 'apache/main' into alamb/add_window_func…
alamb 0d6e587
Update docs
alamb b6ef1aa
updates
alamb 5e0217b
fix doc link
alamb d56a5b2
Change resorting --> re-sorting
alamb File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
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
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
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
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
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
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -36,12 +36,10 @@ use crate::{ | |
expressions::PhysicalSortExpr, reverse_order_bys, AggregateExpr, PhysicalExpr, | ||
}; | ||
|
||
/// A window expr that takes the form of an aggregate function | ||
/// Aggregate Window Expressions that have the form | ||
/// `OVER({ROWS | RANGE| GROUPS} BETWEEN UNBOUNDED PRECEDING AND ...)` | ||
/// e.g cumulative window frames uses `PlainAggregateWindowExpr`. Where as Aggregate Window Expressions | ||
/// that have the form `OVER({ROWS | RANGE| GROUPS} BETWEEN M {PRECEDING| FOLLOWING} AND ...)` | ||
/// e.g sliding window frames uses `SlidingAggregateWindowExpr`. | ||
/// A window expr that takes the form of an aggregate function that | ||
/// can be incrementally computed over sliding windows. | ||
/// | ||
/// See comments on [`WindowExpr`] for more details. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Consolidated this description into |
||
#[derive(Debug)] | ||
pub struct SlidingAggregateWindowExpr { | ||
aggregate: Arc<dyn AggregateExpr>, | ||
|
@@ -72,10 +70,11 @@ impl SlidingAggregateWindowExpr { | |
} | ||
} | ||
|
||
/// peer based evaluation based on the fact that batch is pre-sorted given the sort columns | ||
/// and then per partition point we'll evaluate the peer group (e.g. SUM or MAX gives the same | ||
/// results for peers) and concatenate the results. | ||
|
||
/// Incrementally update window function using the fact that batch is | ||
/// pre-sorted given the sort columns and then per partition point. | ||
/// | ||
/// Evaluates the peer group (e.g. `SUM` or `MAX` gives the same results | ||
/// for peers) and concatenate the results. | ||
impl WindowExpr for SlidingAggregateWindowExpr { | ||
/// Return a reference to Any that can be used for downcasting | ||
fn as_any(&self) -> &dyn Any { | ||
|
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mustafasrepo / @ozankabak if you have time to help me describe more clearly what Stateful and Stateless
PartitionEvaluator
s are , and specifically what is different between them, I would be most appreciativeThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some builtin window functions use window frame information inside the window expression (those are
FIRST_VALUE
,LAST_VALUE
,NTH_VALUE
). However, for most of the window functions what is in the window frame is not important (those areROW_NUMBER
,RANK
,DENSE_RANK
,PERCENT_RANK
,CUME_DIST
,LEAD
,LAG
). For the ones, using window_framePartitionEvaluator::evaluate_inside_range
is called. For the ones that do not use window framePartitionEvaluator::evaluate
is called (For rank calculations,PartitionEvaluator::evaluate_with_rank
is called since its API is quite different. However, it doesn't use window frame either.)PartitionEvaluator::evaluate_stateful
is used only when we produce window result with bounded memory(When window functions are called from theBoundedWindowAggExec
). In this case window results are calculated in running fashion, hence we need to store previous state, to be able to calculate correct output (For instance, forROW_NUMBER
function the current batch evaluator receive may not be the first batch. Hence we cannot start row_number from 0, we need to start from lastROW_NUMBER
produced for the previous batches received. Similarly, we need to store some information in the state. When we do not receive whole table as a single batch)Currently, we have support for bounded(stateful) execution for
FIRST_VALUE
,LAST_VALUE
,NTH_VALUE
,ROW_NUMBER
,RANK
,DENSE_RANK
,LEAD
,LAG
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you @mustafasrepo -- this is super helpful. I am incorporating this information into this PR