diff --git a/server/src/main/java/org/elasticsearch/search/SearchModule.java b/server/src/main/java/org/elasticsearch/search/SearchModule.java index 36421b5cb9aac..c02462d372288 100644 --- a/server/src/main/java/org/elasticsearch/search/SearchModule.java +++ b/server/src/main/java/org/elasticsearch/search/SearchModule.java @@ -345,9 +345,11 @@ private void registerAggregations(List plugins) { registerAggregation(new AggregationSpec(SumAggregationBuilder.NAME, SumAggregationBuilder::new, SumAggregationBuilder::parse) .addResultReader(InternalSum::new)); registerAggregation(new AggregationSpec(MinAggregationBuilder.NAME, MinAggregationBuilder::new, MinAggregationBuilder::parse) - .addResultReader(InternalMin::new)); + .addResultReader(InternalMin::new) + .setAggregatorRegistrar(MinAggregationBuilder::registerAggregators)); registerAggregation(new AggregationSpec(MaxAggregationBuilder.NAME, MaxAggregationBuilder::new, MaxAggregationBuilder::parse) - .addResultReader(InternalMax::new)); + .addResultReader(InternalMax::new) + .setAggregatorRegistrar(MaxAggregationBuilder::registerAggregators)); registerAggregation(new AggregationSpec(StatsAggregationBuilder.NAME, StatsAggregationBuilder::new, StatsAggregationBuilder::parse) .addResultReader(InternalStats::new)); registerAggregation(new AggregationSpec(ExtendedStatsAggregationBuilder.NAME, ExtendedStatsAggregationBuilder::new, diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/metrics/MaxAggregationBuilder.java b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/MaxAggregationBuilder.java index d32bbf96c450c..09861ae0fc42f 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/metrics/MaxAggregationBuilder.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/MaxAggregationBuilder.java @@ -33,6 +33,7 @@ import org.elasticsearch.search.aggregations.support.ValuesSourceAggregationBuilder; import org.elasticsearch.search.aggregations.support.ValuesSourceConfig; import org.elasticsearch.search.aggregations.support.ValuesSourceParserHelper; +import org.elasticsearch.search.aggregations.support.ValuesSourceRegistry; import org.elasticsearch.search.aggregations.support.ValuesSourceType; import java.io.IOException; @@ -51,6 +52,10 @@ public static AggregationBuilder parse(String aggregationName, XContentParser pa return PARSER.parse(parser, new MaxAggregationBuilder(aggregationName), null); } + public static void registerAggregators(ValuesSourceRegistry valuesSourceRegistry) { + MaxAggregatorFactory.registerAggregators(valuesSourceRegistry); + } + public MaxAggregationBuilder(String name) { super(name); } diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/metrics/MaxAggregatorFactory.java b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/MaxAggregatorFactory.java index f0001b86227b2..7604460fbc741 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/metrics/MaxAggregatorFactory.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/MaxAggregatorFactory.java @@ -25,10 +25,13 @@ import org.elasticsearch.search.aggregations.AggregatorFactories; import org.elasticsearch.search.aggregations.AggregatorFactory; import org.elasticsearch.search.aggregations.pipeline.PipelineAggregator; +import org.elasticsearch.search.aggregations.support.AggregatorSupplier; +import org.elasticsearch.search.aggregations.support.CoreValuesSourceType; import org.elasticsearch.search.aggregations.support.ValuesSource; import org.elasticsearch.search.aggregations.support.ValuesSource.Numeric; import org.elasticsearch.search.aggregations.support.ValuesSourceAggregatorFactory; import org.elasticsearch.search.aggregations.support.ValuesSourceConfig; +import org.elasticsearch.search.aggregations.support.ValuesSourceRegistry; import org.elasticsearch.search.internal.SearchContext; import java.io.IOException; @@ -37,6 +40,23 @@ class MaxAggregatorFactory extends ValuesSourceAggregatorFactory { + static void registerAggregators(ValuesSourceRegistry valuesSourceRegistry) { + valuesSourceRegistry.register(MaxAggregationBuilder.NAME, + List.of(CoreValuesSourceType.NUMERIC, CoreValuesSourceType.DATE, CoreValuesSourceType.BOOLEAN), + new MinMaxAggregatorSupplier() { + @Override + public Aggregator build(String name, + ValuesSourceConfig config, + ValuesSource valuesSource, + SearchContext context, + Aggregator parent, + List pipelineAggregators, + Map metaData) throws IOException { + return new MaxAggregator(name, config, (Numeric) valuesSource, context, parent, pipelineAggregators, metaData); + } + }); + } + MaxAggregatorFactory(String name, ValuesSourceConfig config, QueryShardContext queryShardContext, AggregatorFactory parent, AggregatorFactories.Builder subFactoriesBuilder, Map metaData) throws IOException { @@ -58,10 +78,14 @@ protected Aggregator doCreateInternal(ValuesSource valuesSource, boolean collectsFromSingleBucket, List pipelineAggregators, Map metaData) throws IOException { - if (valuesSource instanceof Numeric == false) { - throw new AggregationExecutionException("ValuesSource type " + valuesSource.toString() + "is not supported for aggregation " + - this.name()); + AggregatorSupplier aggregatorSupplier = queryShardContext.getValuesSourceRegistry().getAggregator(config.valueSourceType(), + MaxAggregationBuilder.NAME); + + if (aggregatorSupplier instanceof MinMaxAggregatorSupplier == false) { + throw new AggregationExecutionException("Registry miss-match - expected MinMaxAggregatorSupplier, found [" + + aggregatorSupplier.getClass().toString() + "]"); } - return new MaxAggregator(name, config, (Numeric) valuesSource, searchContext, parent, pipelineAggregators, metaData); + return ((MinMaxAggregatorSupplier) aggregatorSupplier).build(name, config, valuesSource, searchContext, parent, + pipelineAggregators, metaData); } } diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/metrics/MinAggregationBuilder.java b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/MinAggregationBuilder.java index 4a6662afa9337..0a14f3ae711fe 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/metrics/MinAggregationBuilder.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/MinAggregationBuilder.java @@ -33,6 +33,7 @@ import org.elasticsearch.search.aggregations.support.ValuesSourceAggregationBuilder; import org.elasticsearch.search.aggregations.support.ValuesSourceConfig; import org.elasticsearch.search.aggregations.support.ValuesSourceParserHelper; +import org.elasticsearch.search.aggregations.support.ValuesSourceRegistry; import org.elasticsearch.search.aggregations.support.ValuesSourceType; import java.io.IOException; @@ -64,6 +65,10 @@ protected AggregationBuilder shallowCopy(Builder factoriesBuilder, Map pipelineAggregators, + Map metaData) throws IOException { + return new MinAggregator(name, config, (Numeric) valuesSource, context, parent, pipelineAggregators, metaData); + } + }); + } + MinAggregatorFactory(String name, ValuesSourceConfig config, QueryShardContext queryShardContext, AggregatorFactory parent, AggregatorFactories.Builder subFactoriesBuilder, Map metaData) throws IOException { @@ -58,10 +78,14 @@ protected Aggregator doCreateInternal(ValuesSource valuesSource, boolean collectsFromSingleBucket, List pipelineAggregators, Map metaData) throws IOException { - if (valuesSource instanceof Numeric == false) { - throw new AggregationExecutionException("ValuesSource type " + valuesSource.toString() + "is not supported for aggregation " + - this.name()); + AggregatorSupplier aggregatorSupplier = queryShardContext.getValuesSourceRegistry().getAggregator(config.valueSourceType(), + MinAggregationBuilder.NAME); + + if (aggregatorSupplier instanceof MinMaxAggregatorSupplier == false) { + throw new AggregationExecutionException("Registry miss-match - expected MinMaxAggregatorSupplier, found [" + + aggregatorSupplier.getClass().toString() + "]"); } - return new MinAggregator(name, config, (Numeric) valuesSource, searchContext, parent, pipelineAggregators, metaData); + return ((MinMaxAggregatorSupplier) aggregatorSupplier).build(name, config, valuesSource, searchContext, parent, + pipelineAggregators, metaData); } } diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/metrics/MinMaxAggregatorSupplier.java b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/MinMaxAggregatorSupplier.java new file mode 100644 index 0000000000000..51e4c4d95bd1a --- /dev/null +++ b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/MinMaxAggregatorSupplier.java @@ -0,0 +1,40 @@ +/* + * 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.search.aggregations.metrics; + +import org.elasticsearch.search.aggregations.Aggregator; +import org.elasticsearch.search.aggregations.pipeline.PipelineAggregator; +import org.elasticsearch.search.aggregations.support.AggregatorSupplier; +import org.elasticsearch.search.aggregations.support.ValuesSource; +import org.elasticsearch.search.aggregations.support.ValuesSourceConfig; +import org.elasticsearch.search.internal.SearchContext; + +import java.io.IOException; +import java.util.List; +import java.util.Map; + +public interface MinMaxAggregatorSupplier extends AggregatorSupplier { + Aggregator build(String name, + ValuesSourceConfig config, + ValuesSource valuesSource, + SearchContext context, + Aggregator parent, + List pipelineAggregators, + Map metaData) throws IOException; +} diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/MinAggregatorTests.java b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/MinAggregatorTests.java index 9b2481425f9db..8dc352034c168 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/MinAggregatorTests.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/MinAggregatorTests.java @@ -23,6 +23,7 @@ import org.apache.lucene.document.DoublePoint; import org.apache.lucene.document.Field; import org.apache.lucene.document.FloatPoint; +import org.apache.lucene.document.InetAddressPoint; import org.apache.lucene.document.IntPoint; import org.apache.lucene.document.LongPoint; import org.apache.lucene.document.NumericDocValuesField; @@ -54,6 +55,7 @@ import org.elasticsearch.index.IndexSettings; import org.elasticsearch.index.mapper.ContentPath; import org.elasticsearch.index.mapper.DateFieldMapper; +import org.elasticsearch.index.mapper.IpFieldMapper; import org.elasticsearch.index.mapper.KeywordFieldMapper; import org.elasticsearch.index.mapper.MappedFieldType; import org.elasticsearch.index.mapper.Mapper; @@ -259,6 +261,20 @@ public void testQueryFiltersAll() throws IOException { }); } + public void testIpField() throws IOException { + final String fieldName = "IP_field"; + MinAggregationBuilder aggregationBuilder = new MinAggregationBuilder("min").field(fieldName); + + MappedFieldType fieldType = new IpFieldMapper.IpFieldType(); + fieldType.setName(fieldName); + + boolean v4 = randomBoolean(); + expectThrows(IllegalArgumentException.class, () -> testCase(aggregationBuilder, new MatchAllDocsQuery(), iw -> { + iw.addDocument(singleton(new SortedSetDocValuesField(fieldName, new BytesRef(InetAddressPoint.encode(randomIp(v4)))))); + iw.addDocument(singleton(new SortedSetDocValuesField(fieldName, new BytesRef(InetAddressPoint.encode(randomIp(v4)))))); + }, min -> fail("expected an exception"), fieldType)); + } + public void testUnmappedWithMissingField() throws IOException { MinAggregationBuilder aggregationBuilder = new MinAggregationBuilder("min").field("does_not_exist").missing(0L); @@ -287,7 +303,7 @@ public void testUnsupportedType() { }, (Consumer) min -> { fail("Should have thrown exception"); }, fieldType)); - assertEquals(e.getMessage(), "Expected numeric type on field [not_a_number], but got [keyword]"); + assertEquals("Field [not_a_number] of type [keyword(indexed,tokenized)] is not supported for aggregation [min]", e.getMessage()); } public void testBadMissingField() { diff --git a/test/framework/src/main/java/org/elasticsearch/search/aggregations/AggregatorTestCase.java b/test/framework/src/main/java/org/elasticsearch/search/aggregations/AggregatorTestCase.java index 34763267f1c82..cf050b8e80b30 100644 --- a/test/framework/src/main/java/org/elasticsearch/search/aggregations/AggregatorTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/search/aggregations/AggregatorTestCase.java @@ -93,8 +93,8 @@ import org.elasticsearch.search.aggregations.MultiBucketConsumerService.MultiBucketConsumer; import org.elasticsearch.search.aggregations.pipeline.PipelineAggregator; import org.elasticsearch.search.aggregations.support.CoreValuesSourceType; -import org.elasticsearch.search.aggregations.support.ValuesSourceType; import org.elasticsearch.search.aggregations.support.ValuesSourceRegistry; +import org.elasticsearch.search.aggregations.support.ValuesSourceType; import org.elasticsearch.search.fetch.FetchPhase; import org.elasticsearch.search.fetch.subphase.FetchDocValuesPhase; import org.elasticsearch.search.fetch.subphase.FetchSourcePhase;