Skip to content

Commit 0d8d6d0

Browse files
author
Hendrik Muhs
committed
simplify field type lookup
1 parent dddfe05 commit 0d8d6d0

File tree

3 files changed

+48
-57
lines changed

3 files changed

+48
-57
lines changed

‎x-pack/plugin/transform/src/main/java/org/elasticsearch/xpack/transform/transforms/pivot/AggregationResultUtils.java

+11-12
Original file line numberDiff line numberDiff line change
@@ -96,9 +96,8 @@ public static Stream<Map<String, Object>> extractCompositeAggregationResults(
9696
// present at all in the `bucket.getAggregations`. This could occur in the case of a `bucket_selector` agg, which
9797
// does not calculate a value, but instead manipulates other results.
9898
if (aggResult != null) {
99-
final String fieldType = fieldTypeMap.get(aggName);
10099
AggValueExtractor extractor = getExtractor(aggResult);
101-
updateDocument(document, aggName, extractor.value(aggResult, fieldType, fieldTypeMap, ""));
100+
updateDocument(document, aggName, extractor.value(aggResult, fieldTypeMap, ""));
102101
}
103102
}
104103

@@ -179,17 +178,19 @@ public static class AggregationExtractionException extends ElasticsearchExceptio
179178
}
180179

181180
interface AggValueExtractor {
182-
Object value(Aggregation aggregation, String fieldType, Map<String, String> fieldTypeMap, String lookupFieldPrefix);
181+
Object value(Aggregation aggregation, Map<String, String> fieldTypeMap, String lookupFieldPrefix);
183182
}
184183

185184
static class SingleValueAggExtractor implements AggValueExtractor {
186185
@Override
187-
public Object value(Aggregation agg, String fieldType, Map<String, String> fieldTypeMap, String lookupFieldPrefix) {
186+
public Object value(Aggregation agg, Map<String, String> fieldTypeMap, String lookupFieldPrefix) {
188187
SingleValue aggregation = (SingleValue) agg;
189188
// If the double is invalid, this indicates sparse data
190189
if (Numbers.isValidDouble(aggregation.value()) == false) {
191190
return null;
192191
}
192+
193+
String fieldType = fieldTypeMap.get(lookupFieldPrefix.isEmpty() ? agg.getName() : lookupFieldPrefix + "." + agg.getName());
193194
// If the type is numeric or if the formatted string is the same as simply making the value a string,
194195
// gather the `value` type, otherwise utilize `getValueAsString` so we don't lose formatted outputs.
195196
if (isNumericType(fieldType) || aggregation.getValueAsString().equals(String.valueOf(aggregation.value()))) {
@@ -202,7 +203,7 @@ public Object value(Aggregation agg, String fieldType, Map<String, String> field
202203

203204
static class PercentilesAggExtractor implements AggValueExtractor {
204205
@Override
205-
public Object value(Aggregation agg, String fieldType, Map<String, String> fieldTypeMap, String lookupFieldPrefix) {
206+
public Object value(Aggregation agg, Map<String, String> fieldTypeMap, String lookupFieldPrefix) {
206207
Percentiles aggregation = (Percentiles) agg;
207208

208209
HashMap<String, Double> percentiles = new HashMap<>();
@@ -217,7 +218,7 @@ public Object value(Aggregation agg, String fieldType, Map<String, String> field
217218

218219
static class SingleBucketAggExtractor implements AggValueExtractor {
219220
@Override
220-
public Object value(Aggregation agg, String fieldType, Map<String, String> fieldTypeMap, String lookupFieldPrefix) {
221+
public Object value(Aggregation agg, Map<String, String> fieldTypeMap, String lookupFieldPrefix) {
221222
SingleBucketAggregation aggregation = (SingleBucketAggregation) agg;
222223

223224
if (aggregation.getAggregations().iterator().hasNext() == false) {
@@ -226,14 +227,12 @@ public Object value(Aggregation agg, String fieldType, Map<String, String> field
226227

227228
HashMap<String, Object> nested = new HashMap<>();
228229
for (Aggregation subAgg : aggregation.getAggregations()) {
229-
String subLookupFieldPrefix = lookupFieldPrefix.isEmpty() ? agg.getName() : lookupFieldPrefix + "." + agg.getName();
230230
nested.put(
231231
subAgg.getName(),
232232
getExtractor(subAgg).value(
233233
subAgg,
234-
fieldTypeMap.get(subLookupFieldPrefix + "." + subAgg.getName()),
235234
fieldTypeMap,
236-
subLookupFieldPrefix
235+
lookupFieldPrefix.isEmpty() ? agg.getName() : lookupFieldPrefix + "." + agg.getName()
237236
)
238237
);
239238
}
@@ -244,15 +243,15 @@ public Object value(Aggregation agg, String fieldType, Map<String, String> field
244243

245244
static class ScriptedMetricAggExtractor implements AggValueExtractor {
246245
@Override
247-
public Object value(Aggregation agg, String fieldType, Map<String, String> fieldTypeMap, String lookupFieldPrefix) {
246+
public Object value(Aggregation agg, Map<String, String> fieldTypeMap, String lookupFieldPrefix) {
248247
ScriptedMetric aggregation = (ScriptedMetric) agg;
249248
return aggregation.aggregation();
250249
}
251250
}
252251

253252
static class GeoCentroidAggExtractor implements AggValueExtractor {
254253
@Override
255-
public Object value(Aggregation agg, String fieldType, Map<String, String> fieldTypeMap, String lookupFieldPrefix) {
254+
public Object value(Aggregation agg, Map<String, String> fieldTypeMap, String lookupFieldPrefix) {
256255
GeoCentroid aggregation = (GeoCentroid) agg;
257256
// if the account is `0` iff there is no contained centroid
258257
return aggregation.count() > 0 ? aggregation.centroid().toString() : null;
@@ -261,7 +260,7 @@ public Object value(Aggregation agg, String fieldType, Map<String, String> field
261260

262261
static class GeoBoundsAggExtractor implements AggValueExtractor {
263262
@Override
264-
public Object value(Aggregation agg, String fieldType, Map<String, String> fieldTypeMap, String lookupFieldPrefix) {
263+
public Object value(Aggregation agg, Map<String, String> fieldTypeMap, String lookupFieldPrefix) {
265264
GeoBounds aggregation = (GeoBounds) agg;
266265
if (aggregation.bottomRight() == null || aggregation.topLeft() == null) {
267266
return null;

‎x-pack/plugin/transform/src/test/java/org/elasticsearch/xpack/transform/transforms/pivot/AggregationResultUtilsTests.java

+31-36
Original file line numberDiff line numberDiff line change
@@ -629,33 +629,31 @@ public void testUpdateDocumentWithObjectAndNotObject() {
629629
}
630630

631631
public static NumericMetricsAggregation.SingleValue createSingleMetricAgg(String name, Double value, String valueAsString) {
632-
NumericMetricsAggregation.SingleValue agg = createSingleMetricAgg(value, valueAsString);
633-
when(agg.getName()).thenReturn(name);
634-
return agg;
635-
}
636-
637-
public static NumericMetricsAggregation.SingleValue createSingleMetricAgg(Double value, String valueAsString) {
638632
NumericMetricsAggregation.SingleValue agg = mock(NumericMetricsAggregation.SingleValue.class);
639633
when(agg.value()).thenReturn(value);
640634
when(agg.getValueAsString()).thenReturn(valueAsString);
635+
when(agg.getName()).thenReturn(name);
641636
return agg;
642637
}
643638

644639
public void testSingleValueAggExtractor() {
645-
Aggregation agg = createSingleMetricAgg(Double.NaN, "NaN");
646-
assertThat(AggregationResultUtils.getExtractor(agg).value(agg, "double", Collections.emptyMap(), ""), is(nullValue()));
640+
Aggregation agg = createSingleMetricAgg("metric", Double.NaN, "NaN");
641+
assertThat(AggregationResultUtils.getExtractor(agg).value(agg, Collections.singletonMap("metric", "double"), ""), is(nullValue()));
647642

648-
agg = createSingleMetricAgg(Double.POSITIVE_INFINITY, "NaN");
649-
assertThat(AggregationResultUtils.getExtractor(agg).value(agg, "double", Collections.emptyMap(), ""), is(nullValue()));
643+
agg = createSingleMetricAgg("metric", Double.POSITIVE_INFINITY, "NaN");
644+
assertThat(AggregationResultUtils.getExtractor(agg).value(agg, Collections.singletonMap("metric", "double"), ""), is(nullValue()));
650645

651-
agg = createSingleMetricAgg(100.0, "100.0");
652-
assertThat(AggregationResultUtils.getExtractor(agg).value(agg, "double", Collections.emptyMap(), ""), equalTo(100.0));
646+
agg = createSingleMetricAgg("metric", 100.0, "100.0");
647+
assertThat(AggregationResultUtils.getExtractor(agg).value(agg, Collections.singletonMap("metric", "double"), ""), equalTo(100.0));
653648

654-
agg = createSingleMetricAgg(100.0, "one_hundred");
655-
assertThat(AggregationResultUtils.getExtractor(agg).value(agg, "double", Collections.emptyMap(), ""), equalTo(100.0));
649+
agg = createSingleMetricAgg("metric", 100.0, "one_hundred");
650+
assertThat(AggregationResultUtils.getExtractor(agg).value(agg, Collections.singletonMap("metric", "double"), ""), equalTo(100.0));
656651

657-
agg = createSingleMetricAgg(100.0, "one_hundred");
658-
assertThat(AggregationResultUtils.getExtractor(agg).value(agg, "string", Collections.emptyMap(), ""), equalTo("one_hundred"));
652+
agg = createSingleMetricAgg("metric", 100.0, "one_hundred");
653+
assertThat(
654+
AggregationResultUtils.getExtractor(agg).value(agg, Collections.singletonMap("metric", "string"), ""),
655+
equalTo("one_hundred")
656+
);
659657
}
660658

661659
private ScriptedMetric createScriptedMetric(Object returnValue) {
@@ -667,14 +665,14 @@ private ScriptedMetric createScriptedMetric(Object returnValue) {
667665
@SuppressWarnings("unchecked")
668666
public void testScriptedMetricAggExtractor() {
669667
Aggregation agg = createScriptedMetric(null);
670-
assertThat(AggregationResultUtils.getExtractor(agg).value(agg, "object", Collections.emptyMap(), ""), is(nullValue()));
668+
assertThat(AggregationResultUtils.getExtractor(agg).value(agg, Collections.emptyMap(), ""), is(nullValue()));
671669

672670
agg = createScriptedMetric(Collections.singletonList("values"));
673-
Object val = AggregationResultUtils.getExtractor(agg).value(agg, "object", Collections.emptyMap(), "");
671+
Object val = AggregationResultUtils.getExtractor(agg).value(agg, Collections.emptyMap(), "");
674672
assertThat((List<String>) val, hasItem("values"));
675673

676674
agg = createScriptedMetric(Collections.singletonMap("key", 100));
677-
val = AggregationResultUtils.getExtractor(agg).value(agg, "object", Collections.emptyMap(), "");
675+
val = AggregationResultUtils.getExtractor(agg).value(agg, Collections.emptyMap(), "");
678676
assertThat(((Map<String, Object>) val).get("key"), equalTo(100));
679677
}
680678

@@ -687,13 +685,13 @@ private GeoCentroid createGeoCentroid(GeoPoint point, long count) {
687685

688686
public void testGeoCentroidAggExtractor() {
689687
Aggregation agg = createGeoCentroid(null, 0);
690-
assertThat(AggregationResultUtils.getExtractor(agg).value(agg, "geo_point", Collections.emptyMap(), ""), is(nullValue()));
688+
assertThat(AggregationResultUtils.getExtractor(agg).value(agg, Collections.emptyMap(), ""), is(nullValue()));
691689

692690
agg = createGeoCentroid(new GeoPoint(100.0, 101.0), 0);
693-
assertThat(AggregationResultUtils.getExtractor(agg).value(agg, "geo_point", Collections.emptyMap(), ""), is(nullValue()));
691+
assertThat(AggregationResultUtils.getExtractor(agg).value(agg, Collections.emptyMap(), ""), is(nullValue()));
694692

695693
agg = createGeoCentroid(new GeoPoint(100.0, 101.0), randomIntBetween(1, 100));
696-
assertThat(AggregationResultUtils.getExtractor(agg).value(agg, "geo_point", Collections.emptyMap(), ""), equalTo("100.0, 101.0"));
694+
assertThat(AggregationResultUtils.getExtractor(agg).value(agg, Collections.emptyMap(), ""), equalTo("100.0, 101.0"));
697695
}
698696

699697
private GeoBounds createGeoBounds(GeoPoint tl, GeoPoint br) {
@@ -707,10 +705,10 @@ private GeoBounds createGeoBounds(GeoPoint tl, GeoPoint br) {
707705
public void testGeoBoundsAggExtractor() {
708706
final int numberOfRuns = 25;
709707
Aggregation agg = createGeoBounds(null, new GeoPoint(100.0, 101.0));
710-
assertThat(AggregationResultUtils.getExtractor(agg).value(agg, "geo_shape", Collections.emptyMap(), ""), is(nullValue()));
708+
assertThat(AggregationResultUtils.getExtractor(agg).value(agg, Collections.emptyMap(), ""), is(nullValue()));
711709

712710
agg = createGeoBounds(new GeoPoint(100.0, 101.0), null);
713-
assertThat(AggregationResultUtils.getExtractor(agg).value(agg, "geo_shape", Collections.emptyMap(), ""), is(nullValue()));
711+
assertThat(AggregationResultUtils.getExtractor(agg).value(agg, Collections.emptyMap(), ""), is(nullValue()));
714712

715713
String type = "point";
716714
for (int i = 0; i < numberOfRuns; i++) {
@@ -720,10 +718,7 @@ public void testGeoBoundsAggExtractor() {
720718
double lon = randomDoubleBetween(-180.0, 180.0, false);
721719
expectedObject.put("coordinates", Arrays.asList(lon, lat));
722720
agg = createGeoBounds(new GeoPoint(lat, lon), new GeoPoint(lat, lon));
723-
assertThat(
724-
AggregationResultUtils.getExtractor(agg).value(agg, "geo_shape", Collections.emptyMap(), ""),
725-
equalTo(expectedObject)
726-
);
721+
assertThat(AggregationResultUtils.getExtractor(agg).value(agg, Collections.emptyMap(), ""), equalTo(expectedObject));
727722
}
728723

729724
type = "linestring";
@@ -738,7 +733,7 @@ public void testGeoBoundsAggExtractor() {
738733
lon2 = randomDoubleBetween(-180.0, 180.0, false);
739734
}
740735
agg = createGeoBounds(new GeoPoint(lat, lon), new GeoPoint(lat2, lon2));
741-
Object val = AggregationResultUtils.getExtractor(agg).value(agg, "geo_shape", Collections.emptyMap(), "");
736+
Object val = AggregationResultUtils.getExtractor(agg).value(agg, Collections.emptyMap(), "");
742737
Map<String, Object> geoJson = (Map<String, Object>) val;
743738
assertThat(geoJson.get("type"), equalTo(type));
744739
List<Double[]> coordinates = (List<Double[]>) geoJson.get("coordinates");
@@ -762,7 +757,7 @@ public void testGeoBoundsAggExtractor() {
762757
lon2 = randomDoubleBetween(-180.0, 180.0, false);
763758
}
764759
agg = createGeoBounds(new GeoPoint(lat, lon), new GeoPoint(lat2, lon2));
765-
Object val = AggregationResultUtils.getExtractor(agg).value(agg, "geo_shape", Collections.emptyMap(), "");
760+
Object val = AggregationResultUtils.getExtractor(agg).value(agg, Collections.emptyMap(), "");
766761
Map<String, Object> geoJson = (Map<String, Object>) val;
767762
assertThat(geoJson.get("type"), equalTo(type));
768763
List<List<Double[]>> coordinates = (List<List<Double[]>>) geoJson.get("coordinates");
@@ -798,7 +793,7 @@ public void testPercentilesAggExtractor() {
798793
Arrays.asList(new Percentile(1, 0), new Percentile(50, 22.2), new Percentile(99, 43.3), new Percentile(99.5, 100.3))
799794
);
800795
assertThat(
801-
AggregationResultUtils.getExtractor(agg).value(agg, "object", Collections.emptyMap(), ""),
796+
AggregationResultUtils.getExtractor(agg).value(agg, Collections.emptyMap(), ""),
802797
equalTo(asMap("1", 0.0, "50", 22.2, "99", 43.3, "99_5", 100.3))
803798
);
804799
}
@@ -820,11 +815,11 @@ public static SingleBucketAggregation createSingleBucketAgg(String name, long do
820815

821816
public void testSingleBucketAggExtractor() {
822817
Aggregation agg = createSingleBucketAgg("sba", 42L);
823-
assertThat(AggregationResultUtils.getExtractor(agg).value(agg, "long", Collections.emptyMap(), ""), equalTo(42L));
818+
assertThat(AggregationResultUtils.getExtractor(agg).value(agg, Collections.emptyMap(), ""), equalTo(42L));
824819

825820
agg = createSingleBucketAgg("sba1", 42L, createSingleMetricAgg("sub1", 100.0, "100.0"));
826821
assertThat(
827-
AggregationResultUtils.getExtractor(agg).value(agg, "object", Collections.emptyMap(), ""),
822+
AggregationResultUtils.getExtractor(agg).value(agg, Collections.emptyMap(), ""),
828823
equalTo(Collections.singletonMap("sub1", 100.0))
829824
);
830825

@@ -835,7 +830,7 @@ public void testSingleBucketAggExtractor() {
835830
createSingleMetricAgg("sub2", 33.33, "thirty_three")
836831
);
837832
assertThat(
838-
AggregationResultUtils.getExtractor(agg).value(agg, "object", asStringMap("sba2.sub1", "long", "sba2.sub2", "float"), ""),
833+
AggregationResultUtils.getExtractor(agg).value(agg, asStringMap("sba2.sub1", "long", "sba2.sub2", "float"), ""),
839834
equalTo(asMap("sub1", 100.0, "sub2", 33.33))
840835
);
841836

@@ -847,7 +842,7 @@ public void testSingleBucketAggExtractor() {
847842
createSingleBucketAgg("sub3", 42L)
848843
);
849844
assertThat(
850-
AggregationResultUtils.getExtractor(agg).value(agg, "object", asStringMap("sba3.sub1", "long", "sba3.sub2", "double"), ""),
845+
AggregationResultUtils.getExtractor(agg).value(agg, asStringMap("sba3.sub1", "long", "sba3.sub2", "double"), ""),
851846
equalTo(asMap("sub1", 100.0, "sub2", 33.33, "sub3", 42L))
852847
);
853848

@@ -860,7 +855,7 @@ public void testSingleBucketAggExtractor() {
860855
);
861856
assertThat(
862857
AggregationResultUtils.getExtractor(agg)
863-
.value(agg, "object", asStringMap("sba4.sub3.subsub1", "double", "sba4.sub2", "float", "sba4.sub1", "long"), ""),
858+
.value(agg, asStringMap("sba4.sub3.subsub1", "double", "sba4.sub2", "float", "sba4.sub1", "long"), ""),
864859
equalTo(asMap("sub1", 100.0, "sub2", 33.33, "sub3", asMap("subsub1", 11.1)))
865860
);
866861
}

‎x-pack/plugin/transform/src/test/java/org/elasticsearch/xpack/transform/transforms/pivot/AggregationSchemaAndResultTests.java

+6-9
Original file line numberDiff line numberDiff line change
@@ -143,14 +143,14 @@ public void testBasic() throws InterruptedException {
143143
assertEquals("double", mappings.get("p_rating.99_9"));
144144

145145
Aggregation agg = AggregationResultUtilsTests.createSingleMetricAgg("avg_rating", 33.3, "33.3");
146-
assertThat(AggregationResultUtils.getExtractor(agg).value(agg, mappings.get("avg_rating"), mappings, ""), equalTo(33.3));
146+
assertThat(AggregationResultUtils.getExtractor(agg).value(agg, mappings, ""), equalTo(33.3));
147147

148148
agg = AggregationResultUtilsTests.createPercentilesAgg(
149149
"p_agg",
150150
Arrays.asList(new Percentile(1, 0), new Percentile(50, 1.2), new Percentile(99, 2.4), new Percentile(99.5, 4.3))
151151
);
152152
assertThat(
153-
AggregationResultUtils.getExtractor(agg).value(agg, mappings.get("p_rating"), mappings, ""),
153+
AggregationResultUtils.getExtractor(agg).value(agg, mappings, ""),
154154
equalTo(asMap("1", 0.0, "50", 1.2, "99", 2.4, "99_5", 4.3))
155155
);
156156
}
@@ -208,17 +208,14 @@ public void testNested() throws InterruptedException {
208208
assertEquals("double", mappings.get("filter_4.filter_4_1.filter_4_1_2.min_drinks_4"));
209209

210210
Aggregation agg = AggregationResultUtilsTests.createSingleBucketAgg("filter_1", 36363);
211-
assertThat(AggregationResultUtils.getExtractor(agg).value(agg, mappings.get("filter_1"), mappings, ""), equalTo(36363L));
211+
assertThat(AggregationResultUtils.getExtractor(agg).value(agg, mappings, ""), equalTo(36363L));
212212

213213
agg = AggregationResultUtilsTests.createSingleBucketAgg(
214214
"filter_2",
215215
23144,
216216
AggregationResultUtilsTests.createSingleMetricAgg("max_drinks_2", 45.0, "forty_five")
217217
);
218-
assertThat(
219-
AggregationResultUtils.getExtractor(agg).value(agg, mappings.get("filter_2"), mappings, ""),
220-
equalTo(asMap("max_drinks_2", 45.0))
221-
);
218+
assertThat(AggregationResultUtils.getExtractor(agg).value(agg, mappings, ""), equalTo(asMap("max_drinks_2", 45.0)));
222219

223220
agg = AggregationResultUtilsTests.createSingleBucketAgg(
224221
"filter_3",
@@ -230,7 +227,7 @@ public void testNested() throws InterruptedException {
230227
)
231228
);
232229
assertThat(
233-
AggregationResultUtils.getExtractor(agg).value(agg, mappings.get("filter_3"), mappings, ""),
230+
AggregationResultUtils.getExtractor(agg).value(agg, mappings, ""),
234231
equalTo(asMap("filter_3_1", asMap("max_drinks_3", 35.0)))
235232
);
236233

@@ -253,7 +250,7 @@ public void testNested() throws InterruptedException {
253250
)
254251
);
255252
assertThat(
256-
AggregationResultUtils.getExtractor(agg).value(agg, mappings.get("filter_4"), mappings, ""),
253+
AggregationResultUtils.getExtractor(agg).value(agg, mappings, ""),
257254
equalTo(
258255
asMap(
259256
"filter_4_1",

0 commit comments

Comments
 (0)