Skip to content

Commit

Permalink
Support common format geo point (#2801) (#2896)
Browse files Browse the repository at this point in the history
---------
Signed-off-by: panguixin <[email protected]>
(cherry picked from commit 82ef68e)
  • Loading branch information
bugmakerrrrrr authored Aug 6, 2024
1 parent 992e2f5 commit 2df3855
Show file tree
Hide file tree
Showing 10 changed files with 256 additions and 68 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -730,7 +731,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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,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));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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'";
Expand Down
Original file line number Diff line number Diff line change
@@ -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);
}
}
12 changes: 12 additions & 0 deletions integ-test/src/test/resources/geopoints.json
Original file line number Diff line number Diff line change
@@ -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"}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
{
"mappings": {
"properties": {
"point": {
"type": "geo_point"
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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");
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,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;
Expand All @@ -63,7 +64,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;
Expand Down Expand Up @@ -137,10 +137,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()))
Expand Down Expand Up @@ -189,8 +185,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))
Expand Down Expand Up @@ -357,6 +356,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.
*
Expand All @@ -370,8 +412,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);
Expand Down
Original file line number Diff line number Diff line change
@@ -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"));
}
}
Loading

0 comments on commit 2df3855

Please sign in to comment.