[ES|QL] Add a TS variation of GenerativeIT #133804
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
This commit adds a variation of the GenerativeRestTest in ES|QL that queries indices marked as time series indices and runs time series aggregations on them (amongst all the other commands already supported in the Generative tests)
limotova
force-pushed
the
ts-in-generative-tests
branch
from
e3a4b22 to
82296eb
Compare
|
Edit: This was a comment with a list of bugs, these have either been dealt with or triaged with new individual issues, so I am hiding the original contents of this comment in a drop-down Existing bug 1:Can't combine Example stack traceExisting bug 2:After a few pipes, the date field being used as a timestamp gets lost (note that the example query has many layers of stats commands, meaning it actually runs successfully for the first 4 out of 5 stats commands) Example stack traceAnother (shorter) failing query is Existing bug 3:Verification fails because field being grouped on changes from field to reference ( Example stack trace |
kkrik-es
reviewed
Sep 10, 2025
...e/src/javaRestTest/java/org/elasticsearch/xpack/esql/qa/single_node/GenerativeMetricsIT.java
Show resolved
Hide resolved
kkrik-es
reviewed
Sep 10, 2025
...server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/generative/EsqlQueryGenerator.java
Outdated
Show resolved
Hide resolved
kkrik-es
reviewed
Sep 10, 2025
| } | ||
|
|
||
| public static String metricsAgg(List<Column> previousOutput) { | ||
| String outerCommand = randomFrom("min", "max", "sum", "count", "avg"); |
There was a problem hiding this comment.
Is there an easy way to include a wider range of aggregation functions that apply on numerics? E.g. std_dev and top should be applicable.
There was a problem hiding this comment.
I think this list is probably the closest thing we'd have to that? Could exclude the commands that don't apply? (*_over_time functions, spatial, i/rate, and those 3 internal functions?)
There was a problem hiding this comment.
Nice, let's give it a shot.
There was a problem hiding this comment.
reviewing the list a little bit more closely, it looks like some of the names (as NamedWriteableRegistry.Entry) might be a little bit different vs the ES|QL names (CountDistinct vs count_distinct) so this might not work actually....
kkrik-es
reviewed
Sep 10, 2025
...server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/generative/EsqlQueryGenerator.java
Show resolved
Hide resolved
kkrik-es
reviewed
Sep 10, 2025
| if (counterField == null) { | ||
| yield null; | ||
| } | ||
| yield "rate(" + counterField + ")"; |
There was a problem hiding this comment.
or last|first_over_time, once supported.
kkrik-es
reviewed
Sep 10, 2025
| }; | ||
| if (innerCommand == null) { | ||
| // TODO: figure out a default that maybe makes more sense than using a timestamp field | ||
| innerCommand = "count_over_time(" + randomDateField(previousOutput) + ")"; |
There was a problem hiding this comment.
Maybe * works, though not great.
There was a problem hiding this comment.
count(*) is the default in the normal Stats generator but it breaks when combined with time_series aggregations (bug 1), so until we fix that I don't think we can/should set that as the default here
There was a problem hiding this comment.
Yeah we'd need to think of the semantics.. Let's file an issue for the bug btw, good catch :)
There was a problem hiding this comment.
I think I will double-check the state of these bugs first before opening an issue; running a few queries I'm getting some completely new bugs so it's possible that some of them might be fixed.
Somewhat related, I think I saw that @not-napoleon was working on something similar to bug 3 (Output has changed)?
kkrik-es
reviewed
Sep 10, 2025
...java/org/elasticsearch/xpack/esql/qa/rest/generative/command/pipe/MetricsStatsGenerator.java
Show resolved
Hide resolved
kkrik-es
reviewed
Sep 10, 2025
|
Quick heads-up: we are moving the query generator to a more convenient place, so that we can reuse it in different contexts, see #134696 It's not merged yet and I can wait a bit, so please let me know if you are close to merging this one. I don't want to block your progress. |
|
@luigidellaquila thank you for letting us know! |
limotova
force-pushed
the
ts-in-generative-tests
branch
from
dfb9d59 to
5235b80
Compare
|
Sorry for the force push; the only thing it should've swallowed was the spotlessApply |
limotova
added
>test
kkrik-es
reviewed
Sep 16, 2025
...server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/generative/EsqlQueryGenerator.java
Show resolved
Hide resolved
|
Pinging @elastic/es-storage-engine (Team:StorageEngine) |
dnhatn
approved these changes
Sep 16, 2025
...a/org/elasticsearch/xpack/esql/qa/rest/generative/command/pipe/TimeSeriesStatsGenerator.java
Outdated
Show resolved
Hide resolved
limotova
force-pushed
the
ts-in-generative-tests
branch
from
cb6429d to
044e127
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
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.
This commit adds a single_node variation of the GenerativeRestTest in ES|QL that queries indices marked as time series indices and runs time series aggregations on them (amongst all the other commands already supported in the Generative tests), with a lot of limitations.