ES|QL - Add scoring for full text functions disjunctions #121793

carlosdelest · 2025-02-05T16:45:11Z

Adds scoring in FTFs disjunctions. This one takes a separate path for evaluating scores for expressions, creating a separate interface and Operator to retrieve them instead of doing it as part of expression evaluation (#121322)

Changes

A new LuceneQueryScoreEvaluator that produces just scores. For now, it's a copy of LuceneQueryExpressionEvaluator that we can refactor afterwards.
A new ExpressionScorer class that provides the way for scoring expressions.
A new ExpressionScoreMapper interface that allows expressions that implement it to provide factories for ExpressionScorer.
A ScoreMapper class that provides factories for ExpressionScorer. This is the same concept that EvalMapper but applied to scores, keeping those concerns separate. This class provides a default evaluation for scores, and delegates to ExpressionScoreMapper interfaces in case the expressions implement them.
A ScoreOperator that retrieves scores from a filter expression via the ScoreMapper. This operator blends the scores retrieved from Lucene with the ones provided by the filter expressions.
LocalExecutionMapper adds a ScoreOperator on top of the FilterOperator when scoring is active.

Pros

Separate logic for scoring and expression evaluation
The ScoreMapper / ExpressionScorer may be reused for other scoring mechanisms (score function, WHERE .. AS s1 construct)
It can be further optimized - scoring could be done before expression evaluation so expression evaluation uses scores as a way of understanding if expression is true or not

Cons

A separate "phase" for scoring, which is less efficient than doing evaluation / scoring in one go. This can be optimized afterwards, so evaluation can use scores for determining if a result is true or not.

View previous PoCs at #121153, #121322

…corers for expressions

…ner when an Filter is being planned

elasticsearchmachine · 2025-02-05T16:46:08Z

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

nik9000

I like this approach because most expressions won't need to worry about score. They can participate in a scorable expression and they just make 0.0. You can't cast a regular expression into something that quacks like a scorable expression. But AND, OR, MATCH, the other full text functions you can.

nik9000 · 2025-02-11T20:08:23Z

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

+ * Evaluates a tree of functions for every position in the block, resulting in a
+ * new block which is appended to the page.
+ */
+public class ScoreOperator extends AbstractPageMappingOperator {


I do wonder if you could adapt ExpressionScorer.Factory into an ExpressionEvaluator.Factory so you could use EvalOperator for this.

The problem I see is that ExpressionEvaluators evaluate expressions - we've made scoring a separate concern from expressions in this PR, so it cannot be directly combined with expressions via evaluation.

That's what the ScoreOperator is for - it evaluates the scores using their own scoring tree and interface, and then adds it to the _score metadata.

To make scoring compatible with evaluations I think we could create an expression evaluation tree that would map the expressions tree to their corresponding expression scoring tree. Then, we could use the EvalOperator to sum that evaluation to the _score metadata.

I like more the idea of keeping expressions and scores separate. ScoringOperator seems like a small price to pay in order to have that, and also has a single, well defined purpose.

nik9000 · 2025-02-11T20:10:35Z

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

+
+        Block[] blocks = new Block[page.getBlockCount()];
+        blocks[0] = page.getBlock(0);
+        try (DoubleBlock evalScores = scorer.score(page); DoubleBlock existingScores = page.getBlock(1)) {


If you're going to move forward with your "replace a scorable expression in a WHERE with evaluating it's score and then checking the score" then this'll have to be just like EVAL is - just adding a new column. So you'd not be able to combine the scores. In fact, you could use EVAL to combine the scores later.

Using EVAL for the combination would also let you use AddDoublesEvaluator without much trouble which would get you error handling (NaN and stuff) for free too.

Well, for as "free" as the rest of the code has it.

"replace a scorable expression in a WHERE with evaluating it's score and then checking the score" then this'll have to be just like EVAL is - just adding a new column.

Correct. A new column would be added for each expression to be scored, and then each expression could check its corresponding column for it being > 0.

So you'd not be able to combine the scores. In fact, you could use EVAL to combine the scores later.

The global score would need to be combined as a separate, global column. This is due to scores being part of other non-scorable expressions. The global column would resolve the scoring expression and would be the one added to the _score. But yes, it could be combined via EVAL.

I see this step more as an optimization, as it would build on the new scoring interface and scoring mappers.

…e-disjunctions-scoreing-operator # Conflicts: # x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/planner/PlannerUtils.java

…ses to avoid code duplication

…junctions-scoreing-operator' into enhancement/esql-score-disjunctions-scoreing-operator

carlosdelest · 2025-03-07T08:02:16Z

@elasticmachine run elasticsearch-ci/part-4

fang-xing-esql

Thank you @carlosdelest , it looks pretty good to me, I added some comments around the tests.

fang-xing-esql · 2025-03-10T12:44:02Z

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

+required_capability: metadata_score
+
+from books metadata _score 
+| where length(title) > 100


Is this a typo? It seems like this query is the same as scoresNonPushableFunctions.

It's a mistake - I corrected the test to actually use a pushable function in 7457544. Thanks!

fang-xing-esql · 2025-03-10T13:43:37Z

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

@@ -284,6 +284,11 @@ public final void test() throws Throwable {
                "CSV tests cannot currently handle the _source field mapping directives",
                testCase.requiredCapabilities.contains(EsqlCapabilities.Cap.SOURCE_FIELD_MAPPING.capabilityName())
            );
+            assumeFalse(


I wonder why we need to skip metadata_score tests?

The problem with the scoring tests is that they cannot be done via CsvTests, that are not real integration tests and thus have not a Lucene implementation.

Up until now, every test in the scoring spec required a full text function capability (match, kql, qstr), which effectively disable CsvTests.

Right now we have added more tests that do not depend on full text functions (like scoring for pushable conditions). That's the reason why I needed to add this capability to CsvTests.

I've added some capabilities to the tests for completeness in e3e068c, but it's necessary to keep metadata_score as a capability that disables CsvTests, which makes sense as Lucene is needed for scoring to work when we have pushed down functions.

fang-xing-esql · 2025-03-10T13:52:05Z

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

@@ -1466,11 +1466,12 @@ public void testFullTextFunctionsDisjunctions() {
    private void checkWithFullTextFunctionsDisjunctions(String functionInvocation) {

        // Disjunctions with non-pushable functions - scoring
-        checkdisjunctionScoringError("1:35", functionInvocation + " or length(first_name) > 10");


The checkdisjunctionScoringError method can be safely removed.

Good catch! Removed in e4758d3

…e-disjunctions-scoreing-operator

fang-xing-esql

Thank you @carlosdelest! LGTM. Just a couple of more things that I can think of.

If I remember it correctly, the search functions in ES|QL are still under tech preview, we are free to change the scores later, as you mentioned the non-full text function will have no score. At some point, if we can include some score related examples in the docs that will be great.

I would inform Kibana with the ui label, as more limitation of the full text functions are removed, I'm not sure if any action need to be taken on Kibana side, but it is nice to just FYI.

elasticsearchmachine · 2025-03-11T13:19:15Z

Pinging @elastic/kibana-esql (ES|QL-ui)

carlosdelest · 2025-03-11T13:19:16Z

Thanks @fang-xing-esql for your review! 👍

At some point, if we can include some score related examples in the docs that will be great.

Yes! We're already in conversations with the docs team about this. Expect a PR soon 🙂

I would inform Kibana with the ui label, as more limitation of the full text functions are removed, I'm not sure if any action need to be taken on Kibana side, but it is nice to just FYI.

Sure! Added the label. I'm not sure that any action was taken on the UI side for this, but it doesn't hurt.

…e-disjunctions-scoreing-operator # Conflicts: # 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/planner/LocalExecutionPlanner.java

tteofili

LGTM, great job @carlosdelest

elasticsearchmachine · 2025-03-11T14:30:52Z

💔 Backport failed

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

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

carlosdelest · 2025-03-11T15:37:36Z

💚 All backports created successfully

Status	Branch	Result
✅	8.x
✅	9.0

Questions ?

Please refer to the Backport tool documentation

) (cherry picked from commit 2b40e73) # Conflicts: # 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/fulltext/FullTextFunction.java # x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/planner/LocalExecutionPlanner.java # x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/VerifierTests.java

…124573)

)

carlosdelest added 8 commits February 5, 2025 12:23

LuceneQueryScoreEvaluator first implementation

978770d

Add ScoreOperator and ScoreMapper

cb2c3c4

Add a ExpressionScoreMapper and a ScoreMapper interface to retrieve s…

9ca756a

…corers for expressions

Implement ExpressionScoreMapper for FullTextFunction and BinaryLogic

e4eb86d

Create a ScoreOperator that can be planned via the LocalExecutionPlan…

a437da3

…ner when an Filter is being planned

Fix EvalMapper

aa4ffbf

Add tests

1044bfd

Spotless

5abed67

carlosdelest added >enhancement auto-backport Automatically create backport pull requests when merged :Analytics/ES|QL AKA ESQL Team:Search Relevance Meta label for the Search Relevance team in Elasticsearch v8.19.0 v9.1.0 labels Feb 5, 2025

Update docs/changelog/121793.yaml

8b7fd0a

Fix tests

72bbd5f

carlosdelest requested review from nik9000 and fang-xing-esql February 11, 2025 08:03

nik9000 reviewed Feb 11, 2025

View reviewed changes

carlosdelest changed the title ~~[PoC 3] ES|QL - Add scoring for full text functions disjunctions via a ScoreMapper~~ ES|QL - Add scoring for full text functions disjunctions via a ScoreMapper Mar 3, 2025

carlosdelest mentioned this pull request Mar 3, 2025

ESQL - Add scoring for full text functions disjunctions using ExpressionEvaluator #121322

Closed

carlosdelest added 8 commits March 3, 2025 18:20

Merge remote-tracking branch 'origin/main' into enhancement/esql-scor…

1bfa58f

…e-disjunctions-scoreing-operator # Conflicts: # x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/planner/PlannerUtils.java

Add testing and capabilities

5cb0bfc

Remove disjunction limitations from docs

3cab2dc

Calculate the _score attr position instead of hardcoding it

c639ec6

Refactor LuceneQueryExpressionEvaluator into a superclass and subclas…

3f3b5b7

…ses to avoid code duplication

Fix tests

145955c

Refactor query evaluators to use subclasses instead of interfaces

63ca98b

Merge remote-tracking branch 'carlosdelest/enhancement/esql-score-dis…

0008559

…junctions-scoreing-operator' into enhancement/esql-score-disjunctions-scoreing-operator

carlosdelest requested review from afoucret and svilen-mihaylov-elastic March 6, 2025 17:36

carlosdelest changed the title ~~ES|QL - Add scoring for full text functions disjunctions via a ScoreMapper~~ ES|QL - Add scoring for full text functions disjunctions Mar 6, 2025

carlosdelest mentioned this pull request Mar 7, 2025

ES|QL MATCH roadmap #116261

Open

fang-xing-esql reviewed Mar 10, 2025

View reviewed changes

carlosdelest added 3 commits March 10, 2025 16:04

Fix test

7457544

Remove unnecessary method

e4758d3

Add missing capabilities to tests

e3e068c

carlosdelest requested a review from fang-xing-esql March 10, 2025 15:40

Merge remote-tracking branch 'origin/main' into enhancement/esql-scor…

5859c81

…e-disjunctions-scoreing-operator

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

View reviewed changes

carlosdelest added the ES|QL-ui Impacts ES|QL UI label Mar 11, 2025

carlosdelest enabled auto-merge (squash) March 11, 2025 13:24

tteofili approved these changes Mar 11, 2025

View reviewed changes

carlosdelest merged commit 2b40e73 into elastic:main Mar 11, 2025
17 checks passed

elasticsearchmachine added the backport pending label Mar 11, 2025

This was referenced Mar 11, 2025

[8.x] ES|QL - Add scoring for full text functions disjunctions (#121793) #124572

Merged

[9.0] ES|QL - Add scoring for full text functions disjunctions (#121793) #124573

Merged

carlosdelest added a commit that referenced this pull request Mar 11, 2025

ES|QL - Add scoring for full text functions disjunctions (#121793) (#…

a4af548

…124573)

carlosdelest mentioned this pull request Mar 12, 2025

Scoring support in ES|QL #116599

Open

albertzaharovits pushed a commit to albertzaharovits/elasticsearch that referenced this pull request Mar 13, 2025

ES|QL - Add scoring for full text functions disjunctions (elastic#121793

d8916f3

)

jfreden pushed a commit to jfreden/elasticsearch that referenced this pull request Mar 13, 2025

ES|QL - Add scoring for full text functions disjunctions (elastic#121793

0fc5893

)

carlosdelest mentioned this pull request Mar 21, 2025

ES|QL QSTR roadmap #116930

Open

ES|QL - Add scoring for full text functions disjunctions #121793

ES|QL - Add scoring for full text functions disjunctions #121793

Uh oh!

Conversation

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

Uh oh!

Changes

Pros

Cons

Uh oh!

elasticsearchmachine commented Feb 5, 2025

Uh oh!

nik9000 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!

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!

Choose a reason for hiding this comment

Uh oh!

carlosdelest commented Mar 7, 2025

Uh oh!

fang-xing-esql 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!

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!

Choose a reason for hiding this comment

Uh oh!

fang-xing-esql left a comment

Choose a reason for hiding this comment

Uh oh!

elasticsearchmachine commented Mar 11, 2025

Uh oh!

carlosdelest commented Mar 11, 2025

Uh oh!

tteofili left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

elasticsearchmachine commented Mar 11, 2025

💔 Backport failed

Uh oh!

carlosdelest commented Mar 11, 2025

💚 All backports created successfully

Questions ?

Uh oh!

Uh oh!

carlosdelest commented Feb 5, 2025 •

edited

Loading