[ES|QL] MMR Command - MMROperator and Execution by markjhoy · Pull Request #141324 · elastic/elasticsearch

markjhoy · 2026-01-27T02:13:43Z

Adds the operator and execution flow for the MMR command as well as pertinent CSV tests

Also performs a light refactoring to add a CompleteInputCollectorOperator base class for operations that require the full set of input pages to be input / processed before any output

x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/operator/MMROperator.java

...ck/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/planner/LocalExecutionPlanner.java

markjhoy · 2026-01-30T15:10:45Z

x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plan/logical/MMR.java

 import static org.elasticsearch.xpack.esql.core.type.DataType.INTEGER;

 public class MMR extends UnaryPlan implements TelemetryAware, ExecutesOn.Coordinator, PostAnalysisVerificationAware {
+    public static final NamedWriteableRegistry.Entry ENTRY = new NamedWriteableRegistry.Entry(LogicalPlan.class, "Mmr", MMR::new);


Although serialization is not strictly necessary here as this should only run on the coordinator, certain parent plans (e.g. LIMIT) may try and serialize any child plans - and without this (and the writeTo(), etc. implementations) will throw. It theoretically should not hurt anything to allow this to de/serialize.

I don't think we need serialization, if there's any serialization happening that is a separate bug we need to dig into.

markjhoy · 2026-02-02T15:56:15Z

Note: seeing a weird issue with the output of the dense vectors where it's rounding the values. For example in one of the CSV tests, I'm getting a mismatch error due to the results:

Actual:
    keyword_field:keyword | text_body:text | text_vector:dense_vector
    test1                 | first text     | [0.4, 0.2, 0.4]
    test3                 | third text     | [0.4, 0.1, 0.3]
    test6                 | sixth text     | [0.099999994, 0.099999994, 0.099999994]
Expected:
    keyword_field:keyword | text_body:text | text_vector:dense_vector
    test1                 | first text     | [0.4, 0.2, 0.4]
    test3                 | third text     | [0.4, 0.1, 0.3]
    test6                 | sixth text     | [0.1, 0.1, 0.1]

... the input vectors appear to be rounding on input. Shouldn't affect the functionality of the operator here, but we need to look a bit further into this.

elasticsearchmachine · 2026-02-03T23:31:58Z

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

carlosdelest

Looks really good! 💯

Some questions about exception usage and releasing pages.

carlosdelest · 2026-02-04T10:34:25Z

...k/plugin/esql/compute/src/test/java/org/elasticsearch/compute/operator/MMROperatorTests.java

+
+            @Override
+            protected Page createPage(int positionOffset, int length) {
+                length = Integer.min(length, remaining());


I think it would be useful to add random nulls (rarely) and we check nothing breaks...

carlosdelest · 2026-02-04T10:38:38Z

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

+FROM mmr_text_vector_keyword
+| SORT keyword_field
+| LIMIT 10
+| MMR [0.1, 0.2, 0.3]::dense_vector ON text_vector LIMIT 3 WITH { "lambda": 0.1 }


We could do implicit casting here! Check how it's done now for functions - we may expand that to commands, or just for this specific instance.

I realize that users will probably not use directly a vector as an argument, but the result of some function / expression. Just mentioning it here if you think it would be useful. In that case, we can do that on a separate PR.

x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plan/logical/MMR.java

x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plan/physical/MMRExec.java

...ck/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/planner/LocalExecutionPlanner.java

x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/operator/MMROperator.java

carlosdelest · 2026-02-04T11:05:05Z

x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/operator/MMROperator.java

+    }
+
+    @Override
+    protected void onClose() {


Do we need to override? Is this not handled on the superclass?

It's an abstract function of the superclass, called after any close() items performed there

Yep - I mean, I think the exact same thing is being done on the superclass before invoking onClose()?

x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/operator/MMROperator.java

ioanatia · 2026-02-05T12:26:46Z

x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plan/logical/MMR.java

 import static org.elasticsearch.xpack.esql.core.type.DataType.INTEGER;

 public class MMR extends UnaryPlan implements TelemetryAware, ExecutesOn.Coordinator, PostAnalysisVerificationAware {
+    public static final NamedWriteableRegistry.Entry ENTRY = new NamedWriteableRegistry.Entry(LogicalPlan.class, "Mmr", MMR::new);


I don't think we need serialization, if there's any serialization happening that is a separate bug we need to dig into.

ioanatia · 2026-02-05T12:52:30Z

...ck/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/planner/LocalExecutionPlanner.java

+        Integer scoreChannel = null;
+        for (Attribute input : mmr.inputSet()) {
+            if (input instanceof MetadataAttribute && MetadataAttribute.SCORE.equals(input.name())) {
+                scoreChannel = source.layout.get(input.id()).channel();


do we need to pass in the scoreChannel?
when I have seen this initially, I thought that maybe we use these in the MMR formula, but that's not the case 😌
it seems like we only need this for RankDoc ? I wonder what's the side effect of always choosing a constant score of 1.0f when we create the RankDocs objects 🤔 . Can we do that and simplify the code here a bit so we don't need to pass _score here?

ioanatia · 2026-02-05T12:58:18Z

x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/operator/MMROperator.java

+        return new MMROperator.Status(emitNanos, pagesReceived, pagesProcessed, rowsReceived, rowsEmitted);
+    }
+
+    public record Status(long emitNanos, int pagesReceived, int pagesProcessed, long rowsReceived, long rowsEmitted)


I wonder if we can abstract the Status too in CompleteInputCollectorOperator 🤔 .
Other abstract operators seem to do this:

elasticsearch/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/operator/AbstractPageMappingOperator.java

Line 110 in 8a38cbb

public static class Status implements Operator.Status {

initial MMROperator and base diversification

d2dc737

elasticsearchmachine added the v9.4.0 label Jan 27, 2026

markjhoy added 2 commits January 27, 2026 00:50

add _score block input

c6273e3

Merge branch 'main' into markjhoy/esql_mmr_command_operator_execution

6b6c25a

ioanatia reviewed Jan 27, 2026

View reviewed changes

x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/operator/MMROperator.java Outdated Show resolved Hide resolved

markjhoy added 2 commits January 28, 2026 09:50

working MMR output;CompleteInputCollectorOperator

be4128c

Merge branch 'main' into markjhoy/esql_mmr_command_operator_execution

471cbb1

ioanatia reviewed Jan 29, 2026

View reviewed changes

markjhoy added 11 commits January 29, 2026 09:45

add status; add tests; light refactoring

bc32155

remove dependency on _doc vector input block

aadf47a

Merge branch 'main' into markjhoy/esql_mmr_command_operator_execution

23a7a19

Merge branch 'main' into markjhoy/esql_mmr_command_operator_execution

ab60c91

fix bad merge

70d0987

realize queryVector/lambda as VectorData/Float

5036553

fix MMRExec input params

b3d542e

update grammar to ensure ON is specified

b6b4e1c

add status tests

18ae42b

start of MMR csv tests

d82caf1

Merge branch 'main' into markjhoy/esql_mmr_command_operator_execution

d613fa4

markjhoy commented Jan 30, 2026

View reviewed changes

markjhoy added 7 commits January 30, 2026 10:48

re-add writable for MMRExec

dabd6fa

revert csvtests change

9dd97b4

Merge branch 'main' into markjhoy/esql_mmr_command_operator_execution

66e16d3

regen ANTLR grammar from last merge

4498d96

additional CSV tests

dca5109

light fix csv tests for stability

ee663d9

Merge branch 'main' into markjhoy/esql_mmr_command_operator_execution

311f31e

markjhoy added 2 commits February 2, 2026 11:03

drop text_vector column in output from CSV tests

2719291

add KNN subquery and TEXT_EMBEDDING fxn query vec

f8b2334

markjhoy and others added 7 commits February 2, 2026 11:43

add temp capability for BWC tests

58e5b30

Merge branch 'main' into markjhoy/esql_mmr_command_operator_execution

dc50332

add sort for tests

dabce01

Merge branch 'main' into markjhoy/esql_mmr_command_operator_execution

e369b11

Merge branch 'main' into markjhoy/esql_mmr_command_operator_execution

de7e5a2

[CI] Auto commit changes from spotless

6e9ea12

Merge branch 'main' into markjhoy/esql_mmr_command_operator_execution

d72408f

markjhoy added >non-issue Team:Search Relevance Meta label for the Search Relevance team in Elasticsearch :Search Relevance/ES|QL Search functionality in ES|QL labels Feb 3, 2026

markjhoy marked this pull request as ready for review February 3, 2026 23:31

Merge branch 'main' into markjhoy/esql_mmr_command_operator_execution

ddfbf39

markjhoy self-assigned this Feb 3, 2026

markjhoy requested review from a team and ioanatia February 3, 2026 23:31

comment cleanup

6cae905

carlosdelest reviewed Feb 4, 2026

View reviewed changes

markjhoy and others added 3 commits February 4, 2026 20:43

light cleanups and refactoring

9e2bbcd

Merge branch 'main' into markjhoy/esql_mmr_command_operator_execution

c00fc3d

[CI] Auto commit changes from spotless

86b3538

ioanatia reviewed Feb 5, 2026

View reviewed changes

Conversation

markjhoy commented Jan 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

markjhoy commented Feb 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

elasticsearchmachine commented Feb 3, 2026

Uh oh!

carlosdelest left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

markjhoy commented Jan 27, 2026 •

edited

Loading

markjhoy commented Feb 2, 2026 •

edited

Loading