Skip to content

Commit 2d605ee

Browse files
iverasemridula-s109
iverase
authored and
mridula-s109
committed
Throw better exception for unsupported aggregations over shape fields (elastic#129139)
1 parent f768664 commit 2d605ee

File tree

2 files changed

+122
-29
lines changed

2 files changed

+122
-29
lines changed

‎x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/SpatialPlugin.java‎

Lines changed: 1 addition & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -26,12 +26,8 @@
2626
import org.elasticsearch.search.aggregations.bucket.geogrid.GeoHashGridAggregator;
2727
import org.elasticsearch.search.aggregations.bucket.geogrid.GeoTileGridAggregationBuilder;
2828
import org.elasticsearch.search.aggregations.bucket.geogrid.GeoTileGridAggregator;
29-
import org.elasticsearch.search.aggregations.metrics.CardinalityAggregationBuilder;
30-
import org.elasticsearch.search.aggregations.metrics.CardinalityAggregator;
3129
import org.elasticsearch.search.aggregations.metrics.GeoBoundsAggregationBuilder;
3230
import org.elasticsearch.search.aggregations.metrics.GeoCentroidAggregationBuilder;
33-
import org.elasticsearch.search.aggregations.metrics.ValueCountAggregationBuilder;
34-
import org.elasticsearch.search.aggregations.metrics.ValueCountAggregator;
3531
import org.elasticsearch.search.aggregations.support.CoreValuesSourceType;
3632
import org.elasticsearch.search.aggregations.support.ValuesSource;
3733
import org.elasticsearch.search.aggregations.support.ValuesSourceRegistry;
@@ -162,9 +158,7 @@ public List<Consumer<ValuesSourceRegistry.Builder>> getAggregationExtentions() {
162158
return List.of(
163159
this::registerGeoShapeCentroidAggregator,
164160
this::registerGeoShapeGridAggregators,
165-
SpatialPlugin::registerGeoShapeBoundsAggregator,
166-
SpatialPlugin::registerValueCountAggregator,
167-
SpatialPlugin::registerCardinalityAggregator
161+
SpatialPlugin::registerGeoShapeBoundsAggregator
168162
);
169163
}
170164

@@ -408,28 +402,6 @@ private void registerGeoShapeGridAggregators(ValuesSourceRegistry.Builder builde
408402
);
409403
}
410404

411-
private static void registerValueCountAggregator(ValuesSourceRegistry.Builder builder) {
412-
builder.register(ValueCountAggregationBuilder.REGISTRY_KEY, GeoShapeValuesSourceType.instance(), ValueCountAggregator::new, true);
413-
}
414-
415-
private static void registerCardinalityAggregator(ValuesSourceRegistry.Builder builder) {
416-
builder.register(
417-
CardinalityAggregationBuilder.REGISTRY_KEY,
418-
GeoShapeValuesSourceType.instance(),
419-
(name, valuesSourceConfig, precision, executionMode, context, parent, metadata) -> new CardinalityAggregator(
420-
name,
421-
valuesSourceConfig,
422-
precision,
423-
// Force execution mode to null
424-
null,
425-
context,
426-
parent,
427-
metadata
428-
),
429-
true
430-
);
431-
}
432-
433405
private <T> ContextParser<String, T> checkLicense(ContextParser<String, T> realParser, LicensedFeature.Momentary feature) {
434406
return (parser, name) -> {
435407
if (feature.check(getLicenseState()) == false) {

‎x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/search/aggregations/UnsupportedAggregationsTests.java‎

Lines changed: 121 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,121 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the Elastic License
4+
* 2.0; you may not use this file except in compliance with the Elastic License
5+
* 2.0.
6+
*/
7+
8+
package org.elasticsearch.xpack.spatial.search.aggregations;
9+
10+
import org.apache.lucene.document.Document;
11+
import org.elasticsearch.common.geo.Orientation;
12+
import org.elasticsearch.geometry.Point;
13+
import org.elasticsearch.index.mapper.MappedFieldType;
14+
import org.elasticsearch.lucene.spatial.BinaryShapeDocValuesField;
15+
import org.elasticsearch.plugins.SearchPlugin;
16+
import org.elasticsearch.search.aggregations.AggregatorTestCase;
17+
import org.elasticsearch.search.aggregations.metrics.CardinalityAggregationBuilder;
18+
import org.elasticsearch.search.aggregations.metrics.ValueCountAggregationBuilder;
19+
import org.elasticsearch.xpack.spatial.LocalStateSpatialPlugin;
20+
import org.elasticsearch.xpack.spatial.index.mapper.GeoShapeWithDocValuesFieldMapper;
21+
import org.elasticsearch.xpack.spatial.index.mapper.ShapeFieldMapper;
22+
import org.elasticsearch.xpack.spatial.util.GeoTestUtils;
23+
24+
import java.util.Collections;
25+
import java.util.List;
26+
27+
import static org.hamcrest.Matchers.equalTo;
28+
29+
public class UnsupportedAggregationsTests extends AggregatorTestCase {
30+
31+
@Override
32+
protected List<SearchPlugin> getSearchPlugins() {
33+
return List.of(new LocalStateSpatialPlugin());
34+
}
35+
36+
public void testCardinalityAggregationOnGeoShape() {
37+
MappedFieldType fieldType = new GeoShapeWithDocValuesFieldMapper.GeoShapeWithDocValuesFieldType(
38+
"geometry",
39+
true,
40+
true,
41+
randomBoolean(),
42+
Orientation.RIGHT,
43+
null,
44+
null,
45+
null,
46+
false,
47+
Collections.emptyMap()
48+
);
49+
BinaryShapeDocValuesField field = GeoTestUtils.binaryGeoShapeDocValuesField("geometry", new Point(0, 0));
50+
CardinalityAggregationBuilder builder = new CardinalityAggregationBuilder("cardinality").field("geometry");
51+
IllegalArgumentException exception = expectThrows(IllegalArgumentException.class, () -> testCase(iw -> {
52+
Document doc = new Document();
53+
doc.add(field);
54+
iw.addDocument(doc);
55+
}, agg -> {}, new AggTestConfig(builder, fieldType)));
56+
assertThat(exception.getMessage(), equalTo("Field [geometry] of type [geo_shape] is not supported for aggregation [cardinality]"));
57+
}
58+
59+
public void testCardinalityAggregationOnShape() {
60+
MappedFieldType fieldType = new ShapeFieldMapper.ShapeFieldType(
61+
"geometry",
62+
true,
63+
true,
64+
Orientation.RIGHT,
65+
null,
66+
false,
67+
Collections.emptyMap()
68+
);
69+
BinaryShapeDocValuesField field = GeoTestUtils.binaryCartesianShapeDocValuesField("geometry", new Point(0, 0));
70+
CardinalityAggregationBuilder builder = new CardinalityAggregationBuilder("cardinality").field("geometry");
71+
IllegalArgumentException exception = expectThrows(IllegalArgumentException.class, () -> testCase(iw -> {
72+
Document doc = new Document();
73+
doc.add(field);
74+
iw.addDocument(doc);
75+
}, agg -> {}, new AggTestConfig(builder, fieldType)));
76+
assertThat(exception.getMessage(), equalTo("Field [geometry] of type [shape] is not supported for aggregation [cardinality]"));
77+
}
78+
79+
public void testValueCountAggregationOnGeoShape() {
80+
MappedFieldType fieldType = new GeoShapeWithDocValuesFieldMapper.GeoShapeWithDocValuesFieldType(
81+
"geometry",
82+
true,
83+
true,
84+
randomBoolean(),
85+
Orientation.RIGHT,
86+
null,
87+
null,
88+
null,
89+
false,
90+
Collections.emptyMap()
91+
);
92+
BinaryShapeDocValuesField field = GeoTestUtils.binaryGeoShapeDocValuesField("geometry", new Point(0, 0));
93+
ValueCountAggregationBuilder builder = new ValueCountAggregationBuilder("cardinality").field("geometry");
94+
IllegalArgumentException exception = expectThrows(IllegalArgumentException.class, () -> testCase(iw -> {
95+
Document doc = new Document();
96+
doc.add(field);
97+
iw.addDocument(doc);
98+
}, agg -> {}, new AggTestConfig(builder, fieldType)));
99+
assertThat(exception.getMessage(), equalTo("Field [geometry] of type [geo_shape] is not supported for aggregation [value_count]"));
100+
}
101+
102+
public void testValueCountAggregationOShape() {
103+
MappedFieldType fieldType = new ShapeFieldMapper.ShapeFieldType(
104+
"geometry",
105+
true,
106+
true,
107+
Orientation.RIGHT,
108+
null,
109+
false,
110+
Collections.emptyMap()
111+
);
112+
BinaryShapeDocValuesField field = GeoTestUtils.binaryCartesianShapeDocValuesField("geometry", new Point(0, 0));
113+
ValueCountAggregationBuilder builder = new ValueCountAggregationBuilder("cardinality").field("geometry");
114+
IllegalArgumentException exception = expectThrows(IllegalArgumentException.class, () -> testCase(iw -> {
115+
Document doc = new Document();
116+
doc.add(field);
117+
iw.addDocument(doc);
118+
}, agg -> {}, new AggTestConfig(builder, fieldType)));
119+
assertThat(exception.getMessage(), equalTo("Field [geometry] of type [shape] is not supported for aggregation [value_count]"));
120+
}
121+
}

0 commit comments

Comments
 (0)