Skip to content
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

ES|QL: Improve random query generation tests #121750

Conversation

luigidellaquila
Copy link
Contributor

Fix and enable GenerativeIT, test that generates and executes random ES|QL queries on the sample csv-spec datasets.

For now the test considers a list of errors as acceptable (known bugs/limitations).

@luigidellaquila luigidellaquila added >test Issues or PRs that are addressing/adding tests :Analytics/ES|QL AKA ESQL labels Feb 5, 2025
@elasticsearchmachine elasticsearchmachine added Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) v9.1.0 labels Feb 5, 2025
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-analytical-engine (Team:Analytics)

@luigidellaquila
Copy link
Contributor Author

Apparently it's not time to enable this test yet #121754
I'll disable it and will only commit the improvements.

@luigidellaquila luigidellaquila changed the title ES|QL: enable random query generation tests ES|QL: Improve random query generation tests Feb 6, 2025
@luigidellaquila luigidellaquila added auto-backport Automatically create backport pull requests when merged v9.0.0 v8.18.1 v8.19.0 v9.0.1 labels Feb 7, 2025
@luigidellaquila
Copy link
Contributor Author

Thanks @idegtiarenko!

@elasticsearchmachine
Copy link
Collaborator

💚 Backport successful

Status Branch Result
9.0
8.18
8.x

luigidellaquila added a commit to luigidellaquila/elasticsearch that referenced this pull request Feb 7, 2025
Copy link
Member

@costin costin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor styling suggestions

return previousOutput.get(randomIntBetween(0, previousOutput.size() - 1)).name();
// we need to exclude <all-fields-projected>
// https://github.com/elastic/elasticsearch/issues/121741
return randomFrom(previousOutput.stream().filter(x -> x.name().equals("<all-fields-projected>") == false).toList()).name();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

extract "" into a constant

Comment on lines +236 to +241
return col.type.equals("keyword")
|| col.type.equals("text")
|| col.type.equals("long")
|| col.type.equals("integer")
|| col.type.equals("ip")
|| col.type.equals("version");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

put those types into a set and simply do a map.contains()

}

private static boolean sortable(Column col) {
return col.type.equals("keyword")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same comment as groupable

@luigidellaquila
Copy link
Contributor Author

Thanks @costin, I'll have to iterate on this, so I'll include your suggestions in next batch

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Analytics/ES|QL AKA ESQL auto-backport Automatically create backport pull requests when merged Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) >test Issues or PRs that are addressing/adding tests v8.18.1 v8.19.0 v9.0.0 v9.0.1 v9.1.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants