From abc0bc4c7f79490ca18ecf1814673c57bedfa2f7 Mon Sep 17 00:00:00 2001 From: Adrien Grand Date: Thu, 20 Nov 2014 15:35:31 +0100 Subject: [PATCH] Aggregations: Fix geohash grid doc counts computation on multi-valued fields. Close #8512 --- .../bucket/geogrid/GeoHashGridAggregator.java | 16 ++++++++++------ .../aggregations/bucket/GeoHashGridTests.java | 4 +--- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/src/main/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoHashGridAggregator.java b/src/main/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoHashGridAggregator.java index 84c2f6c28d3..8b4674550a8 100644 --- a/src/main/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoHashGridAggregator.java +++ b/src/main/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoHashGridAggregator.java @@ -74,14 +74,18 @@ public class GeoHashGridAggregator extends BucketsAggregator { values.setDocument(doc); final int valuesCount = values.count(); + long previous = Long.MAX_VALUE; for (int i = 0; i < valuesCount; ++i) { final long val = values.valueAt(i); - long bucketOrdinal = bucketOrds.add(val); - if (bucketOrdinal < 0) { // already seen - bucketOrdinal = - 1 - bucketOrdinal; - collectExistingBucket(doc, bucketOrdinal); - } else { - collectBucket(doc, bucketOrdinal); + if (previous != val || i == 0) { + long bucketOrdinal = bucketOrds.add(val); + if (bucketOrdinal < 0) { // already seen + bucketOrdinal = - 1 - bucketOrdinal; + collectExistingBucket(doc, bucketOrdinal); + } else { + collectBucket(doc, bucketOrdinal); + } + previous = val; } } } diff --git a/src/test/java/org/elasticsearch/search/aggregations/bucket/GeoHashGridTests.java b/src/test/java/org/elasticsearch/search/aggregations/bucket/GeoHashGridTests.java index 050d0435887..5f426efdf44 100644 --- a/src/test/java/org/elasticsearch/search/aggregations/bucket/GeoHashGridTests.java +++ b/src/test/java/org/elasticsearch/search/aggregations/bucket/GeoHashGridTests.java @@ -106,9 +106,7 @@ public class GeoHashGridTests extends ElasticsearchIntegrationTest { for (int i = 0; i < numDocs; i++) { final int numPoints = random.nextInt(4); List points = new ArrayList<>(); - // TODO (#8512): this should be a Set, not a List. Currently if a document has two positions that have - // the same geo hash, it will increase the doc_count for this geo hash by 2 instead of 1 - List geoHashes = new ArrayList<>(); + Set geoHashes = new HashSet<>(); for (int j = 0; j < numPoints; ++j) { double lat = (180d * random.nextDouble()) - 90d; double lng = (360d * random.nextDouble()) - 180d;