Skip to content

Conversation

@ioanatia
Copy link
Contributor

We decided to replace the match command with the qstr function (added in #112590)

@ioanatia ioanatia added >non-issue Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) auto-backport Automatically create backport pull requests when merged :Analytics/ES|QL AKA ESQL v8.16.0 labels Sep 23, 2024
@ioanatia ioanatia marked this pull request as ready for review September 23, 2024 17:23
@elasticsearchmachine
Copy link
Collaborator

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

Copy link
Member

@carlosdelest carlosdelest left a comment

Choose a reason for hiding this comment

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

LGTM ✂️

Copy link
Contributor

@ChrisHegarty ChrisHegarty left a comment

Choose a reason for hiding this comment

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

LGTM

// TODO Keep adding tests for all unsupported commands
}

private void assertMatchCommand(String lineAndColumn, String command, String query) {
Copy link
Member

Choose a reason for hiding this comment

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

If there is no test besides testMatchCommand calls assertMatchCommand, it can also be removed.

Copy link
Member

@fang-xing-esql fang-xing-esql left a comment

Choose a reason for hiding this comment

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

LGTM, just wait for CI to be cleared, thank you!

@astefan astefan added the test-release Trigger CI checks against release build label Sep 25, 2024
@astefan
Copy link
Contributor

astefan commented Sep 25, 2024

@elasticmachine run elasticsearch-ci/release-tests

@ioanatia
Copy link
Contributor Author

For the release-tests, the failures are related to #113496 which don't have to do with the changes we have here.
We don't have any other ES|QL tests failing.
I don't see anything else in the release tests related to ES|QL - we have some other tasks for security/ML etc.
So this should be good to merge!

@ioanatia ioanatia merged commit a8d7f79 into elastic:main Sep 25, 2024
@elasticsearchmachine
Copy link
Collaborator

💚 Backport successful

Status Branch Result
8.x

ioanatia added a commit to ioanatia/elasticsearch that referenced this pull request Sep 25, 2024
* Remove match command

* Remove assertMatchCommand

* Remove unused import
ioanatia added a commit that referenced this pull request Sep 26, 2024
* Remove match command

* Remove assertMatchCommand

* Remove unused import

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
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 >non-issue Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) test-release Trigger CI checks against release build v8.16.0 v9.0.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants