From d241c4898e5d5ab87284cc3b351989d26947b552 Mon Sep 17 00:00:00 2001 From: Jim Ferenczi Date: Fri, 19 May 2017 17:11:23 +0200 Subject: Removes parent child fielddata specialization (#24737) This change removes the field data specialization needed for the parent field and replaces it with a simple DocValuesIndexFieldData. The underlying global ordinals are retrieved via a new function called IndexOrdinalsFieldData#getOrdinalMap. The children aggregation is also modified to use a simple WithOrdinals value source rather than the deleted WithOrdinals.Parent. Relates #20257 --- .../aggregations/ChildrenAggregationBuilder.java | 33 ++++++++++++---------- .../aggregations/ChildrenAggregatorFactory.java | 22 ++++++--------- .../aggregations/ParentToChildrenAggregator.java | 23 +++++++-------- .../join/query/HasChildQueryBuilder.java | 27 +++++++++--------- .../join/query/HasParentQueryBuilder.java | 23 +++++++-------- .../ParentToChildrenAggregatorTests.java | 3 +- .../join/query/ChildQuerySearchIT.java | 4 +-- 7 files changed, 67 insertions(+), 68 deletions(-) (limited to 'modules/parent-join/src') diff --git a/modules/parent-join/src/main/java/org/elasticsearch/join/aggregations/ChildrenAggregationBuilder.java b/modules/parent-join/src/main/java/org/elasticsearch/join/aggregations/ChildrenAggregationBuilder.java index d04b1f0a66..35dc4eacbf 100644 --- a/modules/parent-join/src/main/java/org/elasticsearch/join/aggregations/ChildrenAggregationBuilder.java +++ b/modules/parent-join/src/main/java/org/elasticsearch/join/aggregations/ChildrenAggregationBuilder.java @@ -25,15 +25,16 @@ import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentParser; -import org.elasticsearch.index.fielddata.plain.ParentChildIndexFieldData; +import org.elasticsearch.index.fielddata.plain.SortedSetDVOrdinalsIndexFieldData; import org.elasticsearch.index.mapper.DocumentMapper; +import org.elasticsearch.index.mapper.MappedFieldType; import org.elasticsearch.index.mapper.ParentFieldMapper; import org.elasticsearch.index.query.QueryParseContext; import org.elasticsearch.search.aggregations.AggregatorFactories.Builder; import org.elasticsearch.search.aggregations.AggregatorFactory; import org.elasticsearch.search.aggregations.support.FieldContext; import org.elasticsearch.search.aggregations.support.ValueType; -import org.elasticsearch.search.aggregations.support.ValuesSource.Bytes.ParentChild; +import org.elasticsearch.search.aggregations.support.ValuesSource.Bytes.WithOrdinals; import org.elasticsearch.search.aggregations.support.ValuesSourceAggregationBuilder; import org.elasticsearch.search.aggregations.support.ValuesSourceAggregatorFactory; import org.elasticsearch.search.aggregations.support.ValuesSourceConfig; @@ -43,10 +44,11 @@ import org.elasticsearch.search.internal.SearchContext; import java.io.IOException; import java.util.Objects; -public class ChildrenAggregationBuilder extends ValuesSourceAggregationBuilder { +public class ChildrenAggregationBuilder + extends ValuesSourceAggregationBuilder { + public static final String NAME = "children"; - private String parentType; private final String childType; private Query parentFilter; private Query childFilter; @@ -79,15 +81,17 @@ public class ChildrenAggregationBuilder extends ValuesSourceAggregationBuilder

innerBuild(SearchContext context, - ValuesSourceConfig config, AggregatorFactory parent, Builder subFactoriesBuilder) throws IOException { - return new ChildrenAggregatorFactory(name, config, parentType, childFilter, parentFilter, context, parent, + protected ValuesSourceAggregatorFactory innerBuild(SearchContext context, + ValuesSourceConfig config, + AggregatorFactory parent, + Builder subFactoriesBuilder) throws IOException { + return new ChildrenAggregatorFactory(name, config, childFilter, parentFilter, context, parent, subFactoriesBuilder, metaData); } @Override - protected ValuesSourceConfig resolveConfig(SearchContext context) { - ValuesSourceConfig config = new ValuesSourceConfig<>(ValuesSourceType.BYTES); + protected ValuesSourceConfig resolveConfig(SearchContext context) { + ValuesSourceConfig config = new ValuesSourceConfig<>(ValuesSourceType.BYTES); DocumentMapper childDocMapper = context.mapperService().documentMapper(childType); if (childDocMapper != null) { @@ -95,15 +99,15 @@ public class ChildrenAggregationBuilder extends ValuesSourceAggregationBuilder

{ + extends ValuesSourceAggregatorFactory { - private final String parentType; private final Query parentFilter; private final Query childFilter; - public ChildrenAggregatorFactory(String name, ValuesSourceConfig config, String parentType, Query childFilter, - Query parentFilter, SearchContext context, AggregatorFactory parent, AggregatorFactories.Builder subFactoriesBuilder, - Map metaData) throws IOException { + public ChildrenAggregatorFactory(String name, ValuesSourceConfig config, + Query childFilter, Query parentFilter, SearchContext context, AggregatorFactory parent, + AggregatorFactories.Builder subFactoriesBuilder, Map metaData) throws IOException { super(name, config, context, parent, subFactoriesBuilder, metaData); - this.parentType = parentType; this.childFilter = childFilter; this.parentFilter = parentFilter; } @@ -66,12 +63,11 @@ public class ChildrenAggregatorFactory } @Override - protected Aggregator doCreateInternal(ValuesSource.Bytes.WithOrdinals.ParentChild valuesSource, Aggregator parent, + protected Aggregator doCreateInternal(WithOrdinals valuesSource, Aggregator parent, boolean collectsFromSingleBucket, List pipelineAggregators, Map metaData) throws IOException { - long maxOrd = valuesSource.globalMaxOrd(context.searcher(), parentType); - return new ParentToChildrenAggregator(name, factories, context, parent, parentType, childFilter, parentFilter, valuesSource, maxOrd, - pipelineAggregators, metaData); + long maxOrd = valuesSource.globalMaxOrd(context.searcher()); + return new ParentToChildrenAggregator(name, factories, context, parent, childFilter, + parentFilter, valuesSource, maxOrd, pipelineAggregators, metaData); } - } diff --git a/modules/parent-join/src/main/java/org/elasticsearch/join/aggregations/ParentToChildrenAggregator.java b/modules/parent-join/src/main/java/org/elasticsearch/join/aggregations/ParentToChildrenAggregator.java index c1ffb097ab..93ba1b98da 100644 --- a/modules/parent-join/src/main/java/org/elasticsearch/join/aggregations/ParentToChildrenAggregator.java +++ b/modules/parent-join/src/main/java/org/elasticsearch/join/aggregations/ParentToChildrenAggregator.java @@ -20,7 +20,7 @@ package org.elasticsearch.join.aggregations; import org.apache.lucene.index.IndexReader; import org.apache.lucene.index.LeafReaderContext; -import org.apache.lucene.index.SortedDocValues; +import org.apache.lucene.index.SortedSetDocValues; import org.apache.lucene.search.ConstantScoreScorer; import org.apache.lucene.search.DocIdSetIterator; import org.apache.lucene.search.Query; @@ -52,10 +52,9 @@ public class ParentToChildrenAggregator extends SingleBucketAggregator { static final ParseField TYPE_FIELD = new ParseField("type"); - private final String parentType; private final Weight childFilter; private final Weight parentFilter; - private final ValuesSource.Bytes.WithOrdinals.ParentChild valuesSource; + private final ValuesSource.Bytes.WithOrdinals valuesSource; // Maybe use PagedGrowableWriter? This will be less wasteful than LongArray, // but then we don't have the reuse feature of BigArrays. @@ -72,12 +71,11 @@ public class ParentToChildrenAggregator extends SingleBucketAggregator { private boolean multipleBucketsPerParentOrd = false; public ParentToChildrenAggregator(String name, AggregatorFactories factories, - SearchContext context, Aggregator parent, String parentType, Query childFilter, - Query parentFilter, ValuesSource.Bytes.WithOrdinals.ParentChild valuesSource, + SearchContext context, Aggregator parent, Query childFilter, + Query parentFilter, ValuesSource.Bytes.WithOrdinals valuesSource, long maxOrd, List pipelineAggregators, Map metaData) throws IOException { super(name, factories, context, parent, pipelineAggregators, metaData); - this.parentType = parentType; // these two filters are cached in the parser this.childFilter = context.searcher().createNormalizedWeight(childFilter, false); this.parentFilter = context.searcher().createNormalizedWeight(parentFilter, false); @@ -105,9 +103,7 @@ public class ParentToChildrenAggregator extends SingleBucketAggregator { if (valuesSource == null) { return LeafBucketCollector.NO_OP_COLLECTOR; } - - final SortedDocValues globalOrdinals = valuesSource.globalOrdinalsValues(parentType, ctx); - assert globalOrdinals != null; + final SortedSetDocValues globalOrdinals = valuesSource.globalOrdinalsValues(ctx); Scorer parentScorer = parentFilter.scorer(ctx); final Bits parentDocs = Lucene.asSequentialAccessBits(ctx.reader().maxDoc(), parentScorer); return new LeafBucketCollector() { @@ -115,7 +111,8 @@ public class ParentToChildrenAggregator extends SingleBucketAggregator { @Override public void collect(int docId, long bucket) throws IOException { if (parentDocs.get(docId) && globalOrdinals.advanceExact(docId)) { - long globalOrdinal = globalOrdinals.ordValue(); + long globalOrdinal = globalOrdinals.nextOrd(); + assert globalOrdinals.nextOrd() == SortedSetDocValues.NO_MORE_ORDS; if (globalOrdinal != -1) { if (parentOrdToBuckets.get(globalOrdinal) == -1) { parentOrdToBuckets.set(globalOrdinal, bucket); @@ -147,9 +144,8 @@ public class ParentToChildrenAggregator extends SingleBucketAggregator { DocIdSetIterator childDocsIter = childDocsScorer.iterator(); final LeafBucketCollector sub = collectableSubAggregators.getLeafCollector(ctx); - final SortedDocValues globalOrdinals = valuesSource.globalOrdinalsValues(parentType, - ctx); + final SortedSetDocValues globalOrdinals = valuesSource.globalOrdinalsValues(ctx); // Set the scorer, since we now replay only the child docIds sub.setScorer(new ConstantScoreScorer(null, 1f, childDocsIter)); @@ -161,7 +157,8 @@ public class ParentToChildrenAggregator extends SingleBucketAggregator { continue; } if (globalOrdinals.advanceExact(docId)) { - long globalOrdinal = globalOrdinals.ordValue(); + long globalOrdinal = globalOrdinals.nextOrd(); + assert globalOrdinals.nextOrd() == SortedSetDocValues.NO_MORE_ORDS; long bucketOrd = parentOrdToBuckets.get(globalOrdinal); if (bucketOrd != -1) { collectBucket(sub, docId, bucketOrd); diff --git a/modules/parent-join/src/main/java/org/elasticsearch/join/query/HasChildQueryBuilder.java b/modules/parent-join/src/main/java/org/elasticsearch/join/query/HasChildQueryBuilder.java index 494c5e498e..95d000e3cc 100644 --- a/modules/parent-join/src/main/java/org/elasticsearch/join/query/HasChildQueryBuilder.java +++ b/modules/parent-join/src/main/java/org/elasticsearch/join/query/HasChildQueryBuilder.java @@ -34,9 +34,10 @@ import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.lucene.search.Queries; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentParser; -import org.elasticsearch.index.fielddata.IndexParentChildFieldData; -import org.elasticsearch.index.fielddata.plain.ParentChildIndexFieldData; +import org.elasticsearch.index.fielddata.IndexOrdinalsFieldData; +import org.elasticsearch.index.fielddata.plain.SortedSetDVOrdinalsIndexFieldData; import org.elasticsearch.index.mapper.DocumentMapper; +import org.elasticsearch.index.mapper.MappedFieldType; import org.elasticsearch.index.mapper.ParentFieldMapper; import org.elasticsearch.index.query.AbstractQueryBuilder; import org.elasticsearch.index.query.InnerHitBuilder; @@ -48,7 +49,6 @@ import org.elasticsearch.index.query.QueryShardContext; import org.elasticsearch.index.query.QueryShardException; import java.io.IOException; -import java.util.Locale; import java.util.Map; import java.util.Objects; @@ -324,9 +324,10 @@ public class HasChildQueryBuilder extends AbstractQueryBuilder childTypes = new HashSet<>(); - ParentChildIndexFieldData parentChildIndexFieldData = null; for (DocumentMapper documentMapper : context.getMapperService().docMappers(false)) { ParentFieldMapper parentFieldMapper = documentMapper.parentFieldMapper(); if (parentFieldMapper.active() && type.equals(parentFieldMapper.type())) { childTypes.add(documentMapper.type()); - parentChildIndexFieldData = context.getForField(parentFieldMapper.fieldType()); } } - if (childTypes.isEmpty()) { throw new QueryShardException(context, "[" + NAME + "] no child types found for type [" + type + "]"); } @@ -204,14 +202,17 @@ public class HasParentQueryBuilder extends AbstractQueryBuilder verify) diff --git a/modules/parent-join/src/test/java/org/elasticsearch/join/query/ChildQuerySearchIT.java b/modules/parent-join/src/test/java/org/elasticsearch/join/query/ChildQuerySearchIT.java index ed910ac89e..87efdf0a5c 100644 --- a/modules/parent-join/src/test/java/org/elasticsearch/join/query/ChildQuerySearchIT.java +++ b/modules/parent-join/src/test/java/org/elasticsearch/join/query/ChildQuerySearchIT.java @@ -105,7 +105,7 @@ import static org.hamcrest.Matchers.notNullValue; @ClusterScope(scope = Scope.SUITE) public class ChildQuerySearchIT extends ESIntegTestCase { - + @Override protected boolean ignoreExternalCluster() { return true; @@ -2008,7 +2008,7 @@ public class ChildQuerySearchIT extends ESIntegTestCase { .setParent("parent-id").setSource("searchText", "quick brown fox").get(); refresh(); - String[] highlightTypes = new String[] {"plain", "fvh", "postings"}; + String[] highlightTypes = new String[] {"plain", "fvh", "unified"}; for (String highlightType : highlightTypes) { logger.info("Testing with highlight type [{}]", highlightType); SearchResponse searchResponse = client().prepareSearch("test") -- cgit v1.2.3