Skip to content

Commit 2474a1f

Browse files
iveraseiverase
authored
Throw better exception for unsupported aggregations over shape fields (#129139) (#129194)
1 parent a7f3c4b commit 2474a1f

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
@@ -28,12 +28,8 @@
2828
import org.elasticsearch.search.aggregations.bucket.geogrid.GeoHashGridAggregator;
2929
import org.elasticsearch.search.aggregations.bucket.geogrid.GeoTileGridAggregationBuilder;
3030
import org.elasticsearch.search.aggregations.bucket.geogrid.GeoTileGridAggregator;
31-
import org.elasticsearch.search.aggregations.metrics.CardinalityAggregationBuilder;
32-
import org.elasticsearch.search.aggregations.metrics.CardinalityAggregator;
3331
import org.elasticsearch.search.aggregations.metrics.GeoBoundsAggregationBuilder;
3432
import org.elasticsearch.search.aggregations.metrics.GeoCentroidAggregationBuilder;
35-
import org.elasticsearch.search.aggregations.metrics.ValueCountAggregationBuilder;
36-
import org.elasticsearch.search.aggregations.metrics.ValueCountAggregator;
3733
import org.elasticsearch.search.aggregations.support.CoreValuesSourceType;
3834
import org.elasticsearch.search.aggregations.support.ValuesSource;
3935
import org.elasticsearch.search.aggregations.support.ValuesSourceRegistry;
@@ -164,9 +160,7 @@ public List<Consumer<ValuesSourceRegistry.Builder>> getAggregationExtentions() {
164160
return List.of(
165161
this::registerGeoShapeCentroidAggregator,
166162
this::registerGeoShapeGridAggregators,
167-
SpatialPlugin::registerGeoShapeBoundsAggregator,
168-
SpatialPlugin::registerValueCountAggregator,
169-
SpatialPlugin::registerCardinalityAggregator
163+
SpatialPlugin::registerGeoShapeBoundsAggregator
170164
);
171165
}
172166

@@ -410,28 +404,6 @@ private void registerGeoShapeGridAggregators(ValuesSourceRegistry.Builder builde
410404
);
411405
}
412406

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