Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support geopoint type #309

Merged
merged 30 commits into from
Jul 27, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
ccca0b9
Added changes from POC PR
GumpacG Jul 14, 2023
a3859b6
Added geopoint parser for value factory
GumpacG Jul 17, 2023
eba9fb8
Fixed old tests
GumpacG Jul 18, 2023
1fd9893
Fixed all old tests
GumpacG Jul 18, 2023
912401f
Removed irrelevant changes
GumpacG Jul 18, 2023
0a0be6a
Removed irrelevant changes
GumpacG Jul 18, 2023
abe8924
Added integration tests
GumpacG Jul 19, 2023
66404a8
Fixed not throwing exception for geojson
GumpacG Jul 19, 2023
9d7c5a8
Made GeoPointValue an ExprTupleValue to enable access to lat and lon
GumpacG Jul 21, 2023
31ff2dc
Fixed checkstyle
GumpacG Jul 21, 2023
e714aa5
Added unit tests and removed unnecessary code
GumpacG Jul 21, 2023
a24b7c3
reverted irrelevant changes
GumpacG Jul 21, 2023
2d91d3c
Added to docs
GumpacG Jul 21, 2023
4523804
Fixed doc typo
GumpacG Jul 21, 2023
7688155
Test doctests
GumpacG Jul 24, 2023
2cf022f
Test doctests
GumpacG Jul 24, 2023
cbc837c
Remove geopoint data from doctests
GumpacG Jul 24, 2023
821c44a
Fixed doctest failure
GumpacG Jul 24, 2023
7d0658d
Fixed doctest failure
GumpacG Jul 24, 2023
72d8beb
Fix doctest clean up.
Yury-Fridlyand Jul 25, 2023
2bbda8f
Minimized doc example
GumpacG Jul 25, 2023
dcc1133
Fixed doctest
GumpacG Jul 26, 2023
5ccc14f
Fixed doctest
GumpacG Jul 26, 2023
1e07886
Fixed doctest
GumpacG Jul 26, 2023
3192f52
Fixed doctest
GumpacG Jul 26, 2023
b474b30
Fixed reordering of results
GumpacG Jul 26, 2023
364e63a
Added more rows to doctests and removed IOException:
GumpacG Jul 26, 2023
2f5f759
Fixed doctest
GumpacG Jul 26, 2023
6ea102d
Fixed doctest
GumpacG Jul 26, 2023
cc54f39
Addressed PR comments
GumpacG Jul 27, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion docs/user/dql/metadata.rst
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ Example 1: Show All Indices Information
SQL query::

os> SHOW TABLES LIKE '%'
fetched rows / total rows = 9/9
fetched rows / total rows = 10/10
+----------------+---------------+-----------------+--------------+-----------+------------+--------------+-------------+-----------------------------+------------------+
| TABLE_CAT | TABLE_SCHEM | TABLE_NAME | TABLE_TYPE | REMARKS | TYPE_CAT | TYPE_SCHEM | TYPE_NAME | SELF_REFERENCING_COL_NAME | REF_GENERATION |
|----------------+---------------+-----------------+--------------+-----------+------------+--------------+-------------+-----------------------------+------------------|
Expand All @@ -44,6 +44,7 @@ SQL query::
| docTestCluster | null | accounts | BASE TABLE | null | null | null | null | null | null |
| docTestCluster | null | apache | BASE TABLE | null | null | null | null | null | null |
| docTestCluster | null | books | BASE TABLE | null | null | null | null | null | null |
| docTestCluster | null | geopoint | BASE TABLE | null | null | null | null | null | null |
| docTestCluster | null | nested | BASE TABLE | null | null | null | null | null | null |
| docTestCluster | null | nyc_taxi | BASE TABLE | null | null | null | null | null | null |
| docTestCluster | null | people | BASE TABLE | null | null | null | null | null | null |
Expand Down
17 changes: 17 additions & 0 deletions docs/user/general/datatypes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,8 @@ The table below list the mapping between OpenSearch Data Type, OpenSearch SQL Da
+-----------------+---------------------+-----------+
| nested | array | STRUCT |
+-----------------+---------------------+-----------+
| geo_point | geo_point | STRUCT |
+-----------------+---------------------+-----------+

Notes:
* Not all the OpenSearch SQL Type has correspond OpenSearch Type. e.g. data and time. To use function which required such data type, user should explicitly convert the data type.
Expand Down Expand Up @@ -456,3 +458,18 @@ A boolean can be represented by constant value ``TRUE`` or ``FALSE``. Besides, c
|--------+---------+---------------------------+----------------------------|
| True | False | True | False |
+--------+---------+---------------------------+----------------------------+

Geopoint Data Types
==================

A geopoint has a latitude and a longitude property. Although OpenSearch `supports multiple formats <https://opensearch.org/docs/2.3/opensearch/supported-field-types/geo-point/>`_, the SQL plugin currently only supports the format :code:`{"lat": number, "lon": number}`. The geopoint object can be queried or lat and lon can be specified using dot notation. For example, ::

os> SELECT geo_point_object, geo_point_object.lat, geo_point_object.lon FROM geopoint;
fetched rows / total rows = 3/3
+----------------------------------+------------------------+------------------------+
| geo_point_object | geo_point_object.lat | geo_point_object.lon |
|----------------------------------+------------------------+------------------------|
| {'lat': 40.71, 'lon': 74.0} | 40.71 | 74.0 |
| {'lat': -33.852, 'lon': 151.216} | -33.852 | 151.216 |
| null | null | null |
+----------------------------------+------------------------+------------------------+
3 changes: 3 additions & 0 deletions doctest/test_data/geopoint.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{"geo_point_object": {"lat": 40.71, "lon": 74.00}}
{"geo_point_object": {"lat": -33.852, "lon": 151.216}}
{"geo_point_object": null}
4 changes: 3 additions & 1 deletion doctest/test_docs.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
APACHE = "apache"
WILDCARD = "wildcard"
NESTED = "nested"
GEOPOINT = "geopoint"
DATASOURCES = ".ql-datasources"


Expand Down Expand Up @@ -97,6 +98,7 @@ def set_up_test_indices(test):
load_file("apache.json", index_name=APACHE)
load_file("wildcard.json", index_name=WILDCARD)
load_file("nested_objects.json", index_name=NESTED)
load_file("geopoint.json", index_name=GEOPOINT)
load_file("datasources.json", index_name=DATASOURCES)


Expand Down Expand Up @@ -126,7 +128,7 @@ def set_up(test):

def tear_down(test):
# drop leftover tables after each test
test_data_client.indices.delete(index=[ACCOUNTS, EMPLOYEES, PEOPLE, ACCOUNT2, NYC_TAXI, BOOKS, APACHE, WILDCARD, NESTED], ignore_unavailable=True)
test_data_client.indices.delete(index=[ACCOUNTS, EMPLOYEES, PEOPLE, ACCOUNT2, NYC_TAXI, BOOKS, APACHE, WILDCARD, NESTED, GEOPOINT], ignore_unavailable=True)


docsuite = partial(doctest.DocFileSuite,
Expand Down
9 changes: 9 additions & 0 deletions doctest/test_mapping/geopoint.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
{
"mappings": {
"properties": {
"geo_point_object": {
"type": "geo_point"
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -681,7 +681,11 @@ public enum Index {
NESTED_WITH_NULLS(TestsConstants.TEST_INDEX_NESTED_WITH_NULLS,
"multi_nested",
getNestedTypeIndexMapping(),
"src/test/resources/nested_with_nulls.json");
"src/test/resources/nested_with_nulls.json"),
GEOPOINT(TestsConstants.TEST_INDEX_GEOPOINT,
"geopoint",
getMappingFile("geo_point_mapping.json"),
"src/test/resources/geo_point.json");

private final String name;
private final String type;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ public class TestsConstants {
public final static String TEST_INDEX_WILDCARD = TEST_INDEX + "_wildcard";
public final static String TEST_INDEX_MULTI_NESTED_TYPE = TEST_INDEX + "_multi_nested";
public final static String TEST_INDEX_NESTED_WITH_NULLS = TEST_INDEX + "_nested_with_nulls";
public final static String TEST_INDEX_GEOPOINT = TEST_INDEX + "_geopoint";
public final static String DATASOURCES = ".ql-datasources";

public final static String DATE_FORMAT = "yyyy-MM-dd'T'HH:mm:ss.SSS'Z'";
Expand Down
120 changes: 120 additions & 0 deletions integ-test/src/test/java/org/opensearch/sql/sql/GeoPointIT.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,120 @@
/*
* Copyright OpenSearch Contributors
* SPDX-License-Identifier: Apache-2.0
*/

package org.opensearch.sql.sql;

import org.json.JSONObject;
import org.junit.Test;
import org.opensearch.sql.legacy.SQLIntegTestCase;

import java.util.Map;

import static org.opensearch.sql.legacy.TestsConstants.TEST_INDEX_GEOPOINT;
import static org.opensearch.sql.util.MatcherUtils.rows;
import static org.opensearch.sql.util.MatcherUtils.verifyDataRows;

public class GeoPointIT extends SQLIntegTestCase {
@Override
protected void init() throws Exception {
loadIndex(Index.GEOPOINT);
}

@Test
public void test_geo_point() {
String query = "SELECT geo_point_object FROM " + TEST_INDEX_GEOPOINT;
JSONObject result = executeJdbcRequest(query);
verifyDataRows(result,
rows(new JSONObject(Map.of(
"lat", 40.71,
"lon", 74))),
rows(new JSONObject(Map.of(
"lat", -33.85253637358241,
"lon", 151.21652352950258))),
rows(JSONObject.NULL)
);
}

@Test
public void test_geo_point_unsupported_format() {
String query = "SELECT geo_point_geohash FROM " + TEST_INDEX_GEOPOINT;
Exception exception = assertThrows(RuntimeException.class,
() -> executeJdbcRequest(query));

assertTrue(exception.getMessage().contains(
" \"error\": {\n" +
" \"reason\": \"There was internal problem at backend\",\n" +
" \"details\": \"geo point must be in format of {\\\"lat\\\": number, \\\"lon\\\": number}\",\n" +
" \"type\": \"IllegalStateException\"\n" +
" }"
));
}

@Test
public void test_geo_point_in_objects() {
String query = "SELECT object.geo_point_object FROM " + TEST_INDEX_GEOPOINT;
JSONObject result = executeJdbcRequest(query);
verifyDataRows(result,
rows(
(new JSONObject(Map.of(
"lat", 40.71,
"lon", 74)))),
rows(new JSONObject(Map.of(
"lat", -33.85253637358241,
"lon", 151.21652352950258))),
rows(JSONObject.NULL)
);
}

@Test
public void test_geo_point_lat_in_objects() {
String query = "SELECT object.geo_point_object.lat FROM " + TEST_INDEX_GEOPOINT;
JSONObject result = executeJdbcRequest(query);
verifyDataRows(result,
rows(40.71),
rows( -33.85253637358241),
rows(JSONObject.NULL)
);
}

@Test
public void test_geo_point_lat_and_lon() {
String query = "SELECT geo_point_object.lat, geo_point_object.lon FROM " + TEST_INDEX_GEOPOINT;
JSONObject result = executeJdbcRequest(query);
verifyDataRows(result,
rows(40.71, 74),
rows(-33.85253637358241, 151.21652352950258),
rows(JSONObject.NULL, JSONObject.NULL)
);
}

@Test
public void test_geo_point_object_with_lat_and_lon() {
String query = "SELECT geo_point_object, geo_point_object.lat," +
" geo_point_object.lon FROM " + TEST_INDEX_GEOPOINT;
JSONObject result = executeJdbcRequest(query);
verifyDataRows(result,
rows(new JSONObject(Map.of(
"lat", 40.71,
"lon", 74)),
40.71, 74),
rows(new JSONObject(Map.of(
"lat", -33.85253637358241,
"lon", 151.21652352950258)),
-33.85253637358241, 151.21652352950258),
rows(JSONObject.NULL, JSONObject.NULL, JSONObject.NULL)
);
}

@Test
public void test_geo_point_lat_in_functions() {
String query = "SELECT ABS(geo_point_object.lat) FROM " + TEST_INDEX_GEOPOINT;
JSONObject result = executeJdbcRequest(query);
verifyDataRows(result,
rows(40.71),
rows(33.85253637358241),
rows(JSONObject.NULL)
);
}
}
6 changes: 6 additions & 0 deletions integ-test/src/test/resources/geo_point.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{"index":{"_id":"1"}}
{"geo_point_object": {"lat": 40.71, "lon": 74.00}, "object": {"geo_point_object": {"lat": 40.71, "lon": 74.00}}, "geo_point_string": "40.71, 74.00", "geo_point_geohash": "txhxegj0uyp3", "geo_point_array": [74.00, 40.71], "geo_point_string_point": "POINT (74.00 40.71)", "geo_point_geojson": {"type": "Point", "coordinates": [74.00, 40.71]}}
{"index":{"_id":"2"}}
{"geo_point_object": {"lat": -33.85253637358241, "lon": 151.21652352950258}, "object": {"geo_point_object": {"lat": -33.85253637358241, "lon": 151.21652352950258}},"geo_point_string": "-33.85356510743158, 151.22222172610114", "geo_point_geohash": "txhxegj0uyp3", "geo_point_array": [74.00, 40.71], "geo_point_string_point": "POINT (74.00 40.71)", "geo_point_geojson": {"type": "Point", "coordinates": [74.00, 40.71]}}
{"index":{"_id":"3"}}
{"geo_point_object": null, "object": {"geo_point_object": null},"geo_point_string": null, "geo_point_geohash": null, "geo_point_array": null, "geo_point_string_point": null, "geo_point_geojson": null}
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
{
"mappings": {
"properties": {
"geo_point_object": {
"type": "geo_point"
},
"object": {
"type": "object",
"properties": {
"geo_point_object": {
"type": "geo_point"
}
}
},
"geo_point_string": {
"type": "geo_point"
},
"geo_point_geohash": {
"type": "geo_point"
},
"geo_point_array": {
"type": "geo_point"
},
"geo_point_string_point": {
"type": "geo_point"
},
"geo_point_geojson": {
"type": "geo_point"
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ public enum MappingType {
ScaledFloat("scaled_float", ExprCoreType.DOUBLE),
Double("double", ExprCoreType.DOUBLE),
Boolean("boolean", ExprCoreType.BOOLEAN);
// TODO: ranges, geo shape, point, shape
// TODO: ranges, geo shape, shape

private final String name;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,11 @@

import static org.opensearch.sql.data.type.ExprCoreType.UNKNOWN;

import java.util.HashMap;
import java.util.Map;
import lombok.EqualsAndHashCode;
import lombok.Getter;
import org.opensearch.sql.data.type.ExprCoreType;

/**
* The type of a geo_point value. See
Expand All @@ -22,8 +26,15 @@ public class OpenSearchGeoPointType extends OpenSearchDataType {
private OpenSearchGeoPointType() {
super(MappingType.GeoPoint);
exprCoreType = UNKNOWN;
this.properties = new HashMap<>();
this.properties.put("lat", new OpenSearchDataType(ExprCoreType.DOUBLE));
this.properties.put("lon", new OpenSearchDataType(ExprCoreType.DOUBLE));
}

@Getter
@EqualsAndHashCode.Exclude
Map<String, OpenSearchDataType> properties;

public static OpenSearchGeoPointType of() {
return OpenSearchGeoPointType.instance;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -126,28 +126,35 @@ 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;
Double lat = null;
Double lon = null;

if (value.has("lat")) {
try {
lat = extractDoubleValue(value.get("lat"));
} catch (Exception exception) {
throw new IllegalStateException(
"latitude must be number value, but got value: " + value.get(
"lat"));
}
}

if (value.has("lon")) {
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\": "
}

if (lat == null && lon == null) {
matthewryanwells marked this conversation as resolved.
Show resolved Hide resolved
throw new IllegalStateException("geo point must be in format of {\"lat\": number, \"lon\": "
+ "number}");
}

return Pair.of(lat, lon);
}

/**
Expand Down
Loading