Add `count` value to rest output of `geo_centroid` (#24387)

Currently we don't write the count value to the geo_centroid aggregation rest response,
but it is provided via the java api and the count() method in the GeoCentroid interface. 
We should add this parameter to the rest output and also provide it via the getProperty()
method.
This commit is contained in:
Christoph Büscher 2017-04-28 16:25:22 +02:00 committed by GitHub
parent 94e3796908
commit 16a7cbe463
4 changed files with 50 additions and 17 deletions

View File

@ -20,6 +20,7 @@
package org.elasticsearch.search.aggregations.metrics.geocentroid;
import org.apache.lucene.geo.GeoEncodingUtils;
import org.elasticsearch.common.ParseField;
import org.elasticsearch.common.geo.GeoPoint;
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput;
@ -36,8 +37,8 @@ import java.util.Objects;
* Serialization and merge logic for {@link GeoCentroidAggregator}.
*/
public class InternalGeoCentroid extends InternalAggregation implements GeoCentroid {
protected final GeoPoint centroid;
protected final long count;
private final GeoPoint centroid;
private final long count;
public static long encodeLatLon(double lat, double lon) {
return (Integer.toUnsignedLong(GeoEncodingUtils.encodeLatitude(lat)) << 32) | Integer.toUnsignedLong(GeoEncodingUtils.encodeLongitude(lon));
@ -136,6 +137,8 @@ public class InternalGeoCentroid extends InternalAggregation implements GeoCentr
return centroid.lat();
case "lon":
return centroid.lon();
case "count":
return count;
default:
throw new IllegalArgumentException("Found unknown path element [" + coordinate + "] in [" + getName() + "]");
}
@ -145,14 +148,27 @@ public class InternalGeoCentroid extends InternalAggregation implements GeoCentr
}
static class Fields {
static final String CENTROID = "location";
static final ParseField CENTROID = new ParseField("location");
static final ParseField CENTROID_LAT = new ParseField("lat");
static final ParseField CENTROID_LON = new ParseField("lon");
static final ParseField COUNT = new ParseField("count");
}
@Override
public XContentBuilder doXContentBody(XContentBuilder builder, Params params) throws IOException {
return renderXContent(builder, params, centroid, count);
}
static XContentBuilder renderXContent(XContentBuilder builder, Params params, GeoPoint centroid, long count) throws IOException {
if (centroid != null) {
builder.startObject(Fields.CENTROID).field("lat", centroid.lat()).field("lon", centroid.lon()).endObject();
builder.startObject(Fields.CENTROID.getPreferredName());
{
builder.field(Fields.CENTROID_LAT.getPreferredName(), centroid.lat());
builder.field(Fields.CENTROID_LON.getPreferredName(), centroid.lon());
}
builder.endObject();
}
builder.field(Fields.COUNT.getPreferredName(), count);
return builder;
}

View File

@ -59,6 +59,7 @@ public class GeoCentroidIT extends AbstractGeoTestCase {
assertThat(geoCentroid.getName(), equalTo(aggName));
GeoPoint centroid = geoCentroid.centroid();
assertThat(centroid, equalTo(null));
assertEquals(0, geoCentroid.count());
}
public void testUnmapped() throws Exception {
@ -72,6 +73,7 @@ public class GeoCentroidIT extends AbstractGeoTestCase {
assertThat(geoCentroid.getName(), equalTo(aggName));
GeoPoint centroid = geoCentroid.centroid();
assertThat(centroid, equalTo(null));
assertEquals(0, geoCentroid.count());
}
public void testPartiallyUnmapped() throws Exception {
@ -86,6 +88,7 @@ public class GeoCentroidIT extends AbstractGeoTestCase {
GeoPoint centroid = geoCentroid.centroid();
assertThat(centroid.lat(), closeTo(singleCentroid.lat(), GEOHASH_TOLERANCE));
assertThat(centroid.lon(), closeTo(singleCentroid.lon(), GEOHASH_TOLERANCE));
assertEquals(numDocs, geoCentroid.count());
}
public void testSingleValuedField() throws Exception {
@ -101,6 +104,7 @@ public class GeoCentroidIT extends AbstractGeoTestCase {
GeoPoint centroid = geoCentroid.centroid();
assertThat(centroid.lat(), closeTo(singleCentroid.lat(), GEOHASH_TOLERANCE));
assertThat(centroid.lon(), closeTo(singleCentroid.lon(), GEOHASH_TOLERANCE));
assertEquals(numDocs, geoCentroid.count());
}
public void testSingleValueFieldGetProperty() throws Exception {
@ -130,6 +134,7 @@ public class GeoCentroidIT extends AbstractGeoTestCase {
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"));
}
public void testMultiValuedField() throws Exception {
@ -145,6 +150,7 @@ public class GeoCentroidIT extends AbstractGeoTestCase {
GeoPoint centroid = geoCentroid.centroid();
assertThat(centroid.lat(), closeTo(multiCentroid.lat(), GEOHASH_TOLERANCE));
assertThat(centroid.lon(), closeTo(multiCentroid.lon(), GEOHASH_TOLERANCE));
assertEquals(2 * numDocs, geoCentroid.count());
}
public void testSingleValueFieldAsSubAggToGeohashGrid() throws Exception {

View File

@ -42,8 +42,11 @@ public class InternalGeoCentroidTests extends InternalAggregationTestCase<Intern
centroid.resetLon(GeoEncodingUtils.decodeLongitude(encodedLon));
int encodedLat = GeoEncodingUtils.encodeLatitude(centroid.lat());
centroid.resetLat(GeoEncodingUtils.decodeLatitude(encodedLat));
return new InternalGeoCentroid("_name", centroid, 1, Collections.emptyList(), Collections.emptyMap());
long count = randomIntBetween(0, 1000);
if (count == 0) {
centroid = null;
}
return new InternalGeoCentroid("_name", centroid, count, Collections.emptyList(), Collections.emptyMap());
}
@Override
@ -53,14 +56,18 @@ public class InternalGeoCentroidTests extends InternalAggregationTestCase<Intern
@Override
protected void assertReduced(InternalGeoCentroid reduced, List<InternalGeoCentroid> inputs) {
GeoPoint expected = new GeoPoint(0, 0);
int i = 0;
double lonSum = 0;
double latSum = 0;
int totalCount = 0;
for (InternalGeoCentroid input : inputs) {
expected.reset(expected.lat() + (input.centroid().lat() - expected.lat()) / (i+1),
expected.lon() + (input.centroid().lon() - expected.lon()) / (i+1));
i++;
if (input.count() > 0) {
lonSum += (input.count() * input.centroid().getLon());
latSum += (input.count() * input.centroid().getLat());
}
totalCount += input.count();
}
assertEquals(expected.getLat(), reduced.centroid().getLat(), 1E-5D);
assertEquals(expected.getLon(), reduced.centroid().getLon(), 1E-5D);
assertEquals(latSum/totalCount, reduced.centroid().getLat(), 1E-5D);
assertEquals(lonSum/totalCount, reduced.centroid().getLon(), 1E-5D);
assertEquals(totalCount, reduced.count());
}
}

View File

@ -62,7 +62,8 @@ The response for the above aggregation:
"location": {
"lat": 51.00982963107526,
"lon": 3.9662130922079086
}
},
"count": 6
}
}
}
@ -114,7 +115,8 @@ The response for the above aggregation:
"location": {
"lat": 52.371655656024814,
"lon": 4.909563297405839
}
},
"count": 3
}
},
{
@ -124,7 +126,8 @@ The response for the above aggregation:
"location": {
"lat": 48.86055548675358,
"lon": 2.3316944623366
}
},
"count": 2
}
},
{
@ -134,7 +137,8 @@ The response for the above aggregation:
"location": {
"lat": 51.22289997059852,
"lon": 4.40519998781383
}
},
"count": 1
}
}
]