diff --git a/elasticsearch/src/main/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/data/utils/ElasticsearchJsonContent.java b/elasticsearch/src/main/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/data/utils/ElasticsearchJsonContent.java index e5b0ae64cb..1e0f5d467d 100644 --- a/elasticsearch/src/main/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/data/utils/ElasticsearchJsonContent.java +++ b/elasticsearch/src/main/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/data/utils/ElasticsearchJsonContent.java @@ -110,7 +110,29 @@ public Object objectValue() { @Override public Pair geoValue() { - return Pair.of(value().get("lat").doubleValue(), value().get("lon").doubleValue()); + final JsonNode value = value(); + if (value.has("lat") && value.has("lon")) { + Double lat = 0d; + Double lon = 0d; + try { + lat = extractDoubleValue(value.get("lat")); + } catch (Exception exception) { + throw new IllegalStateException( + "latitude must be number value, but got value: " + value.get( + "lat")); + } + try { + lon = extractDoubleValue(value.get("lon")); + } catch (Exception exception) { + throw new IllegalStateException( + "longitude must be number value, but got value: " + value.get( + "lon")); + } + return Pair.of(lat, lon); + } else { + throw new IllegalStateException("geo point must in format of {\"lat\": number, \"lon\": " + + "number}"); + } } /** @@ -120,4 +142,18 @@ public Pair geoValue() { private JsonNode value() { return value.isArray() ? value.get(0) : value; } + + /** + * Get doubleValue from JsonNode if possible. + */ + private Double extractDoubleValue(JsonNode node) { + if (node.isTextual()) { + return Double.valueOf(node.textValue()); + } + if (node.isNumber()) { + return node.doubleValue(); + } else { + throw new IllegalStateException("node must be a number"); + } + } } diff --git a/elasticsearch/src/test/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/data/value/ElasticsearchExprValueFactoryTest.java b/elasticsearch/src/test/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/data/value/ElasticsearchExprValueFactoryTest.java index 9fb03562bb..badf6afb57 100644 --- a/elasticsearch/src/test/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/data/value/ElasticsearchExprValueFactoryTest.java +++ b/elasticsearch/src/test/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/data/value/ElasticsearchExprValueFactoryTest.java @@ -283,10 +283,43 @@ public void constructIP() { public void constructGeoPoint() { assertEquals(new ElasticsearchExprGeoPointValue(42.60355556, -97.25263889), tupleValue("{\"geoV\":{\"lat\":42.60355556,\"lon\":-97.25263889}}").get("geoV")); + assertEquals(new ElasticsearchExprGeoPointValue(42.60355556, -97.25263889), + tupleValue("{\"geoV\":{\"lat\":\"42.60355556\",\"lon\":\"-97.25263889\"}}").get("geoV")); assertEquals(new ElasticsearchExprGeoPointValue(42.60355556, -97.25263889), constructFromObject("geoV", "42.60355556,-97.25263889")); } + @Test + public void constructGeoPointFromUnsupportedFormatShouldThrowException() { + IllegalStateException exception = + assertThrows(IllegalStateException.class, + () -> tupleValue("{\"geoV\":[42.60355556,-97.25263889]}").get("geoV")); + assertEquals("geo point must in format of {\"lat\": number, \"lon\": number}", + exception.getMessage()); + + exception = + assertThrows(IllegalStateException.class, + () -> tupleValue("{\"geoV\":{\"lon\":-97.25263889}}").get("geoV")); + assertEquals("geo point must in format of {\"lat\": number, \"lon\": number}", + exception.getMessage()); + + exception = + assertThrows(IllegalStateException.class, + () -> tupleValue("{\"geoV\":{\"lat\":-97.25263889}}").get("geoV")); + assertEquals("geo point must in format of {\"lat\": number, \"lon\": number}", + exception.getMessage()); + + exception = + assertThrows(IllegalStateException.class, + () -> tupleValue("{\"geoV\":{\"lat\":true,\"lon\":-97.25263889}}").get("geoV")); + assertEquals("latitude must be number value, but got value: true", exception.getMessage()); + + exception = + assertThrows(IllegalStateException.class, + () -> tupleValue("{\"geoV\":{\"lat\":42.60355556,\"lon\":false}}").get("geoV")); + assertEquals("longitude must be number value, but got value: false", exception.getMessage()); + } + @Test public void constructBinary() { assertEquals(new ElasticsearchExprBinaryValue("U29tZSBiaW5hcnkgYmxvYg=="), @@ -298,7 +331,7 @@ public void constructBinary() { * https://www.elastic.co/guide/en/elasticsearch/reference/current/array.html. */ @Test - public void constructFromElasticsearcyArrayReturnFirstElement() { + public void constructFromElasticsearchArrayReturnFirstElement() { assertEquals(integerValue(1), tupleValue("{\"intV\":[1, 2, 3]}").get("intV")); assertEquals(new ExprTupleValue( new LinkedHashMap() {