From 07710a4456fc32fb38f49c19cb8a1dfc605ab3d9 Mon Sep 17 00:00:00 2001 From: Christos Soulios Date: Mon, 5 Jul 2021 19:01:09 +0300 Subject: [PATCH 01/12] Add constraints to dimension fields: - It must be either indexed or doc_value - It cannot be multi-valued (WIP) --- .../index/mapper/IpFieldMapper.java | 15 +++++++--- .../index/mapper/KeywordFieldMapper.java | 30 ++++++++++++++++--- .../index/mapper/NumberFieldMapper.java | 6 ++++ .../index/mapper/IpFieldMapperTests.java | 9 ++++++ .../index/mapper/KeywordFieldMapperTests.java | 20 +++++++++++++ .../mapper/WholeNumberFieldMapperTests.java | 11 +++++++ 6 files changed, 83 insertions(+), 8 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/index/mapper/IpFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/IpFieldMapper.java index 5dc7a6cea5c78..6564a434a1af3 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/IpFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/IpFieldMapper.java @@ -18,11 +18,11 @@ import org.apache.lucene.util.ArrayUtil; import org.apache.lucene.util.BytesRef; import org.elasticsearch.Version; -import org.elasticsearch.core.Nullable; -import org.elasticsearch.core.Tuple; import org.elasticsearch.common.logging.DeprecationCategory; import org.elasticsearch.common.logging.DeprecationLogger; import org.elasticsearch.common.network.InetAddresses; +import org.elasticsearch.core.Nullable; +import org.elasticsearch.core.Tuple; import org.elasticsearch.index.fielddata.IndexFieldData; import org.elasticsearch.index.fielddata.ScriptDocValues; import org.elasticsearch.index.fielddata.plain.SortedSetOrdinalsIndexFieldData; @@ -72,8 +72,7 @@ public static class Builder extends FieldMapper.Builder { private final Parameter onScriptError = Parameter.onScriptErrorParam(m -> toType(m).onScriptError, script); private final Parameter> meta = Parameter.metaParam(); - private final Parameter dimension - = Parameter.boolParam("dimension", false, m -> toType(m).dimension, false); + private final Parameter dimension; private final boolean ignoreMalformedByDefault; private final Version indexCreatedVersion; @@ -88,6 +87,14 @@ public Builder(String name, ScriptCompiler scriptCompiler, boolean ignoreMalform = Parameter.boolParam("ignore_malformed", true, m -> toType(m).ignoreMalformed, ignoreMalformedByDefault); this.script.precludesParameters(nullValue, ignoreMalformed); addScriptValidation(script, indexed, hasDocValues); + this.dimension = Parameter.boolParam("dimension", false, m -> toType(m).dimension, false) + .setValidator(v -> { + if (v && indexed.getValue() == false && hasDocValues.getValue() == false) { + throw new IllegalArgumentException( + "Field [dimension] requires one of [" + indexed.name + "] or [" + hasDocValues.name + "] to be true" + ); + } + }); } Builder nullValue(String nullValue) { diff --git a/server/src/main/java/org/elasticsearch/index/mapper/KeywordFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/KeywordFieldMapper.java index a5c010764d0d9..782f3c49fd470 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/KeywordFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/KeywordFieldMapper.java @@ -126,8 +126,15 @@ public Builder(String name, IndexAnalyzers indexAnalyzers, ScriptCompiler script this.dimension = Parameter.boolParam("dimension", false, m -> toType(m).dimension, false) .setValidator(v -> { + if (v && indexed.getValue() == false && hasDocValues.getValue() == false) { + throw new IllegalArgumentException( + "Field [dimension] requires one of [" + indexed.name + "] or [" + hasDocValues.name + "] to be true" + ); + } if (v && ignoreAbove.getValue() < ignoreAbove.getDefaultValue()) { - throw new IllegalArgumentException("Field [ignore_above] cannot be set in conjunction with field [dimension]"); + throw new IllegalArgumentException( + "Field [" + ignoreAbove.name + "] cannot be set in conjunction with field [dimension]" + ); } }); } @@ -499,11 +506,15 @@ protected void indexScriptValues(SearchLookup searchLookup, LeafReaderContext re } private void indexValue(ParseContext context, String value) { - if (value == null) { return; } + // Check that a dimension field is single-valued and not an array + if (dimension && context.doc().getByKey(name()) != null) { + throw new IllegalArgumentException("Dimension field [" + fieldType().name() + "] cannot be a multi-valued field."); + } + if (value.length() > ignoreAbove) { context.addIgnoredField(name()); return; @@ -518,7 +529,12 @@ private void indexValue(ParseContext context, String value) { final BytesRef binaryValue = new BytesRef(value); if (fieldType.indexOptions() != IndexOptions.NONE || fieldType.stored()) { Field field = new KeywordField(fieldType().name(), binaryValue, fieldType); - context.doc().add(field); + if (dimension && context.doc().getByKey(name()) == null) { + // Add dimension field with key so that we ensure it is single-valued + context.doc().addWithKey(name(), field); + } else { + context.doc().add(field); + } if (fieldType().hasDocValues() == false && fieldType.omitNorms()) { context.addToFieldNames(fieldType().name()); @@ -526,7 +542,13 @@ private void indexValue(ParseContext context, String value) { } if (fieldType().hasDocValues()) { - context.doc().add(new SortedSetDocValuesField(fieldType().name(), binaryValue)); + Field field = new SortedSetDocValuesField(fieldType().name(), binaryValue); + if (dimension && context.doc().getByKey(name()) == null) { + // Add field with key so that we ensure it is single-valued + context.doc().addWithKey(name(), field); + } else { + context.doc().add(field); + } } } diff --git a/server/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java index 3d124ff5ab6b0..1244ca86178c5 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java @@ -118,6 +118,12 @@ public Builder(String name, NumberType type, ScriptCompiler compiler, boolean ig if (v && EnumSet.of(NumberType.INTEGER, NumberType.LONG, NumberType.BYTE, NumberType.SHORT).contains(type) == false) { throw new IllegalArgumentException("Parameter [dimension] cannot be set to numeric type [" + type.name + "]"); } + + if (v && indexed.getValue() == false && hasDocValues.getValue() == false) { + throw new IllegalArgumentException( + "Field [dimension] requires one of [" + indexed.name + "] or [" + hasDocValues.name + "] to be true" + ); + } }); this.script.precludesParameters(ignoreMalformed, coerce, nullValue); diff --git a/server/src/test/java/org/elasticsearch/index/mapper/IpFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/IpFieldMapperTests.java index 76cb8a915bff8..6eee4bb0843f0 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/IpFieldMapperTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/IpFieldMapperTests.java @@ -215,6 +215,15 @@ public void testDimension() throws IOException { assertDimension(false, IpFieldMapper.IpFieldType::isDimension); } + public void testDimensionAndIndexedOrDocvalues() { + Exception e = expectThrows(MapperParsingException.class, () -> createDocumentMapper(fieldMapping(b -> { + minimalMapping(b); + b.field("dimension", true).field("index", false).field("doc_values", false); + }))); + assertThat(e.getCause().getMessage(), + containsString("Field [dimension] requires one of [index] or [doc_values] to be true")); + } + @Override protected String generateRandomInputValue(MappedFieldType ft) { return NetworkAddress.format(randomIp(randomBoolean())); diff --git a/server/src/test/java/org/elasticsearch/index/mapper/KeywordFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/KeywordFieldMapperTests.java index 92dcca91e47d9..48d4c59bb106c 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/KeywordFieldMapperTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/KeywordFieldMapperTests.java @@ -320,6 +320,26 @@ public void testDimensionAndIgnoreAbove() { containsString("Field [ignore_above] cannot be set in conjunction with field [dimension]")); } + public void testDimensionAndIndexedOrDocvalues() { + Exception e = expectThrows(MapperParsingException.class, () -> createDocumentMapper(fieldMapping(b -> { + minimalMapping(b); + b.field("dimension", true).field("index", false).field("doc_values", false); + }))); + assertThat(e.getCause().getMessage(), + containsString("Field [dimension] requires one of [index] or [doc_values] to be true")); + } + + public void testDimensionMultiValuedField() throws IOException { + DocumentMapper mapper = createDocumentMapper(fieldMapping(b -> { + minimalMapping(b); + b.field("dimension", true); + })); + + Exception e = expectThrows(MapperParsingException.class, () -> mapper.parse(source(b -> b.array("field", "1234", "45678")))); + assertThat(e.getCause().getMessage(), + containsString("Dimension field [field] cannot be a multi-valued field")); + } + public void testConfigureSimilarity() throws IOException { MapperService mapperService = createMapperService( fieldMapping(b -> b.field("type", "keyword").field("similarity", "boolean")) diff --git a/server/src/test/java/org/elasticsearch/index/mapper/WholeNumberFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/WholeNumberFieldMapperTests.java index da8b37a662fdc..c9be479bb009b 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/WholeNumberFieldMapperTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/WholeNumberFieldMapperTests.java @@ -12,6 +12,8 @@ import java.io.IOException; +import static org.hamcrest.Matchers.containsString; + public abstract class WholeNumberFieldMapperTests extends NumberFieldMapperTests { protected void testDecimalCoerce() throws IOException { @@ -33,6 +35,15 @@ public void testDimension() throws IOException { assertDimension(false, NumberFieldMapper.NumberFieldType::isDimension); } + public void testDimensionAndIndexedOrDocvalues() { + Exception e = expectThrows(MapperParsingException.class, () -> createDocumentMapper(fieldMapping(b -> { + minimalMapping(b); + b.field("dimension", true).field("index", false).field("doc_values", false); + }))); + assertThat(e.getCause().getMessage(), + containsString("Field [dimension] requires one of [index] or [doc_values] to be true")); + } + @Override protected void registerParameters(ParameterChecker checker) throws IOException { super.registerParameters(checker); From 1204f573c468923e0eb6cc58397de59488c7b4d4 Mon Sep 17 00:00:00 2001 From: Christos Soulios Date: Mon, 5 Jul 2021 20:57:48 +0300 Subject: [PATCH 02/12] Added single-valued field constraint to ip Cleaned up code --- .../index/mapper/IpFieldMapper.java | 25 +++++++++-- .../index/mapper/KeywordFieldMapper.java | 30 +++++++------- .../index/mapper/IpFieldMapperTests.java | 38 +++++++++++++++++ .../index/mapper/KeywordFieldMapperTests.java | 41 +++++++++++++++---- 4 files changed, 109 insertions(+), 25 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/index/mapper/IpFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/IpFieldMapper.java index 6564a434a1af3..1c83c2ad45e6c 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/IpFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/IpFieldMapper.java @@ -8,6 +8,7 @@ package org.elasticsearch.index.mapper; +import org.apache.lucene.document.Field; import org.apache.lucene.document.InetAddressPoint; import org.apache.lucene.document.SortedSetDocValuesField; import org.apache.lucene.document.StoredField; @@ -472,17 +473,35 @@ protected void parseCreateField(ParseContext context) throws IOException { indexValue(context, address); } + /** + * Adds a field to the current document ensuring that if the field is + * a dimension field, it will be added as single-value. + */ + private void addField(ParseContext context, Field field) { + if (dimension && context.doc().getByKey(name()) == null) { + // Add dimension field with key so that we ensure it is single-valued + context.doc().addWithKey(name(), field); + } else { + context.doc().add(field); + } + } + private void indexValue(ParseContext context, InetAddress address) { + // Check that a dimension field is single-valued and not an array + if (dimension && context.doc().getByKey(name()) != null) { + throw new IllegalArgumentException("Dimension field [" + fieldType().name() + "] cannot be a multi-valued field."); + } + if (indexed) { - context.doc().add(new InetAddressPoint(fieldType().name(), address)); + addField(context, new InetAddressPoint(fieldType().name(), address)); } if (hasDocValues) { - context.doc().add(new SortedSetDocValuesField(fieldType().name(), new BytesRef(InetAddressPoint.encode(address)))); + addField(context, new SortedSetDocValuesField(fieldType().name(), new BytesRef(InetAddressPoint.encode(address)))); } else if (stored || indexed) { context.addToFieldNames(fieldType().name()); } if (stored) { - context.doc().add(new StoredField(fieldType().name(), new BytesRef(InetAddressPoint.encode(address)))); + addField(context, new StoredField(fieldType().name(), new BytesRef(InetAddressPoint.encode(address)))); } } diff --git a/server/src/main/java/org/elasticsearch/index/mapper/KeywordFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/KeywordFieldMapper.java index 782f3c49fd470..acc715772727d 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/KeywordFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/KeywordFieldMapper.java @@ -505,6 +505,19 @@ protected void indexScriptValues(SearchLookup searchLookup, LeafReaderContext re this.scriptValues.valuesForDoc(searchLookup, readerContext, doc, value -> indexValue(parseContext, value)); } + /** + * Adds a field to the current document ensuring that if the field is + * a dimension field, it will be added as single-value. + */ + private void addField(ParseContext context, Field field) { + if (dimension && context.doc().getByKey(name()) == null) { + // Add dimension field with key so that we ensure it is single-valued + context.doc().addWithKey(name(), field); + } else { + context.doc().add(field); + } + } + private void indexValue(ParseContext context, String value) { if (value == null) { return; @@ -528,27 +541,14 @@ private void indexValue(ParseContext context, String value) { // convert to utf8 only once before feeding postings/dv/stored fields final BytesRef binaryValue = new BytesRef(value); if (fieldType.indexOptions() != IndexOptions.NONE || fieldType.stored()) { - Field field = new KeywordField(fieldType().name(), binaryValue, fieldType); - if (dimension && context.doc().getByKey(name()) == null) { - // Add dimension field with key so that we ensure it is single-valued - context.doc().addWithKey(name(), field); - } else { - context.doc().add(field); - } - + addField(context, new KeywordField(fieldType().name(), binaryValue, fieldType)); if (fieldType().hasDocValues() == false && fieldType.omitNorms()) { context.addToFieldNames(fieldType().name()); } } if (fieldType().hasDocValues()) { - Field field = new SortedSetDocValuesField(fieldType().name(), binaryValue); - if (dimension && context.doc().getByKey(name()) == null) { - // Add field with key so that we ensure it is single-valued - context.doc().addWithKey(name(), field); - } else { - context.doc().add(field); - } + addField(context, new SortedSetDocValuesField(fieldType().name(), binaryValue)); } } diff --git a/server/src/test/java/org/elasticsearch/index/mapper/IpFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/IpFieldMapperTests.java index 6eee4bb0843f0..996131f581a87 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/IpFieldMapperTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/IpFieldMapperTests.java @@ -224,6 +224,44 @@ public void testDimensionAndIndexedOrDocvalues() { containsString("Field [dimension] requires one of [index] or [doc_values] to be true")); } + public void testDimensionMultiValuedField() throws IOException { + { + DocumentMapper mapper = createDocumentMapper(fieldMapping(b -> { + minimalMapping(b); + b.field("dimension", true); + })); + + Exception e = expectThrows(MapperParsingException.class, + () -> mapper.parse(source(b -> b.array("field", "192.168.1.1", "192.168.1.1")))); + assertThat(e.getCause().getMessage(), + containsString("Dimension field [field] cannot be a multi-valued field")); + } + { + // Disable doc_values + DocumentMapper mapper = createDocumentMapper(fieldMapping(b -> { + minimalMapping(b); + b.field("dimension", true).field("doc_values",false); + })); + + Exception e = expectThrows(MapperParsingException.class, + () -> mapper.parse(source(b -> b.array("field", "192.168.1.1", "192.168.1.1")))); + assertThat(e.getCause().getMessage(), + containsString("Dimension field [field] cannot be a multi-valued field")); + } + { + // Disable indexed fields + DocumentMapper mapper = createDocumentMapper(fieldMapping(b -> { + minimalMapping(b); + b.field("dimension", true).field("index",false); + })); + + Exception e = expectThrows(MapperParsingException.class, + () -> mapper.parse(source(b -> b.array("field", "192.168.1.1", "192.168.1.1")))); + assertThat(e.getCause().getMessage(), + containsString("Dimension field [field] cannot be a multi-valued field")); + } + } + @Override protected String generateRandomInputValue(MappedFieldType ft) { return NetworkAddress.format(randomIp(randomBoolean())); diff --git a/server/src/test/java/org/elasticsearch/index/mapper/KeywordFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/KeywordFieldMapperTests.java index 48d4c59bb106c..db84986b2186f 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/KeywordFieldMapperTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/KeywordFieldMapperTests.java @@ -330,14 +330,41 @@ public void testDimensionAndIndexedOrDocvalues() { } public void testDimensionMultiValuedField() throws IOException { - DocumentMapper mapper = createDocumentMapper(fieldMapping(b -> { - minimalMapping(b); - b.field("dimension", true); - })); + { + DocumentMapper mapper = createDocumentMapper(fieldMapping(b -> { + minimalMapping(b); + b.field("dimension", true); + })); - Exception e = expectThrows(MapperParsingException.class, () -> mapper.parse(source(b -> b.array("field", "1234", "45678")))); - assertThat(e.getCause().getMessage(), - containsString("Dimension field [field] cannot be a multi-valued field")); + Exception e = expectThrows(MapperParsingException.class, + () -> mapper.parse(source(b -> b.array("field", "1234", "45678")))); + assertThat(e.getCause().getMessage(), + containsString("Dimension field [field] cannot be a multi-valued field")); + } + { + // Disable doc_values + DocumentMapper mapper = createDocumentMapper(fieldMapping(b -> { + minimalMapping(b); + b.field("dimension", true).field("doc_values",false); + })); + + Exception e = expectThrows(MapperParsingException.class, + () -> mapper.parse(source(b -> b.array("field", "1234", "45678")))); + assertThat(e.getCause().getMessage(), + containsString("Dimension field [field] cannot be a multi-valued field")); + } + { + // Disable indexed fields + DocumentMapper mapper = createDocumentMapper(fieldMapping(b -> { + minimalMapping(b); + b.field("dimension", true).field("index",false); + })); + + Exception e = expectThrows(MapperParsingException.class, + () -> mapper.parse(source(b -> b.array("field", "1234", "45678")))); + assertThat(e.getCause().getMessage(), + containsString("Dimension field [field] cannot be a multi-valued field")); + } } public void testConfigureSimilarity() throws IOException { From 979ccb3f41fb1f8877607fb3e7e1ce3b95e57bd9 Mon Sep 17 00:00:00 2001 From: Christos Soulios Date: Mon, 5 Jul 2021 21:04:41 +0300 Subject: [PATCH 03/12] Minor improvement --- .../java/org/elasticsearch/index/mapper/IpFieldMapper.java | 6 +++--- .../org/elasticsearch/index/mapper/KeywordFieldMapper.java | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/index/mapper/IpFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/IpFieldMapper.java index 1c83c2ad45e6c..d287e5b9586d8 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/IpFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/IpFieldMapper.java @@ -478,9 +478,9 @@ protected void parseCreateField(ParseContext context) throws IOException { * a dimension field, it will be added as single-value. */ private void addField(ParseContext context, Field field) { - if (dimension && context.doc().getByKey(name()) == null) { + if (dimension && context.doc().getByKey(fieldType().name()) == null) { // Add dimension field with key so that we ensure it is single-valued - context.doc().addWithKey(name(), field); + context.doc().addWithKey(fieldType().name(), field); } else { context.doc().add(field); } @@ -488,7 +488,7 @@ private void addField(ParseContext context, Field field) { private void indexValue(ParseContext context, InetAddress address) { // Check that a dimension field is single-valued and not an array - if (dimension && context.doc().getByKey(name()) != null) { + if (dimension && context.doc().getByKey(fieldType().name()) != null) { throw new IllegalArgumentException("Dimension field [" + fieldType().name() + "] cannot be a multi-valued field."); } diff --git a/server/src/main/java/org/elasticsearch/index/mapper/KeywordFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/KeywordFieldMapper.java index acc715772727d..d35f77f251972 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/KeywordFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/KeywordFieldMapper.java @@ -510,9 +510,9 @@ protected void indexScriptValues(SearchLookup searchLookup, LeafReaderContext re * a dimension field, it will be added as single-value. */ private void addField(ParseContext context, Field field) { - if (dimension && context.doc().getByKey(name()) == null) { + if (dimension && context.doc().getByKey(fieldType().name()) == null) { // Add dimension field with key so that we ensure it is single-valued - context.doc().addWithKey(name(), field); + context.doc().addWithKey(fieldType().name(), field); } else { context.doc().add(field); } @@ -524,7 +524,7 @@ private void indexValue(ParseContext context, String value) { } // Check that a dimension field is single-valued and not an array - if (dimension && context.doc().getByKey(name()) != null) { + if (dimension && context.doc().getByKey(fieldType().name()) != null) { throw new IllegalArgumentException("Dimension field [" + fieldType().name() + "] cannot be a multi-valued field."); } From 091991f6e690840d1e5cfd0e243b50bcb8f2ca2a Mon Sep 17 00:00:00 2001 From: Christos Soulios Date: Mon, 5 Jul 2021 23:18:57 +0300 Subject: [PATCH 04/12] Added single-value field checks for numbers --- .../index/mapper/IpFieldMapper.java | 8 ++-- .../index/mapper/KeywordFieldMapper.java | 6 +-- .../index/mapper/NumberFieldMapper.java | 16 +++++++- .../mapper/WholeNumberFieldMapperTests.java | 38 +++++++++++++++++++ 4 files changed, 59 insertions(+), 9 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/index/mapper/IpFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/IpFieldMapper.java index d287e5b9586d8..6cb019184f9af 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/IpFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/IpFieldMapper.java @@ -477,7 +477,7 @@ protected void parseCreateField(ParseContext context) throws IOException { * Adds a field to the current document ensuring that if the field is * a dimension field, it will be added as single-value. */ - private void addField(ParseContext context, Field field) { + private void indexField(ParseContext context, Field field) { if (dimension && context.doc().getByKey(fieldType().name()) == null) { // Add dimension field with key so that we ensure it is single-valued context.doc().addWithKey(fieldType().name(), field); @@ -493,15 +493,15 @@ private void indexValue(ParseContext context, InetAddress address) { } if (indexed) { - addField(context, new InetAddressPoint(fieldType().name(), address)); + indexField(context, new InetAddressPoint(fieldType().name(), address)); } if (hasDocValues) { - addField(context, new SortedSetDocValuesField(fieldType().name(), new BytesRef(InetAddressPoint.encode(address)))); + indexField(context, new SortedSetDocValuesField(fieldType().name(), new BytesRef(InetAddressPoint.encode(address)))); } else if (stored || indexed) { context.addToFieldNames(fieldType().name()); } if (stored) { - addField(context, new StoredField(fieldType().name(), new BytesRef(InetAddressPoint.encode(address)))); + indexField(context, new StoredField(fieldType().name(), new BytesRef(InetAddressPoint.encode(address)))); } } diff --git a/server/src/main/java/org/elasticsearch/index/mapper/KeywordFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/KeywordFieldMapper.java index d35f77f251972..4990c5dc48228 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/KeywordFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/KeywordFieldMapper.java @@ -509,7 +509,7 @@ protected void indexScriptValues(SearchLookup searchLookup, LeafReaderContext re * Adds a field to the current document ensuring that if the field is * a dimension field, it will be added as single-value. */ - private void addField(ParseContext context, Field field) { + private void indexField(ParseContext context, Field field) { if (dimension && context.doc().getByKey(fieldType().name()) == null) { // Add dimension field with key so that we ensure it is single-valued context.doc().addWithKey(fieldType().name(), field); @@ -541,14 +541,14 @@ private void indexValue(ParseContext context, String value) { // convert to utf8 only once before feeding postings/dv/stored fields final BytesRef binaryValue = new BytesRef(value); if (fieldType.indexOptions() != IndexOptions.NONE || fieldType.stored()) { - addField(context, new KeywordField(fieldType().name(), binaryValue, fieldType)); + indexField(context, new KeywordField(fieldType().name(), binaryValue, fieldType)); if (fieldType().hasDocValues() == false && fieldType.omitNorms()) { context.addToFieldNames(fieldType().name()); } } if (fieldType().hasDocValues()) { - addField(context, new SortedSetDocValuesField(fieldType().name(), binaryValue)); + indexField(context, new SortedSetDocValuesField(fieldType().name(), binaryValue)); } } diff --git a/server/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java index 1244ca86178c5..f2dd9404a8b61 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java @@ -1182,8 +1182,20 @@ protected void parseCreateField(ParseContext context) throws IOException { } private void indexValue(ParseContext context, Number numericValue) { - context.doc().addAll(fieldType().type.createFields(fieldType().name(), numericValue, - indexed, hasDocValues, stored)); + List fields = fieldType().type.createFields(fieldType().name(), numericValue, indexed, hasDocValues, stored); + if (dimension) { + // Check that a dimension field is single-valued and not an array + if (context.doc().getByKey(fieldType().name()) != null) { + throw new IllegalArgumentException("Dimension field [" + fieldType().name() + "] cannot be a multi-valued field."); + } + if (fields.size() > 0) { + // Add the first field by key so that we can validate if it has been added + context.doc().addWithKey(fieldType().name(), fields.get(0)); + context.doc().addAll(fields.subList(1, fields.size())); + } + } else { + context.doc().addAll(fields); + } if (hasDocValues == false && (stored || indexed)) { context.addToFieldNames(fieldType().name()); diff --git a/server/src/test/java/org/elasticsearch/index/mapper/WholeNumberFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/WholeNumberFieldMapperTests.java index c9be479bb009b..0f73cfbd66113 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/WholeNumberFieldMapperTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/WholeNumberFieldMapperTests.java @@ -44,6 +44,44 @@ public void testDimensionAndIndexedOrDocvalues() { containsString("Field [dimension] requires one of [index] or [doc_values] to be true")); } + public void testDimensionMultiValuedField() throws IOException { + { + DocumentMapper mapper = createDocumentMapper(fieldMapping(b -> { + minimalMapping(b); + b.field("dimension", true); + })); + + Exception e = expectThrows(MapperParsingException.class, + () -> mapper.parse(source(b -> b.array("field", randomNumber(), randomNumber(), randomNumber())))); + assertThat(e.getCause().getMessage(), + containsString("Dimension field [field] cannot be a multi-valued field")); + } + { + // Disable doc_values + DocumentMapper mapper = createDocumentMapper(fieldMapping(b -> { + minimalMapping(b); + b.field("dimension", true).field("doc_values",false); + })); + + Exception e = expectThrows(MapperParsingException.class, + () -> mapper.parse(source(b -> b.array("field", randomNumber(), randomNumber(), randomNumber())))); + assertThat(e.getCause().getMessage(), + containsString("Dimension field [field] cannot be a multi-valued field")); + } + { + // Disable indexed fields + DocumentMapper mapper = createDocumentMapper(fieldMapping(b -> { + minimalMapping(b); + b.field("dimension", true).field("index",false); + })); + + Exception e = expectThrows(MapperParsingException.class, + () -> mapper.parse(source(b -> b.array("field", randomNumber(), randomNumber(), randomNumber())))); + assertThat(e.getCause().getMessage(), + containsString("Dimension field [field] cannot be a multi-valued field")); + } + } + @Override protected void registerParameters(ParameterChecker checker) throws IOException { super.registerParameters(checker); From cc241aab82183d94dc2927421707b51885d0cb82 Mon Sep 17 00:00:00 2001 From: Christos Soulios Date: Tue, 6 Jul 2021 12:05:03 +0300 Subject: [PATCH 05/12] Ensure index and doc_values for dimensions Dimension fields must be both indexed and doc_values --- .../index/mapper/IpFieldMapper.java | 31 +++++----- .../index/mapper/KeywordFieldMapper.java | 31 +++++----- .../index/mapper/NumberFieldMapper.java | 5 +- .../index/mapper/IpFieldMapperTests.java | 58 ++++++++----------- .../index/mapper/KeywordFieldMapperTests.java | 58 ++++++++----------- .../mapper/WholeNumberFieldMapperTests.java | 58 ++++++++----------- 6 files changed, 104 insertions(+), 137 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/index/mapper/IpFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/IpFieldMapper.java index 6cb019184f9af..b7547df5f8c53 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/IpFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/IpFieldMapper.java @@ -90,9 +90,9 @@ public Builder(String name, ScriptCompiler scriptCompiler, boolean ignoreMalform addScriptValidation(script, indexed, hasDocValues); this.dimension = Parameter.boolParam("dimension", false, m -> toType(m).dimension, false) .setValidator(v -> { - if (v && indexed.getValue() == false && hasDocValues.getValue() == false) { + if (v && (indexed.getValue() == false || hasDocValues.getValue() == false)) { throw new IllegalArgumentException( - "Field [dimension] requires one of [" + indexed.name + "] or [" + hasDocValues.name + "] to be true" + "Field [dimension] requires that [" + indexed.name + "] and [" + hasDocValues.name + "] are true" ); } }); @@ -473,19 +473,6 @@ protected void parseCreateField(ParseContext context) throws IOException { indexValue(context, address); } - /** - * Adds a field to the current document ensuring that if the field is - * a dimension field, it will be added as single-value. - */ - private void indexField(ParseContext context, Field field) { - if (dimension && context.doc().getByKey(fieldType().name()) == null) { - // Add dimension field with key so that we ensure it is single-valued - context.doc().addWithKey(fieldType().name(), field); - } else { - context.doc().add(field); - } - } - private void indexValue(ParseContext context, InetAddress address) { // Check that a dimension field is single-valued and not an array if (dimension && context.doc().getByKey(fieldType().name()) != null) { @@ -493,15 +480,23 @@ private void indexValue(ParseContext context, InetAddress address) { } if (indexed) { - indexField(context, new InetAddressPoint(fieldType().name(), address)); + Field field = new InetAddressPoint(fieldType().name(), address); + + if (dimension) { + // Add dimension field with key so that we ensure it is single-valued. + // Dimension fields are always indexed. + context.doc().addWithKey(fieldType().name(), field); + } else { + context.doc().add(field); + } } if (hasDocValues) { - indexField(context, new SortedSetDocValuesField(fieldType().name(), new BytesRef(InetAddressPoint.encode(address)))); + context.doc().add(new SortedSetDocValuesField(fieldType().name(), new BytesRef(InetAddressPoint.encode(address)))); } else if (stored || indexed) { context.addToFieldNames(fieldType().name()); } if (stored) { - indexField(context, new StoredField(fieldType().name(), new BytesRef(InetAddressPoint.encode(address)))); + context.doc().add(new StoredField(fieldType().name(), new BytesRef(InetAddressPoint.encode(address)))); } } diff --git a/server/src/main/java/org/elasticsearch/index/mapper/KeywordFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/KeywordFieldMapper.java index 4990c5dc48228..6e573780de7db 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/KeywordFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/KeywordFieldMapper.java @@ -126,9 +126,9 @@ public Builder(String name, IndexAnalyzers indexAnalyzers, ScriptCompiler script this.dimension = Parameter.boolParam("dimension", false, m -> toType(m).dimension, false) .setValidator(v -> { - if (v && indexed.getValue() == false && hasDocValues.getValue() == false) { + if (v && (indexed.getValue() == false || hasDocValues.getValue() == false)) { throw new IllegalArgumentException( - "Field [dimension] requires one of [" + indexed.name + "] or [" + hasDocValues.name + "] to be true" + "Field [dimension] requires that [" + indexed.name + "] and [" + hasDocValues.name + "] are true" ); } if (v && ignoreAbove.getValue() < ignoreAbove.getDefaultValue()) { @@ -505,20 +505,8 @@ protected void indexScriptValues(SearchLookup searchLookup, LeafReaderContext re this.scriptValues.valuesForDoc(searchLookup, readerContext, doc, value -> indexValue(parseContext, value)); } - /** - * Adds a field to the current document ensuring that if the field is - * a dimension field, it will be added as single-value. - */ - private void indexField(ParseContext context, Field field) { - if (dimension && context.doc().getByKey(fieldType().name()) == null) { - // Add dimension field with key so that we ensure it is single-valued - context.doc().addWithKey(fieldType().name(), field); - } else { - context.doc().add(field); - } - } - private void indexValue(ParseContext context, String value) { + if (value == null) { return; } @@ -541,14 +529,23 @@ private void indexValue(ParseContext context, String value) { // convert to utf8 only once before feeding postings/dv/stored fields final BytesRef binaryValue = new BytesRef(value); if (fieldType.indexOptions() != IndexOptions.NONE || fieldType.stored()) { - indexField(context, new KeywordField(fieldType().name(), binaryValue, fieldType)); + Field field = new KeywordField(fieldType().name(), binaryValue, fieldType); + + if (dimension) { + // Add dimension field with key so that we ensure it is single-valued. + // Dimension fields are always indexed. + context.doc().addWithKey(fieldType().name(), field); + } else { + context.doc().add(field); + } + if (fieldType().hasDocValues() == false && fieldType.omitNorms()) { context.addToFieldNames(fieldType().name()); } } if (fieldType().hasDocValues()) { - indexField(context, new SortedSetDocValuesField(fieldType().name(), binaryValue)); + context.doc().add(new SortedSetDocValuesField(fieldType().name(), binaryValue)); } } diff --git a/server/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java index f2dd9404a8b61..0eed54082f68f 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java @@ -118,10 +118,9 @@ public Builder(String name, NumberType type, ScriptCompiler compiler, boolean ig if (v && EnumSet.of(NumberType.INTEGER, NumberType.LONG, NumberType.BYTE, NumberType.SHORT).contains(type) == false) { throw new IllegalArgumentException("Parameter [dimension] cannot be set to numeric type [" + type.name + "]"); } - - if (v && indexed.getValue() == false && hasDocValues.getValue() == false) { + if (v && (indexed.getValue() == false || hasDocValues.getValue() == false)) { throw new IllegalArgumentException( - "Field [dimension] requires one of [" + indexed.name + "] or [" + hasDocValues.name + "] to be true" + "Field [dimension] requires that [" + indexed.name + "] and [" + hasDocValues.name + "] are true" ); } }); diff --git a/server/src/test/java/org/elasticsearch/index/mapper/IpFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/IpFieldMapperTests.java index 996131f581a87..3373d08d67fc2 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/IpFieldMapperTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/IpFieldMapperTests.java @@ -215,53 +215,45 @@ public void testDimension() throws IOException { assertDimension(false, IpFieldMapper.IpFieldType::isDimension); } - public void testDimensionAndIndexedOrDocvalues() { - Exception e = expectThrows(MapperParsingException.class, () -> createDocumentMapper(fieldMapping(b -> { - minimalMapping(b); - b.field("dimension", true).field("index", false).field("doc_values", false); - }))); - assertThat(e.getCause().getMessage(), - containsString("Field [dimension] requires one of [index] or [doc_values] to be true")); - } - - public void testDimensionMultiValuedField() throws IOException { + public void testDimensionIndexedAndDocvalues() { { - DocumentMapper mapper = createDocumentMapper(fieldMapping(b -> { + Exception e = expectThrows(MapperParsingException.class, () -> createDocumentMapper(fieldMapping(b -> { minimalMapping(b); - b.field("dimension", true); - })); - - Exception e = expectThrows(MapperParsingException.class, - () -> mapper.parse(source(b -> b.array("field", "192.168.1.1", "192.168.1.1")))); + b.field("dimension", true).field("index", false).field("doc_values", false); + }))); assertThat(e.getCause().getMessage(), - containsString("Dimension field [field] cannot be a multi-valued field")); + containsString("Field [dimension] requires that [index] and [doc_values] are true")); } { - // Disable doc_values - DocumentMapper mapper = createDocumentMapper(fieldMapping(b -> { + Exception e = expectThrows(MapperParsingException.class, () -> createDocumentMapper(fieldMapping(b -> { minimalMapping(b); - b.field("dimension", true).field("doc_values",false); - })); - - Exception e = expectThrows(MapperParsingException.class, - () -> mapper.parse(source(b -> b.array("field", "192.168.1.1", "192.168.1.1")))); + b.field("dimension", true).field("index", true).field("doc_values", false); + }))); assertThat(e.getCause().getMessage(), - containsString("Dimension field [field] cannot be a multi-valued field")); + containsString("Field [dimension] requires that [index] and [doc_values] are true")); } { - // Disable indexed fields - DocumentMapper mapper = createDocumentMapper(fieldMapping(b -> { + Exception e = expectThrows(MapperParsingException.class, () -> createDocumentMapper(fieldMapping(b -> { minimalMapping(b); - b.field("dimension", true).field("index",false); - })); - - Exception e = expectThrows(MapperParsingException.class, - () -> mapper.parse(source(b -> b.array("field", "192.168.1.1", "192.168.1.1")))); + b.field("dimension", true).field("index", false).field("doc_values", true); + }))); assertThat(e.getCause().getMessage(), - containsString("Dimension field [field] cannot be a multi-valued field")); + containsString("Field [dimension] requires that [index] and [doc_values] are true")); } } + public void testDimensionMultiValuedField() throws IOException { + DocumentMapper mapper = createDocumentMapper(fieldMapping(b -> { + minimalMapping(b); + b.field("dimension", true); + })); + + Exception e = expectThrows(MapperParsingException.class, + () -> mapper.parse(source(b -> b.array("field", "192.168.1.1", "192.168.1.1")))); + assertThat(e.getCause().getMessage(), + containsString("Dimension field [field] cannot be a multi-valued field")); + } + @Override protected String generateRandomInputValue(MappedFieldType ft) { return NetworkAddress.format(randomIp(randomBoolean())); diff --git a/server/src/test/java/org/elasticsearch/index/mapper/KeywordFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/KeywordFieldMapperTests.java index db84986b2186f..5f7619228f38e 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/KeywordFieldMapperTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/KeywordFieldMapperTests.java @@ -320,53 +320,45 @@ public void testDimensionAndIgnoreAbove() { containsString("Field [ignore_above] cannot be set in conjunction with field [dimension]")); } - public void testDimensionAndIndexedOrDocvalues() { - Exception e = expectThrows(MapperParsingException.class, () -> createDocumentMapper(fieldMapping(b -> { - minimalMapping(b); - b.field("dimension", true).field("index", false).field("doc_values", false); - }))); - assertThat(e.getCause().getMessage(), - containsString("Field [dimension] requires one of [index] or [doc_values] to be true")); - } - - public void testDimensionMultiValuedField() throws IOException { + public void testDimensionIndexedAndDocvalues() { { - DocumentMapper mapper = createDocumentMapper(fieldMapping(b -> { + Exception e = expectThrows(MapperParsingException.class, () -> createDocumentMapper(fieldMapping(b -> { minimalMapping(b); - b.field("dimension", true); - })); - - Exception e = expectThrows(MapperParsingException.class, - () -> mapper.parse(source(b -> b.array("field", "1234", "45678")))); + b.field("dimension", true).field("index", false).field("doc_values", false); + }))); assertThat(e.getCause().getMessage(), - containsString("Dimension field [field] cannot be a multi-valued field")); + containsString("Field [dimension] requires that [index] and [doc_values] are true")); } { - // Disable doc_values - DocumentMapper mapper = createDocumentMapper(fieldMapping(b -> { + Exception e = expectThrows(MapperParsingException.class, () -> createDocumentMapper(fieldMapping(b -> { minimalMapping(b); - b.field("dimension", true).field("doc_values",false); - })); - - Exception e = expectThrows(MapperParsingException.class, - () -> mapper.parse(source(b -> b.array("field", "1234", "45678")))); + b.field("dimension", true).field("index", true).field("doc_values", false); + }))); assertThat(e.getCause().getMessage(), - containsString("Dimension field [field] cannot be a multi-valued field")); + containsString("Field [dimension] requires that [index] and [doc_values] are true")); } { - // Disable indexed fields - DocumentMapper mapper = createDocumentMapper(fieldMapping(b -> { + Exception e = expectThrows(MapperParsingException.class, () -> createDocumentMapper(fieldMapping(b -> { minimalMapping(b); - b.field("dimension", true).field("index",false); - })); - - Exception e = expectThrows(MapperParsingException.class, - () -> mapper.parse(source(b -> b.array("field", "1234", "45678")))); + b.field("dimension", true).field("index", false).field("doc_values", true); + }))); assertThat(e.getCause().getMessage(), - containsString("Dimension field [field] cannot be a multi-valued field")); + containsString("Field [dimension] requires that [index] and [doc_values] are true")); } } + public void testDimensionMultiValuedField() throws IOException { + DocumentMapper mapper = createDocumentMapper(fieldMapping(b -> { + minimalMapping(b); + b.field("dimension", true); + })); + + Exception e = expectThrows(MapperParsingException.class, + () -> mapper.parse(source(b -> b.array("field", "1234", "45678")))); + assertThat(e.getCause().getMessage(), + containsString("Dimension field [field] cannot be a multi-valued field")); + } + public void testConfigureSimilarity() throws IOException { MapperService mapperService = createMapperService( fieldMapping(b -> b.field("type", "keyword").field("similarity", "boolean")) diff --git a/server/src/test/java/org/elasticsearch/index/mapper/WholeNumberFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/WholeNumberFieldMapperTests.java index 0f73cfbd66113..ea41733e59184 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/WholeNumberFieldMapperTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/WholeNumberFieldMapperTests.java @@ -35,53 +35,45 @@ public void testDimension() throws IOException { assertDimension(false, NumberFieldMapper.NumberFieldType::isDimension); } - public void testDimensionAndIndexedOrDocvalues() { - Exception e = expectThrows(MapperParsingException.class, () -> createDocumentMapper(fieldMapping(b -> { - minimalMapping(b); - b.field("dimension", true).field("index", false).field("doc_values", false); - }))); - assertThat(e.getCause().getMessage(), - containsString("Field [dimension] requires one of [index] or [doc_values] to be true")); - } - - public void testDimensionMultiValuedField() throws IOException { + public void testDimensionIndexedAndDocvalues() { { - DocumentMapper mapper = createDocumentMapper(fieldMapping(b -> { + Exception e = expectThrows(MapperParsingException.class, () -> createDocumentMapper(fieldMapping(b -> { minimalMapping(b); - b.field("dimension", true); - })); - - Exception e = expectThrows(MapperParsingException.class, - () -> mapper.parse(source(b -> b.array("field", randomNumber(), randomNumber(), randomNumber())))); + b.field("dimension", true).field("index", false).field("doc_values", false); + }))); assertThat(e.getCause().getMessage(), - containsString("Dimension field [field] cannot be a multi-valued field")); + containsString("Field [dimension] requires that [index] and [doc_values] are true")); } { - // Disable doc_values - DocumentMapper mapper = createDocumentMapper(fieldMapping(b -> { + Exception e = expectThrows(MapperParsingException.class, () -> createDocumentMapper(fieldMapping(b -> { minimalMapping(b); - b.field("dimension", true).field("doc_values",false); - })); - - Exception e = expectThrows(MapperParsingException.class, - () -> mapper.parse(source(b -> b.array("field", randomNumber(), randomNumber(), randomNumber())))); + b.field("dimension", true).field("index", true).field("doc_values", false); + }))); assertThat(e.getCause().getMessage(), - containsString("Dimension field [field] cannot be a multi-valued field")); + containsString("Field [dimension] requires that [index] and [doc_values] are true")); } { - // Disable indexed fields - DocumentMapper mapper = createDocumentMapper(fieldMapping(b -> { + Exception e = expectThrows(MapperParsingException.class, () -> createDocumentMapper(fieldMapping(b -> { minimalMapping(b); - b.field("dimension", true).field("index",false); - })); - - Exception e = expectThrows(MapperParsingException.class, - () -> mapper.parse(source(b -> b.array("field", randomNumber(), randomNumber(), randomNumber())))); + b.field("dimension", true).field("index", false).field("doc_values", true); + }))); assertThat(e.getCause().getMessage(), - containsString("Dimension field [field] cannot be a multi-valued field")); + containsString("Field [dimension] requires that [index] and [doc_values] are true")); } } + public void testDimensionMultiValuedField() throws IOException { + DocumentMapper mapper = createDocumentMapper(fieldMapping(b -> { + minimalMapping(b); + b.field("dimension", true); + })); + + Exception e = expectThrows(MapperParsingException.class, + () -> mapper.parse(source(b -> b.array("field", randomNumber(), randomNumber(), randomNumber())))); + assertThat(e.getCause().getMessage(), + containsString("Dimension field [field] cannot be a multi-valued field")); + } + @Override protected void registerParameters(ParameterChecker checker) throws IOException { super.registerParameters(checker); From 78994339d9acc0db12097372a465e43c0c90ae86 Mon Sep 17 00:00:00 2001 From: Christos Soulios Date: Tue, 6 Jul 2021 18:53:44 +0300 Subject: [PATCH 06/12] Add constraint for dimension fields per index --- .../common/settings/IndexScopedSettings.java | 1 + .../java/org/elasticsearch/index/IndexSettings.java | 12 ++++++++++++ .../elasticsearch/index/mapper/MappedFieldType.java | 7 +++++++ .../elasticsearch/index/mapper/MapperService.java | 3 +++ .../elasticsearch/index/mapper/MappingLookup.java | 8 ++++++++ .../index/mapper/DocumentMapperTests.java | 13 +++++++++++++ 6 files changed, 44 insertions(+) diff --git a/server/src/main/java/org/elasticsearch/common/settings/IndexScopedSettings.java b/server/src/main/java/org/elasticsearch/common/settings/IndexScopedSettings.java index 9745ba80ddc6a..8e0f5de3d9e64 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/IndexScopedSettings.java +++ b/server/src/main/java/org/elasticsearch/common/settings/IndexScopedSettings.java @@ -144,6 +144,7 @@ public final class IndexScopedSettings extends AbstractScopedSettings { MapperService.INDEX_MAPPING_NESTED_DOCS_LIMIT_SETTING, MapperService.INDEX_MAPPING_TOTAL_FIELDS_LIMIT_SETTING, MapperService.INDEX_MAPPING_DEPTH_LIMIT_SETTING, + MapperService.INDEX_MAPPING_DIMENSION_FIELDS_LIMIT_SETTING, MapperService.INDEX_MAPPING_FIELD_NAME_LENGTH_LIMIT_SETTING, BitsetFilterCache.INDEX_LOAD_RANDOM_ACCESS_FILTERS_EAGERLY_SETTING, IndexModule.INDEX_STORE_TYPE_SETTING, diff --git a/server/src/main/java/org/elasticsearch/index/IndexSettings.java b/server/src/main/java/org/elasticsearch/index/IndexSettings.java index f866bb60f883a..c36db7e5a385d 100644 --- a/server/src/main/java/org/elasticsearch/index/IndexSettings.java +++ b/server/src/main/java/org/elasticsearch/index/IndexSettings.java @@ -31,6 +31,7 @@ import java.util.function.Function; import static org.elasticsearch.index.mapper.MapperService.INDEX_MAPPING_DEPTH_LIMIT_SETTING; +import static org.elasticsearch.index.mapper.MapperService.INDEX_MAPPING_DIMENSION_FIELDS_LIMIT_SETTING; import static org.elasticsearch.index.mapper.MapperService.INDEX_MAPPING_FIELD_NAME_LENGTH_LIMIT_SETTING; import static org.elasticsearch.index.mapper.MapperService.INDEX_MAPPING_NESTED_DOCS_LIMIT_SETTING; import static org.elasticsearch.index.mapper.MapperService.INDEX_MAPPING_NESTED_FIELDS_LIMIT_SETTING; @@ -379,6 +380,7 @@ private void setRetentionLeaseMillis(final TimeValue retentionLease) { private volatile long mappingTotalFieldsLimit; private volatile long mappingDepthLimit; private volatile long mappingFieldNameLengthLimit; + private volatile long mappingDimensionFieldsLimit; /** * The maximum number of refresh listeners allows on this shard. @@ -503,6 +505,7 @@ public IndexSettings(final IndexMetadata indexMetadata, final Settings nodeSetti mappingTotalFieldsLimit = scopedSettings.get(INDEX_MAPPING_TOTAL_FIELDS_LIMIT_SETTING); mappingDepthLimit = scopedSettings.get(INDEX_MAPPING_DEPTH_LIMIT_SETTING); mappingFieldNameLengthLimit = scopedSettings.get(INDEX_MAPPING_FIELD_NAME_LENGTH_LIMIT_SETTING); + mappingDimensionFieldsLimit = scopedSettings.get(INDEX_MAPPING_DIMENSION_FIELDS_LIMIT_SETTING); scopedSettings.addSettingsUpdateConsumer(MergePolicyConfig.INDEX_COMPOUND_FORMAT_SETTING, mergePolicyConfig::setNoCFSRatio); scopedSettings.addSettingsUpdateConsumer(MergePolicyConfig.INDEX_MERGE_POLICY_DELETES_PCT_ALLOWED_SETTING, @@ -558,6 +561,7 @@ public IndexSettings(final IndexMetadata indexMetadata, final Settings nodeSetti scopedSettings.addSettingsUpdateConsumer(INDEX_MAPPING_TOTAL_FIELDS_LIMIT_SETTING, this::setMappingTotalFieldsLimit); scopedSettings.addSettingsUpdateConsumer(INDEX_MAPPING_DEPTH_LIMIT_SETTING, this::setMappingDepthLimit); scopedSettings.addSettingsUpdateConsumer(INDEX_MAPPING_FIELD_NAME_LENGTH_LIMIT_SETTING, this::setMappingFieldNameLengthLimit); + scopedSettings.addSettingsUpdateConsumer(INDEX_MAPPING_DIMENSION_FIELDS_LIMIT_SETTING, this::setMappingDimensionFieldsLimit); } private void setSearchIdleAfter(TimeValue searchIdleAfter) { this.searchIdleAfter = searchIdleAfter; } @@ -1021,4 +1025,12 @@ public long getMappingFieldNameLengthLimit() { private void setMappingFieldNameLengthLimit(long value) { this.mappingFieldNameLengthLimit = value; } + + public long getMappingDimensionFieldsLimit() { + return mappingDimensionFieldsLimit; + } + + private void setMappingDimensionFieldsLimit(long value) { + this.mappingDimensionFieldsLimit = value; + } } diff --git a/server/src/main/java/org/elasticsearch/index/mapper/MappedFieldType.java b/server/src/main/java/org/elasticsearch/index/mapper/MappedFieldType.java index 8aed9fdbdb049..0d5edfa025980 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/MappedFieldType.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/MappedFieldType.java @@ -165,6 +165,13 @@ public boolean isAggregatable() { } } + /** + * @return true if field has been marked as a dimension field + */ + public boolean isDimension() { + return false; + } + /** Generates a query that will only match documents that contain the given value. * The default implementation returns a {@link TermQuery} over the value bytes * @throws IllegalArgumentException if {@code value} cannot be converted to the expected data type or if the field is not searchable diff --git a/server/src/main/java/org/elasticsearch/index/mapper/MapperService.java b/server/src/main/java/org/elasticsearch/index/mapper/MapperService.java index dba9be14862fe..d18588c09a38e 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/MapperService.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/MapperService.java @@ -89,6 +89,9 @@ public enum MergeReason { Setting.longSetting("index.mapping.depth.limit", 20L, 1, Property.Dynamic, Property.IndexScope); public static final Setting INDEX_MAPPING_FIELD_NAME_LENGTH_LIMIT_SETTING = Setting.longSetting("index.mapping.field_name_length.limit", Long.MAX_VALUE, 1L, Property.Dynamic, Property.IndexScope); + public static final Setting INDEX_MAPPING_DIMENSION_FIELDS_LIMIT_SETTING = + Setting.longSetting("index.mapping.dimension_fields.limit", 16, 0, Property.Dynamic, Property.IndexScope); + private final IndexAnalyzers indexAnalyzers; private final MappingParser mappingParser; diff --git a/server/src/main/java/org/elasticsearch/index/mapper/MappingLookup.java b/server/src/main/java/org/elasticsearch/index/mapper/MappingLookup.java index 975a3860e42f8..de8879bd36187 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/MappingLookup.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/MappingLookup.java @@ -204,6 +204,7 @@ void checkLimits(IndexSettings settings) { checkObjectDepthLimit(settings.getMappingDepthLimit()); checkFieldNameLengthLimit(settings.getMappingFieldNameLengthLimit()); checkNestedLimit(settings.getMappingNestedFieldsLimit()); + checkDimensionFieldLimit(settings.getMappingDimensionFieldsLimit()); } private void checkFieldLimit(long limit) { @@ -217,6 +218,13 @@ void checkFieldLimit(long limit, int additionalFieldsToAdd) { } } + private void checkDimensionFieldLimit(long limit) { + long dimensionFieldCount = fieldMappers.values().stream().filter(m -> ((FieldMapper) m).fieldType().isDimension()).count(); + if (dimensionFieldCount > limit) { + throw new IllegalArgumentException("Limit of total dimension fields [" + limit + "] has been exceeded"); + } + } + private void checkObjectDepthLimit(long limit) { for (String objectPath : objectMappers.keySet()) { int numDots = 0; diff --git a/server/src/test/java/org/elasticsearch/index/mapper/DocumentMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/DocumentMapperTests.java index 97d20baa6843e..ffa1374a08449 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/DocumentMapperTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/DocumentMapperTests.java @@ -287,4 +287,17 @@ public void testEmptyDocumentMapper() { assertEquals(10, documentMapper.mappers().getMapping().getMetadataMappersMap().size()); assertEquals(10, documentMapper.mappers().getMatchingFieldNames("*").size()); } + + public void testTooManyDimensionFields() { + // By default no more than 16 dimensions per document are supported + IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> createDocumentMapper(mapping(b -> { + for (int i = 0; i < 17; i++) { + b.startObject("field" + i) + .field("type", randomFrom("ip", "keyword", "long", "integer", "byte", "short")) + .field("dimension", true) + .endObject(); + } + }))); + assertThat(e.getMessage(), containsString("Limit of total dimension fields [16] has been exceeded")); + } } From d3a6262269dd8f8a91d10d86227a03fcd5e27398 Mon Sep 17 00:00:00 2001 From: Christos Soulios Date: Tue, 6 Jul 2021 19:34:25 +0300 Subject: [PATCH 07/12] Fixed broken tests --- .../java/org/elasticsearch/index/mapper/MappingLookup.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/server/src/main/java/org/elasticsearch/index/mapper/MappingLookup.java b/server/src/main/java/org/elasticsearch/index/mapper/MappingLookup.java index de8879bd36187..2b0e3ba19be83 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/MappingLookup.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/MappingLookup.java @@ -219,7 +219,10 @@ void checkFieldLimit(long limit, int additionalFieldsToAdd) { } private void checkDimensionFieldLimit(long limit) { - long dimensionFieldCount = fieldMappers.values().stream().filter(m -> ((FieldMapper) m).fieldType().isDimension()).count(); + long dimensionFieldCount = fieldMappers.values() + .stream() + .filter(m -> m instanceof FieldMapper && ((FieldMapper) m).fieldType().isDimension()) + .count(); if (dimensionFieldCount > limit) { throw new IllegalArgumentException("Limit of total dimension fields [" + limit + "] has been exceeded"); } From 43257b150a9f71efbd2977dc98d8f64b78a02ae7 Mon Sep 17 00:00:00 2001 From: Christos Soulios Date: Tue, 6 Jul 2021 20:42:34 +0300 Subject: [PATCH 08/12] Fix broken test --- .../xpack/ccr/action/TransportResumeFollowActionTests.java | 1 + 1 file changed, 1 insertion(+) diff --git a/x-pack/plugin/ccr/src/test/java/org/elasticsearch/xpack/ccr/action/TransportResumeFollowActionTests.java b/x-pack/plugin/ccr/src/test/java/org/elasticsearch/xpack/ccr/action/TransportResumeFollowActionTests.java index c76fa1277f8b4..561015a5888be 100644 --- a/x-pack/plugin/ccr/src/test/java/org/elasticsearch/xpack/ccr/action/TransportResumeFollowActionTests.java +++ b/x-pack/plugin/ccr/src/test/java/org/elasticsearch/xpack/ccr/action/TransportResumeFollowActionTests.java @@ -216,6 +216,7 @@ public void testDynamicIndexSettingsAreClassified() { replicatedSettings.add(MapperService.INDEX_MAPPING_NESTED_FIELDS_LIMIT_SETTING); replicatedSettings.add(MapperService.INDEX_MAPPING_DEPTH_LIMIT_SETTING); replicatedSettings.add(MapperService.INDEX_MAPPING_FIELD_NAME_LENGTH_LIMIT_SETTING); + replicatedSettings.add(MapperService.INDEX_MAPPING_DIMENSION_FIELDS_LIMIT_SETTING); replicatedSettings.add(IndexSettings.MAX_NGRAM_DIFF_SETTING); replicatedSettings.add(IndexSettings.MAX_SHINGLE_DIFF_SETTING); From 12a2f9a9110c95dce1430c700bba071ff45f179b Mon Sep 17 00:00:00 2001 From: Christos Soulios Date: Tue, 6 Jul 2021 21:37:25 +0300 Subject: [PATCH 09/12] Do not allow keywords > 1024 chars long --- .../index/mapper/KeywordFieldMapper.java | 9 +++++++++ .../index/mapper/KeywordFieldMapperTests.java | 12 ++++++++++++ 2 files changed, 21 insertions(+) diff --git a/server/src/main/java/org/elasticsearch/index/mapper/KeywordFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/KeywordFieldMapper.java index 230c258ca8050..f3f496b76d4fb 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/KeywordFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/KeywordFieldMapper.java @@ -443,6 +443,9 @@ public boolean isDimension() { } } + /** The maximum keyword length allowed for a dimension field */ + private static final int DIMENSION_MAX_LENGTH = 1024; + private final boolean indexed; private final boolean hasDocValues; private final String nullValue; @@ -517,6 +520,12 @@ private void indexValue(DocumentParserContext context, String value) { throw new IllegalArgumentException("Dimension field [" + fieldType().name() + "] cannot be a multi-valued field."); } + if (dimension && value.length() > DIMENSION_MAX_LENGTH) { + throw new IllegalArgumentException( + "Dimension field [" + fieldType().name() + "] cannot be more than [" + DIMENSION_MAX_LENGTH + "] characters long." + ); + } + if (value.length() > ignoreAbove) { context.addIgnoredField(name()); return; diff --git a/server/src/test/java/org/elasticsearch/index/mapper/KeywordFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/KeywordFieldMapperTests.java index 5f7619228f38e..a7889d15ff984 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/KeywordFieldMapperTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/KeywordFieldMapperTests.java @@ -359,6 +359,18 @@ public void testDimensionMultiValuedField() throws IOException { containsString("Dimension field [field] cannot be a multi-valued field")); } + public void testDimensionExtraLongKeyword() throws IOException { + DocumentMapper mapper = createDocumentMapper(fieldMapping(b -> { + minimalMapping(b); + b.field("dimension", true); + })); + + Exception e = expectThrows(MapperParsingException.class, + () -> mapper.parse(source(b -> b.field("field", randomAlphaOfLengthBetween(1025, 2048))))); + assertThat(e.getCause().getMessage(), + containsString("Dimension field [field] cannot be more than [1024] characters long.")); + } + public void testConfigureSimilarity() throws IOException { MapperService mapperService = createMapperService( fieldMapping(b -> b.field("type", "keyword").field("similarity", "boolean")) From e4eb87b996e7726716b688c11ff36817cc6b172d Mon Sep 17 00:00:00 2001 From: Christos Soulios Date: Tue, 6 Jul 2021 22:34:57 +0300 Subject: [PATCH 10/12] Do not allow keywords > 1024 bytes long The limit should be 1025 bytes, not chars --- .../index/mapper/KeywordFieldMapper.java | 13 ++++++------- .../index/mapper/KeywordFieldMapperTests.java | 4 ++-- 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/index/mapper/KeywordFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/KeywordFieldMapper.java index f3f496b76d4fb..ab0d197ec5bfa 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/KeywordFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/KeywordFieldMapper.java @@ -444,7 +444,7 @@ public boolean isDimension() { } /** The maximum keyword length allowed for a dimension field */ - private static final int DIMENSION_MAX_LENGTH = 1024; + private static final int DIMENSION_MAX_BYTES = 1024; private final boolean indexed; private final boolean hasDocValues; @@ -520,12 +520,6 @@ private void indexValue(DocumentParserContext context, String value) { throw new IllegalArgumentException("Dimension field [" + fieldType().name() + "] cannot be a multi-valued field."); } - if (dimension && value.length() > DIMENSION_MAX_LENGTH) { - throw new IllegalArgumentException( - "Dimension field [" + fieldType().name() + "] cannot be more than [" + DIMENSION_MAX_LENGTH + "] characters long." - ); - } - if (value.length() > ignoreAbove) { context.addIgnoredField(name()); return; @@ -538,6 +532,11 @@ private void indexValue(DocumentParserContext context, String value) { // convert to utf8 only once before feeding postings/dv/stored fields final BytesRef binaryValue = new BytesRef(value); + if (dimension && binaryValue.length > DIMENSION_MAX_BYTES) { + throw new IllegalArgumentException( + "Dimension field [" + fieldType().name() + "] cannot be more than [" + DIMENSION_MAX_BYTES + "] bytes long." + ); + } if (fieldType.indexOptions() != IndexOptions.NONE || fieldType.stored()) { Field field = new KeywordField(fieldType().name(), binaryValue, fieldType); diff --git a/server/src/test/java/org/elasticsearch/index/mapper/KeywordFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/KeywordFieldMapperTests.java index a7889d15ff984..9610b3974a157 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/KeywordFieldMapperTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/KeywordFieldMapperTests.java @@ -366,9 +366,9 @@ public void testDimensionExtraLongKeyword() throws IOException { })); Exception e = expectThrows(MapperParsingException.class, - () -> mapper.parse(source(b -> b.field("field", randomAlphaOfLengthBetween(1025, 2048))))); + () -> mapper.parse(source(b -> b.field("field", randomAlphaOfLengthBetween(1024, 2048))))); assertThat(e.getCause().getMessage(), - containsString("Dimension field [field] cannot be more than [1024] characters long.")); + containsString("Dimension field [field] cannot be more than [1024] bytes long.")); } public void testConfigureSimilarity() throws IOException { From e4a5f0529ff4f1e27046f3858eef463d4465f875 Mon Sep 17 00:00:00 2001 From: Christos Soulios Date: Tue, 6 Jul 2021 22:45:10 +0300 Subject: [PATCH 11/12] Do not allow normalizer to best for kw --- .../index/mapper/KeywordFieldMapper.java | 20 +++++++------------ .../index/mapper/KeywordFieldMapperTests.java | 9 +++++++++ 2 files changed, 16 insertions(+), 13 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/index/mapper/KeywordFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/KeywordFieldMapper.java index ab0d197ec5bfa..3514766d5ba3b 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/KeywordFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/KeywordFieldMapper.java @@ -124,19 +124,13 @@ public Builder(String name, IndexAnalyzers indexAnalyzers, ScriptCompiler script this.script.precludesParameters(nullValue); addScriptValidation(script, indexed, hasDocValues); - this.dimension = Parameter.boolParam("dimension", false, m -> toType(m).dimension, false) - .setValidator(v -> { - if (v && (indexed.getValue() == false || hasDocValues.getValue() == false)) { - throw new IllegalArgumentException( - "Field [dimension] requires that [" + indexed.name + "] and [" + hasDocValues.name + "] are true" - ); - } - if (v && ignoreAbove.getValue() < ignoreAbove.getDefaultValue()) { - throw new IllegalArgumentException( - "Field [" + ignoreAbove.name + "] cannot be set in conjunction with field [dimension]" - ); - } - }); + this.dimension = Parameter.boolParam("dimension", false, m -> toType(m).dimension, false).setValidator(v -> { + if (v && (indexed.getValue() == false || hasDocValues.getValue() == false)) { + throw new IllegalArgumentException( + "Field [dimension] requires that [" + indexed.name + "] and [" + hasDocValues.name + "] are true" + ); + } + }).precludesParameters(normalizer, ignoreAbove); } public Builder(String name) { diff --git a/server/src/test/java/org/elasticsearch/index/mapper/KeywordFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/KeywordFieldMapperTests.java index 9610b3974a157..4d53b9435934b 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/KeywordFieldMapperTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/KeywordFieldMapperTests.java @@ -320,6 +320,15 @@ public void testDimensionAndIgnoreAbove() { containsString("Field [ignore_above] cannot be set in conjunction with field [dimension]")); } + public void testDimensionAndNormalizer() { + Exception e = expectThrows(MapperParsingException.class, () -> createDocumentMapper(fieldMapping(b -> { + minimalMapping(b); + b.field("dimension", true).field("normalizer", "my_normalizer"); + }))); + assertThat(e.getCause().getMessage(), + containsString("Field [normalizer] cannot be set in conjunction with field [dimension]")); + } + public void testDimensionIndexedAndDocvalues() { { Exception e = expectThrows(MapperParsingException.class, () -> createDocumentMapper(fieldMapping(b -> { From d755c3806d26a99f2778ee229b4998dcb82ded5f Mon Sep 17 00:00:00 2001 From: Christos Soulios Date: Wed, 7 Jul 2021 18:07:29 +0300 Subject: [PATCH 12/12] Move getByKey right above addWithKey for clarity --- .../org/elasticsearch/index/mapper/IpFieldMapper.java | 8 +++----- .../elasticsearch/index/mapper/KeywordFieldMapper.java | 10 ++++------ 2 files changed, 7 insertions(+), 11 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/index/mapper/IpFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/IpFieldMapper.java index 80b92bc30a4ce..608041a4dcd35 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/IpFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/IpFieldMapper.java @@ -474,16 +474,14 @@ protected void parseCreateField(DocumentParserContext context) throws IOExceptio } private void indexValue(DocumentParserContext context, InetAddress address) { - // Check that a dimension field is single-valued and not an array - if (dimension && context.doc().getByKey(fieldType().name()) != null) { - throw new IllegalArgumentException("Dimension field [" + fieldType().name() + "] cannot be a multi-valued field."); - } if (indexed) { Field field = new InetAddressPoint(fieldType().name(), address); - if (dimension) { // Add dimension field with key so that we ensure it is single-valued. // Dimension fields are always indexed. + if (context.doc().getByKey(fieldType().name()) != null) { + throw new IllegalArgumentException("Dimension field [" + fieldType().name() + "] cannot be a multi-valued field."); + } context.doc().addWithKey(fieldType().name(), field); } else { context.doc().add(field); diff --git a/server/src/main/java/org/elasticsearch/index/mapper/KeywordFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/KeywordFieldMapper.java index 3514766d5ba3b..bb5de3d0d0906 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/KeywordFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/KeywordFieldMapper.java @@ -509,11 +509,6 @@ private void indexValue(DocumentParserContext context, String value) { return; } - // Check that a dimension field is single-valued and not an array - if (dimension && context.doc().getByKey(fieldType().name()) != null) { - throw new IllegalArgumentException("Dimension field [" + fieldType().name() + "] cannot be a multi-valued field."); - } - if (value.length() > ignoreAbove) { context.addIgnoredField(name()); return; @@ -533,8 +528,11 @@ private void indexValue(DocumentParserContext context, String value) { } if (fieldType.indexOptions() != IndexOptions.NONE || fieldType.stored()) { Field field = new KeywordField(fieldType().name(), binaryValue, fieldType); - if (dimension) { + // Check that a dimension field is single-valued and not an array + if (context.doc().getByKey(fieldType().name()) != null) { + throw new IllegalArgumentException("Dimension field [" + fieldType().name() + "] cannot be a multi-valued field."); + } // Add dimension field with key so that we ensure it is single-valued. // Dimension fields are always indexed. context.doc().addWithKey(fieldType().name(), field);