Skip to content

Commit

Permalink
Support cartesian_bounds aggregation on point and shape (#91298)
Browse files Browse the repository at this point in the history
* Support cartesian_bounds aggregation on point and shape

* Update docs/changelog/91298.yaml

* Update docs/changelog/91298.yaml

* Fixed cartesian parse exceptions to mimic previous behavior

* Re-added removed interfaces to fix failing tests

* Generalized AbstractGeoTestCase for use in Cartesian tests

And added new CartesianBoundsIT and CartesianCentroidIT

* Fixed bug in encoding missing cartesian points

* Simplify cartesian bounds

* Removing the neg/pos left/right split used in bounds because this is only relevant to GeoBounds
* Revert GeoBounds to no longer share common code with CartesianBounds (including aggregators)
* Make Point and Shape aggregators share common code

* Fixed geo comment

* Delete incorrectly added file

* Set correct minimum version for new cartesian aggs

* Fixed specific types for ShapeValues

* Completed Centroid/Bounds aggregation integration tests refactoring

An earlier commit started generalizing Geo/Cartesian integration tests for
centroid and bounds aggregations, but did not complete the job.

This commit completes the work, since almost all tests are identical between
cartesian and geo, except tests related to the dateline.

* Removed licensing comment

* GeoBoundsIT expects wrapLongitude(false)

* Fixed CartesianBoundsIT flakiness and removed geo-specific code

* Reduced CartesianCentroidIT flakiness by normalizing to floats
  • Loading branch information
craigtaverner committed Nov 14, 2022
1 parent 5cb0905 commit 34e8da6
Show file tree
Hide file tree
Showing 41 changed files with 2,589 additions and 560 deletions.
6 changes: 6 additions & 0 deletions docs/changelog/91298.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
pr: 91298
summary: Support `cartesian_bounds` aggregation on point and shape
area: Geo
type: enhancement
issues:
- 90157

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -10,148 +10,92 @@

import org.elasticsearch.action.search.SearchResponse;
import org.elasticsearch.common.geo.GeoPoint;
import org.elasticsearch.search.aggregations.InternalAggregation;
import org.elasticsearch.common.geo.SpatialPoint;
import org.elasticsearch.search.aggregations.bucket.geogrid.GeoGrid;
import org.elasticsearch.search.aggregations.bucket.global.Global;
import org.elasticsearch.test.ESIntegTestCase;
import org.elasticsearch.test.geo.RandomGeoGenerator;

import java.util.List;

import static org.elasticsearch.index.query.QueryBuilders.matchAllQuery;
import static org.elasticsearch.search.aggregations.AggregationBuilders.geoCentroid;
import static org.elasticsearch.search.aggregations.AggregationBuilders.geohashGrid;
import static org.elasticsearch.search.aggregations.AggregationBuilders.global;
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertSearchResponse;
import static org.hamcrest.Matchers.closeTo;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.notNullValue;
import static org.hamcrest.Matchers.sameInstance;

/**
* Integration Test for GeoCentroid metric aggregator
*/
@ESIntegTestCase.SuiteScopeTestCase
public class GeoCentroidIT extends AbstractGeoTestCase {
private static final String aggName = "geoCentroid";
public class GeoCentroidIT extends CentroidAggregationTestBase {

public void testEmptyAggregation() throws Exception {
SearchResponse response = client().prepareSearch(EMPTY_IDX_NAME)
.setQuery(matchAllQuery())
.addAggregation(geoCentroid(aggName).field(SINGLE_VALUED_FIELD_NAME))
public void testSingleValueFieldAsSubAggToGeohashGrid() {
SearchResponse response = client().prepareSearch(HIGH_CARD_IDX_NAME)
.addAggregation(
geohashGrid("geoGrid").field(SINGLE_VALUED_FIELD_NAME)
.subAggregation(centroidAgg(aggName()).field(SINGLE_VALUED_FIELD_NAME))
)
.get();
assertSearchResponse(response);

GeoCentroid geoCentroid = response.getAggregations().get(aggName);
assertThat(response.getHits().getTotalHits().value, equalTo(0L));
assertThat(geoCentroid, notNullValue());
assertThat(geoCentroid.getName(), equalTo(aggName));
assertThat(geoCentroid.centroid(), equalTo(null));
assertEquals(0, geoCentroid.count());
GeoGrid grid = response.getAggregations().get("geoGrid");
assertThat(grid, notNullValue());
assertThat(grid.getName(), equalTo("geoGrid"));
List<? extends GeoGrid.Bucket> buckets = grid.getBuckets();
for (GeoGrid.Bucket cell : buckets) {
String geohash = cell.getKeyAsString();
SpatialPoint expectedCentroid = expectedCentroidsForGeoHash.get(geohash);
GeoCentroid centroidAgg = cell.getAggregations().get(aggName());
assertSameCentroid(centroidAgg.centroid(), expectedCentroid);
}
}

public void testUnmapped() throws Exception {
SearchResponse response = client().prepareSearch(UNMAPPED_IDX_NAME)
.addAggregation(geoCentroid(aggName).field(SINGLE_VALUED_FIELD_NAME))
.get();
assertSearchResponse(response);

GeoCentroid geoCentroid = response.getAggregations().get(aggName);
assertThat(geoCentroid, notNullValue());
assertThat(geoCentroid.getName(), equalTo(aggName));
assertThat(geoCentroid.centroid(), equalTo(null));
assertEquals(0, geoCentroid.count());
@Override
protected String aggName() {
return "geoCentroid";
}

public void testPartiallyUnmapped() throws Exception {
SearchResponse response = client().prepareSearch(IDX_NAME, UNMAPPED_IDX_NAME)
.addAggregation(geoCentroid(aggName).field(SINGLE_VALUED_FIELD_NAME))
.get();
assertSearchResponse(response);

GeoCentroid geoCentroid = response.getAggregations().get(aggName);
assertThat(geoCentroid, notNullValue());
assertThat(geoCentroid.getName(), equalTo(aggName));
assertSameCentroid(geoCentroid.centroid(), singleCentroid);
assertEquals(numDocs, geoCentroid.count());
@Override
public GeoCentroidAggregationBuilder centroidAgg(String name) {
return new GeoCentroidAggregationBuilder(name);
}

public void testSingleValuedField() throws Exception {
SearchResponse response = client().prepareSearch(IDX_NAME)
.setQuery(matchAllQuery())
.addAggregation(geoCentroid(aggName).field(SINGLE_VALUED_FIELD_NAME))
.get();
assertSearchResponse(response);

GeoCentroid geoCentroid = response.getAggregations().get(aggName);
assertThat(geoCentroid, notNullValue());
assertThat(geoCentroid.getName(), equalTo(aggName));
assertSameCentroid(geoCentroid.centroid(), singleCentroid);
assertEquals(numDocs, geoCentroid.count());
/** Geo has different coordinate names than cartesian */
@Override
protected String coordinateName(String coordinate) {
return switch (coordinate) {
case "x" -> "lon";
case "y" -> "lat";
default -> throw new IllegalArgumentException("Unknown coordinate: " + coordinate);
};
}

public void testSingleValueFieldGetProperty() {
SearchResponse response = client().prepareSearch(IDX_NAME)
.setQuery(matchAllQuery())
.addAggregation(global("global").subAggregation(geoCentroid(aggName).field(SINGLE_VALUED_FIELD_NAME)))
.get();
assertSearchResponse(response);
@Override
protected String fieldTypeName() {
return "geo_point";
}

Global global = response.getAggregations().get("global");
assertThat(global, notNullValue());
assertThat(global.getName(), equalTo("global"));
assertThat(global.getDocCount(), equalTo((long) numDocs));
assertThat(global.getAggregations(), notNullValue());
assertThat(global.getAggregations().asMap().size(), equalTo(1));

GeoCentroid geoCentroid = global.getAggregations().get(aggName);
assertThat(geoCentroid, notNullValue());
assertThat(geoCentroid.getName(), equalTo(aggName));
assertThat((GeoCentroid) ((InternalAggregation) global).getProperty(aggName), sameInstance(geoCentroid));
assertSameCentroid(geoCentroid.centroid(), singleCentroid);
assertThat(
((GeoPoint) ((InternalAggregation) global).getProperty(aggName + ".value")).lat(),
closeTo(singleCentroid.lat(), GEOHASH_TOLERANCE)
);
assertThat(
((GeoPoint) ((InternalAggregation) global).getProperty(aggName + ".value")).lon(),
closeTo(singleCentroid.lon(), GEOHASH_TOLERANCE)
);
assertThat((double) ((InternalAggregation) global).getProperty(aggName + ".lat"), closeTo(singleCentroid.lat(), GEOHASH_TOLERANCE));
assertThat((double) ((InternalAggregation) global).getProperty(aggName + ".lon"), closeTo(singleCentroid.lon(), GEOHASH_TOLERANCE));
assertEquals(numDocs, (long) ((InternalAggregation) global).getProperty(aggName + ".count"));
@Override
protected GeoPoint makePoint(double x, double y) {
return new GeoPoint(y, x);
}

public void testMultiValuedField() throws Exception {
SearchResponse searchResponse = client().prepareSearch(IDX_NAME)
.setQuery(matchAllQuery())
.addAggregation(geoCentroid(aggName).field(MULTI_VALUED_FIELD_NAME))
.get();
assertSearchResponse(searchResponse);
@Override
protected GeoPoint randomPoint() {
return RandomGeoGenerator.randomPoint(random());
}

GeoCentroid geoCentroid = searchResponse.getAggregations().get(aggName);
assertThat(geoCentroid, notNullValue());
assertThat(geoCentroid.getName(), equalTo(aggName));
assertSameCentroid(geoCentroid.centroid(), multiCentroid);
assertEquals(2 * numDocs, geoCentroid.count());
@Override
protected void resetX(SpatialPoint point, double x) {
((GeoPoint) point).resetLon(x);
}

public void testSingleValueFieldAsSubAggToGeohashGrid() {
SearchResponse response = client().prepareSearch(HIGH_CARD_IDX_NAME)
.addAggregation(
geohashGrid("geoGrid").field(SINGLE_VALUED_FIELD_NAME).subAggregation(geoCentroid(aggName).field(SINGLE_VALUED_FIELD_NAME))
)
.get();
assertSearchResponse(response);
@Override
protected void resetY(SpatialPoint point, double y) {
((GeoPoint) point).resetLat(y);
}

GeoGrid grid = response.getAggregations().get("geoGrid");
assertThat(grid, notNullValue());
assertThat(grid.getName(), equalTo("geoGrid"));
List<? extends GeoGrid.Bucket> buckets = grid.getBuckets();
for (GeoGrid.Bucket cell : buckets) {
String geohash = cell.getKeyAsString();
GeoPoint expectedCentroid = expectedCentroidsForGeoHash.get(geohash);
GeoCentroid centroidAgg = cell.getAggregations().get(aggName);
assertSameCentroid(centroidAgg.centroid(), expectedCentroid);
}
@Override
protected GeoPoint reset(SpatialPoint point, double x, double y) {
return ((GeoPoint) point).reset(y, x);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,20 +9,8 @@
package org.elasticsearch.search.aggregations.metrics;

import org.elasticsearch.common.geo.GeoPoint;
import org.elasticsearch.search.aggregations.Aggregation;

/**
* An aggregation that computes a bounding box in which all documents of the current bucket are.
*/
public interface GeoBounds extends Aggregation {

/**
* Get the top-left location of the bounding box.
*/
GeoPoint topLeft();

/**
* Get the bottom-right location of the bounding box.
*/
GeoPoint bottomRight();
}
public interface GeoBounds extends SpatialBounds<GeoPoint> {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,122 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/

package org.elasticsearch.search.aggregations.metrics;

import org.elasticsearch.common.geo.BoundingBox;
import org.elasticsearch.common.geo.GeoBoundingBox;
import org.elasticsearch.common.geo.SpatialPoint;
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.search.aggregations.InternalAggregation;
import org.elasticsearch.search.aggregations.support.SamplingContext;
import org.elasticsearch.xcontent.XContentBuilder;

import java.io.IOException;
import java.util.List;
import java.util.Map;

public abstract class InternalBounds<T extends SpatialPoint> extends InternalAggregation implements SpatialBounds<T> {
public final double top;
public final double bottom;

public InternalBounds(String name, double top, double bottom, Map<String, Object> metadata) {
super(name, metadata);
this.top = top;
this.bottom = bottom;
}

/**
* Read from a stream.
*/
public InternalBounds(StreamInput in) throws IOException {
super(in);
top = in.readDouble();
bottom = in.readDouble();
}

@Override
protected void doWriteTo(StreamOutput out) throws IOException {
out.writeDouble(top);
out.writeDouble(bottom);
}

@Override
public InternalAggregation finalizeSampling(SamplingContext samplingContext) {
return this;
}

@Override
protected boolean mustReduceOnSingleInternalAgg() {
return false;
}

@Override
public Object getProperty(List<String> path) {
if (path.isEmpty()) {
return this;
} else if (path.size() == 1) {
BoundingBox<T> bbox = resolveBoundingBox();
String bBoxSide = path.get(0);
return switch (bBoxSide) {
case "top" -> bbox.top();
case "left" -> bbox.left();
case "bottom" -> bbox.bottom();
case "right" -> bbox.right();
default -> throw new IllegalArgumentException("Found unknown path element [" + bBoxSide + "] in [" + getName() + "]");
};
} else if (path.size() == 2) {
BoundingBox<T> bbox = resolveBoundingBox();
T cornerPoint = null;
String cornerString = path.get(0);
cornerPoint = switch (cornerString) {
case "top_left" -> bbox.topLeft();
case "bottom_right" -> bbox.bottomRight();
default -> throw new IllegalArgumentException("Found unknown path element [" + cornerString + "] in [" + getName() + "]");
};
return selectCoordinate(path.get(1), cornerPoint);
} else {
throw new IllegalArgumentException("path not supported for [" + getName() + "]: " + path);
}
}

protected abstract Object selectCoordinate(String coordinateString, T cornerPoint);

@Override
public XContentBuilder doXContentBody(XContentBuilder builder, Params params) throws IOException {
BoundingBox<T> bbox = resolveBoundingBox();
if (bbox != null) {
builder.startObject(GeoBoundingBox.BOUNDS_FIELD.getPreferredName());
bbox.toXContentFragment(builder);
builder.endObject();
}
return builder;
}

protected abstract BoundingBox<T> resolveBoundingBox();

@Override
public T topLeft() {
BoundingBox<T> bbox = resolveBoundingBox();
if (bbox == null) {
return null;
} else {
return bbox.topLeft();
}
}

@Override
public T bottomRight() {
BoundingBox<T> bbox = resolveBoundingBox();
if (bbox == null) {
return null;
} else {
return bbox.bottomRight();
}
}
}

0 comments on commit 34e8da6

Please sign in to comment.