Skip to content

Commit 69b99c8

Browse files
committed
[Rollup] Histo group config should support scaled_floats
Metric config already whitelist scaled_floats, but it wasn't added to the histo group config. This centralizes the mapping types map so that both metrics and histo (and any future configs) use the same map. Fixes elastic#32035
1 parent b7f07f0 commit 69b99c8

File tree

4 files changed

+18
-16
lines changed

4 files changed

+18
-16
lines changed

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/rollup/RollupField.java

+13
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
package org.elasticsearch.xpack.core.rollup;
77

88
import org.elasticsearch.common.ParseField;
9+
import org.elasticsearch.index.mapper.NumberFieldMapper;
910
import org.elasticsearch.search.aggregations.metrics.avg.AvgAggregationBuilder;
1011
import org.elasticsearch.search.aggregations.metrics.max.MaxAggregationBuilder;
1112
import org.elasticsearch.search.aggregations.metrics.min.MinAggregationBuilder;
@@ -15,6 +16,8 @@
1516

1617
import java.util.Arrays;
1718
import java.util.List;
19+
import java.util.stream.Collectors;
20+
import java.util.stream.Stream;
1821

1922
public class RollupField {
2023
// Fields that are used both in core Rollup actions and Rollup plugin
@@ -34,6 +37,16 @@ public class RollupField {
3437
public static final List<String> SUPPORTED_METRICS = Arrays.asList(MaxAggregationBuilder.NAME, MinAggregationBuilder.NAME,
3538
SumAggregationBuilder.NAME, AvgAggregationBuilder.NAME, ValueCountAggregationBuilder.NAME);
3639

40+
// these mapper types are used by the configs (metric, histo, etc) to validate field mappings
41+
public static final List<String> NUMERIC_FIELD_MAPPER_TYPES;
42+
static {
43+
List<String> types = Stream.of(NumberFieldMapper.NumberType.values())
44+
.map(NumberFieldMapper.NumberType::typeName)
45+
.collect(Collectors.toList());
46+
types.add("scaled_float"); // have to add manually since scaled_float is in a module
47+
NUMERIC_FIELD_MAPPER_TYPES = types;
48+
}
49+
3750
/**
3851
* Format to the appropriate Rollup field name convention
3952
*

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/rollup/job/HistoGroupConfig.java

+1-5
Original file line numberDiff line numberDiff line change
@@ -51,10 +51,6 @@ public class HistoGroupConfig implements Writeable, ToXContentFragment {
5151

5252
private static final ParseField INTERVAL = new ParseField("interval");
5353
private static final ParseField FIELDS = new ParseField("fields");
54-
private static final List<String> MAPPER_TYPES = Stream.of(NumberFieldMapper.NumberType.values())
55-
.map(NumberFieldMapper.NumberType::typeName)
56-
.collect(Collectors.toList());
57-
5854

5955
private final long interval;
6056
private final String[] fields;
@@ -126,7 +122,7 @@ public void validateMappings(Map<String, Map<String, FieldCapabilities>> fieldCa
126122
Map<String, FieldCapabilities> fieldCaps = fieldCapsResponse.get(field);
127123
if (fieldCaps != null && fieldCaps.isEmpty() == false) {
128124
fieldCaps.forEach((key, value) -> {
129-
if (MAPPER_TYPES.contains(key)) {
125+
if (RollupField.NUMERIC_FIELD_MAPPER_TYPES.contains(key)) {
130126
if (value.isAggregatable() == false) {
131127
validationException.addValidationError("The field [" + field + "] must be aggregatable across all indices, " +
132128
"but is not.");

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/rollup/job/MetricConfig.java

+1-10
Original file line numberDiff line numberDiff line change
@@ -66,15 +66,6 @@ public class MetricConfig implements Writeable, ToXContentFragment {
6666
private static final ParseField AVG = new ParseField("avg");
6767
private static final ParseField VALUE_COUNT = new ParseField("value_count");
6868

69-
private static final List<String> MAPPER_TYPES;
70-
static {
71-
List<String> types = Stream.of(NumberFieldMapper.NumberType.values())
72-
.map(NumberFieldMapper.NumberType::typeName)
73-
.collect(Collectors.toList());
74-
types.add("scaled_float"); // have to add manually since scaled_float is in a module
75-
MAPPER_TYPES = types;
76-
}
77-
7869
public static final ObjectParser<MetricConfig.Builder, Void> PARSER = new ObjectParser<>(NAME, MetricConfig.Builder::new);
7970

8071
static {
@@ -153,7 +144,7 @@ public void validateMappings(Map<String, Map<String, FieldCapabilities>> fieldCa
153144
Map<String, FieldCapabilities> fieldCaps = fieldCapsResponse.get(field);
154145
if (fieldCaps != null && fieldCaps.isEmpty() == false) {
155146
fieldCaps.forEach((key, value) -> {
156-
if (MAPPER_TYPES.contains(key)) {
147+
if (RollupField.NUMERIC_FIELD_MAPPER_TYPES.contains(key)) {
157148
if (value.isAggregatable() == false) {
158149
validationException.addValidationError("The field [" + field + "] must be aggregatable across all indices, " +
159150
"but is not.");

x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/rollup/job/HistoGroupConfigSerializingTests.java

+3-1
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
import org.elasticsearch.common.xcontent.XContentParser;
1212
import org.elasticsearch.test.AbstractSerializingTestCase;
1313
import org.elasticsearch.xpack.core.rollup.ConfigTestHelpers;
14+
import org.elasticsearch.xpack.core.rollup.RollupField;
1415

1516
import java.io.IOException;
1617
import java.util.Collections;
@@ -111,7 +112,8 @@ public void testValidateMatchingField() throws IOException {
111112
// Have to mock fieldcaps because the ctor's aren't public...
112113
FieldCapabilities fieldCaps = mock(FieldCapabilities.class);
113114
when(fieldCaps.isAggregatable()).thenReturn(true);
114-
responseMap.put("my_field", Collections.singletonMap("long", fieldCaps));
115+
String mappingType = randomFrom(RollupField.NUMERIC_FIELD_MAPPER_TYPES);
116+
responseMap.put("my_field", Collections.singletonMap(mappingType, fieldCaps));
115117

116118
HistoGroupConfig config = new HistoGroupConfig.Builder()
117119
.setFields(Collections.singletonList("my_field"))

0 commit comments

Comments
 (0)