Skip to content

Commit

Permalink
[7.x] Backport ValuesSourceRegistry and related work (elastic#54922)
Browse files Browse the repository at this point in the history
* Add ValuesSource Registry and associated logic (elastic#54281)

* Remove ValuesSourceType argument to ValuesSourceAggregationBuilder (elastic#48638)

* ValuesSourceRegistry Prototype (elastic#48758)

* Remove generics from ValuesSource related classes (elastic#49606)

* fix percentile aggregation tests (elastic#50712)

* Basic thread safety for ValuesSourceRegistry (elastic#50340)

* Remove target value type from ValuesSourceAggregationBuilder (elastic#49943)

* Cleanup default values source type (elastic#50992)

* CoreValuesSourceType no longer implements Writable (elastic#51276)

* Remove genereics & hard coded ValuesSource references from Matrix Stats (elastic#51131)

* Put values source types on fields (elastic#51503)

* Remove VST Any (elastic#51539)

* Rewire terms agg to use new VS registry (elastic#51182)

Also adds some basic AggTestCases for untested code
paths (and boilerplate for future tests once the IT are
converted over)

* Wire Cardinality aggregation to work with the ValuesSourceRegistry (elastic#51337)

* Wire Percentiles aggregator into new VS framework (elastic#51639)

This required a bit of a refactor to percentiles itself.  Before,
the Builder would switch on the chosen algo to generate an
algo-specific factory.  This doesn't work (or at least, would be
difficult) in the new VS framework.

This refactor consolidates both factories together and introduces
a PercentilesConfig object to act as a standardized way to pass
algo-specific parameters through the factory.  This object
is then used when deciding which kind of aggregator to create

Note: CoreValuesSourceType.HISTOGRAM still lives in core, and will
be moved in a subsequent PR.

* Remove generics and target value type from MultiVSAB (elastic#51647)

* fix checkstyle after merge (elastic#52008)

* Plumb ValuesSourceRegistry through to QuerySearchContext (elastic#51710)

* Convert RareTerms to new VS registry (elastic#52166)

* Wire up Value Count (elastic#52225)

* Wire up Max & Min aggregations (elastic#52219)

* ValuesSource refactoring: Wire up Sum aggregation (elastic#52571)

* ValuesSource refactoring: Wire up SigTerms aggregation (elastic#52590)

* Soft immutability for VSConfig (elastic#52729)

* Unmute testSupportedFieldTypes, fix Percentiles/Ranks/Terms tests (elastic#52734)

Also fixes Percentiles which was incorrectly specified to only accept
numeric, but in fact also accepts Boolean and Date (because those are
numeric on master - thanks `testSupportedFieldTypes` for catching it!)

* VS refactoring: Wire up stats aggregation (elastic#52891)

* ValuesSource refactoring: Wire up string_stats aggregation (elastic#52875)

* VS refactoring: Wire up median (MAD) aggregation (elastic#52945)

* fix valuesourcetype issue with constant_keyword field (elastic#53041)x-pack/plugin/rollup/src/main/java/org/elasticsearch/xpack/rollup/job/RollupIndexer.java

this commit implements `getValuesSourceType` for
the ConstantKeyword field type.

master was merged into feature/extensible-values-source
introducing a new field type that was not implementing
`getValuesSourceType`.

* ValuesSource refactoring: Wire up Avg aggregation (elastic#52752)

* Wire PercentileRanks aggregator into new VS framework  (elastic#51693)

* Add a VSConfig resolver for aggregations not using the registry (elastic#53038)

* Vs refactor wire up ranges and date ranges (elastic#52918)

* Wire up geo_bounds aggregation to ValuesSourceRegistry (elastic#53034)

This commit updates the geo_bounds aggregation to depend
on registering itself in the ValuesSourceRegistry

relates elastic#42949.

* VS refactoring: convert Boxplot to new registry (elastic#53132)

* Wire-up geotile_grid and geohash_grid to ValuesSourceRegistry (elastic#53037)

This commit updates the geo*_grid aggregations to depend
on registering itself in the ValuesSourceRegistry

relates to the values-source refactoring meta issue elastic#42949.

* Wire-up geo_centroid agg to ValuesSourceRegistry (elastic#53040)

This commit updates the geo_centroid aggregation to depend
on registering itself in the ValuesSourceRegistry.

relates to the values-source refactoring meta issue elastic#42949.

* Fix type tests for Missing aggregation (elastic#53501)

* ValuesSource Refactor: move histo VSType into XPack module (elastic#53298)

- Introduces a new API (`getBareAggregatorRegistrar()`) which allows plugins to register aggregations against existing agg definitions defined in Core.
- This moves the histogram VSType over to XPack where it belongs. `getHistogramValues()` still remains as a Core concept
- Moves the histo-specific bits over to xpack (e.g. the actual aggregator logic). This requires extra boilerplate since we need to create a new "Analytics" Percentile/Rank aggregators to deal with the histo field. Doubly-so since percentiles/ranks are extra boiler-plate'y... should be much lighter for other aggs

* Wire up DateHistogram to the ValuesSourceRegistry (elastic#53484)

* Vs refactor parser cleanup (elastic#53198)

Co-authored-by: Zachary Tong <[email protected]>
Co-authored-by: Zachary Tong <[email protected]>
Co-authored-by: Christos Soulios <[email protected]>
Co-authored-by: Tal Levy <[email protected]>

* First batch of easy fixes

* Remove List.of from ValuesSourceRegistry

Note that we intend to have a follow up PR dealing with the mutability
of the registry, so I didn't even try to address that here.

* More compiler fixes

* More compiler fixes

* More compiler fixes

* Precommit is happy and so am I

* Add new Core VSTs to tests

* Disabled supported type test on SigTerms until we can backport it's fix

* fix checkstyle

* Fix test failure from semantic merge issue

* Fix some metaData->metadata replacements that got lost

* Fix list of supported types for MinAggregator

* Fix list of supported types for Avg

* remove unused import

Co-authored-by: Zachary Tong <[email protected]>
Co-authored-by: Zachary Tong <[email protected]>
Co-authored-by: Christos Soulios <[email protected]>
Co-authored-by: Tal Levy <[email protected]>
  • Loading branch information
5 people authored Apr 16, 2020
1 parent 8abdf7c commit 22c5518
Show file tree
Hide file tree
Showing 261 changed files with 5,619 additions and 2,379 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,10 @@
import org.elasticsearch.search.aggregations.AggregatorFactories.Builder;
import org.elasticsearch.search.aggregations.AggregatorFactory;
import org.elasticsearch.search.aggregations.support.CoreValuesSourceType;
import org.elasticsearch.search.aggregations.support.ValueType;
import org.elasticsearch.search.aggregations.support.ValuesSource;
import org.elasticsearch.search.aggregations.support.ValuesSource.Bytes;
import org.elasticsearch.search.aggregations.support.ValuesSourceAggregationBuilder;
import org.elasticsearch.search.aggregations.support.ValuesSourceAggregatorFactory;
import org.elasticsearch.search.aggregations.support.ValuesSourceConfig;
import org.elasticsearch.search.aggregations.support.ValuesSourceType;
import org.elasticsearch.search.builder.SearchSourceBuilder;

import java.io.IOException;
Expand All @@ -52,14 +50,14 @@
* {@linkplain AggregationBuilder#rewrite(QueryRewriteContext)}, or
* {@linkplain AbstractAggregationBuilder#build(QueryShardContext, AggregatorFactory)}.
*/
public class StringStatsAggregationBuilder extends ValuesSourceAggregationBuilder<ValuesSource.Bytes, StringStatsAggregationBuilder> {
public class StringStatsAggregationBuilder extends ValuesSourceAggregationBuilder<StringStatsAggregationBuilder> {
public static final String NAME = "string_stats";
private static final ParseField SHOW_DISTRIBUTION_FIELD = new ParseField("show_distribution");

private boolean showDistribution = false;

public StringStatsAggregationBuilder(String name) {
super(name, CoreValuesSourceType.BYTES, ValueType.STRING);
super(name);
}

/**
Expand All @@ -71,6 +69,11 @@ public StringStatsAggregationBuilder showDistribution(boolean showDistribution)
return this;
}

@Override
protected ValuesSourceType defaultValueSourceType() {
return CoreValuesSourceType.BYTES;
}

@Override
public String getType() {
return NAME;
Expand All @@ -92,7 +95,7 @@ public BucketCardinality bucketCardinality() {
}

@Override
protected ValuesSourceAggregatorFactory<Bytes> innerBuild(QueryShardContext queryShardContext, ValuesSourceConfig<Bytes> config,
protected ValuesSourceAggregatorFactory innerBuild(QueryShardContext queryShardContext, ValuesSourceConfig config,
AggregatorFactory parent, Builder subFactoriesBuilder) throws IOException {
throw new UnsupportedOperationException();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1229,7 +1229,8 @@ public static SearchSourceBuilder createTestSearchSourceBuilder() {
searchSourceBuilder.query(new TermQueryBuilder(randomAlphaOfLengthBetween(3, 10), randomAlphaOfLengthBetween(3, 10)));
}
if (randomBoolean()) {
searchSourceBuilder.aggregation(new TermsAggregationBuilder(randomAlphaOfLengthBetween(3, 10), ValueType.STRING)
searchSourceBuilder.aggregation(new TermsAggregationBuilder(randomAlphaOfLengthBetween(3, 10))
.userValueTypeHint(ValueType.STRING)
.field(randomAlphaOfLengthBetween(3, 10)));
}
if (randomBoolean()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,8 @@ public void testSearchMatchQuery() throws IOException {
public void testSearchWithTermsAgg() throws IOException {
SearchRequest searchRequest = new SearchRequest();
SearchSourceBuilder searchSourceBuilder = new SearchSourceBuilder();
searchSourceBuilder.aggregation(new TermsAggregationBuilder("agg1", ValueType.STRING).field("type.keyword"));
searchSourceBuilder.aggregation(new TermsAggregationBuilder("agg1").userValueTypeHint(ValueType.STRING)
.field("type.keyword"));
searchSourceBuilder.size(0);
searchRequest.source(searchSourceBuilder);
SearchResponse searchResponse = execute(searchRequest, highLevelClient()::search, highLevelClient()::searchAsync);
Expand Down Expand Up @@ -358,7 +359,7 @@ public void testSearchWithRangeAgg() throws IOException {
public void testSearchWithTermsAndRangeAgg() throws IOException {
SearchRequest searchRequest = new SearchRequest("index");
SearchSourceBuilder searchSourceBuilder = new SearchSourceBuilder();
TermsAggregationBuilder agg = new TermsAggregationBuilder("agg1", ValueType.STRING).field("type.keyword");
TermsAggregationBuilder agg = new TermsAggregationBuilder("agg1").userValueTypeHint(ValueType.STRING).field("type.keyword");
agg.subAggregation(new RangeAggregationBuilder("subagg").field("num")
.addRange("first", 0, 30).addRange("second", 31, 200));
searchSourceBuilder.aggregation(agg);
Expand Down Expand Up @@ -412,7 +413,7 @@ public void testSearchWithTermsAndRangeAgg() throws IOException {
public void testSearchWithTermsAndWeightedAvg() throws IOException {
SearchRequest searchRequest = new SearchRequest("index");
SearchSourceBuilder searchSourceBuilder = new SearchSourceBuilder();
TermsAggregationBuilder agg = new TermsAggregationBuilder("agg1", ValueType.STRING).field("type.keyword");
TermsAggregationBuilder agg = new TermsAggregationBuilder("agg1").userValueTypeHint(ValueType.STRING).field("type.keyword");
agg.subAggregation(new WeightedAvgAggregationBuilder("subagg")
.value(new MultiValuesSourceFieldConfig.Builder().setFieldName("num").build())
.weight(new MultiValuesSourceFieldConfig.Builder().setFieldName("num2").build())
Expand Down Expand Up @@ -538,10 +539,12 @@ public void testSearchWithParentJoin() throws IOException {
client().performRequest(answerDoc2);
client().performRequest(new Request(HttpPost.METHOD_NAME, "/_refresh"));

TermsAggregationBuilder leafTermAgg = new TermsAggregationBuilder("top-names", ValueType.STRING)
TermsAggregationBuilder leafTermAgg = new TermsAggregationBuilder("top-names")
.userValueTypeHint(ValueType.STRING)
.field("owner.display_name.keyword").size(10);
ChildrenAggregationBuilder childrenAgg = new ChildrenAggregationBuilder("to-answers", "answer").subAggregation(leafTermAgg);
TermsAggregationBuilder termsAgg = new TermsAggregationBuilder("top-tags", ValueType.STRING).field("tags.keyword")
TermsAggregationBuilder termsAgg = new TermsAggregationBuilder("top-tags").userValueTypeHint(ValueType.STRING)
.field("tags.keyword")
.size(10).subAggregation(childrenAgg);
SearchSourceBuilder searchSourceBuilder = new SearchSourceBuilder();
searchSourceBuilder.size(0).aggregation(termsAgg);
Expand Down Expand Up @@ -753,15 +756,18 @@ public void testMultiSearch() throws Exception {
public void testMultiSearch_withAgg() throws Exception {
MultiSearchRequest multiSearchRequest = new MultiSearchRequest();
SearchRequest searchRequest1 = new SearchRequest("index1");
searchRequest1.source().size(0).aggregation(new TermsAggregationBuilder("name", ValueType.STRING).field("field.keyword")
searchRequest1.source().size(0).aggregation(new TermsAggregationBuilder("name").userValueTypeHint(ValueType.STRING)
.field("field.keyword")
.order(BucketOrder.key(true)));
multiSearchRequest.add(searchRequest1);
SearchRequest searchRequest2 = new SearchRequest("index2");
searchRequest2.source().size(0).aggregation(new TermsAggregationBuilder("name", ValueType.STRING).field("field.keyword")
searchRequest2.source().size(0).aggregation(new TermsAggregationBuilder("name").userValueTypeHint(ValueType.STRING)
.field("field.keyword")
.order(BucketOrder.key(true)));
multiSearchRequest.add(searchRequest2);
SearchRequest searchRequest3 = new SearchRequest("index3");
searchRequest3.source().size(0).aggregation(new TermsAggregationBuilder("name", ValueType.STRING).field("field.keyword")
searchRequest3.source().size(0).aggregation(new TermsAggregationBuilder("name").userValueTypeHint(ValueType.STRING)
.field("field.keyword")
.order(BucketOrder.key(true)));
multiSearchRequest.add(searchRequest3);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,23 +28,18 @@
import org.elasticsearch.search.aggregations.AggregatorFactories;
import org.elasticsearch.search.aggregations.AggregatorFactory;
import org.elasticsearch.search.aggregations.support.ArrayValuesSourceAggregationBuilder;
import org.elasticsearch.search.aggregations.support.CoreValuesSourceType;
import org.elasticsearch.search.aggregations.support.ValueType;
import org.elasticsearch.search.aggregations.support.ValuesSource;
import org.elasticsearch.search.aggregations.support.ValuesSource.Numeric;
import org.elasticsearch.search.aggregations.support.ValuesSourceConfig;

import java.io.IOException;
import java.util.Map;

public class MatrixStatsAggregationBuilder
extends ArrayValuesSourceAggregationBuilder.LeafOnly<ValuesSource.Numeric, MatrixStatsAggregationBuilder> {
public class MatrixStatsAggregationBuilder extends ArrayValuesSourceAggregationBuilder.LeafOnly<MatrixStatsAggregationBuilder> {
public static final String NAME = "matrix_stats";

private MultiValueMode multiValueMode = MultiValueMode.AVG;

public MatrixStatsAggregationBuilder(String name) {
super(name, CoreValuesSourceType.NUMERIC, ValueType.NUMERIC);
super(name);
}

protected MatrixStatsAggregationBuilder(MatrixStatsAggregationBuilder clone,
Expand All @@ -62,7 +57,7 @@ protected AggregationBuilder shallowCopy(AggregatorFactories.Builder factoriesBu
* Read from a stream.
*/
public MatrixStatsAggregationBuilder(StreamInput in) throws IOException {
super(in, CoreValuesSourceType.NUMERIC, ValueType.NUMERIC);
super(in);
}

@Override
Expand All @@ -81,7 +76,7 @@ public MultiValueMode multiValueMode() {

@Override
protected MatrixStatsAggregatorFactory innerBuild(QueryShardContext queryShardContext,
Map<String, ValuesSourceConfig<Numeric>> configs,
Map<String, ValuesSourceConfig> configs,
AggregatorFactory parent,
AggregatorFactories.Builder subFactoriesBuilder) throws IOException {
return new MatrixStatsAggregatorFactory(name, configs, multiValueMode, queryShardContext, parent, subFactoriesBuilder, metadata);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@

import org.elasticsearch.index.query.QueryShardContext;
import org.elasticsearch.search.MultiValueMode;
import org.elasticsearch.search.aggregations.AggregationExecutionException;
import org.elasticsearch.search.aggregations.Aggregator;
import org.elasticsearch.search.aggregations.AggregatorFactories;
import org.elasticsearch.search.aggregations.AggregatorFactory;
Expand All @@ -29,14 +30,15 @@
import org.elasticsearch.search.internal.SearchContext;

import java.io.IOException;
import java.util.HashMap;
import java.util.Map;

final class MatrixStatsAggregatorFactory extends ArrayValuesSourceAggregatorFactory<ValuesSource.Numeric> {
final class MatrixStatsAggregatorFactory extends ArrayValuesSourceAggregatorFactory {

private final MultiValueMode multiValueMode;

MatrixStatsAggregatorFactory(String name,
Map<String, ValuesSourceConfig<ValuesSource.Numeric>> configs,
Map<String, ValuesSourceConfig> configs,
MultiValueMode multiValueMode,
QueryShardContext queryShardContext,
AggregatorFactory parent,
Expand All @@ -55,11 +57,20 @@ protected Aggregator createUnmapped(SearchContext searchContext,
}

@Override
protected Aggregator doCreateInternal(Map<String, ValuesSource.Numeric> valuesSources,
protected Aggregator doCreateInternal(Map<String, ValuesSource> valuesSources,
SearchContext searchContext,
Aggregator parent,
boolean collectsFromSingleBucket,
Map<String, Object> metadata) throws IOException {
return new MatrixStatsAggregator(name, valuesSources, searchContext, parent, multiValueMode, metadata);
Map<String, ValuesSource.Numeric> typedValuesSources = new HashMap<>(valuesSources.size());
for (Map.Entry<String, ValuesSource> entry : valuesSources.entrySet()) {
if (entry.getValue() instanceof ValuesSource.Numeric == false) {
throw new AggregationExecutionException("ValuesSource type " + entry.getValue().toString() +
"is not supported for aggregation " + this.name());
}
// TODO: There must be a better option than this.
typedValuesSources.put(entry.getKey(), (ValuesSource.Numeric) entry.getValue());
}
return new MatrixStatsAggregator(name, typedValuesSources, searchContext, parent, multiValueMode, metadata);
}
}
Loading

0 comments on commit 22c5518

Please sign in to comment.