Fix legacy GeoPointField decoding in FieldStats

LegacyGeoPointField was using the wrong decoding for min/max prefix coded GeoPoint Terms. This commit applies the correcct decoding. Note that the min/max values, though, are likely useless anyway since they map to a low resolution morton encoded version of the point; not something that is really of value for field stats.

Author: nknize

core/src/main/java/org/elasticsearch/index/mapper/BaseGeoPointFieldMapper.java

@@ -25,8 +25,9 @@
 import org.apache.lucene.index.IndexableField;
 import org.apache.lucene.index.Terms;
 import org.apache.lucene.search.Query;
+import org.apache.lucene.spatial.util.MortonEncoder;
+import org.apache.lucene.util.BytesRef;
 import org.apache.lucene.util.LegacyNumericUtils;
-import org.apache.lucene.util.NumericUtils;
 import org.elasticsearch.ElasticsearchParseException;
 import org.elasticsearch.Version;
 import org.elasticsearch.action.fieldstats.FieldStats;
@@ -306,6 +307,7 @@ public static class LegacyGeoPointFieldType extends GeoPointFieldType {
 
         protected MappedFieldType latFieldType;
         protected MappedFieldType lonFieldType;
+        protected boolean numericEncoded;
 
         LegacyGeoPointFieldType() {}
 
@@ -316,6 +318,7 @@ public static class LegacyGeoPointFieldType extends GeoPointFieldType {
             this.geoHashPrefixEnabled = ref.geoHashPrefixEnabled;
             this.latFieldType = ref.latFieldType; // copying ref is ok, this can never be modified
             this.lonFieldType = ref.lonFieldType; // copying ref is ok, this can never be modified
+            this.numericEncoded = ref.numericEncoded;
         }
 
         @Override
@@ -329,15 +332,16 @@ public boolean equals(Object o) {
             LegacyGeoPointFieldType that = (LegacyGeoPointFieldType) o;
             return  geoHashPrecision == that.geoHashPrecision &&
                     geoHashPrefixEnabled == that.geoHashPrefixEnabled &&
+                    numericEncoded == that.numericEncoded &&
                     java.util.Objects.equals(geoHashFieldType, that.geoHashFieldType) &&
                     java.util.Objects.equals(latFieldType, that.latFieldType) &&
                     java.util.Objects.equals(lonFieldType, that.lonFieldType);
         }
 
         @Override
         public int hashCode() {
-            return java.util.Objects.hash(super.hashCode(), geoHashFieldType, geoHashPrecision, geoHashPrefixEnabled, latFieldType,
-                    lonFieldType);
+            return java.util.Objects.hash(super.hashCode(), geoHashFieldType, geoHashPrecision, geoHashPrefixEnabled,
+                    numericEncoded, latFieldType, lonFieldType);
         }
 
         @Override
@@ -437,10 +441,9 @@ public FieldStats.GeoPoint stats(IndexReader reader) throws IOException {
             if (terms == null) {
                 return new FieldStats.GeoPoint(reader.maxDoc(), 0L, -1L, -1L, isSearchable(), isAggregatable());
             }
-            GeoPoint minPt = GeoPoint.fromGeohash(NumericUtils.sortableBytesToLong(terms.getMin().bytes, terms.getMin().offset));
-            GeoPoint maxPt = GeoPoint.fromGeohash(NumericUtils.sortableBytesToLong(terms.getMax().bytes, terms.getMax().offset));
             return new FieldStats.GeoPoint(reader.maxDoc(), terms.getDocCount(), -1L, terms.getSumTotalTermFreq(), isSearchable(),
-                isAggregatable(), minPt, maxPt);
+                isAggregatable(), prefixCodedToGeoPoint(terms.getMin(), numericEncoded),
+                prefixCodedToGeoPoint(terms.getMax(), numericEncoded));
         }
     }
 
@@ -657,4 +660,19 @@ public FieldMapper updateFieldType(Map<String, MappedFieldType> fullNameToFieldT
         updated.lonMapper = lonUpdated;
         return updated;
     }
+
+    private static GeoPoint prefixCodedToGeoPoint(BytesRef val, boolean isGeoCoded) {
+        final long encoded = isGeoCoded ? prefixCodedToGeoCoded(val) : LegacyNumericUtils.prefixCodedToLong(val);
+        return new GeoPoint(MortonEncoder.decodeLatitude(encoded), MortonEncoder.decodeLongitude(encoded));
+    }
+
+    private static long prefixCodedToGeoCoded(BytesRef val) {
+        long result = fromBytes((byte)0, (byte)0, (byte)0, (byte)0, val.bytes[val.offset + 0], val.bytes[val.offset + 1],
+            val.bytes[val.offset + 2], val.bytes[val.offset + 3]);
+        return result << 32;
+    }
+
+    private static long fromBytes(byte b1, byte b2, byte b3, byte b4, byte b5, byte b6, byte b7, byte b8) {
+        return ((long)b1 & 255L) << 56 | ((long)b2 & 255L) << 48 | ((long)b3 & 255L) << 40 | ((long)b4 & 255L) << 32 | ((long)b5 & 255L) << 24 | ((long)b6 & 255L) << 16 | ((long)b7 & 255L) << 8 | (long)b8 & 255L;
+    }
 }

core/src/main/java/org/elasticsearch/index/mapper/GeoPointFieldMapper.java

@@ -79,6 +79,7 @@ public GeoPointFieldMapper build(BuilderContext context, String simpleName, Mapp
             if (context.indexCreatedVersion().before(Version.V_2_3_0)) {
                 fieldType.setNumericPrecisionStep(GeoPointField.PRECISION_STEP);
                 fieldType.setNumericType(FieldType.LegacyNumericType.LONG);
+                ((LegacyGeoPointFieldType)fieldType).numericEncoded = true;
             }
             setupFieldType(context);
             return new GeoPointFieldMapper(simpleName, fieldType, defaultFieldType, indexSettings, latMapper, lonMapper,

core/src/test/java/org/elasticsearch/fieldstats/FieldStatsTests.java

@@ -19,24 +19,30 @@
 
 package org.elasticsearch.fieldstats;
 
+import org.apache.lucene.geo.GeoTestUtil;
 import org.apache.lucene.util.BytesRef;
 import org.elasticsearch.Version;
 import org.elasticsearch.action.fieldstats.FieldStats;
 import org.elasticsearch.action.fieldstats.FieldStatsResponse;
 import org.elasticsearch.action.fieldstats.IndexConstraint;
+import org.elasticsearch.cluster.metadata.IndexMetaData;
 import org.elasticsearch.common.io.stream.BytesStreamOutput;
 import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.joda.Joda;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.index.mapper.DateFieldMapper;
+import org.elasticsearch.plugins.Plugin;
 import org.elasticsearch.test.ESSingleNodeTestCase;
+import org.elasticsearch.test.InternalSettingsPlugin;
+import org.elasticsearch.test.VersionUtils;
 import org.joda.time.DateTime;
 import org.joda.time.DateTimeZone;
 
 import java.io.IOException;
 import java.net.InetAddress;
 import java.net.UnknownHostException;
 import java.util.ArrayList;
+import java.util.Collection;
 import java.util.Date;
 import java.util.List;
 import java.util.Locale;
@@ -52,6 +58,11 @@
 /**
  */
 public class FieldStatsTests extends ESSingleNodeTestCase {
+    @Override
+    protected Collection<Class<? extends Plugin>> getPlugins() {
+        return pluginList(InternalSettingsPlugin.class);
+    }
+
     public void testByte() {
         testNumberRange("field1", "byte", 12, 18);
         testNumberRange("field1", "byte", -5, 5);
@@ -676,6 +687,31 @@ public static FieldStats randomFieldStats(boolean withNullMinMax) throws Unknown
         }
     }
 
+    public void testGeopoint() {
+        Version version = VersionUtils.randomVersionBetween(random(), Version.V_2_0_0, Version.CURRENT);
+        Settings settings = Settings.builder().put(IndexMetaData.SETTING_VERSION_CREATED, version).build();
+        createIndex("test", settings, "test",
+            "field_index", makeType("geo_point", true, false, false));
+        version = Version.CURRENT;
+        settings = Settings.builder().put(IndexMetaData.SETTING_VERSION_CREATED, version).build();
+        createIndex("test5x", settings, "test",
+            "field_index", makeType("geo_point", true, false, false));
+        int numDocs = random().nextInt(20);
+        for (int i = 0; i <= numDocs; ++i) {
+            double lat = GeoTestUtil.nextLatitude();
+            double lon = GeoTestUtil.nextLongitude();
+            client().prepareIndex(random().nextBoolean() ? "test" : "test5x", "test").setSource("field_index", lat + "," + lon).get();
+        }
+
+        client().admin().indices().prepareRefresh().get();
+        FieldStatsResponse result = client().prepareFieldStats().setFields("field_index").get();
+        FieldStats stats = result.getAllFieldStats().get("field_index");
+        assertEquals(stats.getDisplayType(), "geo_point");
+        // min/max random testing is not straightforward; there are 3 different encodings since V_2_0
+        // e.g., before V2_3 used legacy numeric encoding which is wildly different from V_2_3 which is morton encoded
+        // which is wildly different from V_5_0 which is point encoded. Skipping min/max in favor of testing
+    }
+
     private void assertSerialization(FieldStats stats, Version version) throws IOException {
         BytesStreamOutput output = new BytesStreamOutput();
         output.setVersion(version);