Browse Source

Cleanup of IndexFieldDataService.getForField.

This method has 2 signatures and one of them is dangerous since it allows to
discard fielddata configuration of the field mapper. This commit changes the
percolator so that it uses fielddata configuration of the _id field mapper
instead of forcing the paged_bytes format.

Closes #4270
Adrien Grand 12 năm trước cách đây
mục cha
commit
596d511466

+ 3 - 4
src/main/java/org/elasticsearch/index/fielddata/IndexFieldDataService.java

@@ -133,10 +133,9 @@ public class IndexFieldDataService extends AbstractIndexComponent {
     }
 
     public <IFD extends IndexFieldData<?>> IFD getForField(FieldMapper<?> mapper) {
-        return getForField(mapper.names(), mapper.fieldDataType(), mapper.hasDocValues());
-    }
-
-    public <IFD extends IndexFieldData<?>> IFD getForField(FieldMapper.Names fieldNames, FieldDataType type, boolean docValues) {
+        final FieldMapper.Names fieldNames = mapper.names();
+        final FieldDataType type = mapper.fieldDataType();
+        final boolean docValues = mapper.hasDocValues();
         IndexFieldData<?> fieldData = loadedFieldData.get(fieldNames.indexName());
         if (fieldData == null) {
             synchronized (loadedFieldData) {

+ 1 - 1
src/main/java/org/elasticsearch/index/percolator/PercolatorQueriesRegistry.java

@@ -244,7 +244,7 @@ public class PercolatorQueriesRegistry extends AbstractIndexShardComponent {
                                     new TermFilter(new Term(TypeFieldMapper.NAME, PercolatorService.TYPE_NAME))
                             )
                     );
-                    QueriesLoaderCollector queryCollector = new QueriesLoaderCollector(PercolatorQueriesRegistry.this, logger, indexFieldDataService);
+                    QueriesLoaderCollector queryCollector = new QueriesLoaderCollector(PercolatorQueriesRegistry.this, logger, mapperService, indexFieldDataService);
                     searcher.searcher().search(query, queryCollector);
                     Map<HashedBytesRef, Query> queries = queryCollector.queries();
                     for (Map.Entry<HashedBytesRef, Query> entry : queries.entrySet()) {

+ 4 - 8
src/main/java/org/elasticsearch/index/percolator/QueriesLoaderCollector.java

@@ -9,13 +9,12 @@ import org.apache.lucene.search.Scorer;
 import org.apache.lucene.util.BytesRef;
 import org.elasticsearch.common.logging.ESLogger;
 import org.elasticsearch.common.lucene.HashedBytesRef;
-import org.elasticsearch.common.settings.ImmutableSettings;
 import org.elasticsearch.index.fielddata.BytesValues;
-import org.elasticsearch.index.fielddata.FieldDataType;
 import org.elasticsearch.index.fielddata.IndexFieldData;
 import org.elasticsearch.index.fielddata.IndexFieldDataService;
 import org.elasticsearch.index.fieldvisitor.JustSourceFieldsVisitor;
 import org.elasticsearch.index.mapper.FieldMapper;
+import org.elasticsearch.index.mapper.MapperService;
 import org.elasticsearch.index.mapper.internal.IdFieldMapper;
 
 import java.io.IOException;
@@ -34,14 +33,11 @@ final class QueriesLoaderCollector extends Collector {
     private BytesValues idValues;
     private AtomicReader reader;
 
-    QueriesLoaderCollector(PercolatorQueriesRegistry percolator, ESLogger logger, IndexFieldDataService indexFieldDataService) {
+    QueriesLoaderCollector(PercolatorQueriesRegistry percolator, ESLogger logger, MapperService mapperService, IndexFieldDataService indexFieldDataService) {
         this.percolator = percolator;
         this.logger = logger;
-        this.idFieldData = indexFieldDataService.getForField(
-                new FieldMapper.Names(IdFieldMapper.NAME),
-                new FieldDataType("string", ImmutableSettings.builder().put("format", "paged_bytes")),
-                false
-        );
+        final FieldMapper<?> idMapper = mapperService.smartNameFieldMapper(IdFieldMapper.NAME);
+        this.idFieldData = indexFieldDataService.getForField(idMapper);
     }
 
     public Map<HashedBytesRef, Query> queries() {

+ 6 - 8
src/main/java/org/elasticsearch/percolator/PercolatorService.java

@@ -47,7 +47,6 @@ import org.elasticsearch.common.lucene.HashedBytesRef;
 import org.elasticsearch.common.lucene.Lucene;
 import org.elasticsearch.common.lucene.search.XCollector;
 import org.elasticsearch.common.lucene.search.XConstantScoreQuery;
-import org.elasticsearch.common.settings.ImmutableSettings;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.text.BytesText;
 import org.elasticsearch.common.text.StringText;
@@ -60,7 +59,6 @@ import org.elasticsearch.common.xcontent.XContentParser;
 import org.elasticsearch.common.xcontent.XContentType;
 import org.elasticsearch.index.engine.Engine;
 import org.elasticsearch.index.fielddata.BytesValues;
-import org.elasticsearch.index.fielddata.FieldDataType;
 import org.elasticsearch.index.fielddata.IndexFieldData;
 import org.elasticsearch.index.mapper.DocumentMapper;
 import org.elasticsearch.index.mapper.FieldMapper;
@@ -73,7 +71,10 @@ import org.elasticsearch.index.query.ParsedQuery;
 import org.elasticsearch.index.service.IndexService;
 import org.elasticsearch.index.shard.service.IndexShard;
 import org.elasticsearch.indices.IndicesService;
-import org.elasticsearch.percolator.QueryCollector.*;
+import org.elasticsearch.percolator.QueryCollector.Count;
+import org.elasticsearch.percolator.QueryCollector.Match;
+import org.elasticsearch.percolator.QueryCollector.MatchAndScore;
+import org.elasticsearch.percolator.QueryCollector.MatchAndSort;
 import org.elasticsearch.search.SearchParseElement;
 import org.elasticsearch.search.SearchShardTarget;
 import org.elasticsearch.search.facet.Facet;
@@ -702,11 +703,8 @@ public class PercolatorService extends AbstractComponent {
                     hls = new ArrayList<Map<String, HighlightField>>(topDocs.scoreDocs.length);
                 }
 
-                IndexFieldData idFieldData = context.fieldData().getForField(
-                        new FieldMapper.Names(IdFieldMapper.NAME),
-                        new FieldDataType("string", ImmutableSettings.builder().put("format", "paged_bytes")),
-                        false
-                );
+                final FieldMapper<?> idMapper = context.mapperService().smartNameFieldMapper(IdFieldMapper.NAME);
+                final IndexFieldData<?> idFieldData = context.fieldData().getForField(idMapper);
                 int i = 0;
                 final HashedBytesRef spare = new HashedBytesRef(new BytesRef());
                 for (ScoreDoc scoreDoc : topDocs.scoreDocs) {

+ 3 - 8
src/main/java/org/elasticsearch/percolator/QueryCollector.java

@@ -28,9 +28,7 @@ import org.elasticsearch.common.logging.ESLogger;
 import org.elasticsearch.common.lucene.HashedBytesRef;
 import org.elasticsearch.common.lucene.Lucene;
 import org.elasticsearch.common.lucene.search.FilteredCollector;
-import org.elasticsearch.common.settings.ImmutableSettings;
 import org.elasticsearch.index.fielddata.BytesValues;
-import org.elasticsearch.index.fielddata.FieldDataType;
 import org.elasticsearch.index.fielddata.IndexFieldData;
 import org.elasticsearch.index.mapper.FieldMapper;
 import org.elasticsearch.index.mapper.internal.IdFieldMapper;
@@ -50,7 +48,7 @@ import java.util.concurrent.ConcurrentMap;
  */
 abstract class QueryCollector extends Collector {
 
-    final IndexFieldData idFieldData;
+    final IndexFieldData<?> idFieldData;
     final IndexSearcher searcher;
     final ConcurrentMap<HashedBytesRef, Query> queries;
     final ESLogger logger;
@@ -67,11 +65,8 @@ abstract class QueryCollector extends Collector {
         this.logger = logger;
         this.queries = context.percolateQueries();
         this.searcher = context.docSearcher();
-        this.idFieldData = context.fieldData().getForField(
-                new FieldMapper.Names(IdFieldMapper.NAME),
-                new FieldDataType("string", ImmutableSettings.builder().put("format", "paged_bytes")),
-                false
-        );
+        final FieldMapper<?> idMapper = context.mapperService().smartNameFieldMapper(IdFieldMapper.NAME);
+        this.idFieldData = context.fieldData().getForField(idMapper);
 
         if (context.facets() != null) {
             for (SearchContextFacets.Entry entry : context.facets().entries()) {

+ 5 - 3
src/test/java/org/elasticsearch/benchmark/fielddata/LongFieldDataBenchmark.java

@@ -32,10 +32,11 @@ import org.apache.lucene.util.RamUsageEstimator;
 import org.elasticsearch.common.lucene.Lucene;
 import org.elasticsearch.index.Index;
 import org.elasticsearch.index.fielddata.AtomicNumericFieldData;
-import org.elasticsearch.index.fielddata.FieldDataType;
 import org.elasticsearch.index.fielddata.IndexFieldDataService;
 import org.elasticsearch.index.fielddata.IndexNumericFieldData;
-import org.elasticsearch.index.mapper.FieldMapper;
+import org.elasticsearch.index.mapper.ContentPath;
+import org.elasticsearch.index.mapper.Mapper.BuilderContext;
+import org.elasticsearch.index.mapper.core.LongFieldMapper;
 
 import java.util.Random;
 
@@ -135,7 +136,8 @@ public class LongFieldDataBenchmark {
 
             final DirectoryReader dr = DirectoryReader.open(dir);
             final IndexFieldDataService fds = new IndexFieldDataService(new Index("dummy"));
-            final IndexNumericFieldData<AtomicNumericFieldData> fd = fds.getForField(new FieldMapper.Names(fieldName), new FieldDataType("long"), false);
+            final LongFieldMapper mapper = new LongFieldMapper.Builder(fieldName).build(new BuilderContext(null, new ContentPath(1)));
+            final IndexNumericFieldData<AtomicNumericFieldData> fd = fds.getForField(mapper);
             final long start = System.nanoTime();
             final AtomicNumericFieldData afd = fd.loadDirect(SlowCompositeReaderWrapper.wrap(dr).getContext());
             final long loadingTimeMs = (System.nanoTime() - start) / 1000 / 1000;

+ 28 - 2
src/test/java/org/elasticsearch/index/fielddata/AbstractFieldDataTests.java

@@ -24,7 +24,10 @@ import org.apache.lucene.index.*;
 import org.apache.lucene.store.RAMDirectory;
 import org.elasticsearch.common.lucene.Lucene;
 import org.elasticsearch.index.Index;
+import org.elasticsearch.index.mapper.ContentPath;
 import org.elasticsearch.index.mapper.FieldMapper;
+import org.elasticsearch.index.mapper.Mapper.BuilderContext;
+import org.elasticsearch.index.mapper.MapperBuilders;
 import org.elasticsearch.test.ElasticsearchTestCase;
 import org.junit.After;
 import org.junit.Before;
@@ -42,8 +45,31 @@ public abstract class AbstractFieldDataTests extends ElasticsearchTestCase {
         return false;
     }
 
-    public <IFD extends IndexFieldData> IFD getForField(String fieldName) {
-        return ifdService.getForField(new FieldMapper.Names(fieldName), getFieldDataType(), hasDocValues());
+    public <IFD extends IndexFieldData<?>> IFD getForField(String fieldName) {
+        return getForField(getFieldDataType(), fieldName);
+    }
+
+    public <IFD extends IndexFieldData<?>> IFD getForField(FieldDataType type, String fieldName) {
+        final FieldMapper<?> mapper;
+        final BuilderContext context = new BuilderContext(null, new ContentPath(1));
+        if (type.getType().equals("string")) {
+            mapper = MapperBuilders.stringField(fieldName).tokenized(false).fieldDataSettings(type.getSettings()).build(context);
+        } else if (type.getType().equals("float")) {
+            mapper = MapperBuilders.floatField(fieldName).fieldDataSettings(type.getSettings()).build(context);
+        } else if (type.getType().equals("double")) {
+            mapper = MapperBuilders.doubleField(fieldName).fieldDataSettings(type.getSettings()).build(context);
+        } else if (type.getType().equals("long")) {
+            mapper = MapperBuilders.longField(fieldName).fieldDataSettings(type.getSettings()).build(context);
+        } else if (type.getType().equals("int")) {
+            mapper = MapperBuilders.integerField(fieldName).fieldDataSettings(type.getSettings()).build(context);
+        } else if (type.getType().equals("short")) {
+            mapper = MapperBuilders.shortField(fieldName).fieldDataSettings(type.getSettings()).build(context);
+        } else if (type.getType().equals("byte")) {
+            mapper = MapperBuilders.byteField(fieldName).fieldDataSettings(type.getSettings()).build(context);
+        } else {
+            throw new UnsupportedOperationException(type.getType());
+        }
+        return ifdService.getForField(mapper);
     }
 
     @Before

+ 15 - 18
src/test/java/org/elasticsearch/index/fielddata/DuelFieldDataTests.java

@@ -27,15 +27,12 @@ import org.apache.lucene.util.English;
 import org.apache.lucene.util.LuceneTestCase;
 import org.apache.lucene.util.NumericUtils;
 import org.elasticsearch.common.settings.ImmutableSettings;
-import org.elasticsearch.index.mapper.FieldMapper;
 import org.junit.Test;
 
 import java.util.*;
 import java.util.Map.Entry;
 
-import static org.hamcrest.Matchers.equalTo;
-import static org.hamcrest.Matchers.is;
-import static org.hamcrest.Matchers.lessThan;
+import static org.hamcrest.Matchers.*;
 
 public class DuelFieldDataTests extends AbstractFieldDataTests {
 
@@ -127,10 +124,11 @@ public class DuelFieldDataTests extends AbstractFieldDataTests {
             } else {
                 right = left = list.remove(0);
             }
+
             ifdService.clear();
-            IndexFieldData leftFieldData = ifdService.getForField(new FieldMapper.Names(left.getValue().name().toLowerCase(Locale.ROOT)), left.getKey(), true);
+            IndexFieldData<?> leftFieldData = getForField(left.getKey(), left.getValue().name().toLowerCase(Locale.ROOT));
             ifdService.clear();
-            IndexFieldData rightFieldData = ifdService.getForField(new FieldMapper.Names(right.getValue().name().toLowerCase(Locale.ROOT)), right.getKey(), true);
+            IndexFieldData<?> rightFieldData = getForField(right.getKey(), right.getValue().name().toLowerCase(Locale.ROOT));
             duelFieldDataBytes(random, context, leftFieldData, rightFieldData, pre);
             duelFieldDataBytes(random, context, rightFieldData, leftFieldData, pre);
 
@@ -190,11 +188,10 @@ public class DuelFieldDataTests extends AbstractFieldDataTests {
                 right = left = list.remove(0);
             }
             ifdService.clear();
-            IndexNumericFieldData leftFieldData = ifdService.getForField(new FieldMapper.Names(left.getValue().name().toLowerCase(Locale.ROOT)),
-                    left.getKey(), true);
+            IndexNumericFieldData<?> leftFieldData = getForField(left.getKey(), left.getValue().name().toLowerCase(Locale.ROOT));
             ifdService.clear();
-            IndexNumericFieldData rightFieldData = ifdService.getForField(new FieldMapper.Names(right.getValue().name().toLowerCase(Locale.ROOT)),
-                    right.getKey(), true);
+            IndexNumericFieldData<?> rightFieldData = getForField(right.getKey(), right.getValue().name().toLowerCase(Locale.ROOT));
+
             duelFieldDataLong(random, context, leftFieldData, rightFieldData);
             duelFieldDataLong(random, context, rightFieldData, leftFieldData);
 
@@ -250,11 +247,11 @@ public class DuelFieldDataTests extends AbstractFieldDataTests {
                 right = left = list.remove(0);
             }
             ifdService.clear();
-            IndexNumericFieldData leftFieldData = ifdService.getForField(new FieldMapper.Names(left.getValue().name().toLowerCase(Locale.ROOT)),
-                    left.getKey(), true);
+            IndexNumericFieldData<?> leftFieldData = getForField(left.getKey(), left.getValue().name().toLowerCase(Locale.ROOT));
+
             ifdService.clear();
-            IndexNumericFieldData rightFieldData = ifdService.getForField(new FieldMapper.Names(right.getValue().name().toLowerCase(Locale.ROOT)),
-                    right.getKey(), true);
+            IndexNumericFieldData<?> rightFieldData = getForField(right.getKey(), right.getValue().name().toLowerCase(Locale.ROOT));
+
             assertOrder(left.getValue().order(), leftFieldData, context);
             assertOrder(right.getValue().order(), rightFieldData, context);
             duelFieldDataDouble(random, context, leftFieldData, rightFieldData);
@@ -319,11 +316,11 @@ public class DuelFieldDataTests extends AbstractFieldDataTests {
                 right = left = list.remove(0);
             }
             ifdService.clear();
-            IndexFieldData leftFieldData = ifdService.getForField(new FieldMapper.Names(left.getValue().name().toLowerCase(Locale.ROOT)),
-                    left.getKey(), true);
+            IndexFieldData<?> leftFieldData = getForField(left.getKey(), left.getValue().name().toLowerCase(Locale.ROOT));
+
             ifdService.clear();
-            IndexFieldData rightFieldData = ifdService.getForField(new FieldMapper.Names(right.getValue().name().toLowerCase(Locale.ROOT)),
-                    right.getKey(), true);
+            IndexFieldData<?> rightFieldData = getForField(right.getKey(), right.getValue().name().toLowerCase(Locale.ROOT));
+
             duelFieldDataBytes(random, context, leftFieldData, rightFieldData, pre);
             duelFieldDataBytes(random, context, rightFieldData, leftFieldData, pre);
 

+ 7 - 8
src/test/java/org/elasticsearch/index/fielddata/FilterFieldDataTest.java

@@ -26,7 +26,6 @@ import org.elasticsearch.common.settings.ImmutableSettings;
 import org.elasticsearch.index.fielddata.AtomicFieldData.WithOrdinals;
 import org.elasticsearch.index.fielddata.ScriptDocValues.Strings;
 import org.elasticsearch.index.fielddata.ordinals.Ordinals.Docs;
-import org.elasticsearch.index.mapper.FieldMapper;
 import org.junit.Test;
 
 import java.util.Random;
@@ -70,7 +69,7 @@ public class FilterFieldDataTest extends AbstractFieldDataTests {
                 ifdService.clear();
                 FieldDataType fieldDataType = new FieldDataType("string", ImmutableSettings.builder().put("format", format)
                         .put("filter.frequency.min_segment_size", 100).put("filter.frequency.min", 0.0d).put("filter.frequency.max", random.nextBoolean() ? 100 : 0.5d));
-                IndexFieldData fieldData = ifdService.getForField(new FieldMapper.Names("high_freq"), fieldDataType, false);
+                IndexFieldData<?> fieldData = getForField(fieldDataType, "high_freq");
                 AtomicFieldData.WithOrdinals<ScriptDocValues.Strings> loadDirect = (WithOrdinals<Strings>) fieldData.loadDirect(context);
                 BytesValues.WithOrdinals bytesValues = loadDirect.getBytesValues(randomBoolean());
                 Docs ordinals = bytesValues.ordinals();
@@ -83,7 +82,7 @@ public class FilterFieldDataTest extends AbstractFieldDataTests {
                 ifdService.clear();
                 FieldDataType fieldDataType = new FieldDataType("string", ImmutableSettings.builder().put("format", format)
                         .put("filter.frequency.min_segment_size", 100).put("filter.frequency.min",  random.nextBoolean() ? 101 : 101d/200.0d).put("filter.frequency.max", 201));
-                IndexFieldData fieldData = ifdService.getForField(new FieldMapper.Names("high_freq"), fieldDataType, false);
+                IndexFieldData<?> fieldData = getForField(fieldDataType, "high_freq");
                 AtomicFieldData.WithOrdinals<ScriptDocValues.Strings> loadDirect = (WithOrdinals<Strings>) fieldData.loadDirect(context);
                 BytesValues.WithOrdinals bytesValues = loadDirect.getBytesValues(randomBoolean());
                 Docs ordinals = bytesValues.ordinals();
@@ -96,7 +95,7 @@ public class FilterFieldDataTest extends AbstractFieldDataTests {
                 ifdService.clear(); // test # docs with value
                 FieldDataType fieldDataType = new FieldDataType("string", ImmutableSettings.builder().put("format", format)
                         .put("filter.frequency.min_segment_size", 101).put("filter.frequency.min", random.nextBoolean() ? 101 : 101d/200.0d));
-                IndexFieldData fieldData = ifdService.getForField(new FieldMapper.Names("med_freq"), fieldDataType, false);
+                IndexFieldData<?> fieldData = getForField(fieldDataType, "med_freq");
                 AtomicFieldData.WithOrdinals<ScriptDocValues.Strings> loadDirect = (WithOrdinals<Strings>) fieldData.loadDirect(context);
                 BytesValues.WithOrdinals bytesValues = loadDirect.getBytesValues(randomBoolean());
                 Docs ordinals = bytesValues.ordinals();
@@ -110,7 +109,7 @@ public class FilterFieldDataTest extends AbstractFieldDataTests {
                 ifdService.clear();
                 FieldDataType fieldDataType = new FieldDataType("string", ImmutableSettings.builder().put("format", format)
                         .put("filter.frequency.min_segment_size", 101).put("filter.frequency.min", random.nextBoolean() ? 101 : 101d/200.0d));
-                IndexFieldData fieldData = ifdService.getForField(new FieldMapper.Names("med_freq"), fieldDataType, false);
+                IndexFieldData<?> fieldData = getForField(fieldDataType, "med_freq");
                 AtomicFieldData.WithOrdinals<ScriptDocValues.Strings> loadDirect = (WithOrdinals<Strings>) fieldData.loadDirect(context);
                 BytesValues.WithOrdinals bytesValues = loadDirect.getBytesValues(randomBoolean());
                 Docs ordinals = bytesValues.ordinals();
@@ -127,7 +126,7 @@ public class FilterFieldDataTest extends AbstractFieldDataTests {
                         .put("filter.frequency.min_segment_size", 0)
                         .put("filter.frequency.min", random.nextBoolean() ? 1 : 1d/200.0d) // 100, 10, 5
                         .put("filter.frequency.max", random.nextBoolean() ? 99 : 99d/200.0d)); // 100
-                IndexFieldData fieldData = ifdService.getForField(new FieldMapper.Names("high_freq"), fieldDataType, false);
+                IndexFieldData<?> fieldData = getForField(fieldDataType, "high_freq");
                 AtomicFieldData.WithOrdinals<ScriptDocValues.Strings> loadDirect = (WithOrdinals<Strings>) fieldData.loadDirect(context);
                 BytesValues.WithOrdinals bytesValues = loadDirect.getBytesValues(randomBoolean());
                 Docs ordinals = bytesValues.ordinals();
@@ -172,7 +171,7 @@ public class FilterFieldDataTest extends AbstractFieldDataTests {
                 ifdService.clear();
                 FieldDataType fieldDataType = new FieldDataType("string", ImmutableSettings.builder().put("format", format)
                         .put("filter.regex.pattern", "\\d"));
-                IndexFieldData fieldData = ifdService.getForField(new FieldMapper.Names("high_freq"), fieldDataType, false);
+                IndexFieldData<?> fieldData = getForField(fieldDataType, "high_freq");
                 AtomicFieldData.WithOrdinals<ScriptDocValues.Strings> loadDirect = (WithOrdinals<Strings>) fieldData.loadDirect(context);
                 BytesValues.WithOrdinals bytesValues = loadDirect.getBytesValues(randomBoolean());
                 Docs ordinals = bytesValues.ordinals();
@@ -184,7 +183,7 @@ public class FilterFieldDataTest extends AbstractFieldDataTests {
                 ifdService.clear();
                 FieldDataType fieldDataType = new FieldDataType("string", ImmutableSettings.builder().put("format", format)
                         .put("filter.regex.pattern", "\\d{1,2}"));
-                IndexFieldData fieldData = ifdService.getForField(new FieldMapper.Names("high_freq"), fieldDataType, false);
+                IndexFieldData<?> fieldData = getForField(fieldDataType, "high_freq");
                 AtomicFieldData.WithOrdinals<ScriptDocValues.Strings> loadDirect = (WithOrdinals<Strings>) fieldData.loadDirect(context);
                 BytesValues.WithOrdinals bytesValues = loadDirect.getBytesValues(randomBoolean());
                 Docs ordinals = bytesValues.ordinals();