From d8ce9ae44732fde6a7c096ed349f8784bcf02257 Mon Sep 17 00:00:00 2001 From: Luca Cavanna Date: Tue, 20 Oct 2020 14:24:36 +0200 Subject: [PATCH 1/2] Move indexAnalyzer logic to FieldTypeLookup MappingLookup currently holds and exposes a FieldNameAnalyzer. Given that it is built based on the field types and their configured field analyzers, it makes more sense to have as part of FieldTypeLookup. As a follow-up, I plan to make FieldTypeLookup public and expose it directly to QueryShardContext, hence this change helps so that PercolateQueryBuilder no longer needs to go through MappingLookup to retrieve the index analyzer. Also I plan to remove the indexAnalyzer method from MappingLookup. --- .../percolator/PercolateQueryBuilder.java | 2 +- .../index/mapper/FieldTypeLookup.java | 29 ++++++++++++------- .../index/mapper/MappingLookup.java | 27 +++++------------ .../index/mapper/FieldTypeLookupTests.java | 16 +++++----- .../mapper/FlatObjectFieldLookupTests.java | 11 +++---- 5 files changed, 43 insertions(+), 42 deletions(-) diff --git a/modules/percolator/src/main/java/org/elasticsearch/percolator/PercolateQueryBuilder.java b/modules/percolator/src/main/java/org/elasticsearch/percolator/PercolateQueryBuilder.java index a583c31831eb6..dbc4008da93ca 100644 --- a/modules/percolator/src/main/java/org/elasticsearch/percolator/PercolateQueryBuilder.java +++ b/modules/percolator/src/main/java/org/elasticsearch/percolator/PercolateQueryBuilder.java @@ -471,7 +471,7 @@ protected Query doToQuery(QueryShardContext context) throws IOException { docs.add(docMapper.parse(new SourceToParse(context.index().getName(), "_temp_id", document, documentXContentType))); } - FieldNameAnalyzer fieldNameAnalyzer = (FieldNameAnalyzer) docMapper.mappers().indexAnalyzer(); + FieldNameAnalyzer fieldNameAnalyzer = docMapper.mappers().indexAnalyzer(); // Need to this custom impl because FieldNameAnalyzer is strict and the percolator sometimes isn't when // 'index.percolator.map_unmapped_fields_as_string' is enabled: Analyzer analyzer = new DelegatingAnalyzerWrapper(Analyzer.PER_FIELD_REUSE_STRATEGY) { diff --git a/server/src/main/java/org/elasticsearch/index/mapper/FieldTypeLookup.java b/server/src/main/java/org/elasticsearch/index/mapper/FieldTypeLookup.java index b142a6f45c21c..dc4642d6b529d 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/FieldTypeLookup.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/FieldTypeLookup.java @@ -19,11 +19,12 @@ package org.elasticsearch.index.mapper; +import org.apache.lucene.analysis.Analyzer; import org.elasticsearch.common.collect.Iterators; import org.elasticsearch.common.regex.Regex; +import org.elasticsearch.index.analysis.FieldNameAnalyzer; import java.util.Collection; -import java.util.Collections; import java.util.HashMap; import java.util.HashSet; import java.util.Iterator; @@ -33,7 +34,7 @@ /** * An immutable container for looking up {@link MappedFieldType}s by their name. */ -class FieldTypeLookup implements Iterable { +final class FieldTypeLookup implements Iterable { private final Map fullNameToFieldType = new HashMap<>(); private final Map aliasToConcreteName = new HashMap<>(); @@ -47,18 +48,17 @@ class FieldTypeLookup implements Iterable { */ private final Map> fieldToCopiedFields = new HashMap<>(); private final DynamicKeyFieldTypeLookup dynamicKeyLookup; - - FieldTypeLookup() { - this(Collections.emptyList(), Collections.emptyList()); - } + private final FieldNameAnalyzer indexAnalyzer; FieldTypeLookup(Collection fieldMappers, - Collection fieldAliasMappers) { + Collection fieldAliasMappers, + Analyzer defaultIndexAnalyzer) { Map dynamicKeyMappers = new HashMap<>(); - + Map indexAnalyzers = new HashMap<>(); for (FieldMapper fieldMapper : fieldMappers) { String fieldName = fieldMapper.name(); MappedFieldType fieldType = fieldMapper.fieldType(); + indexAnalyzers.put(fieldType.name(), fieldType.indexAnalyzer() != null ? fieldType.indexAnalyzer() : defaultIndexAnalyzer); fullNameToFieldType.put(fieldType.name(), fieldType); if (fieldMapper instanceof DynamicKeyFieldMapper) { dynamicKeyMappers.put(fieldName, (DynamicKeyFieldMapper) fieldMapper); @@ -82,6 +82,7 @@ class FieldTypeLookup implements Iterable { } this.dynamicKeyLookup = new DynamicKeyFieldTypeLookup(dynamicKeyMappers, aliasToConcreteName); + this.indexAnalyzer = new FieldNameAnalyzer(indexAnalyzers); } /** @@ -99,10 +100,18 @@ public MappedFieldType get(String field) { return dynamicKeyLookup.get(field); } + /** + * A smart analyzer used for indexing that takes into account specific analyzers configured + * per {@link FieldMapper}. + */ + public FieldNameAnalyzer indexAnalyzer() { + return this.indexAnalyzer; + } + /** * Returns a list of the full names of a simple match regex like pattern against full name and index name. */ - public Set simpleMatchToFullName(String pattern) { + Set simpleMatchToFullName(String pattern) { Set fields = new HashSet<>(); for (MappedFieldType fieldType : this) { if (Regex.simpleMatch(pattern, fieldType.name())) { @@ -129,7 +138,7 @@ public Set simpleMatchToFullName(String pattern) { * should be a concrete field and *not* an alias. * @return A set of paths in the _source that contain the field's values. */ - public Set sourcePaths(String field) { + Set sourcePaths(String field) { String resolvedField = field; int lastDotIndex = field.lastIndexOf('.'); if (lastDotIndex > 0) { 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 64cd4fc4cb9e2..b3c566048fee4 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/MappingLookup.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/MappingLookup.java @@ -40,16 +40,8 @@ public final class MappingLookup implements Iterable { private final boolean hasNested; private final FieldTypeLookup fieldTypeLookup; private final int metadataFieldCount; - private final FieldNameAnalyzer indexAnalyzer; - private static void put(Map analyzers, String key, Analyzer value, Analyzer defaultValue) { - if (value == null) { - value = defaultValue; - } - analyzers.put(key, value); - } - - public static MappingLookup fromMapping(Mapping mapping, Analyzer defaultIndex) { + public static MappingLookup fromMapping(Mapping mapping, Analyzer defaultIndexAnalyzer) { List newObjectMappers = new ArrayList<>(); List newFieldMappers = new ArrayList<>(); List newFieldAliasMappers = new ArrayList<>(); @@ -59,7 +51,8 @@ public static MappingLookup fromMapping(Mapping mapping, Analyzer defaultIndex) } } collect(mapping.root, newObjectMappers, newFieldMappers, newFieldAliasMappers); - return new MappingLookup(newFieldMappers, newObjectMappers, newFieldAliasMappers, mapping.metadataMappers.length, defaultIndex); + return new MappingLookup(newFieldMappers, newObjectMappers, newFieldAliasMappers, mapping.metadataMappers.length, + defaultIndexAnalyzer); } private static void collect(Mapper mapper, Collection objectMappers, @@ -87,9 +80,8 @@ public MappingLookup(Collection mappers, Collection objectMappers, Collection aliasMappers, int metadataFieldCount, - Analyzer defaultIndex) { + Analyzer defaultIndexAnalyzer) { Map fieldMappers = new HashMap<>(); - Map indexAnalyzers = new HashMap<>(); Map objects = new HashMap<>(); boolean hasNested = false; @@ -110,8 +102,7 @@ public MappingLookup(Collection mappers, if (fieldMappers.put(mapper.name(), mapper) != null) { throw new MapperParsingException("Field [" + mapper.name() + "] is defined more than once"); } - MappedFieldType fieldType = mapper.fieldType(); - put(indexAnalyzers, fieldType.name(), fieldType.indexAnalyzer(), defaultIndex); + } this.metadataFieldCount = metadataFieldCount; @@ -124,10 +115,8 @@ public MappingLookup(Collection mappers, } } - this.fieldTypeLookup = new FieldTypeLookup(mappers, aliasMappers); - + this.fieldTypeLookup = new FieldTypeLookup(mappers, aliasMappers, defaultIndexAnalyzer); this.fieldMappers = Collections.unmodifiableMap(fieldMappers); - this.indexAnalyzer = new FieldNameAnalyzer(indexAnalyzers); this.objectMappers = Collections.unmodifiableMap(objects); } @@ -149,8 +138,8 @@ public FieldTypeLookup fieldTypes() { * A smart analyzer used for indexing that takes into account specific analyzers configured * per {@link FieldMapper}. */ - public Analyzer indexAnalyzer() { - return this.indexAnalyzer; + public FieldNameAnalyzer indexAnalyzer() { + return this.fieldTypeLookup.indexAnalyzer(); } @Override diff --git a/server/src/test/java/org/elasticsearch/index/mapper/FieldTypeLookupTests.java b/server/src/test/java/org/elasticsearch/index/mapper/FieldTypeLookupTests.java index 6344691695d1e..dec058bc6ebec 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/FieldTypeLookupTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/FieldTypeLookupTests.java @@ -19,6 +19,7 @@ package org.elasticsearch.index.mapper; +import org.elasticsearch.common.lucene.Lucene; import org.elasticsearch.test.ESTestCase; import java.util.Arrays; @@ -33,7 +34,7 @@ public class FieldTypeLookupTests extends ESTestCase { public void testEmpty() { - FieldTypeLookup lookup = new FieldTypeLookup(); + FieldTypeLookup lookup = new FieldTypeLookup(Collections.emptyList(), Collections.emptyList(), null); assertNull(lookup.get("foo")); Collection names = lookup.simpleMatchToFullName("foo"); assertNotNull(names); @@ -45,7 +46,7 @@ public void testEmpty() { public void testAddNewField() { MockFieldMapper f = new MockFieldMapper("foo"); - FieldTypeLookup lookup = new FieldTypeLookup(Collections.singletonList(f), emptyList()); + FieldTypeLookup lookup = new FieldTypeLookup(Collections.singletonList(f), emptyList(), Lucene.KEYWORD_ANALYZER); assertNull(lookup.get("bar")); assertEquals(f.fieldType(), lookup.get("foo")); assertEquals(1, size(lookup.iterator())); @@ -55,7 +56,8 @@ public void testAddFieldAlias() { MockFieldMapper field = new MockFieldMapper("foo"); FieldAliasMapper alias = new FieldAliasMapper("alias", "alias", "foo"); - FieldTypeLookup lookup = new FieldTypeLookup(Collections.singletonList(field), Collections.singletonList(alias)); + FieldTypeLookup lookup = new FieldTypeLookup(Collections.singletonList(field), Collections.singletonList(alias), + Lucene.KEYWORD_ANALYZER); MappedFieldType aliasType = lookup.get("alias"); assertEquals(field.fieldType(), aliasType); @@ -68,7 +70,7 @@ public void testSimpleMatchToFullName() { FieldAliasMapper alias1 = new FieldAliasMapper("food", "food", "foo"); FieldAliasMapper alias2 = new FieldAliasMapper("barometer", "barometer", "bar"); - FieldTypeLookup lookup = new FieldTypeLookup(Arrays.asList(field1, field2), Arrays.asList(alias1, alias2)); + FieldTypeLookup lookup = new FieldTypeLookup(Arrays.asList(field1, field2), Arrays.asList(alias1, alias2), Lucene.KEYWORD_ANALYZER); Collection names = lookup.simpleMatchToFullName("b*"); @@ -88,7 +90,7 @@ public void testSourcePathWithMultiFields() { .addMultiField(new MockFieldMapper.Builder("field.subfield2")) .build(context); - FieldTypeLookup lookup = new FieldTypeLookup(singletonList(field), emptyList()); + FieldTypeLookup lookup = new FieldTypeLookup(singletonList(field), emptyList(), Lucene.KEYWORD_ANALYZER); assertEquals(Set.of("field"), lookup.sourcePaths("field")); assertEquals(Set.of("field"), lookup.sourcePaths("field.subfield1")); @@ -107,7 +109,7 @@ public void testSourcePathsWithCopyTo() { .copyTo("field") .build(context); - FieldTypeLookup lookup = new FieldTypeLookup(Arrays.asList(field, otherField), emptyList()); + FieldTypeLookup lookup = new FieldTypeLookup(Arrays.asList(field, otherField), emptyList(), Lucene.KEYWORD_ANALYZER); assertEquals(Set.of("other_field", "field"), lookup.sourcePaths("field")); assertEquals(Set.of("other_field", "field"), lookup.sourcePaths("field.subfield1")); @@ -115,7 +117,7 @@ public void testSourcePathsWithCopyTo() { public void testIteratorImmutable() { MockFieldMapper f1 = new MockFieldMapper("foo"); - FieldTypeLookup lookup = new FieldTypeLookup(Collections.singletonList(f1), emptyList()); + FieldTypeLookup lookup = new FieldTypeLookup(Collections.singletonList(f1), emptyList(), Lucene.KEYWORD_ANALYZER); try { Iterator itr = lookup.iterator(); diff --git a/x-pack/plugin/mapper-flattened/src/test/java/org/elasticsearch/index/mapper/FlatObjectFieldLookupTests.java b/x-pack/plugin/mapper-flattened/src/test/java/org/elasticsearch/index/mapper/FlatObjectFieldLookupTests.java index 420b0d99f2d81..cb606b7b6ad60 100644 --- a/x-pack/plugin/mapper-flattened/src/test/java/org/elasticsearch/index/mapper/FlatObjectFieldLookupTests.java +++ b/x-pack/plugin/mapper-flattened/src/test/java/org/elasticsearch/index/mapper/FlatObjectFieldLookupTests.java @@ -7,6 +7,7 @@ package org.elasticsearch.index.mapper; import org.elasticsearch.Version; +import org.elasticsearch.common.lucene.Lucene; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.index.fielddata.IndexFieldData; import org.elasticsearch.index.fielddata.LeafFieldData; @@ -41,7 +42,7 @@ public void testFieldTypeLookup() { String fieldName = "object1.object2.field"; FlatObjectFieldMapper mapper = createFlatObjectMapper(fieldName); - FieldTypeLookup lookup = new FieldTypeLookup(singletonList(mapper), emptyList()); + FieldTypeLookup lookup = new FieldTypeLookup(singletonList(mapper), emptyList(), Lucene.KEYWORD_ANALYZER); assertEquals(mapper.fieldType(), lookup.get(fieldName)); String objectKey = "key1.key2"; @@ -62,7 +63,7 @@ public void testFieldTypeLookupWithAlias() { String aliasName = "alias"; FieldAliasMapper alias = new FieldAliasMapper(aliasName, aliasName, fieldName); - FieldTypeLookup lookup = new FieldTypeLookup(singletonList(mapper), singletonList(alias)); + FieldTypeLookup lookup = new FieldTypeLookup(singletonList(mapper), singletonList(alias), Lucene.KEYWORD_ANALYZER); assertEquals(mapper.fieldType(), lookup.get(aliasName)); String objectKey = "key1.key2"; @@ -85,11 +86,11 @@ public void testFieldTypeLookupWithMultipleFields() { FlatObjectFieldMapper mapper2 = createFlatObjectMapper(field2); FlatObjectFieldMapper mapper3 = createFlatObjectMapper(field3); - FieldTypeLookup lookup = new FieldTypeLookup(Arrays.asList(mapper1, mapper2), emptyList()); + FieldTypeLookup lookup = new FieldTypeLookup(Arrays.asList(mapper1, mapper2), emptyList(), Lucene.KEYWORD_ANALYZER); assertNotNull(lookup.get(field1 + ".some.key")); assertNotNull(lookup.get(field2 + ".some.key")); - lookup = new FieldTypeLookup(Arrays.asList(mapper1, mapper2, mapper3), emptyList()); + lookup = new FieldTypeLookup(Arrays.asList(mapper1, mapper2, mapper3), emptyList(), Lucene.KEYWORD_ANALYZER); assertNotNull(lookup.get(field1 + ".some.key")); assertNotNull(lookup.get(field2 + ".some.key")); assertNotNull(lookup.get(field3 + ".some.key")); @@ -126,7 +127,7 @@ public void testFieldLookupIterator() { MockFieldMapper mapper = new MockFieldMapper("foo"); FlatObjectFieldMapper flatObjectMapper = createFlatObjectMapper("object1.object2.field"); - FieldTypeLookup lookup = new FieldTypeLookup(Arrays.asList(mapper, flatObjectMapper), emptyList()); + FieldTypeLookup lookup = new FieldTypeLookup(Arrays.asList(mapper, flatObjectMapper), emptyList(), Lucene.KEYWORD_ANALYZER); Set fieldNames = new HashSet<>(); for (MappedFieldType fieldType : lookup) { From 5d98316c2d113272309577a9fabcf4c93a41fa35 Mon Sep 17 00:00:00 2001 From: Luca Cavanna Date: Tue, 20 Oct 2020 14:25:58 +0200 Subject: [PATCH 2/2] iter --- .../java/org/elasticsearch/index/mapper/FieldTypeLookup.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/index/mapper/FieldTypeLookup.java b/server/src/main/java/org/elasticsearch/index/mapper/FieldTypeLookup.java index dc4642d6b529d..835a7d0e0161a 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/FieldTypeLookup.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/FieldTypeLookup.java @@ -88,7 +88,7 @@ final class FieldTypeLookup implements Iterable { /** * Returns the mapped field type for the given field name. */ - public MappedFieldType get(String field) { + MappedFieldType get(String field) { String concreteField = aliasToConcreteName.getOrDefault(field, field); MappedFieldType fieldType = fullNameToFieldType.get(concreteField); if (fieldType != null) { @@ -104,7 +104,7 @@ public MappedFieldType get(String field) { * A smart analyzer used for indexing that takes into account specific analyzers configured * per {@link FieldMapper}. */ - public FieldNameAnalyzer indexAnalyzer() { + FieldNameAnalyzer indexAnalyzer() { return this.indexAnalyzer; }