From b101b6a79ccbe7a1173ae50a1675c5fe46079aa6 Mon Sep 17 00:00:00 2001 From: normanj-bitquill <78755797+normanj-bitquill@users.noreply.github.com> Date: Thu, 24 Oct 2024 15:21:37 -0700 Subject: [PATCH] Array values are preserved (#1300) (#3095) Signed-off-by: Norman Jordan (cherry picked from commit e109417bb3ce770fb54cfd7b231b8219a22ed46f) --- .../sql/common/setting/Settings.java | 3 + docs/user/beyond/partiql.rst | 10 +- docs/user/ppl/general/datatypes.rst | 47 +--- .../sql/legacy/ObjectFieldSelectIT.java | 16 +- .../org/opensearch/sql/ppl/StandaloneIT.java | 1 + .../java/org/opensearch/sql/sql/NestedIT.java | 2 +- .../sql/sql/StandalonePaginationIT.java | 1 + .../value/OpenSearchExprValueFactory.java | 9 +- .../request/OpenSearchQueryRequest.java | 4 +- .../request/OpenSearchScrollRequest.java | 4 +- .../setting/OpenSearchSettings.java | 18 +- .../opensearch/storage/OpenSearchIndex.java | 7 +- .../storage/scan/OpenSearchIndexScan.java | 2 - .../storage/script/core/ExpressionScript.java | 2 +- .../client/OpenSearchNodeClientTest.java | 1 - .../client/OpenSearchRestClientTest.java | 1 - .../value/OpenSearchExprValueFactoryTest.java | 220 +++++++++++++++--- .../request/OpenSearchQueryRequestTest.java | 3 +- .../request/OpenSearchRequestBuilderTest.java | 1 + .../response/OpenSearchResponseTest.java | 16 +- .../storage/OpenSearchIndexTest.java | 11 + .../OpenSearchIndexScanPaginationTest.java | 4 +- .../storage/scan/OpenSearchIndexScanTest.java | 4 +- 23 files changed, 289 insertions(+), 98 deletions(-) diff --git a/common/src/main/java/org/opensearch/sql/common/setting/Settings.java b/common/src/main/java/org/opensearch/sql/common/setting/Settings.java index 0037032d22..a9fa693a22 100644 --- a/common/src/main/java/org/opensearch/sql/common/setting/Settings.java +++ b/common/src/main/java/org/opensearch/sql/common/setting/Settings.java @@ -28,6 +28,9 @@ public enum Key { /** PPL Settings. */ PPL_ENABLED("plugins.ppl.enabled"), + /** Query Settings. */ + FIELD_TYPE_TOLERANCE("plugins.query.field_type_tolerance"), + /** Common Settings for SQL and PPL. */ QUERY_MEMORY_LIMIT("plugins.query.memory_limit"), QUERY_SIZE_LIMIT("plugins.query.size_limit"), diff --git a/docs/user/beyond/partiql.rst b/docs/user/beyond/partiql.rst index 76fec8405d..6ad93ddeaf 100644 --- a/docs/user/beyond/partiql.rst +++ b/docs/user/beyond/partiql.rst @@ -202,11 +202,11 @@ Selecting top level for object fields, object fields of array value and nested f os> SELECT city, accounts, projects FROM people; fetched rows / total rows = 1/1 - +-----------------------------------------------------+------------+----------------------------------------------------------------------------------------------------------------+ - | city | accounts | projects | - |-----------------------------------------------------+------------+----------------------------------------------------------------------------------------------------------------| - | {'name': 'Seattle', 'location': {'latitude': 10.5}} | {'id': 1} | [{'name': 'AWS Redshift Spectrum querying'},{'name': 'AWS Redshift security'},{'name': 'AWS Aurora security'}] | - +-----------------------------------------------------+------------+----------------------------------------------------------------------------------------------------------------+ + +-----------------------------------------------------+-----------------------+----------------------------------------------------------------------------------------------------------------+ + | city | accounts | projects | + |-----------------------------------------------------+-----------------------+----------------------------------------------------------------------------------------------------------------| + | {'name': 'Seattle', 'location': {'latitude': 10.5}} | [{'id': 1},{'id': 2}] | [{'name': 'AWS Redshift Spectrum querying'},{'name': 'AWS Redshift security'},{'name': 'AWS Aurora security'}] | + +-----------------------------------------------------+-----------------------+----------------------------------------------------------------------------------------------------------------+ Example 2: Selecting Deeper Levels ---------------------------------- diff --git a/docs/user/ppl/general/datatypes.rst b/docs/user/ppl/general/datatypes.rst index cabc689526..cb653d9173 100644 --- a/docs/user/ppl/general/datatypes.rst +++ b/docs/user/ppl/general/datatypes.rst @@ -39,8 +39,6 @@ The PPL support the following data types. +---------------+ | timestamp | +---------------+ -| datetime | -+---------------+ | date | +---------------+ | time | @@ -114,7 +112,7 @@ Numeric values ranged from -2147483648 to +2147483647 are recognized as integer Date and Time Data Types ======================== -The date and time data types are the types that represent temporal values and PPL plugin supports types including DATE, TIME, DATETIME, TIMESTAMP and INTERVAL. By default, the OpenSearch DSL uses date type as the only date and time related type, which has contained all information about an absolute time point. To integrate with PPL language, each of the types other than timestamp is holding part of temporal or timezone information, and the usage to explicitly clarify the date and time types is reflected in the datetime functions (see `Functions `_ for details), where some functions might have restrictions in the input argument type. +The date and time data types are the types that represent temporal values and PPL plugin supports types including DATE, TIME, TIMESTAMP and INTERVAL. By default, the OpenSearch DSL uses date type as the only date and time related type, which has contained all information about an absolute time point. To integrate with PPL language, each of the types other than timestamp is holding part of temporal or timezone information, and the usage to explicitly clarify the date and time types is reflected in the datetime functions (see `Functions `_ for details), where some functions might have restrictions in the input argument type. Date @@ -141,19 +139,6 @@ Time represents the time on the clock or watch with no regard for which timezone +------+-----------------------+----------------------------------------+ -Datetime --------- - -Datetime type is the combination of date and time. The conversion rule of date or time to datetime is described in `Conversion between date and time types`_. Datetime type does not contain timezone information. For an absolute time point that contains both date time and timezone information, see `Timestamp`_. - -+----------+----------------------------------+--------------------------------------------------------------+ -| Type | Syntax | Range | -+==========+==================================+==============================================================+ -| Datetime | 'yyyy-MM-dd hh:mm:ss[.fraction]' | '0001-01-01 00:00:00.000000' to '9999-12-31 23:59:59.999999' | -+----------+----------------------------------+--------------------------------------------------------------+ - - - Timestamp --------- @@ -183,38 +168,26 @@ The expr is any expression that can be iterated to a quantity value eventually, Conversion between date and time types -------------------------------------- -Basically the date and time types except interval can be converted to each other, but might suffer some alteration of the value or some information loss, for example extracting the time value from a datetime value, or convert a date value to a datetime value and so forth. Here lists the summary of the conversion rules that PPL plugin supports for each of the types: +Basically the date and time types except interval can be converted to each other, but might suffer some alteration of the value or some information loss, for example extracting the time value from a timestamp value, or convert a date value to a timestamp value and so forth. Here lists the summary of the conversion rules that PPL plugin supports for each of the types: Conversion from DATE >>>>>>>>>>>>>>>>>>>> - Since the date value does not have any time information, conversion to `Time`_ type is not useful, and will always return a zero time value '00:00:00'. -- Conversion from date to datetime has a data fill-up due to the lack of time information, and it attaches the time '00:00:00' to the original date by default and forms a datetime instance. For example, the result to covert date '2020-08-17' to datetime type is datetime '2020-08-17 00:00:00'. - -- Conversion to timestamp is to alternate both the time value and the timezone information, and it attaches the zero time value '00:00:00' and the session timezone (UTC by default) to the date. For example, the result to covert date '2020-08-17' to datetime type with session timezone UTC is datetime '2020-08-17 00:00:00' UTC. +- Conversion to timestamp is to alternate both the time value and the timezone information, and it attaches the zero time value '00:00:00' and the session timezone (UTC by default) to the date. For example, the result to covert date '2020-08-17' to timestamp type with session timezone UTC is timestamp '2020-08-17 00:00:00' UTC. Conversion from TIME >>>>>>>>>>>>>>>>>>>> -- Time value cannot be converted to any other date and time types since it does not contain any date information, so it is not meaningful to give no date info to a date/datetime/timestamp instance. - - -Conversion from DATETIME ->>>>>>>>>>>>>>>>>>>>>>>> - -- Conversion from datetime to date is to extract the date part from the datetime value. For example, the result to convert datetime '2020-08-17 14:09:00' to date is date '2020-08-08'. - -- Conversion to time is to extract the time part from the datetime value. For example, the result to convert datetime '2020-08-17 14:09:00' to time is time '14:09:00'. - -- Since the datetime type does not contain timezone information, the conversion to timestamp needs to fill up the timezone part with the session timezone. For example, the result to convert datetime '2020-08-17 14:09:00' with system timezone of UTC, to timestamp is timestamp '2020-08-17 14:09:00' UTC. +- Time value cannot be converted to any other date and time types since it does not contain any date information, so it is not meaningful to give no date info to a date/timestamp instance. Conversion from TIMESTAMP >>>>>>>>>>>>>>>>>>>>>>>>> -- Conversion from timestamp is much more straightforward. To convert it to date is to extract the date value, and conversion to time is to extract the time value. Conversion to datetime, it will extracts the datetime value and leave the timezone information over. For example, the result to convert datetime '2020-08-17 14:09:00' UTC to date is date '2020-08-17', to time is '14:09:00' and to datetime is datetime '2020-08-17 14:09:00'. +- Conversion from timestamp is much more straightforward. To convert it to date is to extract the date value, and conversion to time is to extract the time value. For example, the result to convert timestamp '2020-08-17 14:09:00' UTC to date is date '2020-08-17', to time is '14:09:00'. String Data Types @@ -412,8 +385,8 @@ Select deeper level for object fields of array value which returns the first ele os> source = people | fields accounts, accounts.id; fetched rows / total rows = 1/1 - +------------+---------------+ - | accounts | accounts.id | - |------------+---------------| - | {'id': 1} | 1 | - +------------+---------------+ \ No newline at end of file + +-----------------------+-------------+ + | accounts | accounts.id | + |-----------------------+-------------| + | [{'id': 1},{'id': 2}] | 1 | + +-----------------------+-------------+ diff --git a/integ-test/src/test/java/org/opensearch/sql/legacy/ObjectFieldSelectIT.java b/integ-test/src/test/java/org/opensearch/sql/legacy/ObjectFieldSelectIT.java index 3a2f48d497..aadd79469d 100644 --- a/integ-test/src/test/java/org/opensearch/sql/legacy/ObjectFieldSelectIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/legacy/ObjectFieldSelectIT.java @@ -14,6 +14,7 @@ import org.json.JSONArray; import org.json.JSONObject; import org.junit.Test; +import org.opensearch.sql.common.setting.Settings; import org.opensearch.sql.legacy.utils.StringUtils; /** @@ -79,9 +80,20 @@ public void testSelectNestedFieldItself() { @Test public void testSelectObjectFieldOfArrayValuesItself() { JSONObject response = new JSONObject(query("SELECT accounts FROM %s")); + verifyDataRows(response, rows(new JSONArray("[{\"id\":1},{\"id\":2}]"))); + } - // Only the first element of the list of is returned. - verifyDataRows(response, rows(new JSONObject("{\"id\": 1}"))); + @Test + public void testSelectObjectFieldOfArrayValuesItselfNoFieldTypeTolerance() throws Exception { + updateClusterSettings( + new ClusterSetting(PERSISTENT, Settings.Key.FIELD_TYPE_TOLERANCE.getKeyValue(), "false")); + try { + JSONObject response = new JSONObject(query("SELECT accounts FROM %s")); + verifyDataRows(response, rows(new JSONObject("{\"id\":1}"))); + } finally { + updateClusterSettings( + new ClusterSetting(PERSISTENT, Settings.Key.FIELD_TYPE_TOLERANCE.getKeyValue(), "true")); + } } @Test diff --git a/integ-test/src/test/java/org/opensearch/sql/ppl/StandaloneIT.java b/integ-test/src/test/java/org/opensearch/sql/ppl/StandaloneIT.java index 66f85b0754..d484f3c4d0 100644 --- a/integ-test/src/test/java/org/opensearch/sql/ppl/StandaloneIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/ppl/StandaloneIT.java @@ -153,6 +153,7 @@ private Settings defaultSettings() { new ImmutableMap.Builder() .put(Key.QUERY_SIZE_LIMIT, 200) .put(Key.SQL_PAGINATION_API_SEARCH_AFTER, true) + .put(Key.FIELD_TYPE_TOLERANCE, true) .build(); @Override diff --git a/integ-test/src/test/java/org/opensearch/sql/sql/NestedIT.java b/integ-test/src/test/java/org/opensearch/sql/sql/NestedIT.java index 96bbae94e5..9e7a5be2b2 100644 --- a/integ-test/src/test/java/org/opensearch/sql/sql/NestedIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/sql/NestedIT.java @@ -423,7 +423,7 @@ public void test_nested_in_where_as_predicate_expression_with_multiple_condition + " nested(message.dayOfWeek) >= 4"; JSONObject result = executeJdbcRequest(query); assertEquals(2, result.getInt("total")); - verifyDataRows(result, rows("c", "ab", 4), rows("zz", "aa", 6)); + verifyDataRows(result, rows("c", "ab", 4), rows("zz", new JSONArray(List.of("aa", "bb")), 6)); } @Test diff --git a/integ-test/src/test/java/org/opensearch/sql/sql/StandalonePaginationIT.java b/integ-test/src/test/java/org/opensearch/sql/sql/StandalonePaginationIT.java index 698e185abb..f6951f4a2c 100644 --- a/integ-test/src/test/java/org/opensearch/sql/sql/StandalonePaginationIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/sql/StandalonePaginationIT.java @@ -167,6 +167,7 @@ private Settings defaultSettings() { .put(Key.QUERY_SIZE_LIMIT, 200) .put(Key.SQL_CURSOR_KEEP_ALIVE, TimeValue.timeValueMinutes(1)) .put(Key.SQL_PAGINATION_API_SEARCH_AFTER, true) + .put(Key.FIELD_TYPE_TOLERANCE, true) .build(); @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 e736663f60..2c82d3bef3 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 @@ -75,6 +75,9 @@ public class OpenSearchExprValueFactory { /** The Mapping of Field and ExprType. */ private final Map typeMapping; + /** Whether to support nested value types (such as arrays) */ + private final boolean fieldTypeTolerance; + /** * Extend existing mapping by new data without overwrite. Called from aggregation only {@see * AggregationQueryBuilder#buildTypeMapping}. @@ -143,8 +146,10 @@ public void extendTypeMapping(Map typeMapping) { .build(); /** Constructor of OpenSearchExprValueFactory. */ - public OpenSearchExprValueFactory(Map typeMapping) { + public OpenSearchExprValueFactory( + Map typeMapping, boolean fieldTypeTolerance) { this.typeMapping = OpenSearchDataType.traverseAndFlatten(typeMapping); + this.fieldTypeTolerance = fieldTypeTolerance; } /** @@ -160,7 +165,7 @@ public ExprValue construct(String jsonString, boolean supportArrays) { new OpenSearchJsonContent(OBJECT_MAPPER.readTree(jsonString)), TOP_PATH, Optional.of(STRUCT), - supportArrays); + fieldTypeTolerance || supportArrays); } catch (JsonProcessingException e) { throw new IllegalStateException(String.format("invalid json: %s.", jsonString), e); } diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/request/OpenSearchQueryRequest.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/request/OpenSearchQueryRequest.java index fff252f3b4..cc2b2d781c 100644 --- a/opensearch/src/main/java/org/opensearch/sql/opensearch/request/OpenSearchQueryRequest.java +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/request/OpenSearchQueryRequest.java @@ -150,7 +150,9 @@ public OpenSearchQueryRequest(StreamInput in, OpenSearchStorageEngine engine) th } OpenSearchIndex index = (OpenSearchIndex) engine.getTable(null, indexName.toString()); - exprValueFactory = new OpenSearchExprValueFactory(index.getFieldOpenSearchTypes()); + exprValueFactory = + new OpenSearchExprValueFactory( + index.getFieldOpenSearchTypes(), index.isFieldTypeTolerance()); } @Override diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/request/OpenSearchScrollRequest.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/request/OpenSearchScrollRequest.java index c9490f0767..d793b53fca 100644 --- a/opensearch/src/main/java/org/opensearch/sql/opensearch/request/OpenSearchScrollRequest.java +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/request/OpenSearchScrollRequest.java @@ -178,6 +178,8 @@ public OpenSearchScrollRequest(StreamInput in, OpenSearchStorageEngine engine) includes = in.readStringList(); indexName = new IndexName(in); OpenSearchIndex index = (OpenSearchIndex) engine.getTable(null, indexName.toString()); - exprValueFactory = new OpenSearchExprValueFactory(index.getFieldOpenSearchTypes()); + exprValueFactory = + new OpenSearchExprValueFactory( + index.getFieldOpenSearchTypes(), index.isFieldTypeTolerance()); } } diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/setting/OpenSearchSettings.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/setting/OpenSearchSettings.java index 1083dbd836..612771eea4 100644 --- a/opensearch/src/main/java/org/opensearch/sql/opensearch/setting/OpenSearchSettings.java +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/setting/OpenSearchSettings.java @@ -229,6 +229,13 @@ public class OpenSearchSettings extends Settings { Setting.Property.NodeScope, Setting.Property.Dynamic); + public static final Setting FIELD_TYPE_TOLERANCE_SETTING = + Setting.boolSetting( + Key.FIELD_TYPE_TOLERANCE.getKeyValue(), + true, + Setting.Property.NodeScope, + Setting.Property.Dynamic); + /** Construct OpenSearchSetting. The OpenSearchSetting must be singleton. */ @SuppressWarnings("unchecked") public OpenSearchSettings(ClusterSettings clusterSettings) { @@ -372,13 +379,19 @@ public OpenSearchSettings(ClusterSettings clusterSettings) { clusterSettings, Key.SESSION_INACTIVITY_TIMEOUT_MILLIS, SESSION_INACTIVITY_TIMEOUT_MILLIS_SETTING, - new Updater((Key.SESSION_INACTIVITY_TIMEOUT_MILLIS))); + new Updater(Key.SESSION_INACTIVITY_TIMEOUT_MILLIS)); register( settingBuilder, clusterSettings, Key.STREAMING_JOB_HOUSEKEEPER_INTERVAL, STREAMING_JOB_HOUSEKEEPER_INTERVAL_SETTING, - new Updater((Key.STREAMING_JOB_HOUSEKEEPER_INTERVAL))); + new Updater(Key.STREAMING_JOB_HOUSEKEEPER_INTERVAL)); + register( + settingBuilder, + clusterSettings, + Key.FIELD_TYPE_TOLERANCE, + FIELD_TYPE_TOLERANCE_SETTING, + new Updater(Key.FIELD_TYPE_TOLERANCE)); defaultSettings = settingBuilder.build(); } @@ -455,6 +468,7 @@ public static List> pluginSettings() { .add(DATASOURCES_LIMIT_SETTING) .add(SESSION_INACTIVITY_TIMEOUT_MILLIS_SETTING) .add(STREAMING_JOB_HOUSEKEEPER_INTERVAL_SETTING) + .add(FIELD_TYPE_TOLERANCE_SETTING) .build(); } diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/OpenSearchIndex.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/OpenSearchIndex.java index a6fe83c8c4..b8822cd1e8 100644 --- a/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/OpenSearchIndex.java +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/OpenSearchIndex.java @@ -177,7 +177,12 @@ private OpenSearchExprValueFactory createExprValueFactory() { Map allFields = new HashMap<>(); getReservedFieldTypes().forEach((k, v) -> allFields.put(k, OpenSearchDataType.of(v))); allFields.putAll(getFieldOpenSearchTypes()); - return new OpenSearchExprValueFactory(allFields); + return new OpenSearchExprValueFactory( + allFields, settings.getSettingValue(Settings.Key.FIELD_TYPE_TOLERANCE)); + } + + public boolean isFieldTypeTolerance() { + return settings.getSettingValue(Settings.Key.FIELD_TYPE_TOLERANCE); } @VisibleForTesting diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/scan/OpenSearchIndexScan.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/scan/OpenSearchIndexScan.java index b17773cb03..74cbd1f167 100644 --- a/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/scan/OpenSearchIndexScan.java +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/scan/OpenSearchIndexScan.java @@ -47,8 +47,6 @@ public class OpenSearchIndexScan extends TableScanOperator implements Serializab /** Search response for current batch. */ private Iterator iterator; - private Settings pluginSettings; - /** Creates index scan based on a provided OpenSearchRequestBuilder. */ public OpenSearchIndexScan( OpenSearchClient client, int maxResponseSize, OpenSearchRequest request) { diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/core/ExpressionScript.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/core/ExpressionScript.java index 3a9ff02ba0..460a9b4567 100644 --- a/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/core/ExpressionScript.java +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/core/ExpressionScript.java @@ -102,7 +102,7 @@ private OpenSearchExprValueFactory buildValueFactory(Set fi Map typeEnv = fields.stream() .collect(toMap(ReferenceExpression::getAttr, e -> OpenSearchDataType.of(e.type()))); - return new OpenSearchExprValueFactory(typeEnv); + return new OpenSearchExprValueFactory(typeEnv, false); } private Environment buildValueEnv( 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 ba0fb85422..12a906b25e 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 @@ -297,7 +297,6 @@ void search() { new SearchHits( new SearchHit[] {searchHit}, new TotalHits(1L, TotalHits.Relation.EQUAL_TO), 1.0F)); when(searchHit.getSourceAsString()).thenReturn("{\"id\", 1}"); - when(searchHit.getInnerHits()).thenReturn(null); when(factory.construct(any(), anyBoolean())).thenReturn(exprTupleValue); // Mock second scroll request followed 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 82d9e74422..eb2355a36b 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 @@ -286,7 +286,6 @@ void search() throws IOException { new SearchHits( new SearchHit[] {searchHit}, new TotalHits(1L, TotalHits.Relation.EQUAL_TO), 1.0F)); when(searchHit.getSourceAsString()).thenReturn("{\"id\", 1}"); - when(searchHit.getInnerHits()).thenReturn(null); when(factory.construct(any(), anyBoolean())).thenReturn(exprTupleValue); // Mock second scroll request followed 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 d1bc3c500b..59484f1f17 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 @@ -121,7 +121,10 @@ class OpenSearchExprValueFactoryTest { .build(); private final OpenSearchExprValueFactory exprValueFactory = - new OpenSearchExprValueFactory(MAPPING); + new OpenSearchExprValueFactory(MAPPING, true); + + private final OpenSearchExprValueFactory exprValueFactoryNoArrays = + new OpenSearchExprValueFactory(MAPPING, false); @Test public void constructNullValue() { @@ -478,28 +481,40 @@ public void constructArrayOfStrings() { constructFromObject("arrayV", List.of("zz", "au"))); } + @Test + public void constructArrayOfStringsWithArrays() { + assertEquals( + new ExprCollectionValue(List.of(stringValue("zz"), stringValue("au"))), + constructFromObjectWithArraySupport("arrayV", List.of("zz", "au"))); + } + @Test public void constructNestedArraysOfStrings() { assertEquals( new ExprCollectionValue( List.of(collectionValue(List.of("zz", "au")), collectionValue(List.of("ss")))), - tupleValueWithArraySupport("{\"stringV\":[" + "[\"zz\", \"au\"]," + "[\"ss\"]" + "]}") - .get("stringV")); + tupleValueWithArraySupport("{\"stringV\":[ [\"zz\", \"au\"], [\"ss\"] ]}").get("stringV")); } @Test - public void constructNestedArraysOfStringsReturnsFirstIndex() { + public void constructNestedArraysOfStringsReturnsAll() { assertEquals( - stringValue("zz"), - tupleValue("{\"stringV\":[" + "[\"zz\", \"au\"]," + "[\"ss\"]" + "]}").get("stringV")); + new ExprCollectionValue( + List.of( + new ExprCollectionValue(List.of(stringValue("zz"), stringValue("au"))), + new ExprCollectionValue(List.of(stringValue("ss"))))), + tupleValue("{\"stringV\":[[\"zz\", \"au\"],[\"ss\"]]}").get("stringV")); } @Test - public void constructMultiNestedArraysOfStringsReturnsFirstIndex() { + public void constructMultiNestedArraysOfStringsReturnsAll() { assertEquals( - stringValue("z"), - tupleValue("{\"stringV\":" + "[\"z\"," + "[\"s\"]," + "[\"zz\", \"au\"]" + "]}") - .get("stringV")); + new ExprCollectionValue( + List.of( + stringValue("z"), + new ExprCollectionValue(List.of(stringValue("s"))), + new ExprCollectionValue(List.of(stringValue("zz"), stringValue("au"))))), + tupleValue("{\"stringV\":[\"z\",[\"s\"],[\"zz\", \"au\"]]}").get("stringV")); } @Test @@ -594,6 +609,20 @@ public void constructNestedArrayNode() { tupleValueWithArraySupport("{\"nestedV\":[1969,2011]}").get("nestedV")); } + @Test + public void constructNestedArrayNodeNotSupported() { + assertEquals( + Map.of("stringV", stringValue("foo")), + tupleValueWithoutArraySupport("[{\"stringV\":\"foo\"}]")); + } + + @Test + public void constructNestedArrayNodeNotSupportedNoFieldTolerance() { + assertEquals( + Map.of("stringV", stringValue("foo")), + tupleValueWithoutArraySupportNoFieldTolerance("{\"stringV\":\"foo\"}")); + } + @Test public void constructNestedObjectNode() { assertEquals( @@ -617,9 +646,24 @@ public void constructArrayOfGeoPoints() { } @Test - public void constructArrayOfGeoPointsReturnsFirstIndex() { + public void constructArrayOfGeoPointsNoArrays() { assertEquals( new OpenSearchExprGeoPointValue(42.60355556, -97.25263889), + tupleValueWithoutArraySupport( + "{\"geoV\":[" + + "{\"lat\":42.60355556,\"lon\":-97.25263889}," + + "{\"lat\":-33.6123556,\"lon\":66.287449}" + + "]}") + .get("geoV")); + } + + @Test + public void constructArrayOfGeoPointsReturnsAll() { + assertEquals( + new ExprCollectionValue( + List.of( + new OpenSearchExprGeoPointValue(42.60355556, -97.25263889), + new OpenSearchExprGeoPointValue(-33.6123556, 66.287449))), tupleValue( "{\"geoV\":[" + "{\"lat\":42.60355556,\"lon\":-97.25263889}," @@ -629,46 +673,60 @@ public void constructArrayOfGeoPointsReturnsFirstIndex() { } @Test - public void constructArrayOfIPsReturnsFirstIndex() { + public void constructArrayOfIPsReturnsAll() { assertEquals( - new OpenSearchExprIpValue("192.168.0.1"), + new ExprCollectionValue( + List.of( + new OpenSearchExprIpValue("192.168.0.1"), + new OpenSearchExprIpValue("192.168.0.2"))), tupleValue("{\"ipV\":[\"192.168.0.1\",\"192.168.0.2\"]}").get("ipV")); } @Test - public void constructBinaryArrayReturnsFirstIndex() { + public void constructBinaryArrayReturnsAll() { assertEquals( - new OpenSearchExprBinaryValue("U29tZSBiaWsdfsdfgYmxvYg=="), + new ExprCollectionValue( + List.of( + new OpenSearchExprBinaryValue("U29tZSBiaWsdfsdfgYmxvYg=="), + new OpenSearchExprBinaryValue("U987yuhjjiy8jhk9vY+98jjdf"))), tupleValue("{\"binaryV\":[\"U29tZSBiaWsdfsdfgYmxvYg==\",\"U987yuhjjiy8jhk9vY+98jjdf\"]}") .get("binaryV")); } @Test - public void constructArrayOfCustomEpochMillisReturnsFirstIndex() { + public void constructArrayOfCustomEpochMillisReturnsAll() { assertEquals( - new ExprDatetimeValue("2015-01-01 12:10:30"), + new ExprCollectionValue( + List.of( + new ExprTimestampValue("2015-01-01 12:10:30"), + new ExprTimestampValue("1999-11-09 01:09:44"))), tupleValue("{\"customAndEpochMillisV\":[\"2015-01-01 12:10:30\",\"1999-11-09 01:09:44\"]}") .get("customAndEpochMillisV")); } @Test - public void constructArrayOfDateStringsReturnsFirstIndex() { + public void constructArrayOfDateStringsReturnsAll() { assertEquals( - new ExprDateValue("1984-04-12"), + new ExprCollectionValue( + List.of(new ExprDateValue("1984-04-12"), new ExprDateValue("2033-05-03"))), tupleValue("{\"dateStringV\":[\"1984-04-12\",\"2033-05-03\"]}").get("dateStringV")); } @Test - public void constructArrayOfTimeStringsReturnsFirstIndex() { + public void constructArrayOfTimeStringsReturnsAll() { assertEquals( - new ExprTimeValue("12:10:30"), + new ExprCollectionValue( + List.of(new ExprTimeValue("12:10:30"), new ExprTimeValue("18:33:55"))), tupleValue("{\"timeStringV\":[\"12:10:30.000Z\",\"18:33:55.000Z\"]}").get("timeStringV")); } @Test public void constructArrayOfEpochMillis() { assertEquals( - new ExprTimestampValue(Instant.ofEpochMilli(1420070400001L)), + new ExprCollectionValue( + List.of( + new ExprTimestampValue(Instant.ofEpochMilli(1420070400001L)), + new ExprTimestampValue(Instant.ofEpochMilli(1454251113333L)))), tupleValue("{\"dateOrEpochMillisV\":[\"1420070400001\",\"1454251113333\"]}") .get("dateOrEpochMillisV")); } @@ -780,12 +838,75 @@ public void constructBinary() { } /** - * Return the first element if is OpenSearch Array. + * Return the all elements if is OpenSearch Array. * https://www.elastic.co/guide/en/elasticsearch/reference/current/array.html. */ @Test - public void constructFromOpenSearchArrayReturnFirstElement() { - assertEquals(integerValue(1), tupleValue("{\"intV\":[1, 2, 3]}").get("intV")); + public void constructFromOpenSearchArrayReturnAll() { + assertEquals( + new ExprCollectionValue(List.of(integerValue(1), integerValue(2), integerValue(3))), + tupleValue("{\"intV\":[1, 2, 3]}").get("intV")); + assertEquals( + new ExprCollectionValue( + List.of( + new ExprTupleValue( + new LinkedHashMap() { + { + put("id", integerValue(1)); + put("state", stringValue("WA")); + } + }), + new ExprTupleValue( + new LinkedHashMap() { + { + put("id", integerValue(2)); + put("state", stringValue("CA")); + } + }))), + tupleValue("{\"structV\":[{\"id\":1,\"state\":\"WA\"},{\"id\":2,\"state\":\"CA\"}]}}") + .get("structV")); + } + + /** + * Return the all elements if is OpenSearch Array. + * https://www.elastic.co/guide/en/elasticsearch/reference/current/array.html. + */ + @Test + public void constructFromOpenSearchArrayReturnAllWithArraySupport() { + assertEquals( + new ExprCollectionValue(List.of(integerValue(1), integerValue(2), integerValue(3))), + tupleValue("{\"intV\":[1, 2, 3]}").get("intV")); + assertEquals( + new ExprCollectionValue( + List.of( + new ExprTupleValue( + new LinkedHashMap() { + { + put("id", integerValue(1)); + put("state", stringValue("WA")); + } + }), + new ExprTupleValue( + new LinkedHashMap() { + { + put("id", integerValue(2)); + put("state", stringValue("CA")); + } + }))), + tupleValueWithArraySupport( + "{\"structV\":[{\"id\":1,\"state\":\"WA\"},{\"id\":2,\"state\":\"CA\"}]}}") + .get("structV")); + } + + /** + * Return only the first element if is OpenSearch Array. + * https://www.elastic.co/guide/en/elasticsearch/reference/current/array.html. + */ + @Test + public void constructFromOpenSearchArrayReturnAllWithoutArraySupport() { + assertEquals( + new ExprCollectionValue(List.of(integerValue(1), integerValue(2), integerValue(3))), + tupleValue("{\"intV\":[1, 2, 3]}").get("intV")); assertEquals( new ExprTupleValue( new LinkedHashMap() { @@ -794,7 +915,39 @@ public void constructFromOpenSearchArrayReturnFirstElement() { put("state", stringValue("WA")); } }), - tupleValue("{\"structV\":[{\"id\":1,\"state\":\"WA\"},{\"id\":2,\"state\":\"CA\"}]}}") + tupleValueWithoutArraySupport( + "{\"structV\":[{\"id\":1,\"state\":\"WA\"},{\"id\":2,\"state\":\"CA\"}]}}") + .get("structV")); + } + + /** + * Return only the first element if is OpenSearch Array. + * https://www.elastic.co/guide/en/elasticsearch/reference/current/array.html. + */ + @Test + public void constructFromOpenSearchArrayReturnAllWithoutArraySupportNoFieldTolerance() { + assertEquals( + new ExprCollectionValue(List.of(integerValue(1), integerValue(2), integerValue(3))), + tupleValue("{\"intV\":[1, 2, 3]}").get("intV")); + assertEquals( + new ExprCollectionValue( + List.of( + new ExprTupleValue( + new LinkedHashMap() { + { + put("id", integerValue(1)); + put("state", stringValue("WA")); + } + }), + new ExprTupleValue( + new LinkedHashMap() { + { + put("id", integerValue(2)); + put("state", stringValue("CA")); + } + }))), + tupleValueWithoutArraySupportNoFieldTolerance( + "{\"structV\":[{\"id\":1,\"state\":\"WA\"},{\"id\":2,\"state\":\"CA\"}]}}") .get("structV")); } @@ -815,7 +968,7 @@ public void noTypeFoundForMapping() { @Test public void constructUnsupportedTypeThrowException() { OpenSearchExprValueFactory exprValueFactory = - new OpenSearchExprValueFactory(Map.of("type", new TestType())); + new OpenSearchExprValueFactory(Map.of("type", new TestType()), true); IllegalStateException exception = assertThrows( IllegalStateException.class, () -> exprValueFactory.construct("{\"type\":1}", false)); @@ -832,7 +985,8 @@ public void constructUnsupportedTypeThrowException() { // it is accepted without overwriting existing data. public void factoryMappingsAreExtendableWithoutOverWrite() throws NoSuchFieldException, IllegalAccessException { - var factory = new OpenSearchExprValueFactory(Map.of("value", OpenSearchDataType.of(INTEGER))); + var factory = + new OpenSearchExprValueFactory(Map.of("value", OpenSearchDataType.of(INTEGER)), true); factory.extendTypeMapping( Map.of( "value", OpenSearchDataType.of(DOUBLE), @@ -860,6 +1014,16 @@ public Map tupleValueWithArraySupport(String jsonString) { return construct.tupleValue(); } + public Map tupleValueWithoutArraySupport(String jsonString) { + final ExprValue construct = exprValueFactoryNoArrays.construct(jsonString, false); + return construct.tupleValue(); + } + + public Map tupleValueWithoutArraySupportNoFieldTolerance(String jsonString) { + final ExprValue construct = exprValueFactoryNoArrays.construct(jsonString, true); + return construct.tupleValue(); + } + private ExprValue constructFromObject(String fieldName, Object value) { return exprValueFactory.construct(fieldName, value, false); } diff --git a/opensearch/src/test/java/org/opensearch/sql/opensearch/request/OpenSearchQueryRequestTest.java b/opensearch/src/test/java/org/opensearch/sql/opensearch/request/OpenSearchQueryRequestTest.java index 89b51207b5..0847e520cc 100644 --- a/opensearch/src/test/java/org/opensearch/sql/opensearch/request/OpenSearchQueryRequestTest.java +++ b/opensearch/src/test/java/org/opensearch/sql/opensearch/request/OpenSearchQueryRequestTest.java @@ -20,7 +20,6 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; -import org.mockito.InjectMocks; import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; import org.opensearch.action.search.SearchRequest; @@ -72,7 +71,7 @@ public class OpenSearchQueryRequestTest { @Mock private OpenSearchStorageEngine engine; @Mock private PointInTimeBuilder pointInTimeBuilder; - @InjectMocks private OpenSearchQueryRequest serializationRequest; + private OpenSearchQueryRequest serializationRequest; private SearchSourceBuilder sourceBuilderForSerializer; diff --git a/opensearch/src/test/java/org/opensearch/sql/opensearch/request/OpenSearchRequestBuilderTest.java b/opensearch/src/test/java/org/opensearch/sql/opensearch/request/OpenSearchRequestBuilderTest.java index bf87840b60..a2430a671d 100644 --- a/opensearch/src/test/java/org/opensearch/sql/opensearch/request/OpenSearchRequestBuilderTest.java +++ b/opensearch/src/test/java/org/opensearch/sql/opensearch/request/OpenSearchRequestBuilderTest.java @@ -86,6 +86,7 @@ void setup() { lenient() .when(settings.getSettingValue(Settings.Key.SQL_PAGINATION_API_SEARCH_AFTER)) .thenReturn(true); + lenient().when(settings.getSettingValue(Settings.Key.FIELD_TYPE_TOLERANCE)).thenReturn(false); } @Test diff --git a/opensearch/src/test/java/org/opensearch/sql/opensearch/response/OpenSearchResponseTest.java b/opensearch/src/test/java/org/opensearch/sql/opensearch/response/OpenSearchResponseTest.java index 6f4605bc2f..217145a052 100644 --- a/opensearch/src/test/java/org/opensearch/sql/opensearch/response/OpenSearchResponseTest.java +++ b/opensearch/src/test/java/org/opensearch/sql/opensearch/response/OpenSearchResponseTest.java @@ -14,10 +14,12 @@ import static org.mockito.ArgumentMatchers.anyBoolean; import static org.mockito.ArgumentMatchers.anyInt; import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; import com.google.common.collect.ImmutableMap; import java.util.Arrays; +import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.stream.Collectors; @@ -163,7 +165,8 @@ void iterator_metafields() { "_sort", new ExprLongValue(123456L), "_score", new ExprFloatValue(3.75F), "_maxscore", new ExprFloatValue(3.75F))); - List includes = List.of("id1", "_index", "_id", "_routing", "_sort", "_score", "_maxscore"); + List includes = + List.of("id1", "_index", "_id", "_routing", "_sort", "_score", "_maxscore"); int i = 0; for (ExprValue hit : new OpenSearchResponse(searchResponse, factory, includes)) { if (i == 0) { @@ -248,20 +251,15 @@ void iterator_metafields_scoreNaN() { @Test void iterator_with_inner_hits() { + Map innerHits = new HashMap<>(); + innerHits.put("a", mock(SearchHits.class)); + when(searchHit1.getInnerHits()).thenReturn(innerHits); when(searchResponse.getHits()) .thenReturn( new SearchHits( new SearchHit[] {searchHit1}, new TotalHits(2L, TotalHits.Relation.EQUAL_TO), 1.0F)); - when(searchHit1.getInnerHits()) - .thenReturn( - Map.of( - "innerHit", - new SearchHits( - new SearchHit[] {searchHit1}, - new TotalHits(2L, TotalHits.Relation.EQUAL_TO), - 1.0F))); when(factory.construct(any(), anyBoolean())).thenReturn(exprTupleValue1); diff --git a/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/OpenSearchIndexTest.java b/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/OpenSearchIndexTest.java index ef6b86c42a..3f8a07f495 100644 --- a/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/OpenSearchIndexTest.java +++ b/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/OpenSearchIndexTest.java @@ -10,6 +10,7 @@ import static org.hamcrest.Matchers.allOf; import static org.hamcrest.Matchers.hasEntry; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.doNothing; @@ -82,6 +83,7 @@ void setUp() { lenient() .when(settings.getSettingValue(Settings.Key.SQL_PAGINATION_API_SEARCH_AFTER)) .thenReturn(true); + lenient().when(settings.getSettingValue(Settings.Key.FIELD_TYPE_TOLERANCE)).thenReturn(true); } @Test @@ -270,4 +272,13 @@ void implementOtherLogicalOperators() { include), index.implement(plan)); } + + @Test + void isFieldTypeTolerance() { + when(settings.getSettingValue(Settings.Key.FIELD_TYPE_TOLERANCE)) + .thenReturn(true) + .thenReturn(false); + assertTrue(index.isFieldTypeTolerance()); + assertFalse(index.isFieldTypeTolerance()); + } } diff --git a/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/scan/OpenSearchIndexScanPaginationTest.java b/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/scan/OpenSearchIndexScanPaginationTest.java index e6a17aceaf..6f923cf5c4 100644 --- a/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/scan/OpenSearchIndexScanPaginationTest.java +++ b/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/scan/OpenSearchIndexScanPaginationTest.java @@ -59,6 +59,7 @@ void setup() { lenient() .when(settings.getSettingValue(Settings.Key.SQL_PAGINATION_API_SEARCH_AFTER)) .thenReturn(true); + lenient().when(settings.getSettingValue(Settings.Key.FIELD_TYPE_TOLERANCE)).thenReturn(true); } @Mock private OpenSearchClient client; @@ -67,7 +68,8 @@ void setup() { new OpenSearchExprValueFactory( Map.of( "name", OpenSearchDataType.of(STRING), - "department", OpenSearchDataType.of(STRING))); + "department", OpenSearchDataType.of(STRING)), + true); @Test void query_empty_result() { diff --git a/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/scan/OpenSearchIndexScanTest.java b/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/scan/OpenSearchIndexScanTest.java index e680c6b3a6..da30442bae 100644 --- a/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/scan/OpenSearchIndexScanTest.java +++ b/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/scan/OpenSearchIndexScanTest.java @@ -78,13 +78,15 @@ class OpenSearchIndexScanTest { private final OpenSearchExprValueFactory exprValueFactory = new OpenSearchExprValueFactory( Map.of( - "name", OpenSearchDataType.of(STRING), "department", OpenSearchDataType.of(STRING))); + "name", OpenSearchDataType.of(STRING), "department", OpenSearchDataType.of(STRING)), + true); @BeforeEach void setup() { lenient() .when(settings.getSettingValue(Settings.Key.SQL_PAGINATION_API_SEARCH_AFTER)) .thenReturn(true); + lenient().when(settings.getSettingValue(Settings.Key.FIELD_TYPE_TOLERANCE)).thenReturn(true); } @Test