浏览代码

Fix string terms get key as number to see integers

Currently this method parses the string as a double. This means that it
might lose accuracy if the value is a long that is greater than
2^52. This commit changes this method to try to detect whether the
string represents a long first.
Antonio Matarrese 7 年之前
父节点
当前提交
f61591c6ec

+ 10 - 2
server/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/StringTerms.java

@@ -64,10 +64,18 @@ public class StringTerms extends InternalMappedTerms<StringTerms, StringTerms.Bu
             return getKeyAsString();
         }
 
+        // this method is needed for scripted numeric aggs
         @Override
         public Number getKeyAsNumber() {
-            // this method is needed for scripted numeric aggs
-            return Double.parseDouble(termBytes.utf8ToString());
+            /*
+             * If the term is a long greater than 2^52 then parsing as a double would lose accuracy. Therefore, we first parse as a long and
+             * if this fails then we attempt to parse the term as a double.
+             */
+            try {
+                return Long.parseLong(termBytes.utf8ToString());
+            } catch (final NumberFormatException ignored) {
+                return Double.parseDouble(termBytes.utf8ToString());
+            }
         }
 
         @Override

+ 2 - 0
server/src/test/java/org/elasticsearch/search/aggregations/bucket/DoubleTermsIT.java

@@ -70,6 +70,7 @@ import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcke
 import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertSearchResponse;
 import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.equalTo;
+import static org.hamcrest.Matchers.instanceOf;
 import static org.hamcrest.core.IsNull.notNullValue;
 
 @ESIntegTestCase.SuiteScopeTestCase
@@ -313,6 +314,7 @@ public class DoubleTermsIT extends AbstractTermsTestCase {
             assertThat(terms.getName(), equalTo("terms"));
             for (Bucket bucket : terms.getBuckets()) {
                 assertTrue(foundTerms.add(bucket.getKeyAsNumber()));
+                assertThat(bucket.getKeyAsNumber(), instanceOf(Double.class));
             }
         }
         assertEquals(expectedCardinality, foundTerms.size());

+ 2 - 0
server/src/test/java/org/elasticsearch/search/aggregations/bucket/LongTermsIT.java

@@ -67,6 +67,7 @@ import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcke
 import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertSearchResponse;
 import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.equalTo;
+import static org.hamcrest.Matchers.instanceOf;
 import static org.hamcrest.core.IsNull.notNullValue;
 
 @ESIntegTestCase.SuiteScopeTestCase
@@ -431,6 +432,7 @@ public class LongTermsIT extends AbstractTermsTestCase {
             assertThat(bucket, notNullValue());
             assertThat(key(bucket), equalTo("" + i));
             assertThat(bucket.getKeyAsNumber().intValue(), equalTo(i));
+            assertThat(bucket.getKeyAsNumber(), instanceOf(Long.class));
             assertThat(bucket.getDocCount(), equalTo(1L));
         }
     }