From 6fa239b9a90641a8c8f875913b0d214cd79a6421 Mon Sep 17 00:00:00 2001 From: Vamsi Manohar Date: Wed, 17 Sep 2025 18:37:03 -0700 Subject: [PATCH 1/2] Fix geopoiint issue in complex data types (#4325) Signed-off-by: Vamsi Manohar (cherry picked from commit d527bbd93cef38b0b92ce80e2e86d0a4051997e8) --- .../remote/CalciteGeoPointFormatsIT.java | 14 -- .../sql/legacy/SQLIntegTestCase.java | 5 + .../org/opensearch/sql/legacy/TestUtils.java | 5 + .../opensearch/sql/legacy/TestsConstants.java | 1 + .../opensearch/sql/ppl/GeoPointFormatsIT.java | 159 ++++++++++++++++++ .../src/test/resources/complex_geo.json | 12 ++ .../complex_geo_index_mapping.json | 73 ++++++++ .../util/JdbcOpenSearchDataTypeConvertor.java | 40 ++++- 8 files changed, 294 insertions(+), 15 deletions(-) create mode 100644 integ-test/src/test/resources/complex_geo.json create mode 100644 integ-test/src/test/resources/indexDefinitions/complex_geo_index_mapping.json diff --git a/integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteGeoPointFormatsIT.java b/integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteGeoPointFormatsIT.java index d03b6fab9a2..d1bcfefceb4 100644 --- a/integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteGeoPointFormatsIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteGeoPointFormatsIT.java @@ -5,7 +5,6 @@ package org.opensearch.sql.calcite.remote; -import org.junit.Ignore; import org.opensearch.sql.ppl.GeoPointFormatsIT; import java.io.IOException; @@ -16,17 +15,4 @@ public void init() throws Exception { super.init(); enableCalcite(); } - - @Override - public void testReadingGeoHash() throws IOException { - withFallbackEnabled( - () -> { - try { - super.testReadingGeoHash(); - } catch (IOException e) { - throw new RuntimeException(e); - } - }, - "Need to support metadata, https://github.com/opensearch-project/sql/issues/3333"); - } } 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 5530dd678d7..f1ce9ac0636 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 @@ -781,6 +781,11 @@ public enum Index { "dates", getGeopointIndexMapping(), "src/test/resources/geopoints.json"), + COMPLEX_GEO( + TestsConstants.TEST_INDEX_COMPLEX_GEO, + "complex_geo", + getComplexGeoIndexMapping(), + "src/test/resources/complex_geo.json"), STATE_COUNTRY( TestsConstants.TEST_INDEX_STATE_COUNTRY, "state_country", diff --git a/integ-test/src/test/java/org/opensearch/sql/legacy/TestUtils.java b/integ-test/src/test/java/org/opensearch/sql/legacy/TestUtils.java index 6df29273b6a..de0d99434c2 100644 --- a/integ-test/src/test/java/org/opensearch/sql/legacy/TestUtils.java +++ b/integ-test/src/test/java/org/opensearch/sql/legacy/TestUtils.java @@ -254,6 +254,11 @@ public static String getGeopointIndexMapping() { return getMappingFile(mappingFile); } + public static String getComplexGeoIndexMapping() { + String mappingFile = "complex_geo_index_mapping.json"; + return getMappingFile(mappingFile); + } + public static String getStateCountryIndexMapping() { String mappingFile = "state_country_index_mapping.json"; return getMappingFile(mappingFile); 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 1aaaf62e012..071ff724705 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 static final String TEST_INDEX_MULTI_NESTED_TYPE = TEST_INDEX + "_multi_nested"; public static final String TEST_INDEX_NESTED_WITH_NULLS = TEST_INDEX + "_nested_with_nulls"; public static final String TEST_INDEX_GEOPOINT = TEST_INDEX + "_geopoint"; + public static final String TEST_INDEX_COMPLEX_GEO = TEST_INDEX + "_complex_geo"; public static final String TEST_INDEX_JSON_TEST = TEST_INDEX + "_json_test"; public static final String TEST_INDEX_ALIAS = TEST_INDEX + "_alias"; public static final String TEST_INDEX_FLATTENED_VALUE = TEST_INDEX + "_flattened_value"; diff --git a/integ-test/src/test/java/org/opensearch/sql/ppl/GeoPointFormatsIT.java b/integ-test/src/test/java/org/opensearch/sql/ppl/GeoPointFormatsIT.java index c97a0a434d9..3f2c6ccbb62 100644 --- a/integ-test/src/test/java/org/opensearch/sql/ppl/GeoPointFormatsIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/ppl/GeoPointFormatsIT.java @@ -5,6 +5,7 @@ package org.opensearch.sql.ppl; +import static org.opensearch.sql.legacy.TestsConstants.TEST_INDEX_COMPLEX_GEO; 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.schema; @@ -14,6 +15,7 @@ import java.io.IOException; import java.util.Map; import org.apache.commons.lang3.tuple.Pair; +import org.json.JSONArray; import org.json.JSONObject; import org.junit.jupiter.api.Test; import org.opensearch.sql.sql.GeopointFormatsIT; @@ -23,6 +25,7 @@ public class GeoPointFormatsIT extends PPLIntegTestCase { public void init() throws Exception { super.init(); loadIndex(Index.GEOPOINTS); + loadIndex(Index.COMPLEX_GEO); } @Test @@ -51,4 +54,160 @@ public void testReadingGeoHash() throws IOException { assertEquals(40.71, point.getLeft(), GeopointFormatsIT.TOLERANCE); assertEquals(74, point.getRight(), GeopointFormatsIT.TOLERANCE); } + + // Complex geo tests - geo points within complex types (Maps) + @Test + public void testGeoPointInSimpleMap() throws IOException { + String query = + String.format( + "search source=%s | where id = '1' | fields location", TEST_INDEX_COMPLEX_GEO); + + JSONObject result = executeQuery(query); + verifySchema(result, schema("location", null, "struct")); + + // Verify the map contains the geo point properly converted + // Using exact precision from complex_geo.json: {"lat": 47.6062, "lon": -122.3321} + verifyDataRows( + result, + rows( + Map.of( + "name", "Seattle Office", + "point", Map.of("lat", 47.6062, "lon", -122.3321), + "city", "Seattle", + "country", "USA"))); + } + + @Test + public void testGeoPointInMapWithStringFormat() throws IOException { + String query = + String.format( + "search source=%s | where id = '2' | fields location", TEST_INDEX_COMPLEX_GEO); + + JSONObject result = executeQuery(query); + verifySchema(result, schema("location", null, "struct")); + + // Verify the map contains geo point parsed from string format + // Using exact precision from complex_geo.json: "35.6762,139.6503" + verifyDataRows( + result, + rows( + Map.of( + "name", "Tokyo Office", + "point", Map.of("lat", 35.6762, "lon", 139.6503), + "city", "Tokyo", + "country", "Japan"))); + } + + @Test + public void testNestedMapsWithGeoPoints() throws IOException { + String query = + String.format( + "search source=%s | where id = '3' | fields nested_locations", TEST_INDEX_COMPLEX_GEO); + + JSONObject result = executeQuery(query); + verifySchema(result, schema("nested_locations", null, "struct")); + + // Verify nested structure with multiple geo points + // Using exact precision from complex_geo.json + verifyDataRows( + result, + rows( + Map.of( + "primary", + Map.of( + "office", Map.of("lat", 37.7749, "lon", -122.4194), + "warehouse", Map.of("lat", 37.4419, "lon", -122.143)), + "secondary", + Map.of( + "branch", Map.of("lat", 37.3382, "lon", -121.8863), + "store", Map.of("lat", 37.3688, "lon", -122.0363))))); + } + + @Test + public void testNestedMapsWithStringGeoPoints() throws IOException { + String query = + String.format( + "search source=%s | where id = '4' | fields nested_locations", TEST_INDEX_COMPLEX_GEO); + + JSONObject result = executeQuery(query); + verifySchema(result, schema("nested_locations", null, "struct")); + + // Verify nested structure with geo points in string format + // Using exact precision from complex_geo.json: "40.7128,-74.0060" etc. + verifyDataRows( + result, + rows( + Map.of( + "primary", + Map.of( + "office", Map.of("lat", 40.7128, "lon", -74.006), + "warehouse", Map.of("lat", 40.758, "lon", -73.9855)), + "secondary", + Map.of( + "branch", Map.of("lat", 40.7489, "lon", -73.968), + "store", Map.of("lat", 40.7614, "lon", -73.9776))))); + } + + @Test + public void testMultipleOfficesWithGeoPoints() throws IOException { + String query = + String.format( + "search source=%s | where id = '5' | fields multiple_offices", TEST_INDEX_COMPLEX_GEO); + + JSONObject result = executeQuery(query); + verifySchema(result, schema("multiple_offices", null, "struct")); + + // Verify multiple offices structure + verifyDataRows( + result, + rows( + Map.of( + "headquarters", + Map.of( + "location", Map.of("lat", 51.5074, "lon", -0.1278), "address", "London HQ"), + "regional", + Map.of( + "location", + Map.of("lat", 48.8566, "lon", 2.3522), + "address", + "Paris Regional")))); + } + + @Test + public void testGeoHashInMap() throws IOException { + String query = + String.format( + "search source=%s | where id = '6' | fields location", TEST_INDEX_COMPLEX_GEO); + + JSONObject result = executeQuery(query); + verifySchema(result, schema("location", null, "struct")); + + // Verify geo point converted from geohash "u33dc0cpke7v" + // Using tolerance for geohash conversion which has precision variations + JSONArray dataRows = (JSONArray) result.get("datarows"); + JSONObject row = (JSONObject) ((JSONArray) dataRows.get(0)).get(0); + + assertEquals("Berlin Office", row.getString("name")); + assertEquals("Berlin", row.getString("city")); + assertEquals("Germany", row.getString("country")); + + JSONObject point = row.getJSONObject("point"); + double lat = point.getDouble("lat"); + double lon = point.getDouble("lon"); + + // Expected values from geohash decoding with tolerance + assertEquals(52.52003, lat, GeopointFormatsIT.TOLERANCE); + assertEquals(13.40489, lon, GeopointFormatsIT.TOLERANCE); + } + + @Test + public void testComplexGeoAllDocumentsQuery() throws IOException { + String query = String.format("search source=%s | fields id | sort id", TEST_INDEX_COMPLEX_GEO); + + JSONObject result = executeQuery(query); + verifySchema(result, schema("id", null, "string")); + + // Verify all documents are indexed and queryable + verifyDataRows(result, rows("1"), rows("2"), rows("3"), rows("4"), rows("5"), rows("6")); + } } diff --git a/integ-test/src/test/resources/complex_geo.json b/integ-test/src/test/resources/complex_geo.json new file mode 100644 index 00000000000..59adec87b55 --- /dev/null +++ b/integ-test/src/test/resources/complex_geo.json @@ -0,0 +1,12 @@ +{"index": {"_id": "1"}} +{"id": "1", "location": {"name": "Seattle Office", "point": {"lat": 47.6062, "lon": -122.3321}, "city": "Seattle", "country": "USA"}} +{"index": {"_id": "2"}} +{"id": "2", "location": {"name": "Tokyo Office", "point": "35.6762,139.6503", "city": "Tokyo", "country": "Japan"}} +{"index": {"_id": "3"}} +{"id": "3", "nested_locations": {"primary": {"office": {"lat": 37.7749, "lon": -122.4194}, "warehouse": {"lat": 37.4419, "lon": -122.1430}}, "secondary": {"branch": {"lat": 37.3382, "lon": -121.8863}, "store": {"lat": 37.3688, "lon": -122.0363}}}} +{"index": {"_id": "4"}} +{"id": "4", "nested_locations": {"primary": {"office": "40.7128,-74.0060", "warehouse": "40.7580,-73.9855"}, "secondary": {"branch": "40.7489,-73.9680", "store": "40.7614,-73.9776"}}} +{"index": {"_id": "5"}} +{"id": "5", "multiple_offices": {"headquarters": {"location": {"lat": 51.5074, "lon": -0.1278}, "address": "London HQ"}, "regional": {"location": {"lat": 48.8566, "lon": 2.3522}, "address": "Paris Regional"}}} +{"index": {"_id": "6"}} +{"id": "6", "location": {"name": "Berlin Office", "point": "u33dc0cpke7v", "city": "Berlin", "country": "Germany"}} diff --git a/integ-test/src/test/resources/indexDefinitions/complex_geo_index_mapping.json b/integ-test/src/test/resources/indexDefinitions/complex_geo_index_mapping.json new file mode 100644 index 00000000000..10b855f6162 --- /dev/null +++ b/integ-test/src/test/resources/indexDefinitions/complex_geo_index_mapping.json @@ -0,0 +1,73 @@ +{ + "mappings": { + "properties": { + "id": { + "type": "keyword" + }, + "location": { + "properties": { + "name": { + "type": "keyword" + }, + "point": { + "type": "geo_point" + }, + "city": { + "type": "keyword" + }, + "country": { + "type": "keyword" + } + } + }, + "nested_locations": { + "properties": { + "primary": { + "properties": { + "office": { + "type": "geo_point" + }, + "warehouse": { + "type": "geo_point" + } + } + }, + "secondary": { + "properties": { + "branch": { + "type": "geo_point" + }, + "store": { + "type": "geo_point" + } + } + } + } + }, + "multiple_offices": { + "properties": { + "headquarters": { + "properties": { + "location": { + "type": "geo_point" + }, + "address": { + "type": "text" + } + } + }, + "regional": { + "properties": { + "location": { + "type": "geo_point" + }, + "address": { + "type": "text" + } + } + } + } + } + } + } +} \ No newline at end of file diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/util/JdbcOpenSearchDataTypeConvertor.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/util/JdbcOpenSearchDataTypeConvertor.java index 7e026fac829..6f076f14628 100644 --- a/opensearch/src/main/java/org/opensearch/sql/opensearch/util/JdbcOpenSearchDataTypeConvertor.java +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/util/JdbcOpenSearchDataTypeConvertor.java @@ -10,6 +10,8 @@ import java.sql.SQLException; import java.sql.Types; import java.util.Arrays; +import java.util.HashMap; +import java.util.Map; import lombok.experimental.UtilityClass; import org.locationtech.jts.geom.Point; import org.apache.calcite.avatica.util.ArrayImpl; @@ -129,11 +131,47 @@ public static ExprValue getExprValueFromSqlType( "Unchecked sql type: {}, return Object type {}", sqlType, value.getClass().getTypeName()); - return ExprValueUtils.fromObjectValue(value); + return convertComplexValue(value); } } catch (SQLException e) { LOG.error("Error converting SQL type {}: {}", sqlType, e.getMessage()); throw e; } } + + /** + * Convert complex values like Maps that may contain geo points. This method recursively processes + * Maps to handle nested geo points and converts them to appropriate ExprValue representations. + */ + private static ExprValue convertComplexValue(Object value) { + Object converted = processValue(value); + return ExprValueUtils.fromObjectValue(converted); + } + + /** + * Process values recursively, handling geo points and nested maps. Geo points are converted to + * OpenSearchExprGeoPointValue. Maps are recursively processed to handle nested structures. + */ + private static Object processValue(Object value) { + if (value == null) { + return null; + } + + if (value instanceof Point) { + Point point = (Point) value; + return new OpenSearchExprGeoPointValue(point.getY(), point.getX()); + } + + if (value instanceof Map) { + Map map = (Map) value; + Map convertedMap = new HashMap<>(); + for (Map.Entry entry : map.entrySet()) { + convertedMap.put(entry.getKey(), processValue(entry.getValue())); + } + return convertedMap; + } + + // For other types, return as-is + return value; + } } From 19248673d6e7a736f23d972d9b0bf5c6fb14ee63 Mon Sep 17 00:00:00 2001 From: Vamsi Manohar Date: Wed, 17 Sep 2025 18:57:07 -0700 Subject: [PATCH 2/2] 2.19-dev fix Signed-off-by: Vamsi Manohar --- .../sql/legacy/SQLIntegTestCase.java | 46 +------------------ 1 file changed, 1 insertion(+), 45 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 f1ce9ac0636..b86e6b22c84 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 @@ -6,51 +6,7 @@ package org.opensearch.sql.legacy; import static com.google.common.base.Strings.isNullOrEmpty; -import static org.opensearch.sql.legacy.TestUtils.createIndexByRestClient; -import static org.opensearch.sql.legacy.TestUtils.getAccountIndexMapping; -import static org.opensearch.sql.legacy.TestUtils.getAliasIndexMapping; -import static org.opensearch.sql.legacy.TestUtils.getArrayIndexMapping; -import static org.opensearch.sql.legacy.TestUtils.getBankIndexMapping; -import static org.opensearch.sql.legacy.TestUtils.getBankWithNullValuesIndexMapping; -import static org.opensearch.sql.legacy.TestUtils.getBig5MappingFile; -import static org.opensearch.sql.legacy.TestUtils.getClickBenchMappingFile; -import static org.opensearch.sql.legacy.TestUtils.getDataTypeNonnumericIndexMapping; -import static org.opensearch.sql.legacy.TestUtils.getDataTypeNumericIndexMapping; -import static org.opensearch.sql.legacy.TestUtils.getDateIndexMapping; -import static org.opensearch.sql.legacy.TestUtils.getDateTimeIndexMapping; -import static org.opensearch.sql.legacy.TestUtils.getDeepNestedIndexMapping; -import static org.opensearch.sql.legacy.TestUtils.getDogIndexMapping; -import static org.opensearch.sql.legacy.TestUtils.getDogs2IndexMapping; -import static org.opensearch.sql.legacy.TestUtils.getDogs3IndexMapping; -import static org.opensearch.sql.legacy.TestUtils.getDuplicationNullableIndexMapping; -import static org.opensearch.sql.legacy.TestUtils.getEmployeeNestedTypeIndexMapping; -import static org.opensearch.sql.legacy.TestUtils.getGameOfThronesIndexMapping; -import static org.opensearch.sql.legacy.TestUtils.getGeoIpIndexMapping; -import static org.opensearch.sql.legacy.TestUtils.getGeopointIndexMapping; -import static org.opensearch.sql.legacy.TestUtils.getHdfsLogsIndexMapping; -import static org.opensearch.sql.legacy.TestUtils.getHobbiesIndexMapping; -import static org.opensearch.sql.legacy.TestUtils.getJoinTypeIndexMapping; -import static org.opensearch.sql.legacy.TestUtils.getJsonTestIndexMapping; -import static org.opensearch.sql.legacy.TestUtils.getLocationIndexMapping; -import static org.opensearch.sql.legacy.TestUtils.getLogsIndexMapping; -import static org.opensearch.sql.legacy.TestUtils.getMappingFile; -import static org.opensearch.sql.legacy.TestUtils.getNestedSimpleIndexMapping; -import static org.opensearch.sql.legacy.TestUtils.getNestedTypeIndexMapping; -import static org.opensearch.sql.legacy.TestUtils.getOccupationIndexMapping; -import static org.opensearch.sql.legacy.TestUtils.getOdbcIndexMapping; -import static org.opensearch.sql.legacy.TestUtils.getOrderIndexMapping; -import static org.opensearch.sql.legacy.TestUtils.getPeople2IndexMapping; -import static org.opensearch.sql.legacy.TestUtils.getPhraseIndexMapping; -import static org.opensearch.sql.legacy.TestUtils.getResponseBody; -import static org.opensearch.sql.legacy.TestUtils.getStateCountryIndexMapping; -import static org.opensearch.sql.legacy.TestUtils.getStringIndexMapping; -import static org.opensearch.sql.legacy.TestUtils.getTpchMappingFile; -import static org.opensearch.sql.legacy.TestUtils.getUnexpandedObjectIndexMapping; -import static org.opensearch.sql.legacy.TestUtils.getWeblogsIndexMapping; -import static org.opensearch.sql.legacy.TestUtils.getWorkInformationIndexMapping; -import static org.opensearch.sql.legacy.TestUtils.getWorkerIndexMapping; -import static org.opensearch.sql.legacy.TestUtils.isIndexExist; -import static org.opensearch.sql.legacy.TestUtils.loadDataByRestClient; +import static org.opensearch.sql.legacy.TestUtils.*; import static org.opensearch.sql.legacy.plugin.RestSqlAction.CURSOR_CLOSE_ENDPOINT; import static org.opensearch.sql.legacy.plugin.RestSqlAction.EXPLAIN_API_ENDPOINT; import static org.opensearch.sql.legacy.plugin.RestSqlAction.QUERY_API_ENDPOINT;