Added optional parameters to QSTR ES|QL function by svilen-mihaylov-elastic · Pull Request #121787 · elastic/elasticsearch

svilen-mihaylov-elastic · 2025-02-05T16:11:51Z

Adds options to QSTR function.

#118619 added named function parameters. This PR uses this mechanism for allowing query string function parameters, so query string parameters can be used in ES|QL.

Closes #120933

github-actions · 2025-02-10T21:27:35Z

Warning

It looks like this PR modifies one or more .asciidoc files. These files are being migrated to Markdown, and any changes merged now will be lost. See the migration guide for details.

github-actions · 2025-02-10T22:58:57Z

Warning

It looks like this PR modifies one or more .asciidoc files. These files are being migrated to Markdown, and any changes merged now will be lost. See the migration guide for details.

github-actions · 2025-02-11T20:37:54Z

Warning

It looks like this PR modifies one or more .asciidoc files. These files are being migrated to Markdown, and any changes merged now will be lost. See the migration guide for details.

svilen-mihaylov-elastic · 2025-02-11T20:41:38Z

Still TODO: add randomized test, take care of documentation warnings

...sql/src/main/java/org/elasticsearch/xpack/esql/expression/function/fulltext/QueryString.java

elasticsearchmachine · 2025-02-11T22:22:24Z

Pinging @elastic/es-search-relevance (Team:Search Relevance)

carlosdelest

This is looking good! 💯

Some things I miss:

CSV tests for having an end-to-end test of some option
Some testing in MatchTests that adds the function named param types

I left some other comments to address, but this is almost done.

carlosdelest · 2025-02-12T07:19:11Z

docs/reference/esql/functions/functionNamedParams/qstr.asciidoc

+|===
+name | types | description
+fuzziness | [keyword] | Maximum edit distance allowed for matching.
+auto_generate_synonyms_phrase_query | [boolean] | If true, match phrase queries are automatically created for multi-term synonyms.


I realize now that the docs generate options unsorted - we should probably change AbstractFunctionTestCase so it outputs an ordered options list.

No need to do in this PR, can be a follow up.

...sql/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

carlosdelest · 2025-02-12T07:26:45Z

x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/VerifierTests.java

+            String optionName = allowedOptions.getKey();
+            DataType optionType = allowedOptions.getValue();
+            // Check every possible type for the option - we'll try to convert it to the expected type
+            for (DataType currentType : optionTypes) {


We can probably extract this testing code to a separate method so both match and qstr can reuse it

carlosdelest · 2025-02-12T07:33:02Z

x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/querydsl/query/QueryStringQuery.java

This belongs to QL and not ESQL - I'd say let's keep this out of the scope for this PR

I understand that. Since I'm touching a related area, and since there was a TODO to update the constants to fields, I thought it is a good idea to make the change now that the query string builder constants are public.

...sql/src/main/java/org/elasticsearch/xpack/esql/expression/function/fulltext/QueryString.java

carlosdelest · 2025-02-12T08:15:11Z

docs/reference/esql/functions/types/qstr.asciidoc

We're missing the additional named param types in the tests, so the named parameters param is added to the function types. See how match did it here.

...ql/src/test/java/org/elasticsearch/xpack/esql/optimizer/LocalPhysicalPlanOptimizerTests.java

…expression/function/fulltext/QueryString.java Co-authored-by: Carlos Delgado <6339205+carlosdelest@users.noreply.github.com>

github-actions · 2025-02-12T21:16:41Z

Warning

It looks like this PR modifies one or more .asciidoc files. These files are being migrated to Markdown, and any changes merged now will be lost. See the migration guide for details.

carlosdelest

LGTM - Good work! 💯

There's a pending comment about adding default values to the options in the docs

carlosdelest · 2025-03-05T13:48:17Z

@svilen-mihaylov-elastic I'm adding backports for 8.19 and 9.0, plus auto-backport for creating them after merge

svilen-mihaylov-elastic · 2025-03-05T17:40:27Z

@elasticsearchmachine test this

elasticsearchmachine · 2025-03-06T09:31:56Z

Hi @svilen-mihaylov-elastic, I've updated the changelog YAML for you.

fang-xing-esql

LGTM, thank you @svilen-mihaylov-elastic!

elasticsearchmachine · 2025-03-07T18:01:47Z

💔 Backport failed

Status	Branch	Result
❌	9.0	Commit could not be cherrypicked due to conflicts
❌	8.x	Commit could not be cherrypicked due to conflicts

You can use sqren/backport to manually backport by running backport --upstream elastic/elasticsearch --pr 121787

Adds options to QSTR function. elastic#118619 added named function parameters. This PR uses this mechanism for allowing query string function parameters, so query string parameters can be used in ES|QL. Closes elastic#120933

Adds options to QSTR function. elastic#118619 added named function parameters. This PR uses this mechanism for allowing query string function parameters, so query string parameters can be used in ES|QL. Closes elastic#120933 (cherry picked from commit ee4bcac) # Conflicts: # x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/AnalyzerTests.java # x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/VerifierTests.java

svilen-mihaylov-elastic · 2025-03-18T15:08:31Z

💚 All backports created successfully

Status	Branch	Result
✅	8.x
✅	9.0

Questions ?

Please refer to the Backport tool documentation

Adds options to QSTR function. elastic#118619 added named function parameters. This PR uses this mechanism for allowing query string function parameters, so query string parameters can be used in ES|QL. Closes elastic#120933 (cherry picked from commit ee4bcac) # Conflicts: # x-pack/plugin/esql/qa/testFixtures/src/main/resources/qstr-function.csv-spec # x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/AnalyzerTests.java

…5114) * Added optional parameters to QSTR ES|QL function (#121787) Adds options to QSTR function. #118619 added named function parameters. This PR uses this mechanism for allowing query string function parameters, so query string parameters can be used in ES|QL. Closes #120933 (cherry picked from commit ee4bcac) # Conflicts: # x-pack/plugin/esql/qa/testFixtures/src/main/resources/qstr-function.csv-spec # x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/AnalyzerTests.java * [CI] Auto commit changes from spotless * Fix compilation error * [CI] Auto commit changes from spotless --------- Co-authored-by: elasticsearchmachine <infra-root+elasticsearchmachine@elastic.co>

…5112) * Added optional parameters to QSTR ES|QL function (#121787) Adds options to QSTR function. #118619 added named function parameters. This PR uses this mechanism for allowing query string function parameters, so query string parameters can be used in ES|QL. Closes #120933 (cherry picked from commit ee4bcac) # Conflicts: # x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/AnalyzerTests.java # x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/VerifierTests.java * [CI] Auto commit changes from spotless * Fix compilation error * Fix missing import * getFirst() -> get(0) * Make method public * Make asBuilder public in subclasses * Revert "Make asBuilder public in subclasses" This reverts commit f444aa6. * Revert "Make method public" This reverts commit a1d9f56. * .asQueryBuilder() -> .toQueryBuilder() * Remove extraneous change which sneaked in during backport --------- Co-authored-by: elasticsearchmachine <infra-root+elasticsearchmachine@elastic.co>

svilen-mihaylov-elastic marked this pull request as draft February 5, 2025 16:12

elasticsearchmachine added the v9.1.0 label Feb 5, 2025

svilen-mihaylov-elastic force-pushed the svilen/120933 branch 8 times, most recently from d15c264 to 6e70003 Compare February 10, 2025 21:27

svilen-mihaylov-elastic force-pushed the svilen/120933 branch from 6e70003 to 95fb7a0 Compare February 10, 2025 22:58

Initial commit

66ce0bc

svilen-mihaylov-elastic force-pushed the svilen/120933 branch from 95fb7a0 to 66ce0bc Compare February 11, 2025 20:37

svilen-mihaylov-elastic requested a review from carlosdelest February 11, 2025 20:40

svilen-mihaylov-elastic marked this pull request as ready for review February 11, 2025 20:40

elasticsearchmachine added the needs:triage Requires assignment of a team area label label Feb 11, 2025

svilen-mihaylov-elastic commented Feb 11, 2025

View reviewed changes

...sql/src/main/java/org/elasticsearch/xpack/esql/expression/function/fulltext/QueryString.java Show resolved Hide resolved

svilen-mihaylov-elastic added the Team:Search Relevance Meta label for the Search Relevance team in Elasticsearch label Feb 11, 2025

elasticsearchmachine removed the Team:Search Relevance Meta label for the Search Relevance team in Elasticsearch label Feb 11, 2025

svilen-mihaylov-elastic added Team:Search Relevance Meta label for the Search Relevance team in Elasticsearch :Search Relevance/Search Catch all for Search Relevance labels Feb 11, 2025

elasticsearchmachine removed the needs:triage Requires assignment of a team area label label Feb 11, 2025

carlosdelest reviewed Feb 12, 2025

View reviewed changes

Update x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/…

e813749

…expression/function/fulltext/QueryString.java Co-authored-by: Carlos Delgado <6339205+carlosdelest@users.noreply.github.com>

carlosdelest approved these changes Mar 5, 2025

View reviewed changes

carlosdelest requested review from ChrisHegarty, fang-xing-esql, ioanatia and tteofili March 5, 2025 13:47

carlosdelest added auto-backport Automatically create backport pull requests when merged v9.0.0 v8.19.0 labels Mar 5, 2025

Add more defaults to descriptions

af2df81

svilen-mihaylov-elastic added 2 commits March 5, 2025 13:58

Merge branch 'main' into svilen/120933

be64140

Update docs/changelog/121787.yaml

2c10ee9

ChrisHegarty approved these changes Mar 6, 2025

View reviewed changes

fang-xing-esql approved these changes Mar 6, 2025

View reviewed changes

ioanatia approved these changes Mar 6, 2025

View reviewed changes

Merge branch 'main' into svilen/120933

f72e524

svilen-mihaylov-elastic merged commit ee4bcac into elastic:main Mar 7, 2025
16 of 17 checks passed

elasticsearchmachine added the backport pending label Mar 7, 2025

svilen-mihaylov-elastic mentioned this pull request Mar 18, 2025

[8.x] Added optional parameters to QSTR ES|QL function (#121787) #125112

Merged

svilen-mihaylov-elastic mentioned this pull request Mar 18, 2025

[9.0] Added optional parameters to QSTR ES|QL function (#121787) #125114

Merged

carlosdelest mentioned this pull request Mar 21, 2025

ES|QL QSTR roadmap #116930

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added optional parameters to QSTR ES|QL function#121787

Added optional parameters to QSTR ES|QL function#121787
svilen-mihaylov-elastic merged 27 commits intoelastic:mainfrom
svilen-mihaylov-elastic:svilen/120933

svilen-mihaylov-elastic commented Feb 5, 2025 •

edited by carlosdelest

Loading

github-actions bot commented Feb 10, 2025

github-actions bot commented Feb 10, 2025

github-actions bot commented Feb 11, 2025

svilen-mihaylov-elastic commented Feb 11, 2025

Uh oh!

elasticsearchmachine commented Feb 11, 2025

carlosdelest left a comment

carlosdelest Feb 12, 2025

Uh oh!

Uh oh!

carlosdelest Feb 12, 2025

carlosdelest Feb 12, 2025

svilen-mihaylov-elastic Feb 12, 2025 •

edited

Loading

Uh oh!

Uh oh!

carlosdelest Feb 12, 2025

Uh oh!

github-actions bot commented Feb 12, 2025

carlosdelest left a comment

carlosdelest commented Mar 5, 2025

svilen-mihaylov-elastic commented Mar 5, 2025

elasticsearchmachine commented Mar 6, 2025

fang-xing-esql left a comment

Uh oh!

elasticsearchmachine commented Mar 7, 2025

svilen-mihaylov-elastic commented Mar 18, 2025

Labels

6 participants

Conversation

svilen-mihaylov-elastic commented Feb 5, 2025 • edited by carlosdelest Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

github-actions bot commented Feb 10, 2025

github-actions bot commented Feb 10, 2025

github-actions bot commented Feb 11, 2025

svilen-mihaylov-elastic commented Feb 11, 2025

Uh oh!

elasticsearchmachine commented Feb 11, 2025

carlosdelest left a comment

Choose a reason for hiding this comment

carlosdelest Feb 12, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

carlosdelest Feb 12, 2025

Choose a reason for hiding this comment

carlosdelest Feb 12, 2025

Choose a reason for hiding this comment

svilen-mihaylov-elastic Feb 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

carlosdelest Feb 12, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Feb 12, 2025

carlosdelest left a comment

Choose a reason for hiding this comment

carlosdelest commented Mar 5, 2025

svilen-mihaylov-elastic commented Mar 5, 2025

elasticsearchmachine commented Mar 6, 2025

fang-xing-esql left a comment

Choose a reason for hiding this comment

Uh oh!

elasticsearchmachine commented Mar 7, 2025

💔 Backport failed

svilen-mihaylov-elastic commented Mar 18, 2025

💚 All backports created successfully

Questions ?

Labels

6 participants

svilen-mihaylov-elastic commented Feb 5, 2025 •

edited by carlosdelest

Loading

svilen-mihaylov-elastic Feb 12, 2025 •

edited

Loading