Support common format geo point

integ-test/src/test/java/org/opensearch/sql/legacy/SQLIntegTestCase.java

@@ -20,6 +20,7 @@
 import static org.opensearch.sql.legacy.TestUtils.getDogs3IndexMapping;
 import static org.opensearch.sql.legacy.TestUtils.getEmployeeNestedTypeIndexMapping;
 import static org.opensearch.sql.legacy.TestUtils.getGameOfThronesIndexMapping;
+import static org.opensearch.sql.legacy.TestUtils.getGeopointIndexMapping;
 import static org.opensearch.sql.legacy.TestUtils.getJoinTypeIndexMapping;
 import static org.opensearch.sql.legacy.TestUtils.getLocationIndexMapping;
 import static org.opensearch.sql.legacy.TestUtils.getMappingFile;
@@ -724,7 +725,12 @@ public enum Index {
         TestsConstants.TEST_INDEX_NESTED_WITH_NULLS,
         "multi_nested",
         getNestedTypeIndexMapping(),
-        "src/test/resources/nested_with_nulls.json");
+        "src/test/resources/nested_with_nulls.json"),
+    GEOPOINTS(
+        TestsConstants.TEST_INDEX_GEOPOINT,
+        "dates",
+        getGeopointIndexMapping(),
+        "src/test/resources/geopoints.json");
 
     private final String name;
     private final String type;

integ-test/src/test/java/org/opensearch/sql/legacy/TestUtils.java

@@ -245,6 +245,11 @@ public static String getDataTypeNonnumericIndexMapping() {
     return getMappingFile(mappingFile);
   }
 
+  public static String getGeopointIndexMapping() {
+    String mappingFile = "geopoint_index_mapping.json";
+    return getMappingFile(mappingFile);
+  }
+
   public static void loadBulk(Client client, String jsonPath, String defaultIndex)
       throws Exception {
     System.out.println(String.format("Loading file %s into opensearch cluster", jsonPath));

integ-test/src/test/java/org/opensearch/sql/legacy/TestsConstants.java

@@ -57,6 +57,7 @@ public class TestsConstants {
   public static final String TEST_INDEX_WILDCARD = TEST_INDEX + "_wildcard";
   public static final String TEST_INDEX_MULTI_NESTED_TYPE = TEST_INDEX + "_multi_nested";
   public static final String TEST_INDEX_NESTED_WITH_NULLS = TEST_INDEX + "_nested_with_nulls";
+  public static final String TEST_INDEX_GEOPOINT = TEST_INDEX + "_geopoint";
   public static final String DATASOURCES = ".ql-datasources";
 
   public static final String DATE_FORMAT = "yyyy-MM-dd'T'HH:mm:ss.SSS'Z'";

integ-test/src/test/java/org/opensearch/sql/sql/GeopointFormatsIT.java

@@ -0,0 +1,60 @@
+/*
+ * Copyright OpenSearch Contributors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package org.opensearch.sql.sql;
+
+import static org.opensearch.sql.util.MatcherUtils.rows;
+import static org.opensearch.sql.util.MatcherUtils.schema;
+import static org.opensearch.sql.util.MatcherUtils.verifyDataRows;
+import static org.opensearch.sql.util.MatcherUtils.verifySchema;
+
+import java.io.IOException;
+import java.util.Map;
+import org.apache.commons.lang3.tuple.Pair;
+import org.json.JSONArray;
+import org.json.JSONObject;
+import org.junit.jupiter.api.Test;
+import org.opensearch.sql.legacy.SQLIntegTestCase;
+
+public class GeopointFormatsIT extends SQLIntegTestCase {
+
+  @Override
+  public void init() throws Exception {
+    loadIndex(Index.GEOPOINTS);
+  }
+
+  @Test
+  public void testReadingGeopoints() throws IOException {
+    String query = String.format("SELECT point FROM %s LIMIT 5", Index.GEOPOINTS.getName());
+    JSONObject result = executeJdbcRequest(query);
+    verifySchema(result, schema("point", null, "geo_point"));
+    verifyDataRows(
+        result,
+        rows(Map.of("lon", 74, "lat", 40.71)),
+        rows(Map.of("lon", 74, "lat", 40.71)),
+        rows(Map.of("lon", 74, "lat", 40.71)),
+        rows(Map.of("lon", 74, "lat", 40.71)),
+        rows(Map.of("lon", 74, "lat", 40.71)));
+  }
+
+  private static final double TOLERANCE = 1E-5;
+
+  public void testReadingGeoHash() throws IOException {
+    String query = String.format("SELECT point FROM %s WHERE _id='6'", Index.GEOPOINTS.getName());
+    JSONObject result = executeJdbcRequest(query);
+    verifySchema(result, schema("point", null, "geo_point"));
+    Pair<Double, Double> point = getGeoValue(result);
+    assertEquals(40.71, point.getLeft(), TOLERANCE);
+    assertEquals(74, point.getRight(), TOLERANCE);
+  }
+
+  private Pair<Double, Double> getGeoValue(JSONObject result) {
+    JSONObject geoRaw =
+        (JSONObject) ((JSONArray) ((JSONArray) result.get("datarows")).get(0)).get(0);
+    double lat = geoRaw.getDouble("lat");
+    double lon = geoRaw.getDouble("lon");
+    return Pair.of(lat, lon);
+  }
+}

integ-test/src/test/resources/geopoints.json

@@ -0,0 +1,12 @@
+{"index": {"_id": "1"}}
+{"point": {"lat": 40.71, "lon": 74.00}}
+{"index": {"_id": "2"}}
+{"point": "40.71,74.00"}
+{"index": {"_id": "3"}}
+{"point": [74.00, 40.71]}
+{"index": {"_id": "4"}}
+{"point": "POINT (74.00 40.71)"}
+{"index": {"_id": "5"}}
+{"point": {"type": "Point", "coordinates": [74.00, 40.71]}}
+{"index": {"_id": "6"}}
+{"point": "txhxegj0uyp3"}

integ-test/src/test/resources/indexDefinitions/geopoint_index_mapping.json

@@ -0,0 +1,9 @@
+{
+  "mappings": {
+    "properties": {
+      "point": {
+        "type": "geo_point"
+      }
+    }
+  }
+}

opensearch/src/main/java/org/opensearch/sql/opensearch/data/utils/OpenSearchJsonContent.java

@@ -7,11 +7,19 @@
 
 import com.fasterxml.jackson.databind.JsonNode;
 import com.google.common.collect.Iterators;
+import java.io.IOException;
 import java.util.Iterator;
 import java.util.LinkedHashMap;
 import java.util.Map;
 import lombok.RequiredArgsConstructor;
 import org.apache.commons.lang3.tuple.Pair;
+import org.opensearch.OpenSearchParseException;
+import org.opensearch.common.geo.GeoPoint;
+import org.opensearch.common.geo.GeoUtils;
+import org.opensearch.common.xcontent.json.JsonXContentParser;
+import org.opensearch.core.xcontent.DeprecationHandler;
+import org.opensearch.core.xcontent.NamedXContentRegistry;
+import org.opensearch.core.xcontent.XContentParser;
 
 /** The Implementation of Content to represent {@link JsonNode}. */
 @RequiredArgsConstructor
@@ -122,42 +130,22 @@ public Object objectValue() {
   @Override
   public Pair<Double, Double> geoValue() {
     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}");
+    try (XContentParser parser =
+        new JsonXContentParser(
+            NamedXContentRegistry.EMPTY,
+            DeprecationHandler.IGNORE_DEPRECATIONS,
+            value.traverse())) {
+      parser.nextToken();
+      GeoPoint point = new GeoPoint();
+      GeoUtils.parseGeoPoint(parser, point, true);
+      return Pair.of(point.getLat(), point.getLon());
+    } catch (IOException ex) {
+      throw new OpenSearchParseException("error parsing geo point", ex);
     }
   }
 
   /** Getter for value. If value is array the whole array is returned. */
   private JsonNode value() {
     return 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");
-    }
-  }
 }

opensearch/src/main/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprValueFactory.java

@@ -39,6 +39,7 @@
 import java.util.function.BiFunction;
 import lombok.Getter;
 import lombok.Setter;
+import org.opensearch.OpenSearchParseException;
 import org.opensearch.common.time.DateFormatter;
 import org.opensearch.common.time.DateFormatters;
 import org.opensearch.common.time.FormatNames;
@@ -62,7 +63,6 @@
 import org.opensearch.sql.opensearch.data.type.OpenSearchBinaryType;
 import org.opensearch.sql.opensearch.data.type.OpenSearchDataType;
 import org.opensearch.sql.opensearch.data.type.OpenSearchDateType;
-import org.opensearch.sql.opensearch.data.type.OpenSearchGeoPointType;
 import org.opensearch.sql.opensearch.data.type.OpenSearchIpType;
 import org.opensearch.sql.opensearch.data.utils.Content;
 import org.opensearch.sql.opensearch.data.utils.ObjectContent;
@@ -134,10 +134,6 @@ public void extendTypeMapping(Map<String, OpenSearchDataType> typeMapping) {
           .put(
               OpenSearchDataType.of(OpenSearchDataType.MappingType.Ip),
               (c, dt) -> new OpenSearchExprIpValue(c.stringValue()))
-          .put(
-              OpenSearchDataType.of(OpenSearchDataType.MappingType.GeoPoint),
-              (c, dt) ->
-                  new OpenSearchExprGeoPointValue(c.geoValue().getLeft(), c.geoValue().getRight()))
           .put(
               OpenSearchDataType.of(OpenSearchDataType.MappingType.Binary),
               (c, dt) -> new OpenSearchExprBinaryValue(c.stringValue()))
@@ -193,8 +189,11 @@ private ExprValue parse(
       return ExprNullValue.of();
     }
 
-    ExprType type = fieldType.get();
-    if (type.equals(OpenSearchDataType.of(OpenSearchDataType.MappingType.Nested))
+    final ExprType type = fieldType.get();
+
+    if (type.equals(OpenSearchDataType.of(OpenSearchDataType.MappingType.GeoPoint))) {
+      return parseGeoPoint(content, supportArrays);
+    } else if (type.equals(OpenSearchDataType.of(OpenSearchDataType.MappingType.Nested))
         || content.isArray()) {
       return parseArray(content, field, type, supportArrays);
     } else if (type.equals(OpenSearchDataType.of(OpenSearchDataType.MappingType.Object))
@@ -363,6 +362,49 @@ private ExprValue parseArray(
     return new ExprCollectionValue(result);
   }
 
+  /**
+   * Parse geo point content.
+   *
+   * @param content Content to parse.
+   * @param supportArrays Parsing the whole array or not
+   * @return Geo point value parsed from content.
+   */
+  private ExprValue parseGeoPoint(Content content, boolean supportArrays) {
+    // there is only one point in doc.
+    if (content.isArray() == false) {
+      final var pair = content.geoValue();
+      return new OpenSearchExprGeoPointValue(pair.getLeft(), pair.getRight());
+    }
+
+    var elements = content.array();
+    var first = elements.next();
+    // an array in the [longitude, latitude] format.
+    if (first.isNumber()) {
+      double lon = first.doubleValue();
+      var second = elements.next();
+      if (second.isNumber() == false) {
+        throw new OpenSearchParseException("lat must be a number, got " + second.objectValue());
+      }
+      return new OpenSearchExprGeoPointValue(second.doubleValue(), lon);
+    }
+
+    // there are multi points in doc
+    var pair = first.geoValue();
+    var firstPoint = new OpenSearchExprGeoPointValue(pair.getLeft(), pair.getRight());
+    if (supportArrays) {
+      List<ExprValue> result = new ArrayList<>();
+      result.add(firstPoint);
+      elements.forEachRemaining(
+          e -> {
+            var p = e.geoValue();
+            result.add(new OpenSearchExprGeoPointValue(p.getLeft(), p.getRight()));
+          });
+      return new ExprCollectionValue(result);
+    } else {
+      return firstPoint;
+    }
+  }
+
   /**
    * Parse inner array value. Can be object type and recurse continues.
    *
@@ -376,8 +418,7 @@ private ExprValue parseInnerArrayValue(
       Content content, String prefix, ExprType type, boolean supportArrays) {
     if (type instanceof OpenSearchIpType
         || type instanceof OpenSearchBinaryType
-        || type instanceof OpenSearchDateType
-        || type instanceof OpenSearchGeoPointType) {
+        || type instanceof OpenSearchDateType) {
       return parse(content, prefix, Optional.of(type), supportArrays);
     } else if (content.isString()) {
       return parse(content, prefix, Optional.of(OpenSearchDataType.of(STRING)), supportArrays);

opensearch/src/test/java/org/opensearch/sql/opensearch/data/utils/OpenSearchJsonContentTest.java

@@ -0,0 +1,31 @@
+/*
+ * Copyright OpenSearch Contributors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package org.opensearch.sql.opensearch.data.utils;
+
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+import com.fasterxml.jackson.core.JsonParser;
+import com.fasterxml.jackson.databind.JsonNode;
+import java.io.IOException;
+import org.junit.jupiter.api.Test;
+import org.opensearch.OpenSearchParseException;
+
+public class OpenSearchJsonContentTest {
+  @Test
+  public void testGetValueWithIOException() throws IOException {
+    JsonNode jsonNode = mock(JsonNode.class);
+    JsonParser jsonParser = mock(JsonParser.class);
+    when(jsonNode.traverse()).thenReturn(jsonParser);
+    when(jsonParser.nextToken()).thenThrow(new IOException());
+    OpenSearchJsonContent content = new OpenSearchJsonContent(jsonNode);
+    OpenSearchParseException exception =
+        assertThrows(OpenSearchParseException.class, content::geoValue);
+    assertTrue(exception.getMessage().contains("error parsing geo point"));
+  }
+}