More validation for time-series aggregations #134413
Merged
Conversation
This file contains hidden or 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
dnhatn
force-pushed
the
disallow-sorting-ts
branch
from
984cc32 to
5dcb2bf
Compare
dnhatn
changed the title
|
Pinging @elastic/es-storage-engine (Team:StorageEngine) |
kkrik-es
reviewed
Sep 10, 2025
| failures.add( | ||
| fail( | ||
| ts, | ||
| "over-time aggregate function [{}] can only be used with the TS command and inside another aggregate function", |
kkrik-es
reviewed
Sep 10, 2025
| for (Expression g : groupings) { | ||
| boolean timeBucket = g.anyMatch( | ||
| c -> (c instanceof Bucket b && b.field().equals(timestamp.get()) | ||
| || (c instanceof TBucket tb && tb.field().equals(timestamp.get()))) |
There was a problem hiding this comment.
Check for date_trunk too?
kkrik-es
reviewed
Sep 10, 2025
| nested -> failures.add( | ||
| fail( | ||
| this, | ||
| "cannot use aggregate function [{}] inside over-time aggregation function [{}]", |
There was a problem hiding this comment.
Nit: maybe replace over-time with time-series, here and below?
kkrik-es
reviewed
Sep 10, 2025
| 1:22: nested aggregations [avg(rate(network.bytes_in))] \ | ||
| not allowed inside other aggregations [max(avg(rate(network.bytes_in)))] | ||
| line 1:12: cannot use aggregate function [avg(rate(network.bytes_in))] \ | ||
| inside over-time aggregation function [rate(network.bytes_in)]""")); |
There was a problem hiding this comment.
This second error is somewhat confusing - avg is not used inside rate. Overtriggering?
kkrik-es
reviewed
Sep 10, 2025
| 1:23: limiting [LIMIT 10] the time-series source \ | ||
| before the first aggregation [STATS avg(network.connections)] is not allowed; filter data with a WHERE command instead | ||
| line 1:11: sorting [SORT host] between the time-series source \ | ||
| and the first aggregation [STATS avg(network.connections)] is not allowed""")); |
There was a problem hiding this comment.
Consider adding a couple of examples with SORT and LIMIT after STATS to verify that the error doesn't overtrigger.
kkrik-es
reviewed
Sep 10, 2025
| assertThat( | ||
| error("TS test | INLINESTATS v = avg(network.connections) | STATS max(v)", tsdb), | ||
| equalTo("1:11: InlineStats [INLINESTATS v = avg(network.connections)] in time-series is only allowed after an aggregation") | ||
| ); |
There was a problem hiding this comment.
Add a simple example with INLINESTATS after STATS - if that makes sense?
kkrik-es
approved these changes
Sep 10, 2025
This was referenced Sep 15, 2025
gmjehovich
pushed a commit
to gmjehovich/elasticsearch
that referenced
this pull request
Sep 18, 2025
phananh1010
added a commit
to phananh1010/elasticsearch
that referenced
this pull request
Sep 23, 2025
BASE=3c3058e6f63c5ebdcc4513fc5dcc4ffb3ec8a752 HEAD=69b95e47c4d65a8c6903b6f532b08d03d5406360 Branch=main
phananh1010
added a commit
to phananh1010/elasticsearch
that referenced
this pull request
Oct 1, 2025
BASE=3c3058e6f63c5ebdcc4513fc5dcc4ffb3ec8a752 HEAD=69b95e47c4d65a8c6903b6f532b08d03d5406360 Branch=main
phananh1010
added a commit
to phananh1010/elasticsearch
that referenced
this pull request
Oct 7, 2025
BASE=3c3058e6f63c5ebdcc4513fc5dcc4ffb3ec8a752 HEAD=69b95e47c4d65a8c6903b6f532b08d03d5406360 Branch=main
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.
Add validations to reject the following queries until supported:
Limit and sort cannot be used to alter the time-series source:
TS metrics | LIMIT ... | STATS ...TS metrics | SORT BY ... | STATS ...Over-time aggregation without an outer aggregation (to be supported soon):
TS metrics | STATS rate(requests)TS metrics | STATS last_over_time(requests)Reject lookup join, enrich, change point before the first stats.
Closes #134366
Closes #134372