Merge branch 'main' into markjhoy/default_token_pruning_sparse_vector

Author: markjhoy

BUILDING.md

@@ -144,7 +144,7 @@ To wire this registered cluster into a `TestClusterAware` task (e.g. `RestIntegT
 Additional integration tests for a certain Elasticsearch modules that are specific to certain cluster configuration can be declared in a separate so called `qa` subproject of your module.
 
 The benefit of a dedicated project for these tests are:
-- `qa` projects are dedicated two specific use-cases and easier to maintain
+- `qa` projects are dedicated to specific use-cases and easier to maintain
 - It keeps the specific test logic separated from the common test logic.
 - You can run those tests in parallel to other projects of the build.
 

docs/changelog/127229.yaml

@@ -0,0 +1,6 @@
+pr: 127229
+summary: Return BAD_REQUEST when a field scorer references a missing field
+area: Ranking
+type: bug
+issues:
+ - 127162

docs/changelog/127414.yaml

@@ -0,0 +1,5 @@
+pr: 127414
+summary: Fix npe when using source confirmed text query against missing field
+area: Search
+type: bug
+issues: []

modules/data-streams/src/test/java/org/elasticsearch/datastreams/action/TransportGetDataStreamsActionTests.java

@@ -11,6 +11,7 @@
 import org.elasticsearch.action.datastreams.GetDataStreamAction;
 import org.elasticsearch.cluster.ClusterName;
 import org.elasticsearch.cluster.ClusterState;
+import org.elasticsearch.cluster.metadata.ComponentTemplate;
 import org.elasticsearch.cluster.metadata.ComposableIndexTemplate;
 import org.elasticsearch.cluster.metadata.DataStream;
 import org.elasticsearch.cluster.metadata.DataStreamFailureStoreSettings;
@@ -40,6 +41,7 @@
 import java.time.temporal.ChronoUnit;
 import java.util.ArrayList;
 import java.util.List;
+import java.util.Map;
 import java.util.Set;
 import java.util.stream.Collectors;
 
@@ -543,11 +545,44 @@ public void testGetEffectiveSettingsTemplateOnlySettings() {
         GetDataStreamAction.Request req = new GetDataStreamAction.Request(TEST_REQUEST_TIMEOUT, new String[] {});
         final String templatePolicy = "templatePolicy";
         final String templateIndexMode = IndexMode.LOOKUP.getName();
-        final String dataStreamPolicy = "dataStreamPolicy";
-        final String dataStreamIndexMode = IndexMode.LOGSDB.getName();
 
         ClusterState state = getClusterStateWithDataStreamWithSettings(
             projectId,
+            Settings.builder()
+                .put(IndexMetadata.LIFECYCLE_NAME, templatePolicy)
+                .put(IndexSettings.MODE.getKey(), templateIndexMode)
+                .build(),
+            Settings.EMPTY,
+            Settings.EMPTY
+        );
+
+        GetDataStreamAction.Response response = TransportGetDataStreamsAction.innerOperation(
+            state.projectState(projectId),
+            req,
+            resolver,
+            systemIndices,
+            ClusterSettings.createBuiltInClusterSettings(),
+            dataStreamGlobalRetentionSettings,
+            emptyDataStreamFailureStoreSettings,
+            new IndexSettingProviders(Set.of()),
+            null
+        );
+        assertNotNull(response.getDataStreams());
+        assertThat(response.getDataStreams().size(), equalTo(1));
+        assertThat(response.getDataStreams().get(0).getIlmPolicy(), equalTo(templatePolicy));
+        assertThat(response.getDataStreams().get(0).getIndexModeName(), equalTo(templateIndexMode));
+    }
+
+    public void testGetEffectiveSettingsComponentTemplateOnlySettings() {
+        // Set a lifecycle only in the template, and make sure that is in the response:
+        ProjectId projectId = randomProjectIdOrDefault();
+        GetDataStreamAction.Request req = new GetDataStreamAction.Request(TEST_REQUEST_TIMEOUT, new String[] {});
+        final String templatePolicy = "templatePolicy";
+        final String templateIndexMode = IndexMode.LOOKUP.getName();
+
+        ClusterState state = getClusterStateWithDataStreamWithSettings(
+            projectId,
+            Settings.EMPTY,
             Settings.builder()
                 .put(IndexMetadata.LIFECYCLE_NAME, templatePolicy)
                 .put(IndexSettings.MODE.getKey(), templateIndexMode)
@@ -582,6 +617,10 @@ public void testGetEffectiveSettings() {
         // Now set a lifecycle in both the template and the data stream, and make sure the response has the data stream one:
         ClusterState state = getClusterStateWithDataStreamWithSettings(
             projectId,
+            Settings.builder()
+                .put(IndexMetadata.LIFECYCLE_NAME, templatePolicy)
+                .put(IndexSettings.MODE.getKey(), templateIndexMode)
+                .build(),
             Settings.builder()
                 .put(IndexMetadata.LIFECYCLE_NAME, templatePolicy)
                 .put(IndexSettings.MODE.getKey(), templateIndexMode)
@@ -611,6 +650,7 @@ public void testGetEffectiveSettings() {
     private static ClusterState getClusterStateWithDataStreamWithSettings(
         ProjectId projectId,
         Settings templateSettings,
+        Settings componentTemplateSettings,
         Settings dataStreamSettings
     ) {
         String dataStreamName = "data-stream-1";
@@ -625,8 +665,16 @@ private static ClusterState getClusterStateWithDataStreamWithSettings(
                 .indexPatterns(List.of("*"))
                 .template(Template.builder().settings(templateSettings))
                 .dataStreamTemplate(new ComposableIndexTemplate.DataStreamTemplate())
+                .componentTemplates(List.of("component_template_1"))
                 .build()
         );
+        ComponentTemplate componentTemplate = new ComponentTemplate(
+            Template.builder().settings(componentTemplateSettings).build(),
+            null,
+            null,
+            null
+        );
+        builder.componentTemplates(Map.of("component_template_1", componentTemplate));
 
         List<IndexMetadata> backingIndices = new ArrayList<>();
         for (int backingIndexNumber = 1; backingIndexNumber <= numberOfBackingIndices; backingIndexNumber++) {

modules/mapper-extras/src/main/java/org/elasticsearch/index/mapper/extras/SourceConfirmedTextQuery.java

@@ -267,7 +267,11 @@ public boolean isCacheable(LeafReaderContext ctx) {
             @Override
             public Explanation explain(LeafReaderContext context, int doc) throws IOException {
                 NumericDocValues norms = context.reader().getNormValues(field);
-                RuntimePhraseScorer scorer = (RuntimePhraseScorer) scorerSupplier(context).get(0);
+                ScorerSupplier scorerSupplier = scorerSupplier(context);
+                if (scorerSupplier == null) {
+                    return Explanation.noMatch("No matching phrase");
+                }
+                RuntimePhraseScorer scorer = (RuntimePhraseScorer) scorerSupplier.get(0);
                 if (scorer == null) {
                     return Explanation.noMatch("No matching phrase");
                 }
@@ -277,6 +281,7 @@ public Explanation explain(LeafReaderContext context, int doc) throws IOExceptio
                 }
                 float phraseFreq = scorer.freq();
                 Explanation freqExplanation = Explanation.match(phraseFreq, "phraseFreq=" + phraseFreq);
+                assert simScorer != null;
                 Explanation scoreExplanation = simScorer.explain(freqExplanation, getNormValue(norms, doc));
                 return Explanation.match(
                     scoreExplanation.getValue(),
@@ -321,7 +326,11 @@ public Matches matches(LeafReaderContext context, int doc) throws IOException {
                     Weight innerWeight = in.createWeight(searcher, ScoreMode.COMPLETE_NO_SCORES, 1);
                     return innerWeight.matches(context, doc);
                 }
-                RuntimePhraseScorer scorer = (RuntimePhraseScorer) scorerSupplier(context).get(0L);
+                ScorerSupplier scorerSupplier = scorerSupplier(context);
+                if (scorerSupplier == null) {
+                    return null;
+                }
+                RuntimePhraseScorer scorer = (RuntimePhraseScorer) scorerSupplier.get(0L);
                 if (scorer == null) {
                     return null;
                 }

modules/mapper-extras/src/test/java/org/elasticsearch/index/mapper/extras/SourceConfirmedTextQueryTests.java

@@ -24,6 +24,7 @@
 import org.apache.lucene.queries.spans.SpanTermQuery;
 import org.apache.lucene.search.BooleanClause.Occur;
 import org.apache.lucene.search.BooleanQuery;
+import org.apache.lucene.search.Explanation;
 import org.apache.lucene.search.IndexSearcher;
 import org.apache.lucene.search.MatchNoDocsQuery;
 import org.apache.lucene.search.Matches;
@@ -101,6 +102,26 @@ public void testTerm() throws Exception {
         }
     }
 
+    public void testMissingPhrase() throws Exception {
+        try (Directory dir = newDirectory(); IndexWriter w = new IndexWriter(dir, newIndexWriterConfig(Lucene.STANDARD_ANALYZER))) {
+
+            Document doc = new Document();
+            doc.add(new TextField("body", "a b c b a b c", Store.YES));
+            w.addDocument(doc);
+
+            try (IndexReader reader = DirectoryReader.open(w)) {
+                IndexSearcher searcher = newSearcher(reader);
+                PhraseQuery query = new PhraseQuery("missing_field", "b", "c");
+                Query sourceConfirmedPhraseQuery = new SourceConfirmedTextQuery(query, SOURCE_FETCHER_PROVIDER, Lucene.STANDARD_ANALYZER);
+                Explanation explanation = searcher.explain(sourceConfirmedPhraseQuery, 0);
+                assertFalse(explanation.isMatch());
+
+                Weight weight = searcher.createWeight(query, ScoreMode.COMPLETE, 1);
+                assertNull(weight.matches(getOnlyLeafReader(reader).getContext(), 0));
+            }
+        }
+    }
+
     public void testPhrase() throws Exception {
         try (Directory dir = newDirectory(); IndexWriter w = new IndexWriter(dir, newIndexWriterConfig(Lucene.STANDARD_ANALYZER))) {
 

plugins/examples/rescore/src/yamlRestTest/resources/rest-api-spec/test/example-rescore/30_factor_field.yml

@@ -48,6 +48,24 @@ setup:
   - match: { hits.hits.1._score: 20 }
   - match: { hits.hits.2._score: 10 }
 
+---
+"referencing a missing field returns bad request":
+  - requires:
+      cluster_features: [ "search.rescorer.missing.field.bad.request" ]
+      reason: "Testing the behaviour change with this feature"
+  - do:
+      catch: bad_request
+      search:
+        index: test
+        body:
+          rescore:
+            example:
+              factor: 1
+              factor_field: missing
+  - match: { status: 400 }
+  - match: { error.root_cause.0.type: "illegal_argument_exception" }
+  - match: { error.root_cause.0.reason: "Missing value for field [missing]" }
+
 ---
 "sorted based on a numeric field and rescored based on a factor field using a window size":
   - do:

server/src/main/java/org/elasticsearch/cluster/metadata/DataStream.java

@@ -381,12 +381,7 @@ public ComposableIndexTemplate getEffectiveIndexTemplate(ProjectMetadata project
 
     public Settings getEffectiveSettings(ProjectMetadata projectMetadata) {
         ComposableIndexTemplate template = getMatchingIndexTemplate(projectMetadata);
-        final Settings templateSettings;
-        if (template.template() == null || template.template().settings() == null) {
-            templateSettings = Settings.EMPTY;
-        } else {
-            templateSettings = template.template().settings();
-        }
+        Settings templateSettings = MetadataIndexTemplateService.resolveSettings(template, projectMetadata.componentTemplates());
         return templateSettings.merge(settings);
     }
 

server/src/main/java/org/elasticsearch/common/lucene/search/function/FieldValueFactorFunction.java

@@ -11,7 +11,6 @@
 
 import org.apache.lucene.index.LeafReaderContext;
 import org.apache.lucene.search.Explanation;
-import org.elasticsearch.ElasticsearchException;
 import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.io.stream.StreamOutput;
 import org.elasticsearch.common.io.stream.Writeable;
@@ -73,7 +72,7 @@ public double score(int docId, float subQueryScore) throws IOException {
                     if (missing != null) {
                         value = missing;
                     } else {
-                        throw new ElasticsearchException("Missing value for field [" + field + "]");
+                        throw new IllegalArgumentException("Missing value for field [" + field + "]");
                     }
                 }
                 double val = value * boostFactor;

server/src/main/java/org/elasticsearch/search/SearchFeatures.java

@@ -28,9 +28,10 @@ public Set<NodeFeature> getFeatures() {
     public static final NodeFeature COMPLETION_FIELD_SUPPORTS_DUPLICATE_SUGGESTIONS = new NodeFeature(
         "search.completion_field.duplicate.support"
     );
+    public static final NodeFeature RESCORER_MISSING_FIELD_BAD_REQUEST = new NodeFeature("search.rescorer.missing.field.bad.request");
 
     @Override
     public Set<NodeFeature> getTestFeatures() {
-        return Set.of(RETRIEVER_RESCORER_ENABLED, COMPLETION_FIELD_SUPPORTS_DUPLICATE_SUGGESTIONS);
+        return Set.of(RETRIEVER_RESCORER_ENABLED, COMPLETION_FIELD_SUPPORTS_DUPLICATE_SUGGESTIONS, RESCORER_MISSING_FIELD_BAD_REQUEST);
     }
 }