Add multi_match function #121525 #125062

svilen-mihaylov-elastic · 2025-03-17T22:59:27Z

tracked in #121525

svilen-mihaylov-elastic · 2025-03-20T15:29:48Z

TODO: add tests

carlosdelest

This is looking good! 💯

Things we need to check:

QueryBuilder serialization / deserialization
Testing:
- CSV tests, and include scoring test in scoring.csv
- Add this function to the ScoringIT tests so scoring is checked
- The usual suspects: VerifierTests, LocalPhysicalPlanOptimizerTests, MultiMatchTests (new), LogicalPlanOptimizerTests)

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

...c/main/java/org/elasticsearch/xpack/esql/expression/function/fulltext/FullTextWritables.java

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

…expression/function/fulltext/MultiMatch.java Co-authored-by: Carlos Delgado <[email protected]>

…astic/elasticsearch into svilen/121525

carlosdelest

A question on the handling of options, and some final testing that I'd like us to have. The rest LGTM 🙌

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

carlosdelest · 2025-04-09T14:50:21Z

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

@@ -1656,6 +1657,33 @@ public void testQStrOptionsPushDown() {
        assertThat(expectedQStrQuery.toString(), is(planStr.get()));
    }

+    public void testMultiMatchOptionsPushDown() {
+        String query = """


I still don't see validation that the query retrieves options for zero_terms_query or boost - they should be easy to add and we get complete coverage that all options work as expected

x-pack/plugin/esql/qa/testFixtures/src/main/resources/multi-match-function.csv-spec

carlosdelest · 2025-04-09T15:29:12Z

x-pack/plugin/esql/qa/testFixtures/src/main/resources/scoring.csv-spec

+required_capability: metadata_score
+
+from books metadata _score
+| where multi_match("Hobbit", description, title, {"type": "most_fields"})


Nice work on combining the scores via most_fields and best_fields 👏

x-pack/plugin/esql/qa/testFixtures/src/main/resources/multi-match-function.csv-spec

...ugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/fulltext/Match.java

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

carlosdelest · 2025-04-09T16:48:17Z

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

+    }
+
+    private MultiMatch(Source source, Expression query, List<Expression> fields, Expression options, QueryBuilder queryBuilder) {
+        super(source, query, initChildren(query, fields, options), queryBuilder);


I wonder if this would be better handled by just having a List fieldsAndOptions, instead of separating the two. This seems to be making us check for options in a variety of places and do some gymnastics for the constructor.

We could keep a single List and then check for the last element in the options() field to return that or not.

@ioanatia any opinion on this?

The reason why I wanted to keep them separate was because of the way we use the @interfaces like @MapParam from the constructor arguments to generate the docs.
If all we had was a list of children, that might mean changing the way the docs are generated for functions?
I am not sure that would be less awkward - neither way seems ideal.

The docs generation comment is spot on. Let's keep it this way and iterate if needed.

…tch-function.csv-spec Co-authored-by: Carlos Delgado <[email protected]>

…optimizer/LocalPhysicalPlanOptimizerTests.java Co-authored-by: Carlos Delgado <[email protected]>

carlosdelest

Thank you for iterating on this, @svilen-mihaylov-elastic !

I'd like to have the additional tests for phrase and phrase-prefix included, but this LGTM. Great work! 💯

svilen-mihaylov-elastic · 2025-04-10T16:44:01Z

Thank you for iterating on this, @svilen-mihaylov-elastic !

I'd like to have the additional tests for phrase and phrase-prefix included, but this LGTM. Great work! 💯

Is there anything you need on top of this? 937963b

ioanatia · 2025-04-14T16:57:57Z

docs/reference/query-languages/esql/kibana/docs/functions/date_trunc.md

@@ -1,6 +1,5 @@
 % This is generated by ESQL's AbstractFunctionTestCase. Do no edit it. See ../README.md for how to regenerate it.

-### DATE TRUNC


this looks like an unintended change - let's revert it

ioanatia · 2025-04-14T17:10:10Z

...gin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/LogicalPlanOptimizerTests.java

@@ -7378,6 +7379,24 @@ public void testFunctionNamedParamsAsFunctionArgument() {
        assertEquals(DataType.DOUBLE, ee.dataType());
    }

+    public void testFunctionNamedParamsAsFunctionArgument1() {


as a side note I don't see a difference between this test and the one we add to AnalyzerTests.
I guess we just follow an existing pattern - but as a follow up, we should discuss if both are needed.

Implement multi_match function for ESQL. Its currently available on snapshot builds pending refinement of the syntax.

craigtaverner · 2025-04-17T08:58:53Z

I notice that the examples/multi_match.md file contains an example that does not match the example in the csv-spec file. Presumably the csv-spec was changed later, without re-generating the examples. The consequence is that someone else, later, will regenerate examples and get a surprise multi-match change to their PR.

Initial

d392b17

svilen-mihaylov-elastic changed the title ~~Initial~~ Add multi_match function #121525 Mar 17, 2025

github-actions bot deployed to docs-preview March 17, 2025 22:59 View deployment

elasticsearchmachine added the v9.1.0 label Mar 17, 2025

svilen-mihaylov-elastic added 2 commits March 19, 2025 14:22

Apply Ioana's changes

18559d0

incremental work

dd28440

github-actions bot deployed to docs-preview March 19, 2025 21:21 View deployment

Fix tests

4e67cb2

svilen-mihaylov-elastic requested review from carlosdelest and ioanatia March 20, 2025 15:29

svilen-mihaylov-elastic added 5 commits March 20, 2025 14:21

Merge branch 'main' into svilen/121525

e871d38

Fix

6ab6baa

Fix field names

a2a5fde

Merge branch 'main' into svilen/121525

bb57f52

One more function

257caf9

carlosdelest reviewed Mar 21, 2025

View reviewed changes

svilen-mihaylov-elastic and others added 13 commits March 21, 2025 13:12

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

28ca7e7

…expression/function/fulltext/MultiMatch.java Co-authored-by: Carlos Delgado <[email protected]>

cr feedback

78f6c52

Merge branch 'svilen/121525' of https://github.com/svilen-mihaylov-el…

07d6230

…astic/elasticsearch into svilen/121525

Fix serialization/deserialization

d280906

Remove TODO

9a72c76

Add analyzer test

27d4138

Add verifier test

099a555

More tests

9d48454

Add test

651c28f

Merge branch 'main' into svilen/121525

7e06f77

lint

fb53d2b

Add test and updates

422e4dd

Merge branch 'main' into svilen/121525

c56cab6

svilen-mihaylov-elastic added 2 commits April 8, 2025 15:10

Merge branch 'svilen/121525' of https://github.com/svilen-mihaylov-el…

51a7e6b

…astic/elasticsearch into svilen/121525

Share options code between match and multi_match

fa3d50a

svilen-mihaylov-elastic requested a review from carlosdelest April 9, 2025 13:34

carlosdelest reviewed Apr 9, 2025

View reviewed changes

svilen-mihaylov-elastic and others added 11 commits April 9, 2025 14:55

Update x-pack/plugin/esql/qa/testFixtures/src/main/resources/multi-ma…

050f086

…tch-function.csv-spec Co-authored-by: Carlos Delgado <[email protected]>

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

665c32e

…optimizer/LocalPhysicalPlanOptimizerTests.java Co-authored-by: Carlos Delgado <[email protected]>

Add boost field

56888af

Move static function

02e2b75

Update test

4774dbe

Uses populateOptionsMap from QueryString

999b820

Merge branch 'main' into svilen/121525

8a74c79

Extend test

937963b

add test

6703882

Merge branch 'main' into svilen/121525

44d6721

+1

1cdf7ed

carlosdelest approved these changes Apr 10, 2025

View reviewed changes

svilen-mihaylov-elastic added 4 commits April 10, 2025 14:16

Merge branch 'main' into svilen/121525

58d37f8

Add required capability

9b90726

Merge branch 'main' into svilen/121525

9c6b73d

Indicate multimatch is behind a snapshot

b5d6edb

svilen-mihaylov-elastic requested a review from ChrisHegarty April 14, 2025 13:14

svilen-mihaylov-elastic added 2 commits April 14, 2025 12:32

Merge branch 'main' into svilen/121525

0b928d1

Update count

3ea5092

ioanatia approved these changes Apr 14, 2025

View reviewed changes

svilen-mihaylov-elastic added 2 commits April 14, 2025 18:00

revert

b526b81

Merge branch 'main' into svilen/121525

9c37690

svilen-mihaylov-elastic merged commit 02f9af7 into elastic:main Apr 15, 2025
16 of 17 checks passed

afoucret pushed a commit to afoucret/elasticsearch that referenced this pull request Apr 16, 2025

Add multi_match function elastic#121525 (elastic#125062)

cb2d99c

Implement multi_match function for ESQL. Its currently available on snapshot builds pending refinement of the syntax.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add multi_match function #121525 #125062

Add multi_match function #121525 #125062

svilen-mihaylov-elastic commented Mar 17, 2025 •

edited by ioanatia

Loading

svilen-mihaylov-elastic commented Mar 20, 2025

carlosdelest left a comment

carlosdelest left a comment

carlosdelest Apr 9, 2025

carlosdelest Apr 9, 2025

carlosdelest Apr 9, 2025

svilen-mihaylov-elastic Apr 9, 2025

ioanatia Apr 10, 2025 •

edited

Loading

carlosdelest Apr 10, 2025

carlosdelest left a comment

svilen-mihaylov-elastic commented Apr 10, 2025

ioanatia Apr 14, 2025

ioanatia Apr 14, 2025

craigtaverner commented Apr 17, 2025

		@@ -1,6 +1,5 @@
		% This is generated by ESQL's AbstractFunctionTestCase. Do no edit it. See ../README.md for how to regenerate it.

		### DATE TRUNC

Add multi_match function #121525 #125062

Add multi_match function #121525 #125062

Conversation

svilen-mihaylov-elastic commented Mar 17, 2025 • edited by ioanatia Loading

svilen-mihaylov-elastic commented Mar 20, 2025

carlosdelest left a comment

Choose a reason for hiding this comment

carlosdelest left a comment

Choose a reason for hiding this comment

carlosdelest Apr 9, 2025

Choose a reason for hiding this comment

carlosdelest Apr 9, 2025

Choose a reason for hiding this comment

carlosdelest Apr 9, 2025

Choose a reason for hiding this comment

svilen-mihaylov-elastic Apr 9, 2025

Choose a reason for hiding this comment

ioanatia Apr 10, 2025 • edited Loading

Choose a reason for hiding this comment

carlosdelest Apr 10, 2025

Choose a reason for hiding this comment

carlosdelest left a comment

Choose a reason for hiding this comment

svilen-mihaylov-elastic commented Apr 10, 2025

ioanatia Apr 14, 2025

Choose a reason for hiding this comment

ioanatia Apr 14, 2025

Choose a reason for hiding this comment

craigtaverner commented Apr 17, 2025

svilen-mihaylov-elastic commented Mar 17, 2025 •

edited by ioanatia

Loading

ioanatia Apr 10, 2025 •

edited

Loading