Skip to content

Commit d6caf2d

Browse files
authored
Convert RareTerms to new VS registry (#52166)
1 parent a79bc73 commit d6caf2d

File tree

6 files changed

+146
-35
lines changed

6 files changed

+146
-35
lines changed

rest-api-spec/src/main/resources/rest-api-spec/test/search.aggregation/280_rare_terms.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ setup:
112112
- length: { aggregations.ip_terms.buckets: 0 }
113113

114114
- do:
115-
catch: request
115+
catch: /Aggregation \[ip_terms\] cannot support regular expression style include\/exclude settings as they can only be applied to string fields\. Use an array of values for include\/exclude clauses/
116116
search:
117117
index: test_1
118118
body: { "size" : 0, "aggs" : { "ip_terms" : { "rare_terms" : { "field" : "ip", "exclude" : "127.*" } } } }

server/src/main/java/org/elasticsearch/search/SearchModule.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -397,7 +397,8 @@ private void registerAggregations(List<SearchPlugin> plugins) {
397397
RareTermsAggregationBuilder::parse)
398398
.addResultReader(StringRareTerms.NAME, StringRareTerms::new)
399399
.addResultReader(UnmappedRareTerms.NAME, UnmappedRareTerms::new)
400-
.addResultReader(LongRareTerms.NAME, LongRareTerms::new));
400+
.addResultReader(LongRareTerms.NAME, LongRareTerms::new)
401+
.setAggregatorRegistrar(RareTermsAggregationBuilder::registerAggregators));
401402
registerAggregation(new AggregationSpec(SignificantTermsAggregationBuilder.NAME, SignificantTermsAggregationBuilder::new,
402403
SignificantTermsAggregationBuilder::parse)
403404
.addResultReader(SignificantStringTerms.NAME, SignificantStringTerms::new)

server/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/RareTermsAggregationBuilder.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
import org.elasticsearch.search.aggregations.support.ValuesSourceAggregatorFactory;
3535
import org.elasticsearch.search.aggregations.support.ValuesSourceConfig;
3636
import org.elasticsearch.search.aggregations.support.ValuesSourceParserHelper;
37+
import org.elasticsearch.search.aggregations.support.ValuesSourceRegistry;
3738
import org.elasticsearch.search.aggregations.support.ValuesSourceType;
3839

3940
import java.io.IOException;
@@ -66,6 +67,10 @@ public static AggregationBuilder parse(String aggregationName, XContentParser pa
6667
return PARSER.parse(parser, new RareTermsAggregationBuilder(aggregationName), null);
6768
}
6869

70+
public static void registerAggregators(ValuesSourceRegistry valuesSourceRegistry) {
71+
RareTermsAggregatorFactory.registerAggregators(valuesSourceRegistry);
72+
}
73+
6974
private IncludeExclude includeExclude = null;
7075
private int maxDocCount = 1;
7176
private double precision = 0.001;

server/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/RareTermsAggregatorFactory.java

Lines changed: 92 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -30,9 +30,12 @@
3030
import org.elasticsearch.search.aggregations.InternalAggregation;
3131
import org.elasticsearch.search.aggregations.NonCollectingAggregator;
3232
import org.elasticsearch.search.aggregations.pipeline.PipelineAggregator;
33+
import org.elasticsearch.search.aggregations.support.AggregatorSupplier;
34+
import org.elasticsearch.search.aggregations.support.CoreValuesSourceType;
3335
import org.elasticsearch.search.aggregations.support.ValuesSource;
3436
import org.elasticsearch.search.aggregations.support.ValuesSourceAggregatorFactory;
3537
import org.elasticsearch.search.aggregations.support.ValuesSourceConfig;
38+
import org.elasticsearch.search.aggregations.support.ValuesSourceRegistry;
3639
import org.elasticsearch.search.internal.SearchContext;
3740

3841
import java.io.IOException;
@@ -44,6 +47,88 @@ public class RareTermsAggregatorFactory extends ValuesSourceAggregatorFactory {
4447
private final int maxDocCount;
4548
private final double precision;
4649

50+
static void registerAggregators(ValuesSourceRegistry valuesSourceRegistry) {
51+
valuesSourceRegistry.register(RareTermsAggregationBuilder.NAME,
52+
List.of(CoreValuesSourceType.BYTES, CoreValuesSourceType.IP),
53+
RareTermsAggregatorFactory.bytesSupplier());
54+
55+
valuesSourceRegistry.register(RareTermsAggregationBuilder.NAME,
56+
List.of(CoreValuesSourceType.DATE, CoreValuesSourceType.BOOLEAN, CoreValuesSourceType.NUMERIC),
57+
RareTermsAggregatorFactory.numericSupplier());
58+
}
59+
60+
/**
61+
* This supplier is used for all the field types that should be aggregated as bytes/strings,
62+
* including those that need global ordinals
63+
*/
64+
private static RareTermsAggregatorSupplier bytesSupplier() {
65+
return new RareTermsAggregatorSupplier() {
66+
@Override
67+
public Aggregator build(String name,
68+
AggregatorFactories factories,
69+
ValuesSource valuesSource,
70+
DocValueFormat format,
71+
int maxDocCount,
72+
double precision,
73+
IncludeExclude includeExclude,
74+
SearchContext context,
75+
Aggregator parent,
76+
List<PipelineAggregator> pipelineAggregators,
77+
Map<String, Object> metaData) throws IOException {
78+
79+
ExecutionMode execution = ExecutionMode.MAP; //TODO global ords not implemented yet, only supports "map"
80+
81+
if ((includeExclude != null) && (includeExclude.isRegexBased()) && format != DocValueFormat.RAW) {
82+
throw new IllegalArgumentException("Aggregation [" + name + "] cannot support " +
83+
"regular expression style include/exclude settings as they can only be applied to string fields. " +
84+
"Use an array of values for include/exclude clauses");
85+
}
86+
87+
return execution.create(name, factories, valuesSource, format,
88+
includeExclude, context, parent, pipelineAggregators, metaData, maxDocCount, precision);
89+
90+
}
91+
};
92+
}
93+
94+
/**
95+
* This supplier is used for all fields that expect to be aggregated as a numeric value.
96+
* This includes floating points, and formatted types that use numerics internally for storage (date, boolean, etc)
97+
*/
98+
private static RareTermsAggregatorSupplier numericSupplier() {
99+
return new RareTermsAggregatorSupplier() {
100+
@Override
101+
public Aggregator build(String name,
102+
AggregatorFactories factories,
103+
ValuesSource valuesSource,
104+
DocValueFormat format,
105+
int maxDocCount,
106+
double precision,
107+
IncludeExclude includeExclude,
108+
SearchContext context,
109+
Aggregator parent,
110+
List<PipelineAggregator> pipelineAggregators,
111+
Map<String, Object> metaData) throws IOException {
112+
113+
if ((includeExclude != null) && (includeExclude.isRegexBased())) {
114+
throw new IllegalArgumentException("Aggregation [" + name + "] cannot support regular expression " +
115+
"style include/exclude settings as they can only be applied to string fields. Use an array of numeric " +
116+
"values for include/exclude clauses used to filter numeric fields");
117+
}
118+
119+
IncludeExclude.LongFilter longFilter = null;
120+
if (((ValuesSource.Numeric) valuesSource).isFloatingPoint()) {
121+
throw new IllegalArgumentException("RareTerms aggregation does not support floating point fields.");
122+
}
123+
if (includeExclude != null) {
124+
longFilter = includeExclude.convertToLongFilter(format);
125+
}
126+
return new LongRareTermsAggregator(name, factories, (ValuesSource.Numeric) valuesSource, format,
127+
context, parent, longFilter, maxDocCount, precision, pipelineAggregators, metaData);
128+
}
129+
};
130+
}
131+
47132
RareTermsAggregatorFactory(String name, ValuesSourceConfig config,
48133
IncludeExclude includeExclude,
49134
QueryShardContext queryShardContext,
@@ -79,40 +164,16 @@ protected Aggregator doCreateInternal(ValuesSource valuesSource,
79164
if (collectsFromSingleBucket == false) {
80165
return asMultiBucketAggregator(this, searchContext, parent);
81166
}
82-
if (valuesSource instanceof ValuesSource.Bytes) {
83-
ExecutionMode execution = ExecutionMode.MAP; //TODO global ords not implemented yet, only supports "map"
84-
85-
DocValueFormat format = config.format();
86-
if ((includeExclude != null) && (includeExclude.isRegexBased()) && format != DocValueFormat.RAW) {
87-
throw new AggregationExecutionException("Aggregation [" + name + "] cannot support " +
88-
"regular expression style include/exclude settings as they can only be applied to string fields. " +
89-
"Use an array of values for include/exclude clauses");
90-
}
91-
92-
return execution.create(name, factories, valuesSource, format,
93-
includeExclude, searchContext, parent, pipelineAggregators, metaData, maxDocCount, precision);
94-
}
95-
96-
if ((includeExclude != null) && (includeExclude.isRegexBased())) {
97-
throw new AggregationExecutionException("Aggregation [" + name + "] cannot support regular expression style include/exclude "
98-
+ "settings as they can only be applied to string fields. Use an array of numeric values for include/exclude clauses " +
99-
"used to filter numeric fields");
100-
}
101167

102-
if (valuesSource instanceof ValuesSource.Numeric) {
103-
IncludeExclude.LongFilter longFilter = null;
104-
if (((ValuesSource.Numeric) valuesSource).isFloatingPoint()) {
105-
throw new AggregationExecutionException("RareTerms aggregation does not support floating point fields.");
106-
}
107-
if (includeExclude != null) {
108-
longFilter = includeExclude.convertToLongFilter(config.format());
109-
}
110-
return new LongRareTermsAggregator(name, factories, (ValuesSource.Numeric) valuesSource, config.format(),
111-
searchContext, parent, longFilter, maxDocCount, precision, pipelineAggregators, metaData);
168+
AggregatorSupplier aggregatorSupplier = queryShardContext.getValuesSourceRegistry().getAggregator(config.valueSourceType(),
169+
RareTermsAggregationBuilder.NAME);
170+
if (aggregatorSupplier instanceof RareTermsAggregatorSupplier == false) {
171+
throw new AggregationExecutionException("Registry miss-match - expected RareTermsAggregatorSupplier, found [" +
172+
aggregatorSupplier.getClass().toString() + "]");
112173
}
113174

114-
throw new AggregationExecutionException("RareTerms aggregation cannot be applied to field [" + config.fieldContext().field()
115-
+ "]. It can only be applied to numeric or string fields.");
175+
return ((RareTermsAggregatorSupplier) aggregatorSupplier).build(name, factories, valuesSource, config.format(),
176+
maxDocCount, precision, includeExclude, searchContext, parent, pipelineAggregators, metaData);
116177
}
117178

118179
public enum ExecutionMode {
Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
/*
2+
* Licensed to Elasticsearch under one or more contributor
3+
* license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright
5+
* ownership. Elasticsearch licenses this file to you under
6+
* the Apache License, Version 2.0 (the "License"); you may
7+
* not use this file except in compliance with the License.
8+
* You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
package org.elasticsearch.search.aggregations.bucket.terms;
20+
21+
import org.elasticsearch.search.DocValueFormat;
22+
import org.elasticsearch.search.aggregations.Aggregator;
23+
import org.elasticsearch.search.aggregations.AggregatorFactories;
24+
import org.elasticsearch.search.aggregations.pipeline.PipelineAggregator;
25+
import org.elasticsearch.search.aggregations.support.AggregatorSupplier;
26+
import org.elasticsearch.search.aggregations.support.ValuesSource;
27+
import org.elasticsearch.search.internal.SearchContext;
28+
29+
import java.io.IOException;
30+
import java.util.List;
31+
import java.util.Map;
32+
33+
interface RareTermsAggregatorSupplier extends AggregatorSupplier {
34+
Aggregator build(String name,
35+
AggregatorFactories factories,
36+
ValuesSource valuesSource,
37+
DocValueFormat format,
38+
int maxDocCount,
39+
double precision,
40+
IncludeExclude includeExclude,
41+
SearchContext context,
42+
Aggregator parent,
43+
List<PipelineAggregator> pipelineAggregators,
44+
Map<String, Object> metaData) throws IOException;
45+
}

server/src/test/java/org/elasticsearch/search/aggregations/bucket/terms/RareTermsAggregatorTests.java

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,6 @@
5252
import org.elasticsearch.index.mapper.Uid;
5353
import org.elasticsearch.search.SearchHit;
5454
import org.elasticsearch.search.aggregations.Aggregation;
55-
import org.elasticsearch.search.aggregations.AggregationExecutionException;
5655
import org.elasticsearch.search.aggregations.Aggregations;
5756
import org.elasticsearch.search.aggregations.Aggregator;
5857
import org.elasticsearch.search.aggregations.AggregatorTestCase;
@@ -311,7 +310,7 @@ public void testRangeField() throws Exception {
311310
IndexSearcher indexSearcher = newIndexSearcher(indexReader);
312311
RareTermsAggregationBuilder aggregationBuilder = new RareTermsAggregationBuilder("_name")
313312
.field("field");
314-
expectThrows(AggregationExecutionException.class,
313+
expectThrows(IllegalArgumentException.class,
315314
() -> createAggregator(aggregationBuilder, indexSearcher, fieldType));
316315
}
317316
}

0 commit comments

Comments
 (0)