Przeglądaj źródła

Remove FieldNameAnalyzer and MapperService.indexAnalyzer() (#65595)

`MapperService.indexAnalyzer(String, Function<String, NamedAnalyzer>)`
gives us a robust way of loading the index analyzer for a field, letting the
caller decide how to deal with unmapped or unindexed fields. Having an
additional `indexAnalyzer()` method on MapperService just confuses things,
and is largely unused anyway.

This commit removes the additional method, and changes MappingLookup
to hold a simple Map of fields to analyzers. FieldNameAnalyzer is no longer
used, and is also removed. IndexShard gets a new static method to build
an index-time analyzer to be set on IndexWriterConfig that reproduces the
old behaviour of FieldNameAnalyzer.
Alan Woodward 4 lat temu
rodzic
commit
c9ecb16a92

+ 0 - 67
server/src/main/java/org/elasticsearch/index/analysis/FieldNameAnalyzer.java

@@ -1,67 +0,0 @@
-/*
- * Licensed to Elasticsearch under one or more contributor
- * license agreements. See the NOTICE file distributed with
- * this work for additional information regarding copyright
- * ownership. Elasticsearch licenses this file to you under
- * the Apache License, Version 2.0 (the "License"); you may
- * not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *    http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing,
- * software distributed under the License is distributed on an
- * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
- * KIND, either express or implied.  See the License for the
- * specific language governing permissions and limitations
- * under the License.
- */
-
-package org.elasticsearch.index.analysis;
-
-import org.apache.lucene.analysis.Analyzer;
-import org.apache.lucene.analysis.DelegatingAnalyzerWrapper;
-import org.elasticsearch.common.collect.CopyOnWriteHashMap;
-
-import java.util.Map;
-
-public final class FieldNameAnalyzer extends DelegatingAnalyzerWrapper {
-
-    private final Map<String, NamedAnalyzer> analyzers;
-
-    public FieldNameAnalyzer(Map<String, NamedAnalyzer> analyzers) {
-        super(Analyzer.PER_FIELD_REUSE_STRATEGY);
-        this.analyzers = CopyOnWriteHashMap.copyOf(analyzers);
-    }
-
-    public Map<String, NamedAnalyzer> analyzers() {
-        return analyzers;
-    }
-
-    @Override
-    protected Analyzer getWrappedAnalyzer(String fieldName) {
-        Analyzer analyzer = analyzers.get(fieldName);
-        if (analyzer != null) {
-            return analyzer;
-        }
-        // Don't be lenient here and return the default analyzer
-        // Fields need to be explicitly added
-        throw new IllegalArgumentException("Field [" + fieldName + "] has no associated analyzer");
-    }
-
-    public boolean containsBrokenAnalysis(String field) {
-        Analyzer analyzer = getWrappedAnalyzer(field);
-        if (analyzer instanceof NamedAnalyzer) {
-            analyzer = ((NamedAnalyzer) analyzer).analyzer();
-        }
-        if (analyzer instanceof AnalyzerComponentsProvider) {
-            final TokenFilterFactory[] tokenFilters = ((AnalyzerComponentsProvider) analyzer).getComponents().getTokenFilters();
-            for (TokenFilterFactory tokenFilterFactory : tokenFilters) {
-                if (tokenFilterFactory.breaksFastVectorHighlighter()) {
-                    return true;
-                }
-            }
-        }
-        return false;
-    }
-}

+ 0 - 22
server/src/main/java/org/elasticsearch/index/mapper/MapperService.java

@@ -20,8 +20,6 @@
 package org.elasticsearch.index.mapper;
 
 import org.apache.logging.log4j.message.ParameterizedMessage;
-import org.apache.lucene.analysis.Analyzer;
-import org.apache.lucene.analysis.DelegatingAnalyzerWrapper;
 import org.elasticsearch.Assertions;
 import org.elasticsearch.Version;
 import org.elasticsearch.cluster.metadata.IndexMetadata;
@@ -108,7 +106,6 @@ public class MapperService extends AbstractIndexComponent implements Closeable {
     private final DocumentMapperParser documentMapperParser;
     private final DocumentParser documentParser;
     private final Version indexVersionCreated;
-    private final MapperAnalyzerWrapper indexAnalyzer;
     private final MapperRegistry mapperRegistry;
     private final Supplier<Mapper.TypeParser.ParserContext> parserContextSupplier;
 
@@ -121,7 +118,6 @@ public class MapperService extends AbstractIndexComponent implements Closeable {
         super(indexSettings);
         this.indexVersionCreated = indexSettings.getIndexVersionCreated();
         this.indexAnalyzers = indexAnalyzers;
-        this.indexAnalyzer = new MapperAnalyzerWrapper();
         this.mapperRegistry = mapperRegistry;
         Function<DateFormatter, Mapper.TypeParser.ParserContext> parserContextFunction =
             dateFormatter -> new Mapper.TypeParser.ParserContext(similarityService::getSimilarity, mapperRegistry.getMapperParsers()::get,
@@ -441,10 +437,6 @@ public class MapperService extends AbstractIndexComponent implements Closeable {
         return this.mapper == null ? null : this.mapper.mappers().objectMappers().get(name);
     }
 
-    public Analyzer indexAnalyzer() {
-        return this.indexAnalyzer;
-    }
-
     /**
      * Return the index-time analyzer associated with a particular field
      * @param field                     the field name
@@ -495,20 +487,6 @@ public class MapperService extends AbstractIndexComponent implements Closeable {
         return mapperRegistry.isMetadataField(indexVersionCreated, field);
     }
 
-    /** An analyzer wrapper that can lookup fields within the index mappings */
-    final class MapperAnalyzerWrapper extends DelegatingAnalyzerWrapper {
-
-        MapperAnalyzerWrapper() {
-            super(Analyzer.PER_FIELD_REUSE_STRATEGY);
-        }
-
-        @Override
-        protected Analyzer getWrappedAnalyzer(String fieldName) {
-            return mapper.mappers().indexAnalyzer();
-        }
-
-    }
-
     public synchronized List<String> reloadSearchAnalyzers(AnalysisRegistry registry) throws IOException {
         logger.info("reloading search analyzers");
         // refresh indexAnalyzers and search analyzers

+ 3 - 14
server/src/main/java/org/elasticsearch/index/mapper/MappingLookup.java

@@ -20,7 +20,6 @@
 package org.elasticsearch.index.mapper;
 
 import org.elasticsearch.index.IndexSettings;
-import org.elasticsearch.index.analysis.FieldNameAnalyzer;
 import org.elasticsearch.index.analysis.NamedAnalyzer;
 
 import java.util.ArrayList;
@@ -39,7 +38,7 @@ public final class MappingLookup {
     private final boolean hasNested;
     private final FieldTypeLookup fieldTypeLookup;
     private final int metadataFieldCount;
-    private final FieldNameAnalyzer indexAnalyzer;
+    private final Map<String, NamedAnalyzer> indexAnalyzers = new HashMap<>();
 
     public static MappingLookup fromMapping(Mapping mapping) {
         List<ObjectMapper> newObjectMappers = new ArrayList<>();
@@ -81,7 +80,6 @@ public final class MappingLookup {
                          Collection<RuntimeFieldType> runtimeFieldTypes,
                          int metadataFieldCount) {
         Map<String, Mapper> fieldMappers = new HashMap<>();
-        Map<String, NamedAnalyzer> indexAnalyzers = new HashMap<>();
         Map<String, ObjectMapper> objects = new HashMap<>();
 
         boolean hasNested = false;
@@ -118,7 +116,6 @@ public final class MappingLookup {
         this.fieldTypeLookup = new FieldTypeLookup(mappers, aliasMappers, runtimeFieldTypes);
 
         this.fieldMappers = Collections.unmodifiableMap(fieldMappers);
-        this.indexAnalyzer = new FieldNameAnalyzer(indexAnalyzers);
         this.objectMappers = Collections.unmodifiableMap(objects);
     }
 
@@ -136,17 +133,9 @@ public final class MappingLookup {
         return fieldTypeLookup;
     }
 
-    /**
-     * A smart analyzer used for indexing that takes into account specific analyzers configured
-     * per {@link FieldMapper}.
-     */
-    public FieldNameAnalyzer indexAnalyzer() {
-        return this.indexAnalyzer;
-    }
-
     public NamedAnalyzer indexAnalyzer(String field, Function<String, NamedAnalyzer> unmappedFieldAnalyzer) {
-        if (this.indexAnalyzer.analyzers().containsKey(field)) {
-            return this.indexAnalyzer.analyzers().get(field);
+        if (this.indexAnalyzers.containsKey(field)) {
+            return this.indexAnalyzers.get(field);
         }
         return unmappedFieldAnalyzer.apply(field);
     }

+ 16 - 1
server/src/main/java/org/elasticsearch/index/shard/IndexShard.java

@@ -22,6 +22,8 @@ package org.elasticsearch.index.shard;
 import com.carrotsearch.hppc.ObjectLongMap;
 import org.apache.logging.log4j.Logger;
 import org.apache.logging.log4j.message.ParameterizedMessage;
+import org.apache.lucene.analysis.Analyzer;
+import org.apache.lucene.analysis.DelegatingAnalyzerWrapper;
 import org.apache.lucene.index.CheckIndex;
 import org.apache.lucene.index.DirectoryReader;
 import org.apache.lucene.index.FilterDirectoryReader;
@@ -2725,6 +2727,19 @@ public class IndexShard extends AbstractIndexShardComponent implements IndicesCl
         }
     }
 
+    public static Analyzer buildIndexAnalyzer(MapperService mapperService) {
+        if (mapperService == null) {
+            return null;
+        }
+        return new DelegatingAnalyzerWrapper(Analyzer.PER_FIELD_REUSE_STRATEGY) {
+            @Override
+            protected Analyzer getWrappedAnalyzer(String fieldName) {
+                return mapperService.indexAnalyzer(fieldName, f -> {
+                    throw new IllegalArgumentException("Field [" + fieldName + "] has no associated analyzer");
+                });
+            }
+        };
+    }
 
     private DocumentMapperForType docMapper() {
         return mapperService.documentMapperWithAutoCreate();
@@ -2741,7 +2756,7 @@ public class IndexShard extends AbstractIndexShardComponent implements IndicesCl
         };
         return new EngineConfig(shardId,
                 threadPool, indexSettings, warmer, store, indexSettings.getMergePolicy(),
-                mapperService != null ? mapperService.indexAnalyzer() : null,
+                buildIndexAnalyzer(mapperService),
                 similarityService.similarity(mapperService == null ? null : mapperService::fieldType), codecService, shardEventListener,
                 indexCache != null ? indexCache.query() : null, cachingPolicy, translogConfig,
                 IndexingMemoryController.SHARD_INACTIVE_TIME_SETTING.get(indexSettings.getSettings()),

+ 6 - 3
server/src/test/java/org/elasticsearch/index/mapper/DocumentFieldMapperTests.java

@@ -121,9 +121,12 @@ public class DocumentFieldMapperTests extends LuceneTestCase {
             Collections.emptyList(),
             0);
 
-        assertAnalyzes(mappingLookup.indexAnalyzer(), "field1", "index1");
-        assertAnalyzes(mappingLookup.indexAnalyzer(), "field2", "index2");
-        expectThrows(IllegalArgumentException.class, () -> mappingLookup.indexAnalyzer().tokenStream("field3", "blah"));
+        assertAnalyzes(mappingLookup.indexAnalyzer("field1", f -> null), "field1", "index1");
+        assertAnalyzes(mappingLookup.indexAnalyzer("field2", f -> null), "field2", "index2");
+        expectThrows(IllegalArgumentException.class,
+            () -> mappingLookup.indexAnalyzer("field3", f -> {
+                throw new IllegalArgumentException();
+            }).tokenStream("field3", "blah"));
     }
 
     private void assertAnalyzes(Analyzer analyzer, String field, String output) throws IOException {

+ 7 - 2
server/src/test/java/org/elasticsearch/index/mapper/DocumentMapperTests.java

@@ -19,6 +19,7 @@
 
 package org.elasticsearch.index.mapper;
 
+import org.apache.lucene.analysis.Analyzer;
 import org.apache.lucene.analysis.core.KeywordAnalyzer;
 import org.apache.lucene.analysis.core.WhitespaceAnalyzer;
 import org.apache.lucene.analysis.standard.StandardAnalyzer;
@@ -153,7 +154,9 @@ public class DocumentMapperTests extends MapperServiceTestCase {
         final DocumentMapper documentMapper = mapperService.documentMapper();
 
         expectThrows(IllegalArgumentException.class,
-            () -> documentMapper.mappers().indexAnalyzer().tokenStream("non_existing_field", "foo"));
+            () -> documentMapper.mappers().indexAnalyzer("non_existing_field", f -> {
+                throw new IllegalArgumentException();
+            }));
 
         final AtomicBoolean stopped = new AtomicBoolean(false);
         final CyclicBarrier barrier = new CyclicBarrier(2);
@@ -189,7 +192,9 @@ public class DocumentMapperTests extends MapperServiceTestCase {
                     // not in the mapping yet, try again
                     continue;
                 }
-                assertNotNull(mapperService.indexAnalyzer().tokenStream(fieldName, "foo"));
+                Analyzer a = mapperService.indexAnalyzer(fieldName, f -> null);
+                assertNotNull(a);
+                assertNotNull(a.tokenStream(fieldName, "foo"));
             }
         } finally {
             stopped.set(true);

+ 5 - 4
server/src/test/java/org/elasticsearch/index/mapper/TextFieldMapperTests.java

@@ -743,7 +743,7 @@ public class TextFieldMapperTests extends MapperTestCase {
         IndexableField[] fields = doc.rootDoc().getFields("field._index_phrase");
         assertEquals(1, fields.length);
 
-        try (TokenStream ts = fields[0].tokenStream(mapperService.indexAnalyzer(), null)) {
+        try (TokenStream ts = fields[0].tokenStream(mapperService.indexAnalyzer(fields[0].name(), f -> null), null)) {
             CharTermAttribute termAtt = ts.addAttribute(CharTermAttribute.class);
             ts.reset();
             assertTrue(ts.incrementToken());
@@ -824,7 +824,7 @@ public class TextFieldMapperTests extends MapperTestCase {
             assertEquals(1, fields.length);
             withLuceneIndex(ms, iw -> iw.addDocument(doc.rootDoc()), ir -> {}); // check we can index
 
-            assertAnalyzesTo(ms.indexAnalyzer(), "field._index_prefix", "tweedledum",
+            assertAnalyzesTo(ms.indexAnalyzer("field._index_prefix", f -> null), "field._index_prefix", "tweedledum",
                 new String[]{ "tw", "twe", "twee", "tweed", "tweedl" });
         }
 
@@ -832,7 +832,7 @@ public class TextFieldMapperTests extends MapperTestCase {
             MapperService ms = createMapperService(
                 fieldMapping(b -> b.field("type", "text").field("analyzer", "standard").startObject("index_prefixes").endObject())
             );
-            assertAnalyzesTo(ms.indexAnalyzer(), "field._index_prefix", "tweedledum",
+            assertAnalyzesTo(ms.indexAnalyzer("field._index_prefix", f -> null), "field._index_prefix", "tweedledum",
                 new String[]{ "tw", "twe", "twee", "tweed" });
         }
 
@@ -840,7 +840,8 @@ public class TextFieldMapperTests extends MapperTestCase {
             MapperService ms = createMapperService(
                 fieldMapping(b -> b.field("type", "text").nullField("index_prefixes"))
             );
-            expectThrows(Exception.class, () -> ms.indexAnalyzer().tokenStream("field._index_prefixes", "test"));
+            expectThrows(Exception.class,
+                () -> ms.indexAnalyzer("field._index_prefixes", f -> null).tokenStream("field._index_prefixes", "test"));
         }
 
         {

+ 3 - 1
test/framework/src/main/java/org/elasticsearch/index/mapper/MapperServiceTestCase.java

@@ -51,6 +51,7 @@ import org.elasticsearch.index.fielddata.IndexFieldDataCache;
 import org.elasticsearch.index.query.QueryBuilder;
 import org.elasticsearch.index.query.QueryShardContext;
 import org.elasticsearch.index.query.support.NestedScope;
+import org.elasticsearch.index.shard.IndexShard;
 import org.elasticsearch.index.similarity.SimilarityService;
 import org.elasticsearch.indices.IndicesModule;
 import org.elasticsearch.indices.breaker.NoneCircuitBreakerService;
@@ -208,9 +209,10 @@ public abstract class MapperServiceTestCase extends ESTestCase {
         CheckedConsumer<RandomIndexWriter, IOException> builder,
         CheckedConsumer<IndexReader, IOException> test
     ) throws IOException {
+        IndexWriterConfig iwc = new IndexWriterConfig(IndexShard.buildIndexAnalyzer(mapperService));
         try (
             Directory dir = newDirectory();
-            RandomIndexWriter iw = new RandomIndexWriter(random(), dir, new IndexWriterConfig(mapperService.indexAnalyzer()))
+            RandomIndexWriter iw = new RandomIndexWriter(random(), dir,iwc)
         ) {
             builder.accept(iw);
             try (IndexReader reader = iw.getReader()) {