Browse Source

Adding check for isIndexed in text fields when generating FieldExistsQuery to avoid ISE when field is stored but not indexed or with doc_values (#130531)

Panagiotis Bailis 3 months ago
parent
commit
dc34f4dee1

+ 6 - 0
docs/changelog/130531.yaml

@@ -0,0 +1,6 @@
+pr: 130531
+summary: Adding check for `isIndexed` in text fields when generating field exists
+  queries to avoid ISE when field is stored but not indexed or with `doc_values`
+area: Analysis
+type: bug
+issues: []

+ 61 - 0
rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/search/160_exists_query.yml

@@ -45,6 +45,10 @@ setup:
                       type: keyword
                 text:
                   type: text
+                text_stored_not_indexed:
+                  type: text
+                  store: true
+                  index: false
 
   - do:
       headers:
@@ -70,6 +74,7 @@ setup:
               inner1: "foo"
               inner2: "bar"
             text: "foo bar"
+            text_stored_not_indexed: "foo bar"
 
   - do:
       headers:
@@ -94,6 +99,7 @@ setup:
             object:
               inner1: "foo"
             text: "foo bar"
+            text_stored_not_indexed: "foo bar"
 
   - do:
       headers:
@@ -119,6 +125,7 @@ setup:
             object:
               inner2: "bar"
             text: "foo bar"
+            text_stored_not_indexed: "foo bar"
 
   - do:
       index:
@@ -184,6 +191,12 @@ setup:
                       doc_values: false
                 text:
                   type: text
+                keyword_stored_norms_not_indexed:
+                  type: keyword
+                  doc_values: false
+                  index: false
+                  store: true
+                  norms: true
 
   - do:
       headers:
@@ -209,6 +222,7 @@ setup:
               inner1: "foo"
               inner2: "bar"
             text: "foo bar"
+            keyword_stored_norms_not_indexed: "foo bar"
 
   - do:
       headers:
@@ -233,6 +247,7 @@ setup:
             object:
               inner1: "foo"
             text: "foo bar"
+            keyword_stored_norms_not_indexed: "foo bar"
 
   - do:
       headers:
@@ -258,6 +273,7 @@ setup:
             object:
               inner2: "bar"
             text: "foo bar"
+            keyword_stored_norms_not_indexed: "foo bar"
 
   - do:
       index:
@@ -1268,3 +1284,48 @@ setup:
                 field: text
 
   - match: {hits.total: 1}
+
+---
+"Test exists query on text field with no dv, that is stored but not indexed":
+  - requires:
+      capabilities:
+        - method: POST
+          path: /_search
+          capabilities: [ field_exists_query_for_text_fields_no_index_or_dv ]
+      test_runner_features: capabilities
+      reason: "Before the fix, this query would throw an ISE because the field is not indexed and has no doc values."
+
+  - do:
+      search:
+        rest_total_hits_as_int: true
+        index: test
+        body:
+          query:
+            exists:
+              field: text_stored_not_indexed
+
+  # this should not throw, but rather return 0 hits, as the field is not indexed nor it has doc values
+  - match: {hits.total: 0}
+
+
+---
+"Test exists query on keyword field with no dv, that is stored, with norms, but not indexed":
+  - requires:
+      capabilities:
+        - method: POST
+          path: /_search
+          capabilities: [ field_exists_query_for_text_fields_no_index_or_dv ]
+      test_runner_features: capabilities
+      reason: "Before the fix, this query would throw an ISE because the field is not indexed and has no doc values."
+
+  - do:
+      search:
+        rest_total_hits_as_int: true
+        index: test-no-dv
+        body:
+          query:
+            exists:
+              field: keyword_stored_norms_not_indexed
+
+  # this should not throw, but rather return 0 hits, as the field is not indexed nor it has doc values
+  - match: {hits.total: 0}

+ 1 - 1
server/src/main/java/org/elasticsearch/index/mapper/MappedFieldType.java

@@ -371,7 +371,7 @@ public abstract class MappedFieldType {
     }
 
     public Query existsQuery(SearchExecutionContext context) {
-        if (hasDocValues() || getTextSearchInfo().hasNorms()) {
+        if (hasDocValues() || (isIndexed() && getTextSearchInfo().hasNorms())) {
             return new FieldExistsQuery(name());
         } else {
             return new TermQuery(new Term(FieldNamesFieldMapper.NAME, name()));

+ 2 - 0
server/src/main/java/org/elasticsearch/rest/action/search/SearchCapabilities.java

@@ -52,6 +52,7 @@ public final class SearchCapabilities {
     private static final String SIGNIFICANT_TERMS_ON_NESTED_FIELDS = "significant_terms_on_nested_fields";
     private static final String EXCLUDE_VECTORS_PARAM = "exclude_vectors_param";
     private static final String DENSE_VECTOR_UPDATABLE_BBQ = "dense_vector_updatable_bbq";
+    private static final String FIELD_EXISTS_QUERY_FOR_TEXT_FIELDS_NO_INDEX_OR_DV = "field_exists_query_for_text_fields_no_index_or_dv";
 
     public static final Set<String> CAPABILITIES;
     static {
@@ -75,6 +76,7 @@ public final class SearchCapabilities {
         capabilities.add(SIGNIFICANT_TERMS_ON_NESTED_FIELDS);
         capabilities.add(EXCLUDE_VECTORS_PARAM);
         capabilities.add(DENSE_VECTOR_UPDATABLE_BBQ);
+        capabilities.add(FIELD_EXISTS_QUERY_FOR_TEXT_FIELDS_NO_INDEX_OR_DV);
         CAPABILITIES = Set.copyOf(capabilities);
     }
 }

+ 3 - 1
server/src/test/java/org/elasticsearch/index/mapper/KeywordFieldTypeTests.java

@@ -137,7 +137,9 @@ public class KeywordFieldTypeTests extends FieldTypeTestCase {
             FieldType fieldType = new FieldType();
             fieldType.setOmitNorms(false);
             KeywordFieldType ft = new KeywordFieldType("field", fieldType);
-            assertEquals(new FieldExistsQuery("field"), ft.existsQuery(MOCK_CONTEXT));
+            // updated in #130531 so that a field that is neither indexed nor has doc values will generate a TermQuery
+            // to avoid ISE from FieldExistsQuery
+            assertEquals(new TermQuery(new Term(FieldNamesFieldMapper.NAME, "field")), ft.existsQuery(MOCK_CONTEXT));
         }
         {
             KeywordFieldType ft = new KeywordFieldType("field", true, false, Collections.emptyMap());