_score should not be a reserved attribute in ES|QL (elastic#118435)

kosabogi · Dec 12, 2024 · 7573312 · 7573312
1 parent a560782
commit 7573312
Show file tree

Hide file tree

Showing 5 changed files with 42 additions and 34 deletions.
diff --git a/docs/changelog/118435.yaml b/docs/changelog/118435.yaml
@@ -0,0 +1,6 @@
+pr: 118435
+summary: '`_score` should not be a reserved attribute in ES|QL'
+area: ES|QL
+type: enhancement
+issues:
+ - 118460
diff --git a/muted-tests.yml b/muted-tests.yml
@@ -180,6 +180,12 @@ tests:
 - class: "org.elasticsearch.xpack.esql.qa.mixed.MixedClusterEsqlSpecIT"
   method: "test {scoring.*}"
   issue: https://github.com/elastic/elasticsearch/issues/117641
+- class: "org.elasticsearch.xpack.esql.qa.mixed.MultilusterEsqlSpecIT"
+  method: "test {scoring.*}"
+  issue: https://github.com/elastic/elasticsearch/issues/118460
+- class: "org.elasticsearch.xpack.esql.ccq.MultiClusterSpecIT"
+  method: "test {scoring.*}"
+  issue: https://github.com/elastic/elasticsearch/issues/118460
 - class: org.elasticsearch.xpack.esql.ccq.MultiClusterSpecIT
   method: test {scoring.QstrWithFieldAndScoringSortedEval}
   issue: https://github.com/elastic/elasticsearch/issues/117751

diff --git a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/scoring.csv-spec b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/scoring.csv-spec
@@ -283,3 +283,33 @@ book_no:keyword | c_score:double
 7350            | 2.0
 7140            | 3.0
 ;
+
+QstrScoreManipulation
+required_capability: metadata_score
+required_capability: qstr_function
+
+from books metadata _score 
+| where qstr("title:rings") 
+| eval _score = _score + 1 
+| keep book_no, title, _score
+| limit 2;
+
+book_no:keyword | title:text                                                                                  | _score:double
+4023            | A Tolkien Compass: Including J. R. R. Tolkien's Guide to the Names in The Lord of the Rings | 2.6404519081115723
+2714            | Return of the King Being the Third Part of The Lord of the Rings                            | 2.9239964485168457
+;
+
+QstrScoreOverride
+required_capability: metadata_score
+required_capability: qstr_function
+
+from books metadata _score 
+| where qstr("title:rings") 
+| eval _score = "foobar"
+| keep book_no, title, _score 
+| limit 2;
+
+book_no:keyword | title:text                                                                                  | _score:keyword
+4023            | A Tolkien Compass: Including J. R. R. Tolkien's Guide to the Names in The Lord of the Rings | foobar
+2714            | Return of the King Being the Third Part of The Lord of the Rings                            | foobar
+;
diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/analysis/Verifier.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/analysis/Verifier.java
@@ -18,7 +18,6 @@
 import org.elasticsearch.xpack.esql.core.expression.Expression;
 import org.elasticsearch.xpack.esql.core.expression.Expressions;
 import org.elasticsearch.xpack.esql.core.expression.FieldAttribute;
-import org.elasticsearch.xpack.esql.core.expression.MetadataAttribute;
 import org.elasticsearch.xpack.esql.core.expression.NamedExpression;
 import org.elasticsearch.xpack.esql.core.expression.TypeResolutions;
 import org.elasticsearch.xpack.esql.core.expression.function.Function;
@@ -208,7 +207,6 @@ else if (p instanceof Lookup lookup) {
             checkJoin(p, failures);
         });
         checkRemoteEnrich(plan, failures);
-        checkMetadataScoreNameReserved(plan, failures);
 
         if (failures.isEmpty()) {
             checkLicense(plan, licenseState, failures);
@@ -222,13 +220,6 @@ else if (p instanceof Lookup lookup) {
         return failures;
     }
 
-    private static void checkMetadataScoreNameReserved(LogicalPlan p, Set<Failure> failures) {
-        // _score can only be set as metadata attribute
-        if (p.inputSet().stream().anyMatch(a -> MetadataAttribute.SCORE.equals(a.name()) && (a instanceof MetadataAttribute) == false)) {
-            failures.add(fail(p, "`" + MetadataAttribute.SCORE + "` is a reserved METADATA attribute"));
-        }
-    }
-
     private void checkSort(LogicalPlan p, Set<Failure> failures) {
         if (p instanceof OrderBy ob) {
             ob.order().forEach(o -> {

diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/VerifierTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/VerifierTests.java
@@ -12,7 +12,6 @@
 import org.elasticsearch.test.ESTestCase;
 import org.elasticsearch.xpack.esql.VerificationException;
 import org.elasticsearch.xpack.esql.action.EsqlCapabilities;
-import org.elasticsearch.xpack.esql.core.expression.Attribute;
 import org.elasticsearch.xpack.esql.core.type.DataType;
 import org.elasticsearch.xpack.esql.core.type.EsField;
 import org.elasticsearch.xpack.esql.core.type.InvalidMappedField;
@@ -22,7 +21,6 @@
 import org.elasticsearch.xpack.esql.parser.EsqlParser;
 import org.elasticsearch.xpack.esql.parser.QueryParam;
 import org.elasticsearch.xpack.esql.parser.QueryParams;
-import org.elasticsearch.xpack.esql.plan.logical.LogicalPlan;
 
 import java.util.ArrayList;
 import java.util.LinkedHashMap;
@@ -1805,29 +1803,6 @@ public void testToDatePeriodToTimeDurationWithInvalidType() {
         );
     }
 
-    public void testNonMetadataScore() {
-        assumeTrue("'METADATA _score' is disabled", EsqlCapabilities.Cap.METADATA_SCORE.isEnabled());
-        assertEquals("1:12: `_score` is a reserved METADATA attribute", error("from foo | eval _score = 10"));
-
-        assertEquals(
-            "1:48: `_score` is a reserved METADATA attribute",
-            error("from foo metadata _score | where qstr(\"bar\") | eval _score = _score + 1")
-        );
-    }
-
-    public void testScoreRenaming() {
-        assumeTrue("'METADATA _score' is disabled", EsqlCapabilities.Cap.METADATA_SCORE.isEnabled());
-        assertEquals("1:33: `_score` is a reserved METADATA attribute", error("from foo METADATA _id, _score | rename _id as _score"));
-
-        assertTrue(passes("from foo metadata _score | rename _score as foo").stream().anyMatch(a -> a.name().equals("foo")));
-    }
-
-    private List<Attribute> passes(String query) {
-        LogicalPlan logicalPlan = defaultAnalyzer.analyze(parser.createStatement(query));
-        assertTrue(logicalPlan.resolved());
-        return logicalPlan.output();
-    }
-
     public void testIntervalAsString() {
         // DateTrunc
         for (String interval : List.of("1 minu", "1 dy", "1.5 minutes", "0.5 days", "minutes 1", "day 5")) {