-
Notifications
You must be signed in to change notification settings - Fork 25.6k
Support window functions in time-series aggregations #138139
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
Conversation
|
Hi @dnhatn, I've created a changelog YAML for you. |
ℹ️ Important: Docs version tagging👋 Thanks for updating the docs! Just a friendly reminder that our docs are now cumulative. This means all 9.x versions are documented on the same page and published off of the main branch, instead of creating separate pages for each minor version. We use applies_to tags to mark version-specific features and changes. Expand for a quick overviewWhen to use applies_to tags:✅ At the page level to indicate which products/deployments the content applies to (mandatory) What NOT to do:❌ Don't remove or replace information that applies to an older version 🤔 Need help?
|
| Interval prev = intervals[i]; | ||
| if (prev.v2 > next.v2) { | ||
| state.resets += prev.v2; | ||
| if (prev.v1 > next.v2) { |
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.
Existing bug in rate that only manifests when using window functions.
|
Pinging @elastic/es-storage-engine (Team:StorageEngine) |
| TimeSeriesGroupingAggregatorEvaluationContext evaluationContext | ||
| ) { | ||
| if (selected.getPositionCount() > 0) { | ||
| // TODO: rewrite to NO_WINDOW in the planner if the bucket and the window are the same |
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.
What happens if the window is smaller than the bucket, e.g.
TS metrics | STATS sum(rate(reqs, 1m) BY TBUCKET(5m)
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.
We should reject this query during translation.
| examples = { @Example(file = "k8s-timeseries", tag = "avg_over_time") } | ||
| examples = { | ||
| @Example(file = "k8s-timeseries", tag = "avg_over_time"), | ||
| @Example(file = "k8s-timeseries-avg-over-time", tag = "avg_over_time_with_window") } |
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.
Can we skip the version with window for now? I think it's gonna show up in the documentation, which will be confusing for users in 9.2.. Let's discuss separate with Liam on how best to approach this, once we have all functions updated to support windows.
We do need to add them in the tests so that Kibana documentation is updated..
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 removed this in 3e42bb7
| int window = between(1, 20); | ||
| var query = String.format(Locale.ROOT, """ | ||
| TS k8s | ||
| | STATS avg(last_over_time(network.bytes_in, %s minute)) BY bucket(@timestamp, 1 minute) |
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.
Nit: use TBUCKET?
| } | ||
| } | ||
| throw new EsqlIllegalArgumentException( | ||
| "Unsupported window [{}] of aggregate function [{}]; the window must be a positive multiple of the time bucket [{}].", |
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.
or smaller than the time bucker?
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 pushed e0a3c5a
kkrik-es
left a comment
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.
Nice, so clean! Mostly a question around the documentation, to avoid confusing users of 9.2.
|
Thanks Kostas! |
This change adds window function support for time-series aggregation functions including
rate,avg_over_time, andlast_over_time. Additional time-series aggregation functions will be supported in a follow-up to keep this PR focused.Examples:
Window 5-minute over 1-minute buckets.
Window 10-minute over 5-minute buckets.
Limitation:
ES|QL currently supports only windows that are multiples of the bucket size.