Respect the ignore_above option. (#57307)

For keyword-style fields, if the source value is larger than `ignore_above` then we don't retrieve the field. In particular, the field is treated as if the value didn't exist.
elastic · Jul 15, 2020 · f1db937 · f1db937
1 parent a33a873
commit f1db937
Show file tree

Hide file tree

Showing 8 changed files with 88 additions and 12 deletions.
diff --git a/...ysis-icu/src/main/java/org/elasticsearch/index/mapper/ICUCollationKeywordFieldMapper.java b/...ysis-icu/src/main/java/org/elasticsearch/index/mapper/ICUCollationKeywordFieldMapper.java
@@ -737,6 +737,11 @@ protected String parseSourceValue(Object value, String format) {
         if (format != null) {
             throw new IllegalArgumentException("Field [" + name() + "] of type [" + typeName() + "] doesn't support formats.");
         }
-        return value.toString();
+
+        String keywordValue = value.toString();
+        if (keywordValue.length() > ignoreAbove) {
+            return null;
+        }
+        return keywordValue;
     }
 }
diff --git a/...icu/src/test/java/org/elasticsearch/index/mapper/ICUCollationKeywordFieldMapperTests.java b/...icu/src/test/java/org/elasticsearch/index/mapper/ICUCollationKeywordFieldMapperTests.java
@@ -494,5 +494,12 @@ public void testParseSourceValue() {
         assertEquals("value", mapper.parseSourceValue("value", null));
         assertEquals("42", mapper.parseSourceValue(42L, null));
         assertEquals("true", mapper.parseSourceValue(true, null));
+
+        ICUCollationKeywordFieldMapper ignoreAboveMapper = new ICUCollationKeywordFieldMapper.Builder("field")
+            .ignoreAbove(4)
+            .build(context);
+        assertNull(ignoreAboveMapper.parseSourceValue("value", null));
+        assertEquals("42", ignoreAboveMapper.parseSourceValue(42L, null));
+        assertEquals("true", ignoreAboveMapper.parseSourceValue(true, null));
     }
 }
diff --git a/server/src/main/java/org/elasticsearch/index/mapper/FieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/FieldMapper.java
@@ -298,7 +298,9 @@ public List<?> lookupValues(SourceLookup lookup, @Nullable String format) {
             List<?> sourceValues = sourceValue instanceof List ? (List<?>) sourceValue : List.of(sourceValue);
             for (Object value : sourceValues) {
                 Object parsedValue = parseSourceValue(value, format);
-                values.add(parsedValue);
+                if (parsedValue != null) {
+                    values.add(parsedValue);
+                }
             }
         }
         return values;

diff --git a/server/src/main/java/org/elasticsearch/index/mapper/KeywordFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/KeywordFieldMapper.java
@@ -227,14 +227,6 @@ public Mapper.Builder<?> parse(String name, Map<String, Object> node, ParserCont
         }
     }
 
-    @Override
-    protected String parseSourceValue(Object value, String format) {
-        if (format != null) {
-            throw new IllegalArgumentException("Field [" + name() + "] of type [" + typeName() + "] doesn't support formats.");
-        }
-        return value.toString();
-    }
-
     public static final class KeywordFieldType extends StringFieldType {
 
         boolean hasNorms;
@@ -405,6 +397,20 @@ protected void parseCreateField(ParseContext context) throws IOException {
             context.doc().add(new SortedSetDocValuesField(fieldType().name(), binaryValue));
         }
     }
+
+    @Override
+    protected String parseSourceValue(Object value, String format) {
+        if (format != null) {
+            throw new IllegalArgumentException("Field [" + name() + "] of type [" + typeName() + "] doesn't support formats.");
+        }
+
+        String keywordValue = value.toString();
+        if (keywordValue.length() > ignoreAbove) {
+            return null;
+        }
+        return keywordValue;
+    }
+
     @Override
     protected String contentType() {
         return CONTENT_TYPE;

diff --git a/server/src/test/java/org/elasticsearch/index/mapper/KeywordFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/KeywordFieldMapperTests.java
@@ -633,13 +633,20 @@ public void testMeta() throws Exception {
     public void testParseSourceValue() {
         Settings settings = Settings.builder().put(IndexMetadata.SETTING_VERSION_CREATED, Version.CURRENT.id).build();
         Mapper.BuilderContext context = new Mapper.BuilderContext(settings, new ContentPath());
-        KeywordFieldMapper mapper = new KeywordFieldMapper.Builder("field").build(context);
 
+        KeywordFieldMapper mapper = new KeywordFieldMapper.Builder("field").build(context);
         assertEquals("value", mapper.parseSourceValue("value", null));
         assertEquals("42", mapper.parseSourceValue(42L, null));
         assertEquals("true", mapper.parseSourceValue(true, null));
 
         IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> mapper.parseSourceValue(true, "format"));
         assertEquals("Field [field] of type [keyword] doesn't support formats.", e.getMessage());
+
+        KeywordFieldMapper ignoreAboveMapper = new KeywordFieldMapper.Builder("field")
+            .ignoreAbove(4)
+            .build(context);
+        assertNull(ignoreAboveMapper.parseSourceValue("value", null));
+        assertEquals("42", ignoreAboveMapper.parseSourceValue(42L, null));
+        assertEquals("true", ignoreAboveMapper.parseSourceValue(true, null));
     }
 }
diff --git a/server/src/test/java/org/elasticsearch/search/fetch/subphase/FieldValueRetrieverTests.java b/server/src/test/java/org/elasticsearch/search/fetch/subphase/FieldValueRetrieverTests.java
@@ -178,6 +178,33 @@ public void testDateFormat() throws IOException {
         assertThat(dateField.getValue(), equalTo("1990/12/29"));
     }
 
+    public void testIgnoreAbove() throws IOException {
+        XContentBuilder mapping = XContentFactory.jsonBuilder().startObject()
+            .startObject("properties")
+                .startObject("field")
+                    .field("type", "keyword")
+                    .field("ignore_above", 20)
+                .endObject()
+            .endObject()
+        .endObject();
+
+        IndexService indexService = createIndex("index", Settings.EMPTY, mapping);
+        MapperService mapperService = indexService.mapperService();
+
+        XContentBuilder source = XContentFactory.jsonBuilder().startObject()
+            .array("field", "value", "other_value", "really_really_long_value")
+            .endObject();
+        Map<String, DocumentField> fields = retrieveFields(mapperService, source, "field");
+        DocumentField field = fields.get("field");
+        assertThat(field.getValues().size(), equalTo(2));
+
+        source = XContentFactory.jsonBuilder().startObject()
+            .array("field", "really_really_long_value")
+            .endObject();
+        fields = retrieveFields(mapperService, source, "field");
+        assertFalse(fields.containsKey("field"));
+    }
+
     public void testFieldAliases() throws IOException {
         XContentBuilder mapping = XContentFactory.jsonBuilder().startObject()
             .startObject("properties")

diff --git a/...n/wildcard/src/main/java/org/elasticsearch/xpack/wildcard/mapper/WildcardFieldMapper.java b/...n/wildcard/src/main/java/org/elasticsearch/xpack/wildcard/mapper/WildcardFieldMapper.java
@@ -953,7 +953,12 @@ protected String parseSourceValue(Object value, String format) {
         if (format != null) {
             throw new IllegalArgumentException("Field [" + name() + "] of type [" + typeName() + "] doesn't support formats.");
         }
-        return value.toString();
+
+        String keywordValue = value.toString();
+        if (keywordValue.length() > ignoreAbove) {
+            return null;
+        }
+        return keywordValue;
     }
 
     void createFields(String value, Document parseDoc, List<IndexableField>fields) throws IOException {

diff --git a/...dcard/src/test/java/org/elasticsearch/xpack/wildcard/mapper/WildcardFieldMapperTests.java b/...dcard/src/test/java/org/elasticsearch/xpack/wildcard/mapper/WildcardFieldMapperTests.java
@@ -774,6 +774,23 @@ protected String convertToRandomRegex(String randomValue) {
         return result.toString();
     }
 
+    public void testParseSourceValue() {
+        Settings settings = Settings.builder().put(IndexMetadata.SETTING_VERSION_CREATED, Version.CURRENT.id).build();
+        Mapper.BuilderContext context = new Mapper.BuilderContext(settings, new ContentPath());
+
+        WildcardFieldMapper mapper = new WildcardFieldMapper.Builder("field").build(context);
+        assertEquals("value", mapper.parseSourceValue("value", null));
+        assertEquals("42", mapper.parseSourceValue(42L, null));
+        assertEquals("true", mapper.parseSourceValue(true, null));
+
+        WildcardFieldMapper ignoreAboveMapper = new WildcardFieldMapper.Builder("field")
+            .ignoreAbove(4)
+            .build(context);
+        assertNull(ignoreAboveMapper.parseSourceValue("value", null));
+        assertEquals("42", ignoreAboveMapper.parseSourceValue(42L, null));
+        assertEquals("true", ignoreAboveMapper.parseSourceValue(true, null));
+    }
+
     protected MappedFieldType provideMappedFieldType(String name) {
         if (name.equals(WILDCARD_FIELD_NAME)) {
             return wildcardFieldType.fieldType();