-
Notifications
You must be signed in to change notification settings - Fork 410
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
Refine window when preceding and following boundary type are unbounded #9916
base: master
Are you sure you want to change the base?
Conversation
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
||
has_shortcut = window_description_.frame.begin_type == WindowFrame::BoundaryType::Unbounded | ||
&& window_description_.frame.end_type == WindowFrame::BoundaryType::Unbounded | ||
&& !aggregation_workspaces.empty(); |
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.
and window_workspaces
should be empty?
@@ -292,6 +295,9 @@ void WindowTransformAction::initialWorkspaces() | |||
} | |||
} | |||
|
|||
// Can not have window and agg functions together in one window | |||
assert((!aggregation_workspaces.empty() && !window_workspaces.empty()) == false); |
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.
why have this restriction?
Block WindowTransformAction::tryGetOutputBlock() | ||
{ | ||
// first try calculate the result based on current data | ||
tryCalculate(); | ||
if (window_description.need_decrease) |
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.
if need_decrease is true, has_shortcut should always be false?
/hold |
What problem does this PR solve?
Issue Number: close #9913
Problem Summary:
What is changed and how it works?
Check List
Tests
Side effects
Documentation
Release note