-
Notifications
You must be signed in to change notification settings - Fork 25.3k
[ES|QL] Add MATCH_PHRASE #127661
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] Add MATCH_PHRASE #127661
Conversation
dabef06
to
13ccda7
Compare
Hi @kderusso, I've created a changelog YAML for you. |
8f01955
to
d02864e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's important that we register the match_phrase
as snapshot and get some tests ready for release builds.
Besides that, some small nits.
...sql/src/main/java/org/elasticsearch/xpack/esql/expression/function/EsqlFunctionRegistry.java
Outdated
Show resolved
Hide resolved
...sql/src/main/java/org/elasticsearch/xpack/esql/expression/function/fulltext/MatchPhrase.java
Outdated
Show resolved
Hide resolved
...sql/src/main/java/org/elasticsearch/xpack/esql/expression/function/fulltext/MatchPhrase.java
Outdated
Show resolved
Hide resolved
...sql/src/main/java/org/elasticsearch/xpack/esql/expression/function/fulltext/MatchPhrase.java
Outdated
Show resolved
Hide resolved
@@ -221,7 +221,7 @@ setup: | |||
- gt: {esql.functions.to_long: $functions_to_long} | |||
- match: {esql.functions.coalesce: $functions_coalesce} | |||
- gt: {esql.functions.categorize: $functions_categorize} | |||
- length: {esql.functions: 134} # check the "sister" test above for a likely update to the same esql.functions length check | |||
- length: {esql.functions: 135} # check the "sister" test above for a likely update to the same esql.functions length check |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hopefully those errors get corrected when registering the function under snapshot functions 🤞
@@ -221,7 +221,7 @@ setup: | |||
- gt: {esql.functions.to_long: $functions_to_long} | |||
- match: {esql.functions.coalesce: $functions_coalesce} | |||
- gt: {esql.functions.categorize: $functions_categorize} | |||
- length: {esql.functions: 134} # check the "sister" test above for a likely update to the same esql.functions length check | |||
- length: {esql.functions: 135} # check the "sister" test above for a likely update to the same esql.functions length check |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you'll need to add match_phrase
capability to these tests, otherwise they will fail for release as the function will not be registered
@elasticmachine test this please |
I checked the release tests failures - there is nothing pertaining to |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, great work! 💯
There are other tests that can be added to VerifierTests
- look for EsqlCapabilities.Cap.MULTI_MATCH_FUNCTION.isEnabled()
and you'll be able to add an equivalent one for match_phrase
💔 Backport failed
You can use sqren/backport to manually backport by running |
* Initial commit of match_phrase * Add MatchPhraseQueryTests * First pass at CSV specs * Update docs/changelog/127661.yaml * Refactor so MatchPhrase doesn't use all fulltext test cases, just text only * Fix tests * Add some CSV test cases * Fix test * Update changelog * Update tests * Comment out MATCH_PHRASE in search-functions Markdown * Minor PR feedback * PR feedback - refactor/consolidate code * Add some more tests * Fix some tests * [CI] Auto commit changes from spotless * Fix tests * PR feedback - add tests, support boost and numeric data * Revert "PR feedback - add tests, support boost and numeric data" This reverts commit 4e7a699. * Apply testing/PR feedback outside numeric support only * Regenerate docs * Add negative test * Update x-pack/plugin/esql/qa/testFixtures/src/main/resources/match-phrase-function.csv-spec Co-authored-by: Carlos Delgado <6339205+carlosdelest@users.noreply.github.com> * Update x-pack/plugin/esql/qa/testFixtures/src/main/resources/match-phrase-function.csv-spec Co-authored-by: Carlos Delgado <6339205+carlosdelest@users.noreply.github.com> * Update x-pack/plugin/esql/qa/testFixtures/src/main/resources/match-phrase-function.csv-spec Co-authored-by: Carlos Delgado <6339205+carlosdelest@users.noreply.github.com> * PR feedback * Fix auto-commit error * Regenerate docs * Update x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/fulltext/MatchPhrase.java Co-authored-by: Liam Thompson <32779855+leemthompo@users.noreply.github.com> * Remove non text field types * Fake test data * Remove tests that no longer should pass without ip/date/version support * Put real data in score tests now that I was able to engineer a failure * Realized the scoring test might be flakey because how it was written, updated * PR feedback * PR feedback * [CI] Auto commit changes from spotless * Add check to MatchPhrase tests * Fix merge errors * [CI] Auto commit changes from spotless * Test generated docs * Add additional verifier tests --------- Co-authored-by: elasticsearchmachine <infra-root+elasticsearchmachine@elastic.co> Co-authored-by: Carlos Delgado <6339205+carlosdelest@users.noreply.github.com> Co-authored-by: Liam Thompson <32779855+leemthompo@users.noreply.github.com> (cherry picked from commit eee423a) # Conflicts: # docs/reference/esql/functions/description/match_phrase.md # docs/reference/esql/functions/examples/match_phrase.md # docs/reference/esql/functions/kibana/definition/match_phrase.json # docs/reference/esql/functions/kibana/docs/match_phrase.md # docs/reference/query-languages/esql/_snippets/lists/search-functions.md # docs/reference/query-languages/esql/functions-operators/search-functions.md # x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlCapabilities.java # x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/EsqlFunctionRegistry.java # x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/fulltext/FullTextFunction.java # x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/fulltext/Match.java # x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/fulltext/QueryString.java # x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/VerifierTests.java
💚 All backports created successfully
Questions ?Please refer to the Backport tool documentation |
Backport PR: #129215 |
* [ES|QL] Add MATCH_PHRASE (#127661) * Initial commit of match_phrase * Add MatchPhraseQueryTests * First pass at CSV specs * Update docs/changelog/127661.yaml * Refactor so MatchPhrase doesn't use all fulltext test cases, just text only * Fix tests * Add some CSV test cases * Fix test * Update changelog * Update tests * Comment out MATCH_PHRASE in search-functions Markdown * Minor PR feedback * PR feedback - refactor/consolidate code * Add some more tests * Fix some tests * [CI] Auto commit changes from spotless * Fix tests * PR feedback - add tests, support boost and numeric data * Revert "PR feedback - add tests, support boost and numeric data" This reverts commit 4e7a699. * Apply testing/PR feedback outside numeric support only * Regenerate docs * Add negative test * Update x-pack/plugin/esql/qa/testFixtures/src/main/resources/match-phrase-function.csv-spec Co-authored-by: Carlos Delgado <6339205+carlosdelest@users.noreply.github.com> * Update x-pack/plugin/esql/qa/testFixtures/src/main/resources/match-phrase-function.csv-spec Co-authored-by: Carlos Delgado <6339205+carlosdelest@users.noreply.github.com> * Update x-pack/plugin/esql/qa/testFixtures/src/main/resources/match-phrase-function.csv-spec Co-authored-by: Carlos Delgado <6339205+carlosdelest@users.noreply.github.com> * PR feedback * Fix auto-commit error * Regenerate docs * Update x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/fulltext/MatchPhrase.java Co-authored-by: Liam Thompson <32779855+leemthompo@users.noreply.github.com> * Remove non text field types * Fake test data * Remove tests that no longer should pass without ip/date/version support * Put real data in score tests now that I was able to engineer a failure * Realized the scoring test might be flakey because how it was written, updated * PR feedback * PR feedback * [CI] Auto commit changes from spotless * Add check to MatchPhrase tests * Fix merge errors * [CI] Auto commit changes from spotless * Test generated docs * Add additional verifier tests --------- Co-authored-by: elasticsearchmachine <infra-root+elasticsearchmachine@elastic.co> Co-authored-by: Carlos Delgado <6339205+carlosdelest@users.noreply.github.com> Co-authored-by: Liam Thompson <32779855+leemthompo@users.noreply.github.com> (cherry picked from commit eee423a) # Conflicts: # docs/reference/esql/functions/description/match_phrase.md # docs/reference/esql/functions/examples/match_phrase.md # docs/reference/esql/functions/kibana/definition/match_phrase.json # docs/reference/esql/functions/kibana/docs/match_phrase.md # docs/reference/query-languages/esql/_snippets/lists/search-functions.md # docs/reference/query-languages/esql/functions-operators/search-functions.md # x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlCapabilities.java # x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/EsqlFunctionRegistry.java # x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/fulltext/FullTextFunction.java # x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/fulltext/Match.java # x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/fulltext/QueryString.java # x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/VerifierTests.java * Take match_phrase out of snapshot and make tech preview (#128925) * Take match_phrase out of snapshot and make tech preview * Update docs/changelog/128925.yaml * PR feedback * Adding regenerated test data * Update docs/changelog/128925.yaml Co-authored-by: Carlos Delgado <6339205+carlosdelest@users.noreply.github.com> * [CI] Auto commit changes from spotless * Checkstyle * Correct docs * Hopefully fix docs build * Found one more bad docs link - here's hoping this now fixes the doc build * OMG bitten by - vs _ --------- Co-authored-by: Carlos Delgado <6339205+carlosdelest@users.noreply.github.com> Co-authored-by: elasticsearchmachine <infra-root+elasticsearchmachine@elastic.co> Co-authored-by: Aurélien FOUCRET <aurelien.foucret@gmail.com> * Remove null example for match_phrase (#129173) * Fix errors from merge * Fix more errors from merge * [CI] Auto commit changes from spotless * Fix compile errors introduced through cherry-pick * Fix test compilation * Generate docs * Register match_phrase as a function not a snapshot function (#129255) * Register match_phrase as a function not a snapshot function * Update usage * Fix usage --------- Co-authored-by: Carlos Delgado <6339205+carlosdelest@users.noreply.github.com> Co-authored-by: elasticsearchmachine <infra-root+elasticsearchmachine@elastic.co> Co-authored-by: Aurélien FOUCRET <aurelien.foucret@gmail.com> Co-authored-by: Ioana Tagirta <ioanatia@users.noreply.github.com>
Adds
MATCH_PHRASE
support to ES|QL.Some examples of how to use
MATCH_PHRASE
: