Skip to content

Commit a895c8c

Browse files
committed
Aggregations: Fix geohash grid doc counts computation on multi-valued fields.
Close #8512
1 parent 040b272 commit a895c8c

File tree

2 files changed

+42
-12
lines changed

2 files changed

+42
-12
lines changed

src/main/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoHashGridAggregator.java

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -72,14 +72,18 @@ public void collect(int doc, long owningBucketOrdinal) throws IOException {
7272
assert owningBucketOrdinal == 0;
7373
final int valuesCount = values.setDocument(doc);
7474

75+
long previous = Long.MAX_VALUE;
7576
for (int i = 0; i < valuesCount; ++i) {
7677
final long val = values.nextValue();
77-
long bucketOrdinal = bucketOrds.add(val);
78-
if (bucketOrdinal < 0) { // already seen
79-
bucketOrdinal = - 1 - bucketOrdinal;
80-
collectExistingBucket(doc, bucketOrdinal);
81-
} else {
82-
collectBucket(doc, bucketOrdinal);
78+
if (previous != val || i == 0) {
79+
long bucketOrdinal = bucketOrds.add(val);
80+
if (bucketOrdinal < 0) { // already seen
81+
bucketOrdinal = - 1 - bucketOrdinal;
82+
collectExistingBucket(doc, bucketOrdinal);
83+
} else {
84+
collectBucket(doc, bucketOrdinal);
85+
}
86+
previous = val;
8387
}
8488
}
8589
}

src/test/java/org/elasticsearch/search/aggregations/bucket/GeoHashGridTests.java

Lines changed: 32 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -32,9 +32,7 @@
3232
import org.elasticsearch.test.ElasticsearchIntegrationTest;
3333
import org.junit.Test;
3434

35-
import java.util.ArrayList;
36-
import java.util.List;
37-
import java.util.Random;
35+
import java.util.*;
3836

3937
import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder;
4038
import static org.elasticsearch.search.aggregations.AggregationBuilders.geohashGrid;
@@ -46,17 +44,18 @@
4644
@ElasticsearchIntegrationTest.SuiteScopeTest
4745
public class GeoHashGridTests extends ElasticsearchIntegrationTest {
4846

49-
private IndexRequestBuilder indexCity(String name, String latLon) throws Exception {
47+
private IndexRequestBuilder indexCity(String index, String name, List<String> latLon) throws Exception {
5048
XContentBuilder source = jsonBuilder().startObject().field("city", name);
5149
if (latLon != null) {
5250
source = source.field("location", latLon);
5351
}
5452
source = source.endObject();
55-
return client().prepareIndex("idx", "type").setSource(source);
53+
return client().prepareIndex(index, "type").setSource(source);
5654
}
5755

5856

5957
static ObjectIntMap<String> expectedDocCountsForGeoHash = null;
58+
static ObjectIntMap<String> multiValuedExpectedDocCountsForGeoHash = null;
6059
static int highestPrecisionGeohash = 12;
6160
static int numRandomPoints = 100;
6261

@@ -78,7 +77,7 @@ public void setupSuiteScopeCluster() throws Exception {
7877
double lng = (360d * random.nextDouble()) - 180d;
7978
String randomGeoHash = GeoHashUtils.encode(lat, lng, highestPrecisionGeohash);
8079
//Index at the highest resolution
81-
cities.add(indexCity(randomGeoHash, lat + ", " + lng));
80+
cities.add(indexCity("idx", randomGeoHash, Collections.singletonList(lat + ", " + lng)));
8281
expectedDocCountsForGeoHash.put(randomGeoHash, expectedDocCountsForGeoHash.getOrDefault(randomGeoHash, 0) + 1);
8382
//Update expected doc counts for all resolutions..
8483
for (int precision = highestPrecisionGeohash - 1; precision > 0; precision--) {
@@ -90,6 +89,33 @@ public void setupSuiteScopeCluster() throws Exception {
9089
}
9190
}
9291
indexRandom(true, cities);
92+
93+
assertAcked(prepareCreate("multi_valued_idx")
94+
.addMapping("type", "location", "type=geo_point", "city", "type=string,index=not_analyzed"));
95+
96+
cities = new ArrayList<>();
97+
multiValuedExpectedDocCountsForGeoHash = new ObjectIntOpenHashMap<>(numRandomPoints * 2);
98+
for (int i = 0; i < numRandomPoints; i++) {
99+
final int numPoints = random.nextInt(4);
100+
List<String> points = new ArrayList<>();
101+
Set<String> geoHashes = new HashSet<>();
102+
for (int j = 0; j < numPoints; ++j) {
103+
double lat = (180d * random.nextDouble()) - 90d;
104+
double lng = (360d * random.nextDouble()) - 180d;
105+
points.add(lat + "," + lng);
106+
// Update expected doc counts for all resolutions..
107+
for (int precision = highestPrecisionGeohash; precision > 0; precision--) {
108+
final String geoHash = GeoHashUtils.encode(lat, lng, precision);
109+
geoHashes.add(geoHash);
110+
}
111+
}
112+
cities.add(indexCity("multi_valued_idx", Integer.toString(i), points));
113+
for (String hash : geoHashes) {
114+
multiValuedExpectedDocCountsForGeoHash.put(hash, multiValuedExpectedDocCountsForGeoHash.getOrDefault(hash, 0) + 1);
115+
}
116+
}
117+
indexRandom(true, cities);
118+
93119
ensureSearchable();
94120
}
95121

0 commit comments

Comments
 (0)