From c8d9d83db9c256b3bc1e970b4c352dc13c392c62 Mon Sep 17 00:00:00 2001 From: jimczi Date: Thu, 28 Mar 2019 16:19:31 +0100 Subject: [PATCH 1/2] Fix merging of search_as_you_type field mapper The merge of the `search_as_you_type` field mapper uses the wrong prefix field and does not update the underlying field types. --- .../mapper/SearchAsYouTypeFieldMapper.java | 21 +++++-- .../SearchAsYouTypeFieldMapperTests.java | 55 +++++++++++++++++++ 2 files changed, 71 insertions(+), 5 deletions(-) diff --git a/modules/mapper-extras/src/main/java/org/elasticsearch/index/mapper/SearchAsYouTypeFieldMapper.java b/modules/mapper-extras/src/main/java/org/elasticsearch/index/mapper/SearchAsYouTypeFieldMapper.java index 69948bf98a6ac..0931b497a4366 100644 --- a/modules/mapper-extras/src/main/java/org/elasticsearch/index/mapper/SearchAsYouTypeFieldMapper.java +++ b/modules/mapper-extras/src/main/java/org/elasticsearch/index/mapper/SearchAsYouTypeFieldMapper.java @@ -516,7 +516,7 @@ protected void parseCreateField(ParseContext context, List field @Override protected String contentType() { - return CONTENT_TYPE; + return "shingle"; } } @@ -663,6 +663,16 @@ public SearchAsYouTypeFieldMapper(String simpleName, this.maxShingleSize = maxShingleSize; } + @Override + public FieldMapper updateFieldType(Map fullNameToFieldType) { + SearchAsYouTypeFieldMapper fieldMapper = (SearchAsYouTypeFieldMapper) super.updateFieldType(fullNameToFieldType); + fieldMapper.prefixField = (PrefixFieldMapper) fieldMapper.prefixField.updateFieldType(fullNameToFieldType); + for (int i = 0; i < fieldMapper.shingleFields.length; i++) { + fieldMapper.shingleFields[i] = (ShingleFieldMapper) fieldMapper.shingleFields[i].updateFieldType(fullNameToFieldType); + } + return fieldMapper; + } + @Override protected void parseCreateField(ParseContext context, List fields) throws IOException { final String value = context.externalValueSet() ? context.externalValue().toString() : context.parser().textOrNull(); @@ -692,10 +702,12 @@ protected void doMerge(Mapper mergeWith) { super.doMerge(mergeWith); SearchAsYouTypeFieldMapper mw = (SearchAsYouTypeFieldMapper) mergeWith; if (mw.maxShingleSize != maxShingleSize) { - throw new IllegalArgumentException("mapper [" + name() + "] has different maxShingleSize setting, current [" + throw new IllegalArgumentException("mapper [" + name() + "] has different [max_shingle_size] setting, current [" + this.maxShingleSize + "], merged [" + mw.maxShingleSize + "]"); } - this.prefixField = (PrefixFieldMapper) this.prefixField.merge(mw); + if (prefixField.equals(mw.prefixField) == false) { + this.prefixField = (PrefixFieldMapper) this.prefixField.merge(mw.prefixField); + } ShingleFieldMapper[] shingleFieldMappers = new ShingleFieldMapper[mw.shingleFields.length]; for (int i = 0; i < shingleFieldMappers.length; i++) { @@ -736,8 +748,7 @@ public Iterator iterator() { List subIterators = new ArrayList<>(); subIterators.add(prefixField); subIterators.addAll(Arrays.asList(shingleFields)); - @SuppressWarnings("unchecked") Iterator concat = Iterators.concat(super.iterator(), subIterators.iterator()); - return concat; + return Iterators.concat(super.iterator(), subIterators.iterator()); } /** diff --git a/modules/mapper-extras/src/test/java/org/elasticsearch/index/mapper/SearchAsYouTypeFieldMapperTests.java b/modules/mapper-extras/src/test/java/org/elasticsearch/index/mapper/SearchAsYouTypeFieldMapperTests.java index 9ed43a9505624..4622b34ea1514 100644 --- a/modules/mapper-extras/src/test/java/org/elasticsearch/index/mapper/SearchAsYouTypeFieldMapperTests.java +++ b/modules/mapper-extras/src/test/java/org/elasticsearch/index/mapper/SearchAsYouTypeFieldMapperTests.java @@ -71,6 +71,7 @@ import static java.util.Arrays.asList; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.hasProperty; import static org.hamcrest.Matchers.containsInAnyOrder; +import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.hasSize; import static org.hamcrest.Matchers.notNullValue; @@ -180,6 +181,60 @@ public void testConfiguration() throws IOException { getShingleFieldMapper(defaultMapper, "a_field._4gram").fieldType(), 4, analyzerName, prefixFieldMapper.fieldType()); } + public void testSimpleMerge() throws IOException { + MapperService mapperService = createIndex("test").mapperService(); + { + String mapping = Strings.toString(XContentFactory.jsonBuilder().startObject() + .startObject("_doc") + .startObject("properties") + .startObject("a_field") + .field("type", "search_as_you_type") + .field("analyzer", "standard") + .endObject() + .endObject() + .endObject().endObject()); + DocumentMapper mapper = mapperService.merge("_doc", + new CompressedXContent(mapping), MapperService.MergeReason.MAPPING_UPDATE); + } + + { + String mapping = Strings.toString(XContentFactory.jsonBuilder().startObject() + .startObject("_doc") + .startObject("properties") + .startObject("a_field") + .field("type", "search_as_you_type") + .field("analyzer", "standard") + .endObject() + .startObject("b_field") + .field("type", "text") + .endObject() + .endObject() + .endObject().endObject()); + DocumentMapper mapper = mapperService.merge("_doc", + new CompressedXContent(mapping), MapperService.MergeReason.MAPPING_UPDATE); + } + + { + String mapping = Strings.toString(XContentFactory.jsonBuilder().startObject() + .startObject("_doc") + .startObject("properties") + .startObject("a_field") + .field("type", "search_as_you_type") + .field("analyzer", "standard") + .field("max_shingle_size", "4") + .endObject() + .startObject("b_field") + .field("type", "text") + .endObject() + .endObject() + .endObject().endObject()); + IllegalArgumentException e = expectThrows(IllegalArgumentException.class, + () -> mapperService.merge("_doc", + new CompressedXContent(mapping), MapperService.MergeReason.MAPPING_UPDATE)); + assertThat(e.getMessage(), containsString("different [max_shingle_size]")); + } + } + public void testIndexOptions() throws IOException { final String mapping = Strings.toString(XContentFactory.jsonBuilder() .startObject() From 9a4bfb0829384b682f2410e19fb852f177a685ba Mon Sep 17 00:00:00 2001 From: jimczi Date: Thu, 28 Mar 2019 17:21:14 +0100 Subject: [PATCH 2/2] address review --- .../index/mapper/SearchAsYouTypeFieldMapper.java | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/modules/mapper-extras/src/main/java/org/elasticsearch/index/mapper/SearchAsYouTypeFieldMapper.java b/modules/mapper-extras/src/main/java/org/elasticsearch/index/mapper/SearchAsYouTypeFieldMapper.java index 0931b497a4366..867e975e9f51c 100644 --- a/modules/mapper-extras/src/main/java/org/elasticsearch/index/mapper/SearchAsYouTypeFieldMapper.java +++ b/modules/mapper-extras/src/main/java/org/elasticsearch/index/mapper/SearchAsYouTypeFieldMapper.java @@ -705,9 +705,7 @@ protected void doMerge(Mapper mergeWith) { throw new IllegalArgumentException("mapper [" + name() + "] has different [max_shingle_size] setting, current [" + this.maxShingleSize + "], merged [" + mw.maxShingleSize + "]"); } - if (prefixField.equals(mw.prefixField) == false) { - this.prefixField = (PrefixFieldMapper) this.prefixField.merge(mw.prefixField); - } + this.prefixField = (PrefixFieldMapper) this.prefixField.merge(mw.prefixField); ShingleFieldMapper[] shingleFieldMappers = new ShingleFieldMapper[mw.shingleFields.length]; for (int i = 0; i < shingleFieldMappers.length; i++) { @@ -748,7 +746,8 @@ public Iterator iterator() { List subIterators = new ArrayList<>(); subIterators.add(prefixField); subIterators.addAll(Arrays.asList(shingleFields)); - return Iterators.concat(super.iterator(), subIterators.iterator()); + @SuppressWarnings("unchecked") Iterator concat = Iterators.concat(super.iterator(), subIterators.iterator()); + return concat; } /**