Aggregations: Fix geohash grid doc counts computation on multi-valued…

… fields. Close elastic#8512
mute · Nov 21, 2014 · 8b2ad3c · 8b2ad3c
1 parent cb7c1b2
commit 8b2ad3c
Show file tree

Hide file tree

Showing 2 changed files with 42 additions and 12 deletions.
diff --git a/...main/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoHashGridAggregator.java b/...main/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoHashGridAggregator.java
@@ -72,14 +72,18 @@ public void collect(int doc, long owningBucketOrdinal) throws IOException {
         assert owningBucketOrdinal == 0;
         final int valuesCount = values.setDocument(doc);
 
+        long previous = Long.MAX_VALUE;
         for (int i = 0; i < valuesCount; ++i) {
             final long val = values.nextValue();
-            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
@@ -32,9 +32,7 @@
 import org.elasticsearch.test.ElasticsearchIntegrationTest;
 import org.junit.Test;
 
-import java.util.ArrayList;
-import java.util.List;
-import java.util.Random;
+import java.util.*;
 
 import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder;
 import static org.elasticsearch.search.aggregations.AggregationBuilders.geohashGrid;
@@ -46,17 +44,18 @@
 @ElasticsearchIntegrationTest.SuiteScopeTest
 public class GeoHashGridTests extends ElasticsearchIntegrationTest {
 
-    private IndexRequestBuilder indexCity(String name, String latLon) throws Exception {
+    private IndexRequestBuilder indexCity(String index, String name, List<String> latLon) throws Exception {
         XContentBuilder source = jsonBuilder().startObject().field("city", name);
         if (latLon != null) {
             source = source.field("location", latLon);
         }
         source = source.endObject();
-        return client().prepareIndex("idx", "type").setSource(source);
+        return client().prepareIndex(index, "type").setSource(source);
     }
 
 
     static ObjectIntMap<String> expectedDocCountsForGeoHash = null;
+    static ObjectIntMap<String> multiValuedExpectedDocCountsForGeoHash = null;
     static int highestPrecisionGeohash = 12;
     static int numRandomPoints = 100;
 
@@ -78,7 +77,7 @@ public void setupSuiteScopeCluster() throws Exception {
             double lng = (360d * random.nextDouble()) - 180d;
             String randomGeoHash = GeoHashUtils.encode(lat, lng, highestPrecisionGeohash);
             //Index at the highest resolution
-            cities.add(indexCity(randomGeoHash, lat + ", " + lng));
+            cities.add(indexCity("idx", randomGeoHash, Collections.singletonList(lat + ", " + lng)));
             expectedDocCountsForGeoHash.put(randomGeoHash, expectedDocCountsForGeoHash.getOrDefault(randomGeoHash, 0) + 1);
             //Update expected doc counts for all resolutions..
             for (int precision = highestPrecisionGeohash - 1; precision > 0; precision--) {
@@ -90,6 +89,33 @@ public void setupSuiteScopeCluster() throws Exception {
             }
         }
         indexRandom(true, cities);
+
+        assertAcked(prepareCreate("multi_valued_idx")
+                .addMapping("type", "location", "type=geo_point", "city", "type=string,index=not_analyzed"));
+
+        cities = new ArrayList<>();
+        multiValuedExpectedDocCountsForGeoHash = new ObjectIntOpenHashMap<>(numRandomPoints * 2);
+        for (int i = 0; i < numRandomPoints; i++) {
+            final int numPoints = random.nextInt(4);
+            List<String> points = new ArrayList<>();
+            Set<String> geoHashes = new HashSet<>();
+            for (int j = 0; j < numPoints; ++j) {
+                double lat = (180d * random.nextDouble()) - 90d;
+                double lng = (360d * random.nextDouble()) - 180d;
+                points.add(lat + "," + lng);
+                // Update expected doc counts for all resolutions..
+                for (int precision = highestPrecisionGeohash; precision > 0; precision--) {
+                    final String geoHash = GeoHashUtils.encode(lat, lng, precision);
+                    geoHashes.add(geoHash);
+                }
+            }
+            cities.add(indexCity("multi_valued_idx", Integer.toString(i), points));
+            for (String hash : geoHashes) {
+                multiValuedExpectedDocCountsForGeoHash.put(hash, multiValuedExpectedDocCountsForGeoHash.getOrDefault(hash, 0) + 1);
+            }
+        }
+        indexRandom(true, cities);
+
         ensureSearchable();
     }