From ccca0b9f21f60d20243d38f25dce0c7a9a24dbc4 Mon Sep 17 00:00:00 2001 From: Guian Gumpac Date: Fri, 14 Jul 2023 10:57:58 -0700 Subject: [PATCH 01/30] Added changes from POC PR Signed-off-by: Guian Gumpac --- .../sql/data/type/ExprCoreType.java | 7 +++- .../org/opensearch/sql/sql/GeoPointIT.java | 40 +++++++++++++++++++ integ-test/src/test/resources/datatypes.json | 2 +- integ-test/src/test/resources/geo_point.json | 4 ++ .../datatypes_index_mapping.json | 25 ++++++++++++ .../indexDefinitions/geo_point_mapping.json | 24 +++++++++++ .../data/type/OpenSearchDataType.java | 4 +- .../data/type/OpenSearchGeoPointType.java | 15 ++++++- .../value/OpenSearchExprValueFactory.java | 2 +- 9 files changed, 117 insertions(+), 6 deletions(-) create mode 100644 integ-test/src/test/java/org/opensearch/sql/sql/GeoPointIT.java create mode 100644 integ-test/src/test/resources/geo_point.json create mode 100644 integ-test/src/test/resources/indexDefinitions/geo_point_mapping.json diff --git a/core/src/main/java/org/opensearch/sql/data/type/ExprCoreType.java b/core/src/main/java/org/opensearch/sql/data/type/ExprCoreType.java index 815f94a9df..b58f5477c2 100644 --- a/core/src/main/java/org/opensearch/sql/data/type/ExprCoreType.java +++ b/core/src/main/java/org/opensearch/sql/data/type/ExprCoreType.java @@ -68,7 +68,12 @@ public enum ExprCoreType implements ExprType { /** * Array. */ - ARRAY(UNDEFINED); + ARRAY(UNDEFINED), + + /** + * GEO_POINT. + */ + GEO_POINT(UNDEFINED); /** * Parents (wider/compatible types) of current base type. diff --git a/integ-test/src/test/java/org/opensearch/sql/sql/GeoPointIT.java b/integ-test/src/test/java/org/opensearch/sql/sql/GeoPointIT.java new file mode 100644 index 0000000000..e59bfd70bd --- /dev/null +++ b/integ-test/src/test/java/org/opensearch/sql/sql/GeoPointIT.java @@ -0,0 +1,40 @@ +/* + * 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.io.IOException; + +import static org.opensearch.sql.legacy.TestsConstants.TEST_INDEX_WILDCARD; +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.WILDCARD); + } + + @Test + public void test_geo_point_in_select() throws IOException { + String query = "SELECT KeywordBody, KeywordBody LIKE 'test wildcard%' FROM " + TEST_INDEX_WILDCARD; + JSONObject result = executeJdbcRequest(query); + verifyDataRows(result, + rows("test wildcard", true), + rows("test wildcard in the end of the text%", true), + rows("%test wildcard in the beginning of the text", false), + rows("test wildcard in % the middle of the text", true), + rows("test wildcard %% beside each other", true), + rows("test wildcard in the end of the text_", true), + rows("_test wildcard in the beginning of the text", false), + rows("test wildcard in _ the middle of the text", true), + rows("test wildcard __ beside each other", true), + rows("test backslash wildcard \\_", false)); + } +} diff --git a/integ-test/src/test/resources/datatypes.json b/integ-test/src/test/resources/datatypes.json index ea3290ee64..a4490dc60f 100644 --- a/integ-test/src/test/resources/datatypes.json +++ b/integ-test/src/test/resources/datatypes.json @@ -1,2 +1,2 @@ {"index":{"_id":"1"}} -{"boolean_value": true, "keyword_value": "keyword", "text_value": "text", "binary_value": "U29tZSBiaW5hcnkgYmxvYg==", "date_value": "2020-10-13 13:00:00", "ip_value": "127.0.0.1", "object_value": {"first": "Dale", "last": "Dale"}, "nested_value": [{"first" : "John", "last" : "Smith"}, {"first" : "Alice", "last" : "White"}], "geo_point_value": { "lat": 40.71, "lon": 74.00 }} +{"boolean_value": true, "keyword_value": "keyword", "text_value": "text", "binary_value": "U29tZSBiaW5hcnkgYmxvYg==", "date_value": "2020-10-13 13:00:00", "ip_value": "127.0.0.1", "object_value": {"first": "Dale", "last": "Dale"}, "nested_value": [{"first" : "John", "last" : "Smith"}, {"first" : "Alice", "last" : "White"}], "geo_point_value": {"lat": -33.94609833, "lon": 151.177002}, "geo_shape_value": {"type": "polygon", "coordinates": [[[74.0060, 40.7128], [71.0589, 42.3601], [73.7562, 42.6526], [74.0060, 40.7128]]]}, "integer_range_value": {"gte": 0, "lte": 10}, "long_range_value": {"gte": 0.0, "lte": 10.0}, "ip_range_value": {"gte": "10.24.34.0", "lte": "10.24.35.255"}, "ip_cidr_range_value": "10.24.34.0/24", "date_range_value": {"gte": "2019-05-01", "lte": "2019-05-15"}, "unsigned_long_value" : 10223372036854775807, "flat_object_value": {"flat_object_string_value": "hello world", "flat_object_int_value": 10, "flat_object_date_value": "2023-10-12"}} diff --git a/integ-test/src/test/resources/geo_point.json b/integ-test/src/test/resources/geo_point.json new file mode 100644 index 0000000000..a937fbb03d --- /dev/null +++ b/integ-test/src/test/resources/geo_point.json @@ -0,0 +1,4 @@ +{"index":{"_id":"1"}} +{"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": null, "geo_point_string": null, "geo_point_geohash": null, "geo_point_array": null, "geo_point_string_point": null, "geo_point_geojson": null} diff --git a/integ-test/src/test/resources/indexDefinitions/datatypes_index_mapping.json b/integ-test/src/test/resources/indexDefinitions/datatypes_index_mapping.json index 8c1759b369..3fe34c611c 100644 --- a/integ-test/src/test/resources/indexDefinitions/datatypes_index_mapping.json +++ b/integ-test/src/test/resources/indexDefinitions/datatypes_index_mapping.json @@ -35,6 +35,31 @@ }, "geo_point_value": { "type": "geo_point" + }, + "geo_shape_value": { + "type": "geo_shape" + }, + "integer_range_value": { + "type": "integer_range" + }, + "long_range_value": { + "type": "long_range" + }, + "ip_range_value": { + "type": "ip_range" + }, + "ip_cidr_range_value": { + "type": "ip_range" + }, + "date_range_value" : { + "type" : "date_range", + "format" : "strict_year_month_day" + }, + "unsigned_long_value" : { + "type" : "unsigned_long" + }, + "flat_object_value": { + "type": "flat_object" } } } diff --git a/integ-test/src/test/resources/indexDefinitions/geo_point_mapping.json b/integ-test/src/test/resources/indexDefinitions/geo_point_mapping.json new file mode 100644 index 0000000000..3063ae8b93 --- /dev/null +++ b/integ-test/src/test/resources/indexDefinitions/geo_point_mapping.json @@ -0,0 +1,24 @@ +{ + "mappings": { + "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" + } + } + } +} diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/data/type/OpenSearchDataType.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/data/type/OpenSearchDataType.java index 273b980d2a..612e4c0bce 100644 --- a/opensearch/src/main/java/org/opensearch/sql/opensearch/data/type/OpenSearchDataType.java +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/data/type/OpenSearchDataType.java @@ -32,7 +32,7 @@ public enum MappingType { Text("text", ExprCoreType.UNKNOWN), Keyword("keyword", ExprCoreType.STRING), Ip("ip", ExprCoreType.UNKNOWN), - GeoPoint("geo_point", ExprCoreType.UNKNOWN), + GeoPoint("geo_point", ExprCoreType.GEO_POINT), Binary("binary", ExprCoreType.UNKNOWN), Date("date", ExprCoreType.TIMESTAMP), Object("object", ExprCoreType.STRUCT), @@ -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; diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/data/type/OpenSearchGeoPointType.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/data/type/OpenSearchGeoPointType.java index c2428a59a8..f04b79aea0 100644 --- a/opensearch/src/main/java/org/opensearch/sql/opensearch/data/type/OpenSearchGeoPointType.java +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/data/type/OpenSearchGeoPointType.java @@ -6,9 +6,15 @@ package org.opensearch.sql.opensearch.data.type; +import static org.opensearch.sql.data.type.ExprCoreType.GEO_POINT; import static org.opensearch.sql.data.type.ExprCoreType.UNKNOWN; import lombok.EqualsAndHashCode; +import lombok.Getter; +import org.opensearch.sql.data.type.ExprCoreType; + +import java.util.HashMap; +import java.util.Map; /** * The type of a geo_point value. See @@ -21,9 +27,16 @@ public class OpenSearchGeoPointType extends OpenSearchDataType { private OpenSearchGeoPointType() { super(MappingType.GeoPoint); - exprCoreType = UNKNOWN; + exprCoreType = GEO_POINT; + this.properties = new HashMap<>(); + this.properties.put("lat", new OpenSearchDataType(ExprCoreType.DOUBLE)); + this.properties.put("lon", new OpenSearchDataType(ExprCoreType.DOUBLE)); } + @Getter + @EqualsAndHashCode.Exclude + Map properties; + public static OpenSearchGeoPointType of() { return OpenSearchGeoPointType.instance; } diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprValueFactory.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprValueFactory.java index 95815d5c38..2f8739edfb 100644 --- a/opensearch/src/main/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprValueFactory.java +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprValueFactory.java @@ -195,7 +195,7 @@ private ExprValue parse( || content.isArray()) { return parseArray(content, field, type, supportArrays); } else if (type.equals(OpenSearchDataType.of(OpenSearchDataType.MappingType.Object)) - || type == STRUCT) { + || type == STRUCT || type.equals(OpenSearchDataType.of(OpenSearchDataType.MappingType.GeoPoint))) { return parseStruct(content, field, supportArrays); } else { if (typeActionMap.containsKey(type)) { From a3859b600d6b16dfc31bee431eab1c15b42b4c88 Mon Sep 17 00:00:00 2001 From: Guian Gumpac Date: Mon, 17 Jul 2023 09:33:04 -0700 Subject: [PATCH 02/30] Added geopoint parser for value factory Signed-off-by: Guian Gumpac --- .../opensearch/sql/analysis/AnalyzerTest.java | 2 +- integ-test/src/test/resources/geo_point.json | 2 +- .../value/OpenSearchExprValueFactory.java | 27 ++++++++++++++++++- 3 files changed, 28 insertions(+), 3 deletions(-) diff --git a/core/src/test/java/org/opensearch/sql/analysis/AnalyzerTest.java b/core/src/test/java/org/opensearch/sql/analysis/AnalyzerTest.java index 100cfd67af..d75fd3c039 100644 --- a/core/src/test/java/org/opensearch/sql/analysis/AnalyzerTest.java +++ b/core/src/test/java/org/opensearch/sql/analysis/AnalyzerTest.java @@ -165,7 +165,7 @@ public void filter_relation_with_invalid_qualifiedName_ExpressionEvaluationExcep "= function expected {[BYTE,BYTE],[SHORT,SHORT],[INTEGER,INTEGER],[LONG,LONG]," + "[FLOAT,FLOAT],[DOUBLE,DOUBLE],[STRING,STRING],[BOOLEAN,BOOLEAN],[DATE,DATE]," + "[TIME,TIME],[DATETIME,DATETIME],[TIMESTAMP,TIMESTAMP],[INTERVAL,INTERVAL]," - + "[STRUCT,STRUCT],[ARRAY,ARRAY]}, but get [STRING,INTEGER]", + + "[STRUCT,STRUCT],[ARRAY,ARRAY],[GEO_POINT,GEO_POINT]}, but get [STRING,INTEGER]", exception.getMessage()); } diff --git a/integ-test/src/test/resources/geo_point.json b/integ-test/src/test/resources/geo_point.json index a937fbb03d..1ad3166fd1 100644 --- a/integ-test/src/test/resources/geo_point.json +++ b/integ-test/src/test/resources/geo_point.json @@ -1,4 +1,4 @@ {"index":{"_id":"1"}} -{"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]}} +{"geo_point_object": {"lat": 40.71}, "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": null, "geo_point_string": null, "geo_point_geohash": null, "geo_point_array": null, "geo_point_string_point": null, "geo_point_geojson": null} diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprValueFactory.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprValueFactory.java index 2f8739edfb..137f97dc69 100644 --- a/opensearch/src/main/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprValueFactory.java +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprValueFactory.java @@ -43,6 +43,7 @@ import java.util.function.BiFunction; import lombok.Getter; import lombok.Setter; +import org.joda.time.IllegalInstantException; import org.opensearch.common.time.DateFormatter; import org.opensearch.common.time.DateFormatters; import org.opensearch.common.time.FormatNames; @@ -195,8 +196,10 @@ private ExprValue parse( || content.isArray()) { return parseArray(content, field, type, supportArrays); } else if (type.equals(OpenSearchDataType.of(OpenSearchDataType.MappingType.Object)) - || type == STRUCT || type.equals(OpenSearchDataType.of(OpenSearchDataType.MappingType.GeoPoint))) { + || type == STRUCT) { return parseStruct(content, field, supportArrays); + } else if (type.equals(OpenSearchDataType.of(OpenSearchDataType.MappingType.GeoPoint))) { + return parseGeoPointObject(content, field); } else { if (typeActionMap.containsKey(type)) { return typeActionMap.get(type).apply(content, type); @@ -313,6 +316,28 @@ private ExprValue parseStruct(Content content, String prefix, boolean supportArr return new ExprTupleValue(result); } + /** + * Parse geo_point object content. + * @param content Content to parse. + * @param prefix Prefix for Level of object depth to parse. + * @return Value parsed from content. + */ + private ExprValue parseGeoPointObject(Content content, String prefix) { + LinkedHashMap result = new LinkedHashMap<>(); + content.map().forEachRemaining(entry -> result.put(entry.getKey(), + parse(entry.getValue(), + makeField(prefix, entry.getKey()), + type(makeField(prefix, entry.getKey())), false))); + + // result is empty when an unsupported format is queried + if (result.isEmpty()) { + throw new IllegalInstantException("geo point must in format of {\"lat\": number, \"lon\": " + + "number}"); + } + + return new ExprTupleValue(result); + } + /** * Parse array content. Can also parse nested which isn't necessarily an array. * @param content Content to parse. From eba9fb836ef2f9806a9ab3ae5f80c65425838587 Mon Sep 17 00:00:00 2001 From: Guian Gumpac Date: Mon, 17 Jul 2023 17:22:28 -0700 Subject: [PATCH 03/30] Fixed old tests Signed-off-by: Guian Gumpac --- integ-test/src/test/resources/geo_point.json | 4 ++-- .../data/value/OpenSearchExprGeoPointValue.java | 6 +++++- .../opensearch/client/OpenSearchNodeClientTest.java | 2 +- .../opensearch/client/OpenSearchRestClientTest.java | 2 +- .../data/type/OpenSearchDataTypeTest.java | 5 +++-- .../data/value/OpenSearchExprValueFactoryTest.java | 13 +------------ 6 files changed, 13 insertions(+), 19 deletions(-) diff --git a/integ-test/src/test/resources/geo_point.json b/integ-test/src/test/resources/geo_point.json index 1ad3166fd1..9e338abaf3 100644 --- a/integ-test/src/test/resources/geo_point.json +++ b/integ-test/src/test/resources/geo_point.json @@ -1,4 +1,4 @@ {"index":{"_id":"1"}} -{"geo_point_object": {"lat": 40.71}, "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]}} +{"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": null, "geo_point_string": null, "geo_point_geohash": null, "geo_point_array": null, "geo_point_string_point": null, "geo_point_geojson": null} +{"geo_point_object": {"lat": "40.71", "lon": "74.00"}, "geo_point_string": null, "geo_point_geohash": null, "geo_point_array": null, "geo_point_string_point": null, "geo_point_geojson": null} diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprGeoPointValue.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprGeoPointValue.java index 72f7f4a4f2..991be34376 100644 --- a/opensearch/src/main/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprGeoPointValue.java +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprGeoPointValue.java @@ -6,6 +6,7 @@ package org.opensearch.sql.opensearch.data.value; +import java.util.Map; import java.util.Objects; import lombok.Data; import org.opensearch.sql.data.model.AbstractExprValue; @@ -43,7 +44,10 @@ public int compare(ExprValue other) { @Override public boolean equal(ExprValue other) { - return geoPoint.equals(((OpenSearchExprGeoPointValue) other).geoPoint); + Map otherTupleValue = other.tupleValue(); + return geoPoint.equals(new OpenSearchExprGeoPointValue( + other.tupleValue().get("lat").doubleValue(), + other.tupleValue().get("lon").doubleValue()).geoPoint); } @Override diff --git a/opensearch/src/test/java/org/opensearch/sql/opensearch/client/OpenSearchNodeClientTest.java b/opensearch/src/test/java/org/opensearch/sql/opensearch/client/OpenSearchNodeClientTest.java index 9417a1de1d..9fce7a0b5d 100644 --- a/opensearch/src/test/java/org/opensearch/sql/opensearch/client/OpenSearchNodeClientTest.java +++ b/opensearch/src/test/java/org/opensearch/sql/opensearch/client/OpenSearchNodeClientTest.java @@ -171,7 +171,7 @@ void get_index_mappings() throws IOException { () -> assertEquals(1, indexMappings.size()), // 10 types extended to 17 after flattening () -> assertEquals(10, mapping.size()), - () -> assertEquals(17, parsedTypes.size()), + () -> assertEquals(19, parsedTypes.size()), () -> assertEquals("TEXT", mapping.get("address").legacyTypeName()), () -> assertEquals(OpenSearchTextType.of(MappingType.Text), parsedTypes.get("address")), diff --git a/opensearch/src/test/java/org/opensearch/sql/opensearch/client/OpenSearchRestClientTest.java b/opensearch/src/test/java/org/opensearch/sql/opensearch/client/OpenSearchRestClientTest.java index cceb6de995..fedf07f0d2 100644 --- a/opensearch/src/test/java/org/opensearch/sql/opensearch/client/OpenSearchRestClientTest.java +++ b/opensearch/src/test/java/org/opensearch/sql/opensearch/client/OpenSearchRestClientTest.java @@ -172,7 +172,7 @@ void get_index_mappings() throws IOException { () -> assertEquals(1, indexMappings.size()), // 10 types extended to 17 after flattening () -> assertEquals(10, mapping.size()), - () -> assertEquals(17, parsedTypes.size()), + () -> assertEquals(19, parsedTypes.size()), () -> assertEquals("TEXT", mapping.get("address").legacyTypeName()), () -> assertEquals(OpenSearchTextType.of(MappingType.Text), parsedTypes.get("address")), diff --git a/opensearch/src/test/java/org/opensearch/sql/opensearch/data/type/OpenSearchDataTypeTest.java b/opensearch/src/test/java/org/opensearch/sql/opensearch/data/type/OpenSearchDataTypeTest.java index 8d69b3d855..c7e8055b45 100644 --- a/opensearch/src/test/java/org/opensearch/sql/opensearch/data/type/OpenSearchDataTypeTest.java +++ b/opensearch/src/test/java/org/opensearch/sql/opensearch/data/type/OpenSearchDataTypeTest.java @@ -22,6 +22,7 @@ import static org.opensearch.sql.data.type.ExprCoreType.DATE; import static org.opensearch.sql.data.type.ExprCoreType.DOUBLE; import static org.opensearch.sql.data.type.ExprCoreType.FLOAT; +import static org.opensearch.sql.data.type.ExprCoreType.GEO_POINT; import static org.opensearch.sql.data.type.ExprCoreType.INTEGER; import static org.opensearch.sql.data.type.ExprCoreType.LONG; import static org.opensearch.sql.data.type.ExprCoreType.SHORT; @@ -109,7 +110,7 @@ private static Stream getTestDataWithType() { Arguments.of(MappingType.Object, "object", STRUCT), Arguments.of(MappingType.Nested, "nested", ARRAY), Arguments.of(MappingType.GeoPoint, "geo_point", - OpenSearchGeoPointType.of()), + GEO_POINT), Arguments.of(MappingType.Binary, "binary", OpenSearchBinaryType.of()), Arguments.of(MappingType.Ip, "ip", @@ -291,7 +292,7 @@ public void traverseAndFlatten() { var flattened = OpenSearchDataType.traverseAndFlatten(getSampleMapping()); var objectType = OpenSearchDataType.of(MappingType.Object); assertAll( - () -> assertEquals(9, flattened.size()), + () -> assertEquals(11, flattened.size()), () -> assertTrue(flattened.get("mapping").getProperties().isEmpty()), () -> assertTrue(flattened.get("mapping.submapping").getProperties().isEmpty()), () -> assertTrue( diff --git a/opensearch/src/test/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprValueFactoryTest.java b/opensearch/src/test/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprValueFactoryTest.java index 827606a961..ba795bcfdd 100644 --- a/opensearch/src/test/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprValueFactoryTest.java +++ b/opensearch/src/test/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprValueFactoryTest.java @@ -59,6 +59,7 @@ import org.opensearch.sql.opensearch.data.type.OpenSearchDataType; import org.opensearch.sql.opensearch.data.type.OpenSearchDateType; import org.opensearch.sql.opensearch.data.type.OpenSearchTextType; +import org.opensearch.sql.opensearch.data.utils.ObjectContent; import org.opensearch.sql.opensearch.data.utils.OpenSearchJsonContent; class OpenSearchExprValueFactoryTest { @@ -733,18 +734,6 @@ public void constructGeoPointFromUnsupportedFormatShouldThrowException() { 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")); From 1fd9893740ec44b297ef8f0f2ece7b5bd09cca8f Mon Sep 17 00:00:00 2001 From: Guian Gumpac Date: Tue, 18 Jul 2023 15:00:58 -0700 Subject: [PATCH 04/30] Fixed all old tests Signed-off-by: Guian Gumpac --- integ-test/src/test/resources/geo_point.json | 2 - .../data/type/OpenSearchGeoPointType.java | 6 +-- .../data/utils/OpenSearchJsonContent.java | 2 +- .../value/OpenSearchExprGeoPointValue.java | 5 +- .../value/OpenSearchExprValueFactory.java | 47 +++++++++---------- .../value/OpenSearchExprValueFactoryTest.java | 2 +- 6 files changed, 26 insertions(+), 38 deletions(-) diff --git a/integ-test/src/test/resources/geo_point.json b/integ-test/src/test/resources/geo_point.json index 9e338abaf3..14a47f5926 100644 --- a/integ-test/src/test/resources/geo_point.json +++ b/integ-test/src/test/resources/geo_point.json @@ -1,4 +1,2 @@ {"index":{"_id":"1"}} {"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": "40.71", "lon": "74.00"}, "geo_point_string": null, "geo_point_geohash": null, "geo_point_array": null, "geo_point_string_point": null, "geo_point_geojson": null} diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/data/type/OpenSearchGeoPointType.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/data/type/OpenSearchGeoPointType.java index f04b79aea0..1848596686 100644 --- a/opensearch/src/main/java/org/opensearch/sql/opensearch/data/type/OpenSearchGeoPointType.java +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/data/type/OpenSearchGeoPointType.java @@ -7,15 +7,13 @@ package org.opensearch.sql.opensearch.data.type; import static org.opensearch.sql.data.type.ExprCoreType.GEO_POINT; -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; -import java.util.HashMap; -import java.util.Map; - /** * The type of a geo_point value. See * doc diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/data/utils/OpenSearchJsonContent.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/data/utils/OpenSearchJsonContent.java index 61da7c3b74..8aa89835b2 100644 --- a/opensearch/src/main/java/org/opensearch/sql/opensearch/data/utils/OpenSearchJsonContent.java +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/data/utils/OpenSearchJsonContent.java @@ -145,7 +145,7 @@ public Pair geoValue() { } return Pair.of(lat, lon); } else { - throw new IllegalStateException("geo point must in format of {\"lat\": number, \"lon\": " + throw new IllegalStateException("geo point must be in format of {\"lat\": number, \"lon\": " + "number}"); } } diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprGeoPointValue.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprGeoPointValue.java index 991be34376..2f4d4c4fdd 100644 --- a/opensearch/src/main/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprGeoPointValue.java +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprGeoPointValue.java @@ -44,10 +44,7 @@ public int compare(ExprValue other) { @Override public boolean equal(ExprValue other) { - Map otherTupleValue = other.tupleValue(); - return geoPoint.equals(new OpenSearchExprGeoPointValue( - other.tupleValue().get("lat").doubleValue(), - other.tupleValue().get("lon").doubleValue()).geoPoint); + return geoPoint.equals(((OpenSearchExprGeoPointValue) other).geoPoint); } @Override diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprValueFactory.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprValueFactory.java index 137f97dc69..e4779dbbf7 100644 --- a/opensearch/src/main/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprValueFactory.java +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprValueFactory.java @@ -25,13 +25,11 @@ import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.ObjectMapper; -import com.fasterxml.jackson.databind.node.ArrayNode; import com.fasterxml.jackson.databind.node.ObjectNode; import com.google.common.collect.ImmutableMap; import java.time.Instant; import java.time.LocalDate; import java.time.LocalTime; -import java.time.ZoneId; import java.time.ZonedDateTime; import java.time.format.DateTimeParseException; import java.time.temporal.TemporalAccessor; @@ -43,7 +41,6 @@ import java.util.function.BiFunction; import lombok.Getter; import lombok.Setter; -import org.joda.time.IllegalInstantException; import org.opensearch.common.time.DateFormatter; import org.opensearch.common.time.DateFormatters; import org.opensearch.common.time.FormatNames; @@ -199,7 +196,27 @@ private ExprValue parse( || type == STRUCT) { return parseStruct(content, field, supportArrays); } else if (type.equals(OpenSearchDataType.of(OpenSearchDataType.MappingType.GeoPoint))) { - return parseGeoPointObject(content, field); + ExprValue result = null; + + // Allows to parse for double values in geo_point object + if (typeActionMap.containsKey(type)) { + try { + result = typeActionMap.get(type).apply(content, type); + } catch (IllegalStateException e) { + if (e.getMessage().contains("must be number value")) { + throw e; + } + result = parseStruct(content, field, supportArrays); + } + } + + // result is empty when an unsupported format is queried + if (result instanceof ExprTupleValue && result.tupleValue().isEmpty()) { + throw new IllegalStateException("geo point must be in format of {\"lat\": number, \"lon\": " + + "number}"); + } + + return result; } else { if (typeActionMap.containsKey(type)) { return typeActionMap.get(type).apply(content, type); @@ -316,28 +333,6 @@ private ExprValue parseStruct(Content content, String prefix, boolean supportArr return new ExprTupleValue(result); } - /** - * Parse geo_point object content. - * @param content Content to parse. - * @param prefix Prefix for Level of object depth to parse. - * @return Value parsed from content. - */ - private ExprValue parseGeoPointObject(Content content, String prefix) { - LinkedHashMap result = new LinkedHashMap<>(); - content.map().forEachRemaining(entry -> result.put(entry.getKey(), - parse(entry.getValue(), - makeField(prefix, entry.getKey()), - type(makeField(prefix, entry.getKey())), false))); - - // result is empty when an unsupported format is queried - if (result.isEmpty()) { - throw new IllegalInstantException("geo point must in format of {\"lat\": number, \"lon\": " - + "number}"); - } - - return new ExprTupleValue(result); - } - /** * Parse array content. Can also parse nested which isn't necessarily an array. * @param content Content to parse. diff --git a/opensearch/src/test/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprValueFactoryTest.java b/opensearch/src/test/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprValueFactoryTest.java index ba795bcfdd..307c50a359 100644 --- a/opensearch/src/test/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprValueFactoryTest.java +++ b/opensearch/src/test/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprValueFactoryTest.java @@ -731,7 +731,7 @@ 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}", + assertEquals("geo point must be in format of {\"lat\": number, \"lon\": number}", exception.getMessage()); exception = From 912401fd05ccbcc1d333c0f0258062bd66a2633f Mon Sep 17 00:00:00 2001 From: Guian Gumpac Date: Tue, 18 Jul 2023 15:12:11 -0700 Subject: [PATCH 05/30] Removed irrelevant changes Signed-off-by: Guian Gumpac --- integ-test/src/test/resources/datatypes.json | 2 +- .../datatypes_index_mapping.json | 25 ------------------- .../value/OpenSearchExprGeoPointValue.java | 1 - 3 files changed, 1 insertion(+), 27 deletions(-) diff --git a/integ-test/src/test/resources/datatypes.json b/integ-test/src/test/resources/datatypes.json index a4490dc60f..02ca4c69fb 100644 --- a/integ-test/src/test/resources/datatypes.json +++ b/integ-test/src/test/resources/datatypes.json @@ -1,2 +1,2 @@ {"index":{"_id":"1"}} -{"boolean_value": true, "keyword_value": "keyword", "text_value": "text", "binary_value": "U29tZSBiaW5hcnkgYmxvYg==", "date_value": "2020-10-13 13:00:00", "ip_value": "127.0.0.1", "object_value": {"first": "Dale", "last": "Dale"}, "nested_value": [{"first" : "John", "last" : "Smith"}, {"first" : "Alice", "last" : "White"}], "geo_point_value": {"lat": -33.94609833, "lon": 151.177002}, "geo_shape_value": {"type": "polygon", "coordinates": [[[74.0060, 40.7128], [71.0589, 42.3601], [73.7562, 42.6526], [74.0060, 40.7128]]]}, "integer_range_value": {"gte": 0, "lte": 10}, "long_range_value": {"gte": 0.0, "lte": 10.0}, "ip_range_value": {"gte": "10.24.34.0", "lte": "10.24.35.255"}, "ip_cidr_range_value": "10.24.34.0/24", "date_range_value": {"gte": "2019-05-01", "lte": "2019-05-15"}, "unsigned_long_value" : 10223372036854775807, "flat_object_value": {"flat_object_string_value": "hello world", "flat_object_int_value": 10, "flat_object_date_value": "2023-10-12"}} +{"boolean_value": true, "keyword_value": "keyword", "text_value": "text", "binary_value": "U29tZSBiaW5hcnkgYmxvYg==", "date_value": "2020-10-13 13:00:00", "ip_value": "127.0.0.1", "object_value": {"first": "Dale", "last": "Dale"}, "nested_value": [{"first" : "John", "last" : "Smith"}, {"first" : "Alice", "last" : "White"}], "geo_point_value": { "lat": 40.71, "lon": 74.00 }} \ No newline at end of file diff --git a/integ-test/src/test/resources/indexDefinitions/datatypes_index_mapping.json b/integ-test/src/test/resources/indexDefinitions/datatypes_index_mapping.json index 3fe34c611c..8c1759b369 100644 --- a/integ-test/src/test/resources/indexDefinitions/datatypes_index_mapping.json +++ b/integ-test/src/test/resources/indexDefinitions/datatypes_index_mapping.json @@ -35,31 +35,6 @@ }, "geo_point_value": { "type": "geo_point" - }, - "geo_shape_value": { - "type": "geo_shape" - }, - "integer_range_value": { - "type": "integer_range" - }, - "long_range_value": { - "type": "long_range" - }, - "ip_range_value": { - "type": "ip_range" - }, - "ip_cidr_range_value": { - "type": "ip_range" - }, - "date_range_value" : { - "type" : "date_range", - "format" : "strict_year_month_day" - }, - "unsigned_long_value" : { - "type" : "unsigned_long" - }, - "flat_object_value": { - "type": "flat_object" } } } diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprGeoPointValue.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprGeoPointValue.java index 2f4d4c4fdd..72f7f4a4f2 100644 --- a/opensearch/src/main/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprGeoPointValue.java +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprGeoPointValue.java @@ -6,7 +6,6 @@ package org.opensearch.sql.opensearch.data.value; -import java.util.Map; import java.util.Objects; import lombok.Data; import org.opensearch.sql.data.model.AbstractExprValue; From 0a0be6a1d472a80fa9d48778bc1b04a4c37e8cde Mon Sep 17 00:00:00 2001 From: Guian Gumpac Date: Tue, 18 Jul 2023 15:12:43 -0700 Subject: [PATCH 06/30] Removed irrelevant changes Signed-off-by: Guian Gumpac --- integ-test/src/test/resources/datatypes.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/integ-test/src/test/resources/datatypes.json b/integ-test/src/test/resources/datatypes.json index 02ca4c69fb..ea3290ee64 100644 --- a/integ-test/src/test/resources/datatypes.json +++ b/integ-test/src/test/resources/datatypes.json @@ -1,2 +1,2 @@ {"index":{"_id":"1"}} -{"boolean_value": true, "keyword_value": "keyword", "text_value": "text", "binary_value": "U29tZSBiaW5hcnkgYmxvYg==", "date_value": "2020-10-13 13:00:00", "ip_value": "127.0.0.1", "object_value": {"first": "Dale", "last": "Dale"}, "nested_value": [{"first" : "John", "last" : "Smith"}, {"first" : "Alice", "last" : "White"}], "geo_point_value": { "lat": 40.71, "lon": 74.00 }} \ No newline at end of file +{"boolean_value": true, "keyword_value": "keyword", "text_value": "text", "binary_value": "U29tZSBiaW5hcnkgYmxvYg==", "date_value": "2020-10-13 13:00:00", "ip_value": "127.0.0.1", "object_value": {"first": "Dale", "last": "Dale"}, "nested_value": [{"first" : "John", "last" : "Smith"}, {"first" : "Alice", "last" : "White"}], "geo_point_value": { "lat": 40.71, "lon": 74.00 }} From abe89249db504cbb029b7d4e5ab280d8cda8f931 Mon Sep 17 00:00:00 2001 From: Guian Gumpac Date: Wed, 19 Jul 2023 13:58:34 -0700 Subject: [PATCH 07/30] Added integration tests Signed-off-by: Guian Gumpac --- .../sql/legacy/SQLIntegTestCase.java | 6 +- .../opensearch/sql/legacy/TestsConstants.java | 1 + .../org/opensearch/sql/sql/GeoPointIT.java | 109 +++++++++++++++--- integ-test/src/test/resources/geo_point.json | 6 +- .../indexDefinitions/geo_point_mapping.json | 8 ++ 5 files changed, 114 insertions(+), 16 deletions(-) diff --git a/integ-test/src/test/java/org/opensearch/sql/legacy/SQLIntegTestCase.java b/integ-test/src/test/java/org/opensearch/sql/legacy/SQLIntegTestCase.java index 7216c03d08..8a78cec91e 100644 --- a/integ-test/src/test/java/org/opensearch/sql/legacy/SQLIntegTestCase.java +++ b/integ-test/src/test/java/org/opensearch/sql/legacy/SQLIntegTestCase.java @@ -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; diff --git a/integ-test/src/test/java/org/opensearch/sql/legacy/TestsConstants.java b/integ-test/src/test/java/org/opensearch/sql/legacy/TestsConstants.java index 338be25a0c..0d7376786a 100644 --- a/integ-test/src/test/java/org/opensearch/sql/legacy/TestsConstants.java +++ b/integ-test/src/test/java/org/opensearch/sql/legacy/TestsConstants.java @@ -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'"; diff --git a/integ-test/src/test/java/org/opensearch/sql/sql/GeoPointIT.java b/integ-test/src/test/java/org/opensearch/sql/sql/GeoPointIT.java index e59bfd70bd..c33580f181 100644 --- a/integ-test/src/test/java/org/opensearch/sql/sql/GeoPointIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/sql/GeoPointIT.java @@ -10,31 +10,112 @@ import org.opensearch.sql.legacy.SQLIntegTestCase; import java.io.IOException; +import java.util.Map; -import static org.opensearch.sql.legacy.TestsConstants.TEST_INDEX_WILDCARD; +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.WILDCARD); + loadIndex(Index.GEOPOINT); } @Test - public void test_geo_point_in_select() throws IOException { - String query = "SELECT KeywordBody, KeywordBody LIKE 'test wildcard%' FROM " + TEST_INDEX_WILDCARD; + public void test_geo_point() throws IOException { + String query = "SELECT geo_point_object FROM " + TEST_INDEX_GEOPOINT; JSONObject result = executeJdbcRequest(query); verifyDataRows(result, - rows("test wildcard", true), - rows("test wildcard in the end of the text%", true), - rows("%test wildcard in the beginning of the text", false), - rows("test wildcard in % the middle of the text", true), - rows("test wildcard %% beside each other", true), - rows("test wildcard in the end of the text_", true), - rows("_test wildcard in the beginning of the text", false), - rows("test wildcard in _ the middle of the text", true), - rows("test wildcard __ beside each other", true), - rows("test backslash wildcard \\_", false)); + 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() throws IOException { + 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() throws IOException { + 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() throws IOException { + 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() throws IOException { + 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_objecte_with_lat_and_lon() throws IOException { + 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() throws IOException { + 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) + ); } } diff --git a/integ-test/src/test/resources/geo_point.json b/integ-test/src/test/resources/geo_point.json index 14a47f5926..4ba2d27828 100644 --- a/integ-test/src/test/resources/geo_point.json +++ b/integ-test/src/test/resources/geo_point.json @@ -1,2 +1,6 @@ {"index":{"_id":"1"}} -{"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]}} +{"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} diff --git a/integ-test/src/test/resources/indexDefinitions/geo_point_mapping.json b/integ-test/src/test/resources/indexDefinitions/geo_point_mapping.json index 3063ae8b93..8dc92fe5c4 100644 --- a/integ-test/src/test/resources/indexDefinitions/geo_point_mapping.json +++ b/integ-test/src/test/resources/indexDefinitions/geo_point_mapping.json @@ -4,6 +4,14 @@ "geo_point_object": { "type": "geo_point" }, + "object": { + "type": "object", + "properties": { + "geo_point_object": { + "type": "geo_point" + } + } + }, "geo_point_string": { "type": "geo_point" }, From 66404a8fde7924596e9ec6629a1373f094fa8b57 Mon Sep 17 00:00:00 2001 From: Guian Gumpac Date: Wed, 19 Jul 2023 14:14:14 -0700 Subject: [PATCH 08/30] Fixed not throwing exception for geojson Signed-off-by: Guian Gumpac --- .../src/test/java/org/opensearch/sql/sql/GeoPointIT.java | 2 +- .../sql/opensearch/data/value/OpenSearchExprValueFactory.java | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/integ-test/src/test/java/org/opensearch/sql/sql/GeoPointIT.java b/integ-test/src/test/java/org/opensearch/sql/sql/GeoPointIT.java index c33580f181..aa798e0632 100644 --- a/integ-test/src/test/java/org/opensearch/sql/sql/GeoPointIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/sql/GeoPointIT.java @@ -91,7 +91,7 @@ public void test_geo_point_lat_and_lon() throws IOException { } @Test - public void test_geo_point_objecte_with_lat_and_lon() throws IOException { + public void test_geo_point_object_with_lat_and_lon() throws IOException { String query = "SELECT geo_point_object, geo_point_object.lat," + " geo_point_object.lon FROM " + TEST_INDEX_GEOPOINT; JSONObject result = executeJdbcRequest(query); diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprValueFactory.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprValueFactory.java index e4779dbbf7..65a88c6a85 100644 --- a/opensearch/src/main/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprValueFactory.java +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprValueFactory.java @@ -211,7 +211,8 @@ private ExprValue parse( } // result is empty when an unsupported format is queried - if (result instanceof ExprTupleValue && result.tupleValue().isEmpty()) { + if (result instanceof ExprTupleValue && (result.tupleValue().isEmpty() || + result.tupleValue().get("type") instanceof ExprNullValue)) { throw new IllegalStateException("geo point must be in format of {\"lat\": number, \"lon\": " + "number}"); } From 9d7c5a87ae19b16abd119360d54d6af221fcf0b9 Mon Sep 17 00:00:00 2001 From: Guian Gumpac Date: Thu, 20 Jul 2023 19:05:26 -0700 Subject: [PATCH 09/30] Made GeoPointValue an ExprTupleValue to enable access to lat and lon Signed-off-by: Guian Gumpac --- .../value/OpenSearchExprGeoPointValue.java | 42 +++---------------- .../value/OpenSearchExprValueFactory.java | 40 ++++++++---------- .../OpenSearchExprGeoPointValueTest.java | 23 ---------- .../value/OpenSearchExprValueFactoryTest.java | 10 ++--- 4 files changed, 27 insertions(+), 88 deletions(-) diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprGeoPointValue.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprGeoPointValue.java index 72f7f4a4f2..2d11d8f271 100644 --- a/opensearch/src/main/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprGeoPointValue.java +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprGeoPointValue.java @@ -6,9 +6,10 @@ package org.opensearch.sql.opensearch.data.value; -import java.util.Objects; -import lombok.Data; -import org.opensearch.sql.data.model.AbstractExprValue; +import java.util.LinkedHashMap; +import java.util.Map; +import org.opensearch.sql.data.model.ExprDoubleValue; +import org.opensearch.sql.data.model.ExprTupleValue; import org.opensearch.sql.data.model.ExprValue; import org.opensearch.sql.data.type.ExprType; import org.opensearch.sql.opensearch.data.type.OpenSearchGeoPointType; @@ -17,45 +18,14 @@ * OpenSearch GeoPointValue. * Todo, add this to avoid the unknown value type exception, the implementation will be changed. */ -public class OpenSearchExprGeoPointValue extends AbstractExprValue { - - private final GeoPoint geoPoint; +public class OpenSearchExprGeoPointValue extends ExprTupleValue { public OpenSearchExprGeoPointValue(Double lat, Double lon) { - this.geoPoint = new GeoPoint(lat, lon); - } - - @Override - public Object value() { - return geoPoint; + super(new LinkedHashMap<>(Map.of("lat", (ExprValue) new ExprDoubleValue(lat), "lon", (ExprValue) new ExprDoubleValue(lon)))); } @Override public ExprType type() { return OpenSearchGeoPointType.of(); } - - @Override - public int compare(ExprValue other) { - return geoPoint.toString() - .compareTo((((OpenSearchExprGeoPointValue) other).geoPoint).toString()); - } - - @Override - public boolean equal(ExprValue other) { - return geoPoint.equals(((OpenSearchExprGeoPointValue) other).geoPoint); - } - - @Override - public int hashCode() { - return Objects.hashCode(geoPoint); - } - - @Data - public static class GeoPoint { - - private final Double lat; - - private final Double lon; - } } diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprValueFactory.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprValueFactory.java index 65a88c6a85..8f65811e49 100644 --- a/opensearch/src/main/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprValueFactory.java +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprValueFactory.java @@ -196,37 +196,33 @@ private ExprValue parse( || type == STRUCT) { return parseStruct(content, field, supportArrays); } else if (type.equals(OpenSearchDataType.of(OpenSearchDataType.MappingType.GeoPoint))) { - ExprValue result = null; - - // Allows to parse for double values in geo_point object - if (typeActionMap.containsKey(type)) { - try { - result = typeActionMap.get(type).apply(content, type); - } catch (IllegalStateException e) { - if (e.getMessage().contains("must be number value")) { - throw e; - } - result = parseStruct(content, field, supportArrays); + try { + if (typeActionMap.containsKey(type)) { + return typeActionMap.get(type).apply(content, type); } } - // result is empty when an unsupported format is queried - if (result instanceof ExprTupleValue && (result.tupleValue().isEmpty() || - result.tupleValue().get("type") instanceof ExprNullValue)) { - throw new IllegalStateException("geo point must be in format of {\"lat\": number, \"lon\": " - + "number}"); - } + // The try block throws an exception for queries accessing lat or lon of geo_point + catch (IllegalStateException e) { + ExprValue result = parseStruct(content, field, supportArrays); - return result; + // result is empty when an unsupported format is queried + if (result.tupleValue().isEmpty() || + result.tupleValue().get("type") instanceof ExprNullValue) { + throw new IllegalStateException("geo point must be in format of {\"lat\": number, \"lon\": " + + "number}"); + } + + return result; + } } else { if (typeActionMap.containsKey(type)) { return typeActionMap.get(type).apply(content, type); - } else { - throw new IllegalStateException( - String.format( - "Unsupported type: %s for value: %s.", type.typeName(), content.objectValue())); } } + throw new IllegalStateException( + String.format( + "Unsupported type: %s for value: %s.", type.typeName(), content.objectValue())); } /** diff --git a/opensearch/src/test/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprGeoPointValueTest.java b/opensearch/src/test/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprGeoPointValueTest.java index 4edb25aff5..2b78c0d352 100644 --- a/opensearch/src/test/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprGeoPointValueTest.java +++ b/opensearch/src/test/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprGeoPointValueTest.java @@ -7,8 +7,6 @@ package org.opensearch.sql.opensearch.data.value; import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertNotNull; -import static org.junit.jupiter.api.Assertions.assertTrue; import org.junit.jupiter.api.Test; import org.opensearch.sql.opensearch.data.type.OpenSearchGeoPointType; @@ -17,29 +15,8 @@ class OpenSearchExprGeoPointValueTest { private OpenSearchExprGeoPointValue geoPointValue = new OpenSearchExprGeoPointValue(1.0, 1.0); - @Test - void value() { - assertEquals(new OpenSearchExprGeoPointValue.GeoPoint(1.0, 1.0), geoPointValue.value()); - } - @Test void type() { assertEquals(OpenSearchGeoPointType.of(), geoPointValue.type()); } - - @Test - void compare() { - assertEquals(0, geoPointValue.compareTo(new OpenSearchExprGeoPointValue(1.0, 1.0))); - assertEquals(geoPointValue, new OpenSearchExprGeoPointValue(1.0, 1.0)); - } - - @Test - void equal() { - assertTrue(geoPointValue.equal(new OpenSearchExprGeoPointValue(1.0, 1.0))); - } - - @Test - void testHashCode() { - assertNotNull(geoPointValue.hashCode()); - } } diff --git a/opensearch/src/test/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprValueFactoryTest.java b/opensearch/src/test/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprValueFactoryTest.java index 307c50a359..9b3eb3b232 100644 --- a/opensearch/src/test/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprValueFactoryTest.java +++ b/opensearch/src/test/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprValueFactoryTest.java @@ -736,13 +736,9 @@ public void constructGeoPointFromUnsupportedFormatShouldThrowException() { 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()); + () -> tupleValue("{\"geoV\":{\"type\": \"Point\", \"coordinates\": [74.00, 40.71]}}").get("geoV")); + assertEquals("geo point must be in format of {\"lat\": number, \"lon\": number}", + exception.getMessage()); } @Test From 31ff2dc912bda7a0bc155a2440ba4c0ee50dc929 Mon Sep 17 00:00:00 2001 From: Guian Gumpac Date: Fri, 21 Jul 2023 09:11:11 -0700 Subject: [PATCH 10/30] Fixed checkstyle Signed-off-by: Guian Gumpac --- .../data/value/OpenSearchExprGeoPointValue.java | 9 ++++++++- .../data/value/OpenSearchExprValueFactory.java | 14 ++++++-------- .../data/value/OpenSearchExprValueFactoryTest.java | 3 ++- 3 files changed, 16 insertions(+), 10 deletions(-) diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprGeoPointValue.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprGeoPointValue.java index 2d11d8f271..89c6347b96 100644 --- a/opensearch/src/main/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprGeoPointValue.java +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprGeoPointValue.java @@ -20,8 +20,15 @@ */ public class OpenSearchExprGeoPointValue extends ExprTupleValue { + /** + * Constructor for OpenSearchExprGeoPointValue. + * @param lat double value of latitude property of geo_point + * @param lon double value of longitude property of geo_point + */ public OpenSearchExprGeoPointValue(Double lat, Double lon) { - super(new LinkedHashMap<>(Map.of("lat", (ExprValue) new ExprDoubleValue(lat), "lon", (ExprValue) new ExprDoubleValue(lon)))); + super(new LinkedHashMap<>(Map.of( + "lat", (ExprValue) new ExprDoubleValue(lat), + "lon", (ExprValue) new ExprDoubleValue(lon)))); } @Override diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprValueFactory.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprValueFactory.java index 8f65811e49..205e55e42b 100644 --- a/opensearch/src/main/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprValueFactory.java +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprValueFactory.java @@ -200,17 +200,15 @@ private ExprValue parse( if (typeActionMap.containsKey(type)) { return typeActionMap.get(type).apply(content, type); } - } - - // The try block throws an exception for queries accessing lat or lon of geo_point - catch (IllegalStateException e) { + } catch (IllegalStateException e) { + // The try block throws an exception for queries accessing lat or lon of geo_point ExprValue result = parseStruct(content, field, supportArrays); // result is empty when an unsupported format is queried - if (result.tupleValue().isEmpty() || - result.tupleValue().get("type") instanceof ExprNullValue) { - throw new IllegalStateException("geo point must be in format of {\"lat\": number, \"lon\": " - + "number}"); + if (result.tupleValue().isEmpty() + || result.tupleValue().get("type") instanceof ExprNullValue) { + throw new IllegalStateException("geo point must be in format of {\"lat\": number," + + " \"lon\": number}"); } return result; diff --git a/opensearch/src/test/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprValueFactoryTest.java b/opensearch/src/test/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprValueFactoryTest.java index 9b3eb3b232..b41ead0848 100644 --- a/opensearch/src/test/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprValueFactoryTest.java +++ b/opensearch/src/test/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprValueFactoryTest.java @@ -736,7 +736,8 @@ public void constructGeoPointFromUnsupportedFormatShouldThrowException() { exception = assertThrows(IllegalStateException.class, - () -> tupleValue("{\"geoV\":{\"type\": \"Point\", \"coordinates\": [74.00, 40.71]}}").get("geoV")); + () -> tupleValue("{\"geoV\":{\"type\": \"Point\"," + + " \"coordinates\": [74.00, 40.71]}}").get("geoV")); assertEquals("geo point must be in format of {\"lat\": number, \"lon\": number}", exception.getMessage()); } From e714aa5a812087adad4186fc33d76f7ea7c02e6f Mon Sep 17 00:00:00 2001 From: Guian Gumpac Date: Fri, 21 Jul 2023 13:14:05 -0700 Subject: [PATCH 11/30] Added unit tests and removed unnecessary code Signed-off-by: Guian Gumpac --- .../data/utils/OpenSearchJsonContent.java | 17 +++++++++---- .../value/OpenSearchExprGeoPointValue.java | 4 ++-- .../value/OpenSearchExprValueFactory.java | 18 -------------- .../value/OpenSearchExprValueFactoryTest.java | 24 +++++++++++++++++++ 4 files changed, 38 insertions(+), 25 deletions(-) diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/data/utils/OpenSearchJsonContent.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/data/utils/OpenSearchJsonContent.java index 8aa89835b2..6cac6de3b6 100644 --- a/opensearch/src/main/java/org/opensearch/sql/opensearch/data/utils/OpenSearchJsonContent.java +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/data/utils/OpenSearchJsonContent.java @@ -126,9 +126,10 @@ public Object objectValue() { @Override public Pair 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) { @@ -136,6 +137,9 @@ public Pair geoValue() { "latitude must be number value, but got value: " + value.get( "lat")); } + } + + if (value.has("lon")) { try { lon = extractDoubleValue(value.get("lon")); } catch (Exception exception) { @@ -143,11 +147,14 @@ public Pair geoValue() { "longitude must be number value, but got value: " + value.get( "lon")); } - return Pair.of(lat, lon); - } else { + } + + if (lat == null && lon == null) { throw new IllegalStateException("geo point must be in format of {\"lat\": number, \"lon\": " + "number}"); } + + return Pair.of(lat, lon); } /** diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprGeoPointValue.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprGeoPointValue.java index 89c6347b96..98a271d8ba 100644 --- a/opensearch/src/main/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprGeoPointValue.java +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprGeoPointValue.java @@ -27,8 +27,8 @@ public class OpenSearchExprGeoPointValue extends ExprTupleValue { */ public OpenSearchExprGeoPointValue(Double lat, Double lon) { super(new LinkedHashMap<>(Map.of( - "lat", (ExprValue) new ExprDoubleValue(lat), - "lon", (ExprValue) new ExprDoubleValue(lon)))); + "lat", new ExprDoubleValue(lat), + "lon", new ExprDoubleValue(lon)))); } @Override diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprValueFactory.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprValueFactory.java index 205e55e42b..81b3ce8c8a 100644 --- a/opensearch/src/main/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprValueFactory.java +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprValueFactory.java @@ -195,24 +195,6 @@ private ExprValue parse( } else if (type.equals(OpenSearchDataType.of(OpenSearchDataType.MappingType.Object)) || type == STRUCT) { return parseStruct(content, field, supportArrays); - } else if (type.equals(OpenSearchDataType.of(OpenSearchDataType.MappingType.GeoPoint))) { - try { - if (typeActionMap.containsKey(type)) { - return typeActionMap.get(type).apply(content, type); - } - } catch (IllegalStateException e) { - // The try block throws an exception for queries accessing lat or lon of geo_point - ExprValue result = parseStruct(content, field, supportArrays); - - // result is empty when an unsupported format is queried - if (result.tupleValue().isEmpty() - || result.tupleValue().get("type") instanceof ExprNullValue) { - throw new IllegalStateException("geo point must be in format of {\"lat\": number," - + " \"lon\": number}"); - } - - return result; - } } else { if (typeActionMap.containsKey(type)) { return typeActionMap.get(type).apply(content, type); diff --git a/opensearch/src/test/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprValueFactoryTest.java b/opensearch/src/test/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprValueFactoryTest.java index b41ead0848..e8873bb741 100644 --- a/opensearch/src/test/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprValueFactoryTest.java +++ b/opensearch/src/test/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprValueFactoryTest.java @@ -726,6 +726,14 @@ public void constructGeoPoint() { constructFromObject("geoV", "42.60355556,-97.25263889")); } + @Test + public void constructGeoPointLat() { + assertEquals(doubleValue(42.60355556), + tupleValue("{\"geoV\":{\"lat\":42.60355556}}").get("geoV").tupleValue().get("lat")); + assertEquals(doubleValue(-97.25263889), + tupleValue("{\"geoV\":{\"lon\":-97.25263889}}").get("geoV").tupleValue().get("lon")); + } + @Test public void constructGeoPointFromUnsupportedFormatShouldThrowException() { IllegalStateException exception = @@ -734,12 +742,28 @@ public void constructGeoPointFromUnsupportedFormatShouldThrowException() { assertEquals("geo point must be in format of {\"lat\": number, \"lon\": number}", exception.getMessage()); + exception = + assertThrows(IllegalStateException.class, + () -> tupleValue("{\"geoV\":\"txhxegj0uyp3\"}").get("geoV")); + assertEquals("geo point must be in format of {\"lat\": number, \"lon\": number}", + exception.getMessage()); + exception = assertThrows(IllegalStateException.class, () -> tupleValue("{\"geoV\":{\"type\": \"Point\"," + " \"coordinates\": [74.00, 40.71]}}").get("geoV")); assertEquals("geo point must be 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 From a24b7c31c1cd46ec71b88da8936dd491c4f0e8c1 Mon Sep 17 00:00:00 2001 From: Guian Gumpac Date: Fri, 21 Jul 2023 13:20:31 -0700 Subject: [PATCH 12/30] reverted irrelevant changes Signed-off-by: Guian Gumpac --- .../data/value/OpenSearchExprValueFactory.java | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprValueFactory.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprValueFactory.java index 81b3ce8c8a..95815d5c38 100644 --- a/opensearch/src/main/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprValueFactory.java +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprValueFactory.java @@ -25,11 +25,13 @@ import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.databind.node.ArrayNode; import com.fasterxml.jackson.databind.node.ObjectNode; import com.google.common.collect.ImmutableMap; import java.time.Instant; import java.time.LocalDate; import java.time.LocalTime; +import java.time.ZoneId; import java.time.ZonedDateTime; import java.time.format.DateTimeParseException; import java.time.temporal.TemporalAccessor; @@ -198,11 +200,12 @@ private ExprValue parse( } else { if (typeActionMap.containsKey(type)) { return typeActionMap.get(type).apply(content, type); + } else { + throw new IllegalStateException( + String.format( + "Unsupported type: %s for value: %s.", type.typeName(), content.objectValue())); } } - throw new IllegalStateException( - String.format( - "Unsupported type: %s for value: %s.", type.typeName(), content.objectValue())); } /** From 2d91d3c9521cbbab5d59da0b19c157cb761c7052 Mon Sep 17 00:00:00 2001 From: Guian Gumpac Date: Fri, 21 Jul 2023 14:05:43 -0700 Subject: [PATCH 13/30] Added to docs Signed-off-by: Guian Gumpac --- docs/user/general/datatypes.rst | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/docs/user/general/datatypes.rst b/docs/user/general/datatypes.rst index a265ffd4c9..bbb734a188 100644 --- a/docs/user/general/datatypes.rst +++ b/docs/user/general/datatypes.rst @@ -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. @@ -456,3 +458,17 @@ 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 `_, the SQL plugin currently only supports the format :code:`{"lat": number, "lon": 74.00}`. 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; + fetched rows / total rows = 1/1 + +-----------------------------+------------------------+------------------------+ + | geo_point_object | geo_point_object.lat | geo_point_object.lon | + |-----------------------------+------------------------+------------------------| + | {'lon': 74.0, 'lat': 40.71} | 40.71 | 74.0 | + +-----------------------------+------------------------+------------------------+ From 4523804f73f308be2f39cb90b955e79091414045 Mon Sep 17 00:00:00 2001 From: Guian Gumpac Date: Fri, 21 Jul 2023 14:48:17 -0700 Subject: [PATCH 14/30] Fixed doc typo Signed-off-by: Guian Gumpac --- docs/user/general/datatypes.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/user/general/datatypes.rst b/docs/user/general/datatypes.rst index bbb734a188..2eaedf8a28 100644 --- a/docs/user/general/datatypes.rst +++ b/docs/user/general/datatypes.rst @@ -462,7 +462,7 @@ A boolean can be represented by constant value ``TRUE`` or ``FALSE``. Besides, c Geopoint Data Types ================== -A geopoint has a latitude and a longitude property. Although OpenSearch `supports multiple formats `_, the SQL plugin currently only supports the format :code:`{"lat": number, "lon": 74.00}`. The geopoint object can be queried or lat and lon can be specified using dot notation. For example, :: +A geopoint has a latitude and a longitude property. Although OpenSearch `supports multiple formats `_, 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 7688155b27f33106fd27454c889e96d3e6734630 Mon Sep 17 00:00:00 2001 From: Guian Gumpac Date: Mon, 24 Jul 2023 11:44:58 -0700 Subject: [PATCH 15/30] Test doctests Signed-off-by: Guian Gumpac --- docs/user/general/datatypes.rst | 3 +-- doctest/test_data/geo_point.json | 6 ++++++ doctest/test_docs.py | 2 ++ doctest/test_mapping/geo_point.json | 32 +++++++++++++++++++++++++++++ 4 files changed, 41 insertions(+), 2 deletions(-) create mode 100644 doctest/test_data/geo_point.json create mode 100644 doctest/test_mapping/geo_point.json diff --git a/docs/user/general/datatypes.rst b/docs/user/general/datatypes.rst index 2eaedf8a28..bbda3465d7 100644 --- a/docs/user/general/datatypes.rst +++ b/docs/user/general/datatypes.rst @@ -464,8 +464,7 @@ Geopoint Data Types A geopoint has a latitude and a longitude property. Although OpenSearch `supports multiple formats `_, 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; + os> SELECT geo_point_object, geo_point_object.lat, geo_point_object.lon; fetched rows / total rows = 1/1 +-----------------------------+------------------------+------------------------+ | geo_point_object | geo_point_object.lat | geo_point_object.lon | diff --git a/doctest/test_data/geo_point.json b/doctest/test_data/geo_point.json new file mode 100644 index 0000000000..4ba2d27828 --- /dev/null +++ b/doctest/test_data/geo_point.json @@ -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} diff --git a/doctest/test_docs.py b/doctest/test_docs.py index 1fedbdf49e..303bbcdbb2 100644 --- a/doctest/test_docs.py +++ b/doctest/test_docs.py @@ -28,6 +28,7 @@ APACHE = "apache" WILDCARD = "wildcard" NESTED = "nested" +GEOPOINT = "geo_point" DATASOURCES = ".ql-datasources" @@ -98,6 +99,7 @@ def set_up_test_indices(test): load_file("wildcard.json", index_name=WILDCARD) load_file("nested_objects.json", index_name=NESTED) load_file("datasources.json", index_name=DATASOURCES) + load_file("geo_point.json", index_name=GEOPOINT) def load_file(filename, index_name): diff --git a/doctest/test_mapping/geo_point.json b/doctest/test_mapping/geo_point.json new file mode 100644 index 0000000000..8dc92fe5c4 --- /dev/null +++ b/doctest/test_mapping/geo_point.json @@ -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" + } + } + } +} From 2cf022f2cc5e0a5319c9c30f6345f5d82f852046 Mon Sep 17 00:00:00 2001 From: Guian Gumpac Date: Mon, 24 Jul 2023 12:08:19 -0700 Subject: [PATCH 16/30] Test doctests Signed-off-by: Guian Gumpac --- doctest/test_docs.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doctest/test_docs.py b/doctest/test_docs.py index 303bbcdbb2..76a55b09ae 100644 --- a/doctest/test_docs.py +++ b/doctest/test_docs.py @@ -28,7 +28,7 @@ APACHE = "apache" WILDCARD = "wildcard" NESTED = "nested" -GEOPOINT = "geo_point" +GEOPOINT = "geopoint" DATASOURCES = ".ql-datasources" From cbc837cfaee903d39bb217aa5c9dc273d23c82b7 Mon Sep 17 00:00:00 2001 From: Guian Gumpac Date: Mon, 24 Jul 2023 12:30:20 -0700 Subject: [PATCH 17/30] Remove geopoint data from doctests Signed-off-by: Guian Gumpac --- doctest/test_data/geo_point.json | 6 ------ doctest/test_docs.py | 2 -- doctest/test_mapping/geo_point.json | 32 ----------------------------- 3 files changed, 40 deletions(-) delete mode 100644 doctest/test_data/geo_point.json delete mode 100644 doctest/test_mapping/geo_point.json diff --git a/doctest/test_data/geo_point.json b/doctest/test_data/geo_point.json deleted file mode 100644 index 4ba2d27828..0000000000 --- a/doctest/test_data/geo_point.json +++ /dev/null @@ -1,6 +0,0 @@ -{"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} diff --git a/doctest/test_docs.py b/doctest/test_docs.py index 76a55b09ae..1fedbdf49e 100644 --- a/doctest/test_docs.py +++ b/doctest/test_docs.py @@ -28,7 +28,6 @@ APACHE = "apache" WILDCARD = "wildcard" NESTED = "nested" -GEOPOINT = "geopoint" DATASOURCES = ".ql-datasources" @@ -99,7 +98,6 @@ def set_up_test_indices(test): load_file("wildcard.json", index_name=WILDCARD) load_file("nested_objects.json", index_name=NESTED) load_file("datasources.json", index_name=DATASOURCES) - load_file("geo_point.json", index_name=GEOPOINT) def load_file(filename, index_name): diff --git a/doctest/test_mapping/geo_point.json b/doctest/test_mapping/geo_point.json deleted file mode 100644 index 8dc92fe5c4..0000000000 --- a/doctest/test_mapping/geo_point.json +++ /dev/null @@ -1,32 +0,0 @@ -{ - "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" - } - } - } -} From 821c44a84536cc904c01fdad2217c519963f49c3 Mon Sep 17 00:00:00 2001 From: Guian Gumpac Date: Mon, 24 Jul 2023 15:09:17 -0700 Subject: [PATCH 18/30] Fixed doctest failure Signed-off-by: Guian Gumpac --- docs/user/general/datatypes.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/user/general/datatypes.rst b/docs/user/general/datatypes.rst index bbda3465d7..614d6de9f9 100644 --- a/docs/user/general/datatypes.rst +++ b/docs/user/general/datatypes.rst @@ -464,7 +464,7 @@ Geopoint Data Types A geopoint has a latitude and a longitude property. Although OpenSearch `supports multiple formats `_, 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; + os> SELECT geo_point_object, geo_point_object.lat, geo_point_object.lon FROM geo_point; fetched rows / total rows = 1/1 +-----------------------------+------------------------+------------------------+ | geo_point_object | geo_point_object.lat | geo_point_object.lon | From 7d0658d06f62f81d60e04f53d29bb37b7ab22549 Mon Sep 17 00:00:00 2001 From: Guian Gumpac Date: Mon, 24 Jul 2023 15:40:33 -0700 Subject: [PATCH 19/30] Fixed doctest failure Signed-off-by: Guian Gumpac --- docs/user/general/datatypes.rst | 2 +- doctest/test_data/geopoint.json | 6 ++++++ doctest/test_docs.py | 2 ++ doctest/test_mapping/geopoint.json | 32 ++++++++++++++++++++++++++++++ 4 files changed, 41 insertions(+), 1 deletion(-) create mode 100644 doctest/test_data/geopoint.json create mode 100644 doctest/test_mapping/geopoint.json diff --git a/docs/user/general/datatypes.rst b/docs/user/general/datatypes.rst index 614d6de9f9..5b6b0c0594 100644 --- a/docs/user/general/datatypes.rst +++ b/docs/user/general/datatypes.rst @@ -464,7 +464,7 @@ Geopoint Data Types A geopoint has a latitude and a longitude property. Although OpenSearch `supports multiple formats `_, 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 geo_point; + os> SELECT geo_point_object, geo_point_object.lat, geo_point_object.lon FROM geopoint; fetched rows / total rows = 1/1 +-----------------------------+------------------------+------------------------+ | geo_point_object | geo_point_object.lat | geo_point_object.lon | diff --git a/doctest/test_data/geopoint.json b/doctest/test_data/geopoint.json new file mode 100644 index 0000000000..4ba2d27828 --- /dev/null +++ b/doctest/test_data/geopoint.json @@ -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} diff --git a/doctest/test_docs.py b/doctest/test_docs.py index 1fedbdf49e..eacee34652 100644 --- a/doctest/test_docs.py +++ b/doctest/test_docs.py @@ -28,6 +28,7 @@ APACHE = "apache" WILDCARD = "wildcard" NESTED = "nested" +GEOPOINT = "geopoint" DATASOURCES = ".ql-datasources" @@ -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) diff --git a/doctest/test_mapping/geopoint.json b/doctest/test_mapping/geopoint.json new file mode 100644 index 0000000000..8dc92fe5c4 --- /dev/null +++ b/doctest/test_mapping/geopoint.json @@ -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" + } + } + } +} From 72d8beb186a53ac6ee4d1cb93290ee1575a012e6 Mon Sep 17 00:00:00 2001 From: Yury-Fridlyand Date: Tue, 25 Jul 2023 14:15:26 -0700 Subject: [PATCH 20/30] Fix doctest clean up. Signed-off-by: Yury-Fridlyand --- doctest/test_docs.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doctest/test_docs.py b/doctest/test_docs.py index eacee34652..93953cf73b 100644 --- a/doctest/test_docs.py +++ b/doctest/test_docs.py @@ -128,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, From 2bbda8f418c055586b1ae2fbb04a3fc5bf6b4210 Mon Sep 17 00:00:00 2001 From: Guian Gumpac Date: Tue, 25 Jul 2023 15:48:07 -0700 Subject: [PATCH 21/30] Minimized doc example Signed-off-by: Guian Gumpac --- doctest/test_data/geopoint.json | 6 +----- doctest/test_mapping/geopoint.json | 23 ----------------------- 2 files changed, 1 insertion(+), 28 deletions(-) diff --git a/doctest/test_data/geopoint.json b/doctest/test_data/geopoint.json index 4ba2d27828..b21d58e200 100644 --- a/doctest/test_data/geopoint.json +++ b/doctest/test_data/geopoint.json @@ -1,6 +1,2 @@ {"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} +{"geo_point_object": {"lat": 40.71, "lon": 74.00}} diff --git a/doctest/test_mapping/geopoint.json b/doctest/test_mapping/geopoint.json index 8dc92fe5c4..034ad6184d 100644 --- a/doctest/test_mapping/geopoint.json +++ b/doctest/test_mapping/geopoint.json @@ -3,29 +3,6 @@ "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" } } } From dcc1133e0ec6e110b9bed6798afa1cf873f45e4c Mon Sep 17 00:00:00 2001 From: Guian Gumpac Date: Tue, 25 Jul 2023 17:07:50 -0700 Subject: [PATCH 22/30] Fixed doctest Signed-off-by: Guian Gumpac --- doctest/test_data/geopoint.json | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/doctest/test_data/geopoint.json b/doctest/test_data/geopoint.json index b21d58e200..3487e41353 100644 --- a/doctest/test_data/geopoint.json +++ b/doctest/test_data/geopoint.json @@ -1,2 +1 @@ -{"index":{"_id":"1"}} -{"geo_point_object": {"lat": 40.71, "lon": 74.00}} +{"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]}} From 5ccc14f844aaa3a17a042d6cfa8293ec9e9efca8 Mon Sep 17 00:00:00 2001 From: Guian Gumpac Date: Tue, 25 Jul 2023 17:31:13 -0700 Subject: [PATCH 23/30] Fixed doctest Signed-off-by: Guian Gumpac --- docs/user/dql/metadata.rst | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/user/dql/metadata.rst b/docs/user/dql/metadata.rst index a02bcf096a..ffee10c817 100644 --- a/docs/user/dql/metadata.rst +++ b/docs/user/dql/metadata.rst @@ -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 | From 1e0788616f4da8a123c32bb206c3858d06f6d1d2 Mon Sep 17 00:00:00 2001 From: Guian Gumpac Date: Wed, 26 Jul 2023 07:11:06 -0700 Subject: [PATCH 24/30] Fixed doctest Signed-off-by: Guian Gumpac --- docs/user/dql/metadata.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/user/dql/metadata.rst b/docs/user/dql/metadata.rst index ffee10c817..60b8af853b 100644 --- a/docs/user/dql/metadata.rst +++ b/docs/user/dql/metadata.rst @@ -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 | |----------------+---------------+-----------------+--------------+-----------+------------+--------------+-------------+-----------------------------+------------------| From 3192f52dc1b3cfa0569fe24c938e1061e659a2c6 Mon Sep 17 00:00:00 2001 From: Guian Gumpac Date: Wed, 26 Jul 2023 07:35:56 -0700 Subject: [PATCH 25/30] Fixed doctest Signed-off-by: Guian Gumpac --- docs/user/general/datatypes.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/user/general/datatypes.rst b/docs/user/general/datatypes.rst index 5b6b0c0594..e2a831afa2 100644 --- a/docs/user/general/datatypes.rst +++ b/docs/user/general/datatypes.rst @@ -469,5 +469,5 @@ A geopoint has a latitude and a longitude property. Although OpenSearch `support +-----------------------------+------------------------+------------------------+ | geo_point_object | geo_point_object.lat | geo_point_object.lon | |-----------------------------+------------------------+------------------------| - | {'lon': 74.0, 'lat': 40.71} | 40.71 | 74.0 | + | {'lat': 40.71, 'lon': 74.0} | 40.71 | 74.0 | +-----------------------------+------------------------+------------------------+ From b474b301a54e97c484d4b81df10b136c5470cfec Mon Sep 17 00:00:00 2001 From: Guian Gumpac Date: Wed, 26 Jul 2023 08:48:48 -0700 Subject: [PATCH 26/30] Fixed reordering of results Signed-off-by: Guian Gumpac --- .../opensearch/data/value/OpenSearchExprGeoPointValue.java | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprGeoPointValue.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprGeoPointValue.java index 98a271d8ba..a15272f594 100644 --- a/opensearch/src/main/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprGeoPointValue.java +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprGeoPointValue.java @@ -6,11 +6,10 @@ package org.opensearch.sql.opensearch.data.value; +import com.google.common.collect.ImmutableMap; import java.util.LinkedHashMap; -import java.util.Map; import org.opensearch.sql.data.model.ExprDoubleValue; import org.opensearch.sql.data.model.ExprTupleValue; -import org.opensearch.sql.data.model.ExprValue; import org.opensearch.sql.data.type.ExprType; import org.opensearch.sql.opensearch.data.type.OpenSearchGeoPointType; @@ -26,7 +25,7 @@ public class OpenSearchExprGeoPointValue extends ExprTupleValue { * @param lon double value of longitude property of geo_point */ public OpenSearchExprGeoPointValue(Double lat, Double lon) { - super(new LinkedHashMap<>(Map.of( + super(new LinkedHashMap<>(ImmutableMap.of( "lat", new ExprDoubleValue(lat), "lon", new ExprDoubleValue(lon)))); } From 364e63a85fceaeae6e75bcb9719fdc18cc3ce91d Mon Sep 17 00:00:00 2001 From: Guian Gumpac Date: Wed, 26 Jul 2023 09:15:20 -0700 Subject: [PATCH 27/30] Added more rows to doctests and removed IOException: Signed-off-by: Guian Gumpac --- docs/user/general/datatypes.rst | 12 +++++++----- doctest/test_data/geopoint.json | 4 +++- .../java/org/opensearch/sql/sql/GeoPointIT.java | 15 +++++++-------- 3 files changed, 17 insertions(+), 14 deletions(-) diff --git a/docs/user/general/datatypes.rst b/docs/user/general/datatypes.rst index e2a831afa2..6c79d6c6e6 100644 --- a/docs/user/general/datatypes.rst +++ b/docs/user/general/datatypes.rst @@ -466,8 +466,10 @@ A geopoint has a latitude and a longitude property. Although OpenSearch `support os> SELECT geo_point_object, geo_point_object.lat, geo_point_object.lon FROM geopoint; fetched rows / total rows = 1/1 - +-----------------------------+------------------------+------------------------+ - | geo_point_object | geo_point_object.lat | geo_point_object.lon | - |-----------------------------+------------------------+------------------------| - | {'lat': 40.71, 'lon': 74.0} | 40.71 | 74.0 | - +-----------------------------+------------------------+------------------------+ + +----------------------------------+------------------------+------------------------+ + | 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 | + +----------------------------------+------------------------+------------------------+ diff --git a/doctest/test_data/geopoint.json b/doctest/test_data/geopoint.json index 3487e41353..26f49506fb 100644 --- a/doctest/test_data/geopoint.json +++ b/doctest/test_data/geopoint.json @@ -1 +1,3 @@ -{"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]}} +{"geo_point_object": {"lat": 40.71, "lon": 74.00}} +{"geo_point_object": {"lat": -33.852, "lon": 151.216}} +{"geo_point_object": null} diff --git a/integ-test/src/test/java/org/opensearch/sql/sql/GeoPointIT.java b/integ-test/src/test/java/org/opensearch/sql/sql/GeoPointIT.java index aa798e0632..e738418290 100644 --- a/integ-test/src/test/java/org/opensearch/sql/sql/GeoPointIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/sql/GeoPointIT.java @@ -9,7 +9,6 @@ import org.junit.Test; import org.opensearch.sql.legacy.SQLIntegTestCase; -import java.io.IOException; import java.util.Map; import static org.opensearch.sql.legacy.TestsConstants.TEST_INDEX_GEOPOINT; @@ -23,7 +22,7 @@ protected void init() throws Exception { } @Test - public void test_geo_point() throws IOException { + public void test_geo_point() { String query = "SELECT geo_point_object FROM " + TEST_INDEX_GEOPOINT; JSONObject result = executeJdbcRequest(query); verifyDataRows(result, @@ -38,7 +37,7 @@ public void test_geo_point() throws IOException { } @Test - public void test_geo_point_unsupported_format() throws IOException { + public void test_geo_point_unsupported_format() { String query = "SELECT geo_point_geohash FROM " + TEST_INDEX_GEOPOINT; Exception exception = assertThrows(RuntimeException.class, () -> executeJdbcRequest(query)); @@ -53,7 +52,7 @@ public void test_geo_point_unsupported_format() throws IOException { } @Test - public void test_geo_point_in_objects() throws IOException { + public void test_geo_point_in_objects() { String query = "SELECT object.geo_point_object FROM " + TEST_INDEX_GEOPOINT; JSONObject result = executeJdbcRequest(query); verifyDataRows(result, @@ -69,7 +68,7 @@ public void test_geo_point_in_objects() throws IOException { } @Test - public void test_geo_point_lat_in_objects() throws IOException { + 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, @@ -80,7 +79,7 @@ public void test_geo_point_lat_in_objects() throws IOException { } @Test - public void test_geo_point_lat_and_lon() throws IOException { + 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, @@ -91,7 +90,7 @@ public void test_geo_point_lat_and_lon() throws IOException { } @Test - public void test_geo_point_object_with_lat_and_lon() throws IOException { + 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); @@ -109,7 +108,7 @@ public void test_geo_point_object_with_lat_and_lon() throws IOException { } @Test - public void test_geo_point_lat_in_functions() throws IOException { + 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, From 2f5f759cc10a16c9c779b1476de54937e521b89e Mon Sep 17 00:00:00 2001 From: Guian Gumpac Date: Wed, 26 Jul 2023 11:37:43 -0700 Subject: [PATCH 28/30] Fixed doctest Signed-off-by: Guian Gumpac --- docs/user/general/datatypes.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/user/general/datatypes.rst b/docs/user/general/datatypes.rst index 6c79d6c6e6..6827d7108c 100644 --- a/docs/user/general/datatypes.rst +++ b/docs/user/general/datatypes.rst @@ -465,7 +465,7 @@ Geopoint Data Types A geopoint has a latitude and a longitude property. Although OpenSearch `supports multiple formats `_, 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 = 1/1 + fetched rows / total rows = 3/3 +----------------------------------+------------------------+------------------------+ | geo_point_object | geo_point_object.lat | geo_point_object.lon | |----------------------------------+------------------------+------------------------| From 6ea102dc37021c3e230cae8b48005db1f36445ff Mon Sep 17 00:00:00 2001 From: Guian Gumpac Date: Wed, 26 Jul 2023 11:48:41 -0700 Subject: [PATCH 29/30] Fixed doctest Signed-off-by: Guian Gumpac --- docs/user/general/datatypes.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/user/general/datatypes.rst b/docs/user/general/datatypes.rst index 6827d7108c..fcb0c4accd 100644 --- a/docs/user/general/datatypes.rst +++ b/docs/user/general/datatypes.rst @@ -470,6 +470,6 @@ A geopoint has a latitude and a longitude property. Although OpenSearch `support | 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 | + | {'lat': -33.852, 'lon': 151.216} | -33.852 | 151.216 | | null | null | null | +----------------------------------+------------------------+------------------------+ From cc54f39106d15d39c1f9b2e947429eacffe20731 Mon Sep 17 00:00:00 2001 From: Guian Gumpac Date: Thu, 27 Jul 2023 10:17:37 -0700 Subject: [PATCH 30/30] Addressed PR comments Signed-off-by: Guian Gumpac --- .../org/opensearch/sql/data/type/ExprCoreType.java | 7 +------ .../java/org/opensearch/sql/analysis/AnalyzerTest.java | 2 +- .../sql/opensearch/data/type/OpenSearchDataType.java | 2 +- .../opensearch/data/type/OpenSearchGeoPointType.java | 4 ++-- .../opensearch/client/OpenSearchNodeClientTest.java | 2 +- .../opensearch/client/OpenSearchRestClientTest.java | 2 +- .../opensearch/data/type/OpenSearchDataTypeTest.java | 3 +-- .../data/value/OpenSearchExprGeoPointValueTest.java | 10 +++++++++- 8 files changed, 17 insertions(+), 15 deletions(-) diff --git a/core/src/main/java/org/opensearch/sql/data/type/ExprCoreType.java b/core/src/main/java/org/opensearch/sql/data/type/ExprCoreType.java index b58f5477c2..815f94a9df 100644 --- a/core/src/main/java/org/opensearch/sql/data/type/ExprCoreType.java +++ b/core/src/main/java/org/opensearch/sql/data/type/ExprCoreType.java @@ -68,12 +68,7 @@ public enum ExprCoreType implements ExprType { /** * Array. */ - ARRAY(UNDEFINED), - - /** - * GEO_POINT. - */ - GEO_POINT(UNDEFINED); + ARRAY(UNDEFINED); /** * Parents (wider/compatible types) of current base type. diff --git a/core/src/test/java/org/opensearch/sql/analysis/AnalyzerTest.java b/core/src/test/java/org/opensearch/sql/analysis/AnalyzerTest.java index d75fd3c039..100cfd67af 100644 --- a/core/src/test/java/org/opensearch/sql/analysis/AnalyzerTest.java +++ b/core/src/test/java/org/opensearch/sql/analysis/AnalyzerTest.java @@ -165,7 +165,7 @@ public void filter_relation_with_invalid_qualifiedName_ExpressionEvaluationExcep "= function expected {[BYTE,BYTE],[SHORT,SHORT],[INTEGER,INTEGER],[LONG,LONG]," + "[FLOAT,FLOAT],[DOUBLE,DOUBLE],[STRING,STRING],[BOOLEAN,BOOLEAN],[DATE,DATE]," + "[TIME,TIME],[DATETIME,DATETIME],[TIMESTAMP,TIMESTAMP],[INTERVAL,INTERVAL]," - + "[STRUCT,STRUCT],[ARRAY,ARRAY],[GEO_POINT,GEO_POINT]}, but get [STRING,INTEGER]", + + "[STRUCT,STRUCT],[ARRAY,ARRAY]}, but get [STRING,INTEGER]", exception.getMessage()); } diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/data/type/OpenSearchDataType.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/data/type/OpenSearchDataType.java index 612e4c0bce..770518e54d 100644 --- a/opensearch/src/main/java/org/opensearch/sql/opensearch/data/type/OpenSearchDataType.java +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/data/type/OpenSearchDataType.java @@ -32,7 +32,7 @@ public enum MappingType { Text("text", ExprCoreType.UNKNOWN), Keyword("keyword", ExprCoreType.STRING), Ip("ip", ExprCoreType.UNKNOWN), - GeoPoint("geo_point", ExprCoreType.GEO_POINT), + GeoPoint("geo_point", ExprCoreType.UNKNOWN), Binary("binary", ExprCoreType.UNKNOWN), Date("date", ExprCoreType.TIMESTAMP), Object("object", ExprCoreType.STRUCT), diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/data/type/OpenSearchGeoPointType.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/data/type/OpenSearchGeoPointType.java index 1848596686..4bccee43ab 100644 --- a/opensearch/src/main/java/org/opensearch/sql/opensearch/data/type/OpenSearchGeoPointType.java +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/data/type/OpenSearchGeoPointType.java @@ -6,7 +6,7 @@ package org.opensearch.sql.opensearch.data.type; -import static org.opensearch.sql.data.type.ExprCoreType.GEO_POINT; +import static org.opensearch.sql.data.type.ExprCoreType.UNKNOWN; import java.util.HashMap; import java.util.Map; @@ -25,7 +25,7 @@ public class OpenSearchGeoPointType extends OpenSearchDataType { private OpenSearchGeoPointType() { super(MappingType.GeoPoint); - exprCoreType = GEO_POINT; + exprCoreType = UNKNOWN; this.properties = new HashMap<>(); this.properties.put("lat", new OpenSearchDataType(ExprCoreType.DOUBLE)); this.properties.put("lon", new OpenSearchDataType(ExprCoreType.DOUBLE)); diff --git a/opensearch/src/test/java/org/opensearch/sql/opensearch/client/OpenSearchNodeClientTest.java b/opensearch/src/test/java/org/opensearch/sql/opensearch/client/OpenSearchNodeClientTest.java index 9fce7a0b5d..ad4b92cc9f 100644 --- a/opensearch/src/test/java/org/opensearch/sql/opensearch/client/OpenSearchNodeClientTest.java +++ b/opensearch/src/test/java/org/opensearch/sql/opensearch/client/OpenSearchNodeClientTest.java @@ -169,7 +169,7 @@ void get_index_mappings() throws IOException { var parsedTypes = OpenSearchDataType.traverseAndFlatten(mapping); assertAll( () -> assertEquals(1, indexMappings.size()), - // 10 types extended to 17 after flattening + // 10 types extended to 19 after flattening () -> assertEquals(10, mapping.size()), () -> assertEquals(19, parsedTypes.size()), () -> assertEquals("TEXT", mapping.get("address").legacyTypeName()), diff --git a/opensearch/src/test/java/org/opensearch/sql/opensearch/client/OpenSearchRestClientTest.java b/opensearch/src/test/java/org/opensearch/sql/opensearch/client/OpenSearchRestClientTest.java index fedf07f0d2..e339baf0ac 100644 --- a/opensearch/src/test/java/org/opensearch/sql/opensearch/client/OpenSearchRestClientTest.java +++ b/opensearch/src/test/java/org/opensearch/sql/opensearch/client/OpenSearchRestClientTest.java @@ -170,7 +170,7 @@ void get_index_mappings() throws IOException { var parsedTypes = OpenSearchDataType.traverseAndFlatten(mapping); assertAll( () -> assertEquals(1, indexMappings.size()), - // 10 types extended to 17 after flattening + // 10 types extended to 19 after flattening () -> assertEquals(10, mapping.size()), () -> assertEquals(19, parsedTypes.size()), () -> assertEquals("TEXT", mapping.get("address").legacyTypeName()), diff --git a/opensearch/src/test/java/org/opensearch/sql/opensearch/data/type/OpenSearchDataTypeTest.java b/opensearch/src/test/java/org/opensearch/sql/opensearch/data/type/OpenSearchDataTypeTest.java index c7e8055b45..20e6f5eeb4 100644 --- a/opensearch/src/test/java/org/opensearch/sql/opensearch/data/type/OpenSearchDataTypeTest.java +++ b/opensearch/src/test/java/org/opensearch/sql/opensearch/data/type/OpenSearchDataTypeTest.java @@ -22,7 +22,6 @@ import static org.opensearch.sql.data.type.ExprCoreType.DATE; import static org.opensearch.sql.data.type.ExprCoreType.DOUBLE; import static org.opensearch.sql.data.type.ExprCoreType.FLOAT; -import static org.opensearch.sql.data.type.ExprCoreType.GEO_POINT; import static org.opensearch.sql.data.type.ExprCoreType.INTEGER; import static org.opensearch.sql.data.type.ExprCoreType.LONG; import static org.opensearch.sql.data.type.ExprCoreType.SHORT; @@ -110,7 +109,7 @@ private static Stream getTestDataWithType() { Arguments.of(MappingType.Object, "object", STRUCT), Arguments.of(MappingType.Nested, "nested", ARRAY), Arguments.of(MappingType.GeoPoint, "geo_point", - GEO_POINT), + OpenSearchGeoPointType.of()), Arguments.of(MappingType.Binary, "binary", OpenSearchBinaryType.of()), Arguments.of(MappingType.Ip, "ip", diff --git a/opensearch/src/test/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprGeoPointValueTest.java b/opensearch/src/test/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprGeoPointValueTest.java index 2b78c0d352..38e0b29702 100644 --- a/opensearch/src/test/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprGeoPointValueTest.java +++ b/opensearch/src/test/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprGeoPointValueTest.java @@ -8,15 +8,23 @@ import static org.junit.jupiter.api.Assertions.assertEquals; +import com.google.common.collect.ImmutableMap; +import java.util.LinkedHashMap; import org.junit.jupiter.api.Test; import org.opensearch.sql.opensearch.data.type.OpenSearchGeoPointType; class OpenSearchExprGeoPointValueTest { - private OpenSearchExprGeoPointValue geoPointValue = new OpenSearchExprGeoPointValue(1.0, 1.0); + private OpenSearchExprGeoPointValue geoPointValue = new OpenSearchExprGeoPointValue(1.0, 2.0); @Test void type() { assertEquals(OpenSearchGeoPointType.of(), geoPointValue.type()); } + + @Test + void value() { + assertEquals(new LinkedHashMap<>(ImmutableMap.of("lat", 1.0, "lon", 2.0)), + geoPointValue.value()); + } }