Skip to content

Conversation

@ivancea
Copy link
Contributor

@ivancea ivancea commented Feb 25, 2025

Fixes #123067

Just like WildcardLike and RLike, some functions can be converted to Lucene queries. Here it's those two, which are nearly identical to WildcardLike

This, like some other functions, needs a FoldContext. I'm using the static one for this here, but it's fixed in #123398, which I kept separated as it changes many files

@ivancea ivancea added >enhancement 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.18.0 v9.0.1 v9.1.0 labels Feb 25, 2025
@elasticsearchmachine
Copy link
Collaborator

Hi @ivancea, I've created a changelog YAML for you.

@ivancea ivancea marked this pull request as ready for review February 25, 2025 17:28
@elasticsearchmachine
Copy link
Collaborator

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

Copy link
Contributor

@astefan astefan left a comment

Choose a reason for hiding this comment

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

This is a good start in terms of testing this, but the tests are the bare minimum.
Let's make this functionality more robust by having stronger tests, please. For CSV (notice also the _index metadata which is a feature of pushable attributes):

  • from employees | where starts_with(first_name, "A") | keep first_name, last_name | where ends_with(last_name, "g")
  • from employees* metadata _index | where starts_with(first_name::keyword, "A") | keep first_name, last_name, _index | where ends_with(last_name::keyword, "g") and starts_with(_index, "e")
  • from e* metadata _index | where starts_with(first_name::keyword, "A") | keep first_name, last_name, languages.long, _index | where ends_with(last_name::keyword, "g") and starts_with(_index, "e")
  • from e* metadata _index | where starts_with(first_name::keyword, "A") | keep first_name, last_name, languages.long, _index | where ends_with(last_name::keyword, "g") and ends_with(_index, "e")
  • from employees metadata _index | where starts_with(first_name, "A") | keep first_name, last_name, languages.long, _index | where ends_with(_index, "s") and (ends_with(last_name, "e") or ends_with(last_name, "m")
  • from employees metadata _index | where starts_with(first_name, "A") | keep first_name, last_name, languages.long, _index | where ends_with(_index, "s") and not (ends_with(last_name, "e") or ends_with(last_name, "m")

For unit testing, please look at PhysicalPlanOptimizerTests.testPushAndInequalitiesFilter and make sure that the EsQueryExec that gets created includes the actual wildcard query in its proper form. Please, also use a query with two statements so we can see the bool query getting created as well.

@ivancea ivancea changed the title ESQL: Push down StarsWith and EndsWith functions to Lucene ESQL: Push down StartsWith and EndsWith functions to Lucene Feb 28, 2025
@ivancea
Copy link
Contributor Author

ivancea commented Mar 5, 2025

@astefan Just added some extra CSV tests with some combinations, with and without _index and wildcards.
For the PhysicalPlanOptimizerTests tests, I just added one with a combination of ANDs and ORs, to ensure it works as it should. I didn't add much more here, as it's generic, already tested code (AND and OR functions), and the StartsWith and EndsWith functions already have unit tests for translations.
WDYT?

@ivancea ivancea requested a review from astefan March 6, 2025 10:08
Copy link
Contributor

Choose a reason for hiding this comment

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

Please, add as a javadoc the expected physicalplan tree. I know some methods in this class do not have this, but I (or we) try to have this consistent throughout the class. Thanks.

Copy link
Contributor

@astefan astefan left a comment

Choose a reason for hiding this comment

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

LGTM with a minor comment regarding the unit test.

@ivancea ivancea enabled auto-merge (squash) March 11, 2025 16:35
@ivancea ivancea merged commit 2fff041 into elastic:main Mar 11, 2025
16 of 17 checks passed
@ivancea ivancea deleted the pushdown-starts-with branch March 11, 2025 18:14
@elasticsearchmachine
Copy link
Collaborator

💚 Backport successful

Status Branch Result
8.x
9.0

ivancea added a commit to ivancea/elasticsearch that referenced this pull request Mar 11, 2025
…123381)

Fixes elastic#123067

Just like WildcardLike and RLike, some functions can be converted to Lucene queries. Here it's those two, which are nearly identical to WildcardLike

This, like some other functions, needs a FoldContext. I'm using the static method for this here, but it's fixed in elastic#123398, which I kept separated as it changes many files
ivancea added a commit to ivancea/elasticsearch that referenced this pull request Mar 11, 2025
…123381)

Fixes elastic#123067

Just like WildcardLike and RLike, some functions can be converted to Lucene queries. Here it's those two, which are nearly identical to WildcardLike

This, like some other functions, needs a FoldContext. I'm using the static method for this here, but it's fixed in elastic#123398, which I kept separated as it changes many files
elasticsearchmachine pushed a commit that referenced this pull request Mar 11, 2025
…#124583)

Fixes #123067

Just like WildcardLike and RLike, some functions can be converted to Lucene queries. Here it's those two, which are nearly identical to WildcardLike

This, like some other functions, needs a FoldContext. I'm using the static method for this here, but it's fixed in #123398, which I kept separated as it changes many files
elasticsearchmachine pushed a commit that referenced this pull request Mar 11, 2025
…#124582)

Fixes #123067

Just like WildcardLike and RLike, some functions can be converted to Lucene queries. Here it's those two, which are nearly identical to WildcardLike

This, like some other functions, needs a FoldContext. I'm using the static method for this here, but it's fixed in #123398, which I kept separated as it changes many files
albertzaharovits pushed a commit to albertzaharovits/elasticsearch that referenced this pull request Mar 13, 2025
…123381)

Fixes elastic#123067

Just like WildcardLike and RLike, some functions can be converted to Lucene queries. Here it's those two, which are nearly identical to WildcardLike

This, like some other functions, needs a FoldContext. I'm using the static method for this here, but it's fixed in elastic#123398, which I kept separated as it changes many files
jfreden pushed a commit to jfreden/elasticsearch that referenced this pull request Mar 13, 2025
…123381)

Fixes elastic#123067

Just like WildcardLike and RLike, some functions can be converted to Lucene queries. Here it's those two, which are nearly identical to WildcardLike

This, like some other functions, needs a FoldContext. I'm using the static method for this here, but it's fixed in elastic#123398, which I kept separated as it changes many files
ivancea added a commit to ivancea/elasticsearch that referenced this pull request Mar 14, 2025
…123381)

Fixes elastic#123067

Just like WildcardLike and RLike, some functions can be converted to Lucene queries. Here it's those two, which are nearly identical to WildcardLike

This, like some other functions, needs a FoldContext. I'm using the static method for this here, but it's fixed in elastic#123398, which I kept separated as it changes many files
@ivancea
Copy link
Contributor Author

ivancea commented Mar 14, 2025

Also backporting to 8.18 in #124871

ivancea added a commit that referenced this pull request Mar 17, 2025
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 >enhancement Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) v8.19.0 v9.0.1 v9.1.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ESQL: pushdown starts_with , ends_with functions

4 participants