-
Notifications
You must be signed in to change notification settings - Fork 74
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
Repeat emit for global aggregation #848
Conversation
@@ -51,6 +59,8 @@ struct AggregatingTransformParams | |||
return res; | |||
} | |||
|
|||
bool repeatEmit() const noexcept { return emit_repeat && !emit_changelog && emit_mode < EmitMode::OnUpdate; } |
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.
I think we don't need it.
Since the global aggregation already checked !emit_changelog
and the AST parsing ensured emit_mode
must be emit periodic as well
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 global aggr cannot emit changelog ?
{ | ||
*rows_since_last_finalizations[current_variant] += rows; | ||
} | ||
void addRowCount(size_t rows, size_t current_variant) { *rows_since_last_finalizations[current_variant] += rows; } | ||
}; |
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.
clang format ?
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.
It is clang-formatted
PR checklist:
proton: starts/ends
for new code in existing community code base ?Please write user-readable short description of the changes:
Closed #847