From 32e7e730a0bc0ae23929c98c76e19d58ce5d446c Mon Sep 17 00:00:00 2001 From: Norman Jordan Date: Tue, 22 Oct 2024 10:52:36 -0700 Subject: [PATCH 1/6] Array values are preserved Signed-off-by: Norman Jordan --- docs/user/beyond/partiql.rst | 10 +++++----- docs/user/ppl/general/datatypes.rst | 10 +++++----- .../org/opensearch/sql/legacy/ObjectFieldSelectIT.java | 2 +- .../src/test/java/org/opensearch/sql/sql/NestedIT.java | 2 +- .../sql/opensearch/response/OpenSearchResponse.java | 7 +------ .../opensearch/client/OpenSearchNodeClientTest.java | 1 - .../opensearch/client/OpenSearchRestClientTest.java | 1 - .../opensearch/response/OpenSearchResponseTest.java | 10 ---------- 8 files changed, 13 insertions(+), 30 deletions(-) diff --git a/docs/user/beyond/partiql.rst b/docs/user/beyond/partiql.rst index 6a93a55c94..d8e4b0722b 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 a205626dbd..ba8281b6a9 100644 --- a/docs/user/ppl/general/datatypes.rst +++ b/docs/user/ppl/general/datatypes.rst @@ -385,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 | + +-----------------------+-------------+ \ No newline at end of file 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..9dd23e57c6 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 @@ -81,7 +81,7 @@ public void testSelectObjectFieldOfArrayValuesItself() { JSONObject response = new JSONObject(query("SELECT accounts FROM %s")); // Only the first element of the list of is returned. - verifyDataRows(response, rows(new JSONObject("{\"id\": 1}"))); + verifyDataRows(response, rows(new JSONArray("[{\"id\":1},{\"id\":2}]"))); } @Test 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 4ae683c229..18d93dbb2a 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 @@ -427,7 +427,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/opensearch/src/main/java/org/opensearch/sql/opensearch/response/OpenSearchResponse.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/response/OpenSearchResponse.java index e43777a740..a77863e7db 100644 --- a/opensearch/src/main/java/org/opensearch/sql/opensearch/response/OpenSearchResponse.java +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/response/OpenSearchResponse.java @@ -116,12 +116,7 @@ public Iterator iterator() { */ private void addParsedHitsToBuilder( ImmutableMap.Builder builder, SearchHit hit) { - builder.putAll( - exprValueFactory - .construct( - hit.getSourceAsString(), - !(hit.getInnerHits() == null || hit.getInnerHits().isEmpty())) - .tupleValue()); + builder.putAll(exprValueFactory.construct(hit.getSourceAsString(), true).tupleValue()); } /** 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/response/OpenSearchResponseTest.java b/opensearch/src/test/java/org/opensearch/sql/opensearch/response/OpenSearchResponseTest.java index 6f4605bc2f..d66087b176 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 @@ -109,8 +109,6 @@ void iterator() { when(searchHit1.getSourceAsString()).thenReturn("{\"id1\", 1}"); when(searchHit2.getSourceAsString()).thenReturn("{\"id1\", 2}"); - when(searchHit1.getInnerHits()).thenReturn(null); - when(searchHit2.getInnerHits()).thenReturn(null); when(factory.construct(any(), anyBoolean())) .thenReturn(exprTupleValue1) .thenReturn(exprTupleValue2); @@ -254,14 +252,6 @@ void iterator_with_inner_hits() { 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); From e93cc5f42f49ed94cf45ad2c387fcb9936806c04 Mon Sep 17 00:00:00 2001 From: Norman Jordan Date: Wed, 23 Oct 2024 14:59:59 -0700 Subject: [PATCH 2/6] Added a setting to control array handling Signed-off-by: Norman Jordan --- .../sql/common/setting/Settings.java | 3 ++ docs/user/beyond/partiql.rst | 10 ++-- docs/user/optimization/optimization.rst | 22 ++++---- docs/user/ppl/general/datatypes.rst | 10 ++-- docs/user/ppl/interfaces/endpoint.rst | 2 +- .../sql/legacy/ObjectFieldSelectIT.java | 2 +- .../org/opensearch/sql/ppl/StandaloneIT.java | 1 + .../java/org/opensearch/sql/sql/NestedIT.java | 2 +- .../sql/sql/StandalonePaginationIT.java | 1 + .../ppl/explain_filter_agg_push.json | 2 +- .../ppl/explain_filter_push.json | 2 +- .../ppl/explain_limit_push.json | 2 +- .../expectedOutput/ppl/explain_output.json | 2 +- .../expectedOutput/ppl/explain_sort_push.json | 2 +- .../request/OpenSearchQueryRequest.java | 50 +++++++++++++++++-- .../request/OpenSearchRequestBuilder.java | 24 +++++++-- .../request/OpenSearchScrollRequest.java | 2 +- .../response/OpenSearchResponse.java | 19 +++++-- .../setting/OpenSearchSettings.java | 18 ++++++- .../OpenSearchExecutionEngineTest.java | 1 + .../response/OpenSearchResponseTest.java | 26 +++++----- .../storage/scan/OpenSearchIndexScanTest.java | 1 + 22 files changed, 148 insertions(+), 56 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 d8e4b0722b..6a93a55c94 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},{'id': 2}] | [{'name': 'AWS Redshift Spectrum querying'},{'name': 'AWS Redshift security'},{'name': 'AWS Aurora security'}] | - +-----------------------------------------------------+-----------------------+----------------------------------------------------------------------------------------------------------------+ + +-----------------------------------------------------+-----------+----------------------------------------------------------------------------------------------------------------+ + | city | accounts | projects | + |-----------------------------------------------------+-----------+----------------------------------------------------------------------------------------------------------------| + | {'name': 'Seattle', 'location': {'latitude': 10.5}} | {'id': 1} | [{'name': 'AWS Redshift Spectrum querying'},{'name': 'AWS Redshift security'},{'name': 'AWS Aurora security'}] | + +-----------------------------------------------------+-----------+----------------------------------------------------------------------------------------------------------------+ Example 2: Selecting Deeper Levels ---------------------------------- diff --git a/docs/user/optimization/optimization.rst b/docs/user/optimization/optimization.rst index 454c9ec066..e96d47c65e 100644 --- a/docs/user/optimization/optimization.rst +++ b/docs/user/optimization/optimization.rst @@ -44,7 +44,7 @@ The consecutive Filter operator will be merged as one Filter operator:: { "name": "OpenSearchIndexScan", "description": { - "request": "OpenSearchQueryRequest(indexName=accounts, sourceBuilder={\"from\":0,\"size\":10000,\"timeout\":\"1m\",\"query\":{\"bool\":{\"filter\":[{\"range\":{\"age\":{\"from\":null,\"to\":20,\"include_lower\":true,\"include_upper\":false,\"boost\":1.0}}},{\"range\":{\"age\":{\"from\":10,\"to\":null,\"include_lower\":false,\"include_upper\":true,\"boost\":1.0}}}],\"adjust_pure_negative\":true,\"boost\":1.0}},\"_source\":{\"includes\":[\"age\"],\"excludes\":[]},\"sort\":[{\"_doc\":{\"order\":\"asc\"}}]}, searchDone=false)" + "request": "OpenSearchQueryRequest(indexName=accounts, sourceBuilder={\"from\":0,\"size\":10000,\"timeout\":\"1m\",\"query\":{\"bool\":{\"filter\":[{\"range\":{\"age\":{\"from\":null,\"to\":20,\"include_lower\":true,\"include_upper\":false,\"boost\":1.0}}},{\"range\":{\"age\":{\"from\":10,\"to\":null,\"include_lower\":false,\"include_upper\":true,\"boost\":1.0}}}],\"adjust_pure_negative\":true,\"boost\":1.0}},\"_source\":{\"includes\":[\"age\"],\"excludes\":[]},\"sort\":[{\"_doc\":{\"order\":\"asc\"}}]}, searchDone=false, fieldTypeTolerance=false)" }, "children": [] } @@ -71,7 +71,7 @@ The Filter operator should be push down under Sort operator:: { "name": "OpenSearchIndexScan", "description": { - "request": "OpenSearchQueryRequest(indexName=accounts, sourceBuilder={\"from\":0,\"size\":10000,\"timeout\":\"1m\",\"query\":{\"range\":{\"age\":{\"from\":null,\"to\":20,\"include_lower\":true,\"include_upper\":false,\"boost\":1.0}}},\"_source\":{\"includes\":[\"age\"],\"excludes\":[]},\"sort\":[{\"age\":{\"order\":\"asc\",\"missing\":\"_first\"}}]}, searchDone=false)" + "request": "OpenSearchQueryRequest(indexName=accounts, sourceBuilder={\"from\":0,\"size\":10000,\"timeout\":\"1m\",\"query\":{\"range\":{\"age\":{\"from\":null,\"to\":20,\"include_lower\":true,\"include_upper\":false,\"boost\":1.0}}},\"_source\":{\"includes\":[\"age\"],\"excludes\":[]},\"sort\":[{\"age\":{\"order\":\"asc\",\"missing\":\"_first\"}}]}, searchDone=false, fieldTypeTolerance=false)" }, "children": [] } @@ -102,7 +102,7 @@ The Project list will push down to Query DSL to `filter the source source = people | fields accounts, accounts.id; fetched rows / total rows = 1/1 - +-----------------------+-------------+ - | accounts | accounts.id | - |-----------------------+-------------| - | [{'id': 1},{'id': 2}] | 1 | - +-----------------------+-------------+ \ No newline at end of file + +-----------+-------------+ + | accounts | accounts.id | + |-----------+-------------| + | {'id': 1} | 1 | + +-----------+-------------+ \ No newline at end of file diff --git a/docs/user/ppl/interfaces/endpoint.rst b/docs/user/ppl/interfaces/endpoint.rst index fb931fb0ba..6477c2fd99 100644 --- a/docs/user/ppl/interfaces/endpoint.rst +++ b/docs/user/ppl/interfaces/endpoint.rst @@ -91,7 +91,7 @@ The following PPL query demonstrated that where and stats command were pushed do { "name": "OpenSearchIndexScan", "description": { - "request": "OpenSearchQueryRequest(indexName=accounts, sourceBuilder={\"from\":0,\"size\":10000,\"timeout\":\"1m\",\"query\":{\"range\":{\"age\":{\"from\":10,\"to\":null,\"include_lower\":false,\"include_upper\":true,\"boost\":1.0}}},\"sort\":[{\"_doc\":{\"order\":\"asc\"}}],\"aggregations\":{\"avg(age)\":{\"avg\":{\"field\":\"age\"}}}}, searchDone=false)" + "request": "OpenSearchQueryRequest(indexName=accounts, sourceBuilder={\"from\":0,\"size\":10000,\"timeout\":\"1m\",\"query\":{\"range\":{\"age\":{\"from\":10,\"to\":null,\"include_lower\":false,\"include_upper\":true,\"boost\":1.0}}},\"sort\":[{\"_doc\":{\"order\":\"asc\"}}],\"aggregations\":{\"avg(age)\":{\"avg\":{\"field\":\"age\"}}}}, searchDone=false, fieldTypeTolerance=false)" }, "children": [] } 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 9dd23e57c6..40ee963654 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 @@ -81,7 +81,7 @@ public void testSelectObjectFieldOfArrayValuesItself() { JSONObject response = new JSONObject(query("SELECT accounts FROM %s")); // Only the first element of the list of is returned. - verifyDataRows(response, rows(new JSONArray("[{\"id\":1},{\"id\":2}]"))); + verifyDataRows(response, rows(new JSONObject("{\"id\":1}"))); } @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..b8ac6f072d 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, false) .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 18d93dbb2a..4ae683c229 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 @@ -427,7 +427,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", new JSONArray(List.of("aa", "bb")), 6)); + verifyDataRows(result, rows("c", "ab", 4), rows("zz", "aa", 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..e4b3da96fc 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, false) .build(); @Override diff --git a/integ-test/src/test/resources/expectedOutput/ppl/explain_filter_agg_push.json b/integ-test/src/test/resources/expectedOutput/ppl/explain_filter_agg_push.json index 8035822357..8b17045306 100644 --- a/integ-test/src/test/resources/expectedOutput/ppl/explain_filter_agg_push.json +++ b/integ-test/src/test/resources/expectedOutput/ppl/explain_filter_agg_push.json @@ -8,7 +8,7 @@ { "name": "OpenSearchIndexScan", "description": { - "request": "OpenSearchQueryRequest(indexName\u003dopensearch-sql_test_index_account, sourceBuilder\u003d{\"from\":0,\"size\":10000,\"timeout\":\"1m\",\"query\":{\"range\":{\"age\":{\"from\":30,\"to\":null,\"include_lower\":false,\"include_upper\":true,\"boost\":1.0}}},\"sort\":[{\"_doc\":{\"order\":\"asc\"}}],\"aggregations\":{\"composite_buckets\":{\"composite\":{\"size\":1000,\"sources\":[{\"state\":{\"terms\":{\"field\":\"state.keyword\",\"missing_bucket\":true,\"missing_order\":\"first\",\"order\":\"asc\"}}},{\"city\":{\"terms\":{\"field\":\"city.keyword\",\"missing_bucket\":true,\"missing_order\":\"first\",\"order\":\"asc\"}}}]},\"aggregations\":{\"avg_age\":{\"avg\":{\"field\":\"age\"}}}}}}, needClean\u003dtrue, searchDone\u003dfalse, pitId\u003dnull, cursorKeepAlive\u003dnull, searchAfter\u003dnull, searchResponse\u003dnull)" + "request": "OpenSearchQueryRequest(indexName\u003dopensearch-sql_test_index_account, sourceBuilder\u003d{\"from\":0,\"size\":10000,\"timeout\":\"1m\",\"query\":{\"range\":{\"age\":{\"from\":30,\"to\":null,\"include_lower\":false,\"include_upper\":true,\"boost\":1.0}}},\"sort\":[{\"_doc\":{\"order\":\"asc\"}}],\"aggregations\":{\"composite_buckets\":{\"composite\":{\"size\":1000,\"sources\":[{\"state\":{\"terms\":{\"field\":\"state.keyword\",\"missing_bucket\":true,\"missing_order\":\"first\",\"order\":\"asc\"}}},{\"city\":{\"terms\":{\"field\":\"city.keyword\",\"missing_bucket\":true,\"missing_order\":\"first\",\"order\":\"asc\"}}}]},\"aggregations\":{\"avg_age\":{\"avg\":{\"field\":\"age\"}}}}}}, needClean\u003dtrue, searchDone\u003dfalse, pitId\u003dnull, cursorKeepAlive\u003dnull, searchAfter\u003dnull, searchResponse\u003dnull, fieldTypeTolerance\u003dfalse)" }, "children": [] } diff --git a/integ-test/src/test/resources/expectedOutput/ppl/explain_filter_push.json b/integ-test/src/test/resources/expectedOutput/ppl/explain_filter_push.json index 3e92a17b97..68a9eb81a0 100644 --- a/integ-test/src/test/resources/expectedOutput/ppl/explain_filter_push.json +++ b/integ-test/src/test/resources/expectedOutput/ppl/explain_filter_push.json @@ -8,7 +8,7 @@ { "name": "OpenSearchIndexScan", "description": { - "request": "OpenSearchQueryRequest(indexName\u003dopensearch-sql_test_index_account, sourceBuilder\u003d{\"from\":0,\"size\":10000,\"timeout\":\"1m\",\"query\":{\"bool\":{\"filter\":[{\"bool\":{\"filter\":[{\"range\":{\"balance\":{\"from\":10000,\"to\":null,\"include_lower\":false,\"include_upper\":true,\"boost\":1.0}}},{\"range\":{\"age\":{\"from\":null,\"to\":40,\"include_lower\":true,\"include_upper\":false,\"boost\":1.0}}}],\"adjust_pure_negative\":true,\"boost\":1.0}},{\"range\":{\"age\":{\"from\":30,\"to\":null,\"include_lower\":false,\"include_upper\":true,\"boost\":1.0}}}],\"adjust_pure_negative\":true,\"boost\":1.0}},\"_source\":{\"includes\":[\"age\"],\"excludes\":[]},\"sort\":[{\"_doc\":{\"order\":\"asc\"}}]}, needClean\u003dtrue, searchDone\u003dfalse, pitId\u003dnull, cursorKeepAlive\u003dnull, searchAfter\u003dnull, searchResponse\u003dnull)" + "request": "OpenSearchQueryRequest(indexName\u003dopensearch-sql_test_index_account, sourceBuilder\u003d{\"from\":0,\"size\":10000,\"timeout\":\"1m\",\"query\":{\"bool\":{\"filter\":[{\"bool\":{\"filter\":[{\"range\":{\"balance\":{\"from\":10000,\"to\":null,\"include_lower\":false,\"include_upper\":true,\"boost\":1.0}}},{\"range\":{\"age\":{\"from\":null,\"to\":40,\"include_lower\":true,\"include_upper\":false,\"boost\":1.0}}}],\"adjust_pure_negative\":true,\"boost\":1.0}},{\"range\":{\"age\":{\"from\":30,\"to\":null,\"include_lower\":false,\"include_upper\":true,\"boost\":1.0}}}],\"adjust_pure_negative\":true,\"boost\":1.0}},\"_source\":{\"includes\":[\"age\"],\"excludes\":[]},\"sort\":[{\"_doc\":{\"order\":\"asc\"}}]}, needClean\u003dtrue, searchDone\u003dfalse, pitId\u003dnull, cursorKeepAlive\u003dnull, searchAfter\u003dnull, searchResponse\u003dnull, fieldTypeTolerance\u003dfalse)" }, "children": [] } diff --git a/integ-test/src/test/resources/expectedOutput/ppl/explain_limit_push.json b/integ-test/src/test/resources/expectedOutput/ppl/explain_limit_push.json index 0a0b58f17d..7481620bbb 100644 --- a/integ-test/src/test/resources/expectedOutput/ppl/explain_limit_push.json +++ b/integ-test/src/test/resources/expectedOutput/ppl/explain_limit_push.json @@ -16,7 +16,7 @@ { "name": "OpenSearchIndexScan", "description": { - "request": "OpenSearchQueryRequest(indexName=opensearch-sql_test_index_account, sourceBuilder={\"from\":0,\"size\":5,\"timeout\":\"1m\"}, needClean\u003dtrue, searchDone\u003dfalse, pitId\u003dnull, cursorKeepAlive\u003dnull, searchAfter\u003dnull, searchResponse\u003dnull)" + "request": "OpenSearchQueryRequest(indexName=opensearch-sql_test_index_account, sourceBuilder={\"from\":0,\"size\":5,\"timeout\":\"1m\"}, needClean\u003dtrue, searchDone\u003dfalse, pitId\u003dnull, cursorKeepAlive\u003dnull, searchAfter\u003dnull, searchResponse\u003dnull, fieldTypeTolerance\u003dfalse)" }, "children": [] } diff --git a/integ-test/src/test/resources/expectedOutput/ppl/explain_output.json b/integ-test/src/test/resources/expectedOutput/ppl/explain_output.json index bd7310810e..ebae60b063 100644 --- a/integ-test/src/test/resources/expectedOutput/ppl/explain_output.json +++ b/integ-test/src/test/resources/expectedOutput/ppl/explain_output.json @@ -31,7 +31,7 @@ { "name": "OpenSearchIndexScan", "description": { - "request": "OpenSearchQueryRequest(indexName\u003dopensearch-sql_test_index_account, sourceBuilder\u003d{\"from\":0,\"size\":10000,\"timeout\":\"1m\",\"query\":{\"range\":{\"age\":{\"from\":30,\"to\":null,\"include_lower\":false,\"include_upper\":true,\"boost\":1.0}}},\"sort\":[{\"_doc\":{\"order\":\"asc\"}}],\"aggregations\":{\"composite_buckets\":{\"composite\":{\"size\":1000,\"sources\":[{\"state\":{\"terms\":{\"field\":\"state.keyword\",\"missing_bucket\":true,\"missing_order\":\"first\",\"order\":\"asc\"}}},{\"city\":{\"terms\":{\"field\":\"city.keyword\",\"missing_bucket\":true,\"missing_order\":\"first\",\"order\":\"asc\"}}}]},\"aggregations\":{\"avg_age\":{\"avg\":{\"field\":\"age\"}}}}}}, needClean\u003dtrue, searchDone\u003dfalse, pitId\u003dnull, cursorKeepAlive\u003dnull, searchAfter\u003dnull, searchResponse\u003dnull)" + "request": "OpenSearchQueryRequest(indexName\u003dopensearch-sql_test_index_account, sourceBuilder\u003d{\"from\":0,\"size\":10000,\"timeout\":\"1m\",\"query\":{\"range\":{\"age\":{\"from\":30,\"to\":null,\"include_lower\":false,\"include_upper\":true,\"boost\":1.0}}},\"sort\":[{\"_doc\":{\"order\":\"asc\"}}],\"aggregations\":{\"composite_buckets\":{\"composite\":{\"size\":1000,\"sources\":[{\"state\":{\"terms\":{\"field\":\"state.keyword\",\"missing_bucket\":true,\"missing_order\":\"first\",\"order\":\"asc\"}}},{\"city\":{\"terms\":{\"field\":\"city.keyword\",\"missing_bucket\":true,\"missing_order\":\"first\",\"order\":\"asc\"}}}]},\"aggregations\":{\"avg_age\":{\"avg\":{\"field\":\"age\"}}}}}}, needClean\u003dtrue, searchDone\u003dfalse, pitId\u003dnull, cursorKeepAlive\u003dnull, searchAfter\u003dnull, searchResponse\u003dnull, fieldTypeTolerance\u003dfalse)" }, "children": [] } diff --git a/integ-test/src/test/resources/expectedOutput/ppl/explain_sort_push.json b/integ-test/src/test/resources/expectedOutput/ppl/explain_sort_push.json index e2630e24f9..10e0b2080f 100644 --- a/integ-test/src/test/resources/expectedOutput/ppl/explain_sort_push.json +++ b/integ-test/src/test/resources/expectedOutput/ppl/explain_sort_push.json @@ -8,7 +8,7 @@ { "name": "OpenSearchIndexScan", "description": { - "request": "OpenSearchQueryRequest(indexName\u003dopensearch-sql_test_index_account, sourceBuilder\u003d{\"from\":0,\"size\":10000,\"timeout\":\"1m\",\"query\":{\"range\":{\"age\":{\"from\":30,\"to\":null,\"include_lower\":false,\"include_upper\":true,\"boost\":1.0}}},\"_source\":{\"includes\":[\"age\"],\"excludes\":[]},\"sort\":[{\"age\":{\"order\":\"asc\",\"missing\":\"_first\"}}]}, needClean\u003dtrue, searchDone\u003dfalse, pitId\u003dnull, cursorKeepAlive\u003dnull, searchAfter\u003dnull, searchResponse\u003dnull)" + "request": "OpenSearchQueryRequest(indexName\u003dopensearch-sql_test_index_account, sourceBuilder\u003d{\"from\":0,\"size\":10000,\"timeout\":\"1m\",\"query\":{\"range\":{\"age\":{\"from\":30,\"to\":null,\"include_lower\":false,\"include_upper\":true,\"boost\":1.0}}},\"_source\":{\"includes\":[\"age\"],\"excludes\":[]},\"sort\":[{\"age\":{\"order\":\"asc\",\"missing\":\"_first\"}}]}, needClean\u003dtrue, searchDone\u003dfalse, pitId\u003dnull, cursorKeepAlive\u003dnull, searchAfter\u003dnull, searchResponse\u003dnull, fieldTypeTolerance\u003dfalse)" }, "children": [] } 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 18ad1809c1..1e8c5e0268 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 @@ -73,6 +73,8 @@ public class OpenSearchQueryRequest implements OpenSearchRequest { private SearchResponse searchResponse = null; + private boolean fieldTypeTolerance; + /** Constructor of OpenSearchQueryRequest. */ public OpenSearchQueryRequest( String indexName, int size, OpenSearchExprValueFactory factory, List includes) { @@ -101,6 +103,21 @@ public OpenSearchQueryRequest( this.sourceBuilder = sourceBuilder; this.exprValueFactory = factory; this.includes = includes; + this.fieldTypeTolerance = false; + } + + /** Constructor of OpenSearchQueryRequest. */ + public OpenSearchQueryRequest( + IndexName indexName, + SearchSourceBuilder sourceBuilder, + OpenSearchExprValueFactory factory, + List includes, + boolean fieldTypeTolerance) { + this.indexName = indexName; + this.sourceBuilder = sourceBuilder; + this.exprValueFactory = factory; + this.includes = includes; + this.fieldTypeTolerance = fieldTypeTolerance; } /** Constructor of OpenSearchQueryRequest with PIT support. */ @@ -117,6 +134,25 @@ public OpenSearchQueryRequest( this.includes = includes; this.cursorKeepAlive = cursorKeepAlive; this.pitId = pitId; + this.fieldTypeTolerance = false; + } + + /** Constructor of OpenSearchQueryRequest with PIT support. */ + public OpenSearchQueryRequest( + IndexName indexName, + SearchSourceBuilder sourceBuilder, + OpenSearchExprValueFactory factory, + List includes, + TimeValue cursorKeepAlive, + String pitId, + boolean fieldTypeTolerance) { + this.indexName = indexName; + this.sourceBuilder = sourceBuilder; + this.exprValueFactory = factory; + this.includes = includes; + this.cursorKeepAlive = cursorKeepAlive; + this.pitId = pitId; + this.fieldTypeTolerance = fieldTypeTolerance; } /** @@ -161,14 +197,16 @@ public OpenSearchResponse search( if (this.pitId == null) { // When SearchRequest doesn't contain PitId, fetch single page request if (searchDone) { - return new OpenSearchResponse(SearchHits.empty(), exprValueFactory, includes); + return new OpenSearchResponse( + SearchHits.empty(), exprValueFactory, includes, fieldTypeTolerance); } else { searchDone = true; return new OpenSearchResponse( searchAction.apply( new SearchRequest().indices(indexName.getIndexNames()).source(sourceBuilder)), exprValueFactory, - includes); + includes, + fieldTypeTolerance); } } else { // Search with PIT instead of scroll API @@ -179,7 +217,9 @@ public OpenSearchResponse search( public OpenSearchResponse searchWithPIT(Function searchAction) { OpenSearchResponse openSearchResponse; if (searchDone) { - openSearchResponse = new OpenSearchResponse(SearchHits.empty(), exprValueFactory, includes); + openSearchResponse = + new OpenSearchResponse( + SearchHits.empty(), exprValueFactory, includes, fieldTypeTolerance); } else { this.sourceBuilder.pointInTimeBuilder(new PointInTimeBuilder(this.pitId)); this.sourceBuilder.timeout(cursorKeepAlive); @@ -197,7 +237,9 @@ public OpenSearchResponse searchWithPIT(Function SearchRequest searchRequest = new SearchRequest().source(this.sourceBuilder); this.searchResponse = searchAction.apply(searchRequest); - openSearchResponse = new OpenSearchResponse(this.searchResponse, exprValueFactory, includes); + openSearchResponse = + new OpenSearchResponse( + this.searchResponse, exprValueFactory, includes, fieldTypeTolerance); needClean = openSearchResponse.isEmpty(); searchDone = openSearchResponse.isEmpty(); diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/request/OpenSearchRequestBuilder.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/request/OpenSearchRequestBuilder.java index 6fa9b17697..0a6a8a953b 100644 --- a/opensearch/src/main/java/org/opensearch/sql/opensearch/request/OpenSearchRequestBuilder.java +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/request/OpenSearchRequestBuilder.java @@ -110,6 +110,7 @@ private OpenSearchRequest buildRequestWithPit( int size = requestedTotalSize; FetchSourceContext fetchSource = this.sourceBuilder.fetchSource(); List includes = fetchSource != null ? Arrays.asList(fetchSource.includes()) : List.of(); + boolean fieldTypeTolerance = settings.getSettingValue(Settings.Key.FIELD_TYPE_TOLERANCE); if (pageSize == null) { if (startFrom + size > maxResultWindow) { @@ -117,12 +118,19 @@ private OpenSearchRequest buildRequestWithPit( // Search with PIT request String pitId = createPit(indexName, cursorKeepAlive, client); return new OpenSearchQueryRequest( - indexName, sourceBuilder, exprValueFactory, includes, cursorKeepAlive, pitId); + indexName, + sourceBuilder, + exprValueFactory, + includes, + cursorKeepAlive, + pitId, + fieldTypeTolerance); } else { sourceBuilder.from(startFrom); sourceBuilder.size(requestedTotalSize); // Search with non-Pit request - return new OpenSearchQueryRequest(indexName, sourceBuilder, exprValueFactory, includes); + return new OpenSearchQueryRequest( + indexName, sourceBuilder, exprValueFactory, includes, fieldTypeTolerance); } } else { if (startFrom != 0) { @@ -132,7 +140,13 @@ private OpenSearchRequest buildRequestWithPit( // Search with PIT request String pitId = createPit(indexName, cursorKeepAlive, client); return new OpenSearchQueryRequest( - indexName, sourceBuilder, exprValueFactory, includes, cursorKeepAlive, pitId); + indexName, + sourceBuilder, + exprValueFactory, + includes, + cursorKeepAlive, + pitId, + fieldTypeTolerance); } } @@ -141,6 +155,7 @@ private OpenSearchRequest buildRequestWithScroll( int size = requestedTotalSize; FetchSourceContext fetchSource = this.sourceBuilder.fetchSource(); List includes = fetchSource != null ? Arrays.asList(fetchSource.includes()) : List.of(); + boolean fieldTypeTolerance = settings.getSettingValue(Settings.Key.FIELD_TYPE_TOLERANCE); if (pageSize == null) { if (startFrom + size > maxResultWindow) { @@ -150,7 +165,8 @@ private OpenSearchRequest buildRequestWithScroll( } else { sourceBuilder.from(startFrom); sourceBuilder.size(requestedTotalSize); - return new OpenSearchQueryRequest(indexName, sourceBuilder, exprValueFactory, includes); + return new OpenSearchQueryRequest( + indexName, sourceBuilder, exprValueFactory, includes, fieldTypeTolerance); } } else { if (startFrom != 0) { 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..7b0fd55aae 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 @@ -102,7 +102,7 @@ public OpenSearchResponse search( openSearchResponse = searchAction.apply(initialSearchRequest); } - var response = new OpenSearchResponse(openSearchResponse, exprValueFactory, includes); + var response = new OpenSearchResponse(openSearchResponse, exprValueFactory, includes, false); needClean = response.isEmpty(); if (!needClean) { setScrollId(openSearchResponse.getScrollId()); diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/response/OpenSearchResponse.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/response/OpenSearchResponse.java index a77863e7db..b6ead66539 100644 --- a/opensearch/src/main/java/org/opensearch/sql/opensearch/response/OpenSearchResponse.java +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/response/OpenSearchResponse.java @@ -48,6 +48,8 @@ public class OpenSearchResponse implements Iterable { /** List of requested include fields. */ private final List includes; + private boolean fieldTypeTolerance; + /** OpenSearchExprValueFactory used to build ExprValue from search result. */ @EqualsAndHashCode.Exclude private final OpenSearchExprValueFactory exprValueFactory; @@ -55,20 +57,26 @@ public class OpenSearchResponse implements Iterable { public OpenSearchResponse( SearchResponse searchResponse, OpenSearchExprValueFactory exprValueFactory, - List includes) { + List includes, + boolean fieldTypeTolerance) { this.hits = searchResponse.getHits(); this.aggregations = searchResponse.getAggregations(); this.exprValueFactory = exprValueFactory; this.includes = includes; + this.fieldTypeTolerance = fieldTypeTolerance; } /** Constructor of OpenSearchResponse with SearchHits. */ public OpenSearchResponse( - SearchHits hits, OpenSearchExprValueFactory exprValueFactory, List includes) { + SearchHits hits, + OpenSearchExprValueFactory exprValueFactory, + List includes, + boolean fieldTypeTolerance) { this.hits = hits; this.aggregations = null; this.exprValueFactory = exprValueFactory; this.includes = includes; + this.fieldTypeTolerance = fieldTypeTolerance; } /** @@ -116,7 +124,12 @@ public Iterator iterator() { */ private void addParsedHitsToBuilder( ImmutableMap.Builder builder, SearchHit hit) { - builder.putAll(exprValueFactory.construct(hit.getSourceAsString(), true).tupleValue()); + builder.putAll( + exprValueFactory + .construct( + hit.getSourceAsString(), + fieldTypeTolerance || !(hit.getInnerHits() == null || hit.getInnerHits().isEmpty())) + .tupleValue()); } /** 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..04e4b3acbd 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(), + false, + 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/test/java/org/opensearch/sql/opensearch/executor/OpenSearchExecutionEngineTest.java b/opensearch/src/test/java/org/opensearch/sql/opensearch/executor/OpenSearchExecutionEngineTest.java index e5cf94eb86..d5eb06cd5a 100644 --- a/opensearch/src/test/java/org/opensearch/sql/opensearch/executor/OpenSearchExecutionEngineTest.java +++ b/opensearch/src/test/java/org/opensearch/sql/opensearch/executor/OpenSearchExecutionEngineTest.java @@ -175,6 +175,7 @@ void explain_successfully() { Settings settings = mock(Settings.class); when(settings.getSettingValue(SQL_CURSOR_KEEP_ALIVE)).thenReturn(TimeValue.timeValueMinutes(1)); when(settings.getSettingValue(Settings.Key.SQL_PAGINATION_API_SEARCH_AFTER)).thenReturn(true); + when(settings.getSettingValue(Settings.Key.FIELD_TYPE_TOLERANCE)).thenReturn(false); OpenSearchExprValueFactory exprValueFactory = mock(OpenSearchExprValueFactory.class); final var name = new OpenSearchRequest.IndexName("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 d66087b176..7b80c5a475 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 @@ -77,24 +77,24 @@ void isEmpty() { new TotalHits(2L, TotalHits.Relation.EQUAL_TO), 1.0F)); - var response = new OpenSearchResponse(searchResponse, factory, includes); + var response = new OpenSearchResponse(searchResponse, factory, includes, false); assertFalse(response.isEmpty()); when(searchResponse.getHits()).thenReturn(SearchHits.empty()); when(searchResponse.getAggregations()).thenReturn(null); - response = new OpenSearchResponse(searchResponse, factory, includes); + response = new OpenSearchResponse(searchResponse, factory, includes, false); assertTrue(response.isEmpty()); when(searchResponse.getHits()) .thenReturn(new SearchHits(null, new TotalHits(0, TotalHits.Relation.EQUAL_TO), 0)); - response = new OpenSearchResponse(searchResponse, factory, includes); + response = new OpenSearchResponse(searchResponse, factory, includes, false); assertTrue(response.isEmpty()); when(searchResponse.getHits()).thenReturn(SearchHits.empty()); when(searchResponse.getAggregations()).thenReturn(new Aggregations(emptyList())); - response = new OpenSearchResponse(searchResponse, factory, includes); + response = new OpenSearchResponse(searchResponse, factory, includes, false); assertFalse(response.isEmpty()); } @@ -114,7 +114,7 @@ void iterator() { .thenReturn(exprTupleValue2); int i = 0; - for (ExprValue hit : new OpenSearchResponse(searchResponse, factory, List.of("id1"))) { + for (ExprValue hit : new OpenSearchResponse(searchResponse, factory, List.of("id1"), false)) { if (i == 0) { assertEquals(exprTupleValue1.tupleValue().get("id"), hit.tupleValue().get("id")); } else if (i == 1) { @@ -163,7 +163,7 @@ void iterator_metafields() { "_maxscore", new ExprFloatValue(3.75F))); List includes = List.of("id1", "_index", "_id", "_routing", "_sort", "_score", "_maxscore"); int i = 0; - for (ExprValue hit : new OpenSearchResponse(searchResponse, factory, includes)) { + for (ExprValue hit : new OpenSearchResponse(searchResponse, factory, includes, false)) { if (i == 0) { assertEquals(exprTupleResponse, hit); } else { @@ -194,7 +194,7 @@ void iterator_metafields_withoutIncludes() { ExprTupleValue exprTupleResponse = ExprTupleValue.fromExprValueMap(ImmutableMap.of("id1", new ExprIntegerValue(1))); int i = 0; - for (ExprValue hit : new OpenSearchResponse(searchResponse, factory, includes)) { + for (ExprValue hit : new OpenSearchResponse(searchResponse, factory, includes, true)) { if (i == 0) { assertEquals(exprTupleResponse, hit); } else { @@ -234,7 +234,7 @@ void iterator_metafields_scoreNaN() { "_id", new ExprStringValue("testId"), "_sort", new ExprLongValue(123456L))); int i = 0; - for (ExprValue hit : new OpenSearchResponse(searchResponse, factory, includes)) { + for (ExprValue hit : new OpenSearchResponse(searchResponse, factory, includes, true)) { if (i == 0) { assertEquals(exprTupleResponse, hit); } else { @@ -255,7 +255,7 @@ void iterator_with_inner_hits() { when(factory.construct(any(), anyBoolean())).thenReturn(exprTupleValue1); - for (ExprValue hit : new OpenSearchResponse(searchResponse, factory, includes)) { + for (ExprValue hit : new OpenSearchResponse(searchResponse, factory, includes, false)) { assertEquals(exprTupleValue1, hit); } } @@ -264,7 +264,7 @@ void iterator_with_inner_hits() { void response_is_aggregation_when_aggregation_not_empty() { when(searchResponse.getAggregations()).thenReturn(aggregations); - OpenSearchResponse response = new OpenSearchResponse(searchResponse, factory, includes); + OpenSearchResponse response = new OpenSearchResponse(searchResponse, factory, includes, false); assertTrue(response.isAggregationResponse()); } @@ -272,7 +272,7 @@ void response_is_aggregation_when_aggregation_not_empty() { void response_isnot_aggregation_when_aggregation_is_empty() { when(searchResponse.getAggregations()).thenReturn(null); - OpenSearchResponse response = new OpenSearchResponse(searchResponse, factory, includes); + OpenSearchResponse response = new OpenSearchResponse(searchResponse, factory, includes, false); assertFalse(response.isAggregationResponse()); } @@ -289,7 +289,7 @@ void aggregation_iterator() { .thenReturn(new ExprIntegerValue(2)); int i = 0; - for (ExprValue hit : new OpenSearchResponse(searchResponse, factory, includes)) { + for (ExprValue hit : new OpenSearchResponse(searchResponse, factory, includes, false)) { if (i == 0) { assertEquals(exprTupleValue1, hit); } else if (i == 1) { @@ -321,7 +321,7 @@ void highlight_iterator() { when(searchHit1.getHighlightFields()).thenReturn(highlightMap); when(factory.construct(any(), anyBoolean())).thenReturn(resultTuple); - for (ExprValue resultHit : new OpenSearchResponse(searchResponse, factory, includes)) { + for (ExprValue resultHit : new OpenSearchResponse(searchResponse, factory, includes, false)) { var expected = ExprValueUtils.collectionValue( Arrays.stream(searchHit.getHighlightFields().get("highlights").getFragments()) 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..2502b39df3 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 @@ -85,6 +85,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 From 1fba9ecaed24101b839b491634491e3639ce1bd9 Mon Sep 17 00:00:00 2001 From: Norman Jordan Date: Wed, 23 Oct 2024 15:27:38 -0700 Subject: [PATCH 3/6] Fixed some more tests Signed-off-by: Norman Jordan --- .../executor/protector/OpenSearchExecutionProtectorTest.java | 1 + .../sql/opensearch/request/OpenSearchQueryRequestTest.java | 3 +-- .../sql/opensearch/request/OpenSearchRequestBuilderTest.java | 1 + .../opensearch/sql/opensearch/storage/OpenSearchIndexTest.java | 1 + .../storage/scan/OpenSearchIndexScanPaginationTest.java | 1 + 5 files changed, 5 insertions(+), 2 deletions(-) diff --git a/opensearch/src/test/java/org/opensearch/sql/opensearch/executor/protector/OpenSearchExecutionProtectorTest.java b/opensearch/src/test/java/org/opensearch/sql/opensearch/executor/protector/OpenSearchExecutionProtectorTest.java index da06c1eb66..767da431aa 100644 --- a/opensearch/src/test/java/org/opensearch/sql/opensearch/executor/protector/OpenSearchExecutionProtectorTest.java +++ b/opensearch/src/test/java/org/opensearch/sql/opensearch/executor/protector/OpenSearchExecutionProtectorTest.java @@ -90,6 +90,7 @@ public void setup() { @Test void test_protect_indexScan() { when(settings.getSettingValue(Settings.Key.SQL_PAGINATION_API_SEARCH_AFTER)).thenReturn(true); + when(settings.getSettingValue(Settings.Key.FIELD_TYPE_TOLERANCE)).thenReturn(false); String indexName = "test"; final int maxResultWindow = 10000; 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/storage/OpenSearchIndexTest.java b/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/OpenSearchIndexTest.java index ef6b86c42a..fc95395b17 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 @@ -82,6 +82,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/storage/scan/OpenSearchIndexScanPaginationTest.java b/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/scan/OpenSearchIndexScanPaginationTest.java index e6a17aceaf..28006894a7 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(false); } @Mock private OpenSearchClient client; From bd462c78c1b06eb21a20ae4b51a2534922b337c1 Mon Sep 17 00:00:00 2001 From: Norman Jordan Date: Wed, 23 Oct 2024 22:42:28 -0700 Subject: [PATCH 4/6] Reworked to move pass the configuration through OpenSearchExprValueFactory Signed-off-by: Norman Jordan --- docs/user/beyond/partiql.rst | 10 +- docs/user/optimization/optimization.rst | 22 +-- docs/user/ppl/general/datatypes.rst | 10 +- docs/user/ppl/interfaces/endpoint.rst | 2 +- .../sql/legacy/ObjectFieldSelectIT.java | 4 +- .../java/org/opensearch/sql/sql/NestedIT.java | 2 +- .../sql/sql/StandalonePaginationIT.java | 2 +- .../ppl/explain_filter_agg_push.json | 2 +- .../ppl/explain_filter_push.json | 2 +- .../ppl/explain_limit_push.json | 2 +- .../expectedOutput/ppl/explain_output.json | 2 +- .../expectedOutput/ppl/explain_sort_push.json | 2 +- .../value/OpenSearchExprValueFactory.java | 8 +- .../request/OpenSearchQueryRequest.java | 57 ++----- .../request/OpenSearchRequestBuilder.java | 22 +-- .../request/OpenSearchScrollRequest.java | 6 +- .../response/OpenSearchResponse.java | 14 +- .../setting/OpenSearchSettings.java | 2 +- .../opensearch/storage/OpenSearchIndex.java | 8 +- .../storage/scan/OpenSearchIndexScan.java | 12 +- .../storage/script/core/ExpressionScript.java | 2 +- .../value/OpenSearchExprValueFactoryTest.java | 141 ++++++++++++++---- .../OpenSearchExecutionEngineTest.java | 3 +- .../OpenSearchExecutionProtectorTest.java | 9 +- .../request/OpenSearchQueryRequestTest.java | 2 +- .../response/OpenSearchResponseTest.java | 36 +++-- .../storage/OpenSearchIndexTest.java | 23 ++- .../OpenSearchIndexScanPaginationTest.java | 7 +- .../storage/scan/OpenSearchIndexScanTest.java | 27 ++-- 29 files changed, 259 insertions(+), 182 deletions(-) diff --git a/docs/user/beyond/partiql.rst b/docs/user/beyond/partiql.rst index 6a93a55c94..d8e4b0722b 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/optimization/optimization.rst b/docs/user/optimization/optimization.rst index e96d47c65e..454c9ec066 100644 --- a/docs/user/optimization/optimization.rst +++ b/docs/user/optimization/optimization.rst @@ -44,7 +44,7 @@ The consecutive Filter operator will be merged as one Filter operator:: { "name": "OpenSearchIndexScan", "description": { - "request": "OpenSearchQueryRequest(indexName=accounts, sourceBuilder={\"from\":0,\"size\":10000,\"timeout\":\"1m\",\"query\":{\"bool\":{\"filter\":[{\"range\":{\"age\":{\"from\":null,\"to\":20,\"include_lower\":true,\"include_upper\":false,\"boost\":1.0}}},{\"range\":{\"age\":{\"from\":10,\"to\":null,\"include_lower\":false,\"include_upper\":true,\"boost\":1.0}}}],\"adjust_pure_negative\":true,\"boost\":1.0}},\"_source\":{\"includes\":[\"age\"],\"excludes\":[]},\"sort\":[{\"_doc\":{\"order\":\"asc\"}}]}, searchDone=false, fieldTypeTolerance=false)" + "request": "OpenSearchQueryRequest(indexName=accounts, sourceBuilder={\"from\":0,\"size\":10000,\"timeout\":\"1m\",\"query\":{\"bool\":{\"filter\":[{\"range\":{\"age\":{\"from\":null,\"to\":20,\"include_lower\":true,\"include_upper\":false,\"boost\":1.0}}},{\"range\":{\"age\":{\"from\":10,\"to\":null,\"include_lower\":false,\"include_upper\":true,\"boost\":1.0}}}],\"adjust_pure_negative\":true,\"boost\":1.0}},\"_source\":{\"includes\":[\"age\"],\"excludes\":[]},\"sort\":[{\"_doc\":{\"order\":\"asc\"}}]}, searchDone=false)" }, "children": [] } @@ -71,7 +71,7 @@ The Filter operator should be push down under Sort operator:: { "name": "OpenSearchIndexScan", "description": { - "request": "OpenSearchQueryRequest(indexName=accounts, sourceBuilder={\"from\":0,\"size\":10000,\"timeout\":\"1m\",\"query\":{\"range\":{\"age\":{\"from\":null,\"to\":20,\"include_lower\":true,\"include_upper\":false,\"boost\":1.0}}},\"_source\":{\"includes\":[\"age\"],\"excludes\":[]},\"sort\":[{\"age\":{\"order\":\"asc\",\"missing\":\"_first\"}}]}, searchDone=false, fieldTypeTolerance=false)" + "request": "OpenSearchQueryRequest(indexName=accounts, sourceBuilder={\"from\":0,\"size\":10000,\"timeout\":\"1m\",\"query\":{\"range\":{\"age\":{\"from\":null,\"to\":20,\"include_lower\":true,\"include_upper\":false,\"boost\":1.0}}},\"_source\":{\"includes\":[\"age\"],\"excludes\":[]},\"sort\":[{\"age\":{\"order\":\"asc\",\"missing\":\"_first\"}}]}, searchDone=false)" }, "children": [] } @@ -102,7 +102,7 @@ The Project list will push down to Query DSL to `filter the source 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 | + +-----------------------+-------------+ \ No newline at end of file diff --git a/docs/user/ppl/interfaces/endpoint.rst b/docs/user/ppl/interfaces/endpoint.rst index 6477c2fd99..fb931fb0ba 100644 --- a/docs/user/ppl/interfaces/endpoint.rst +++ b/docs/user/ppl/interfaces/endpoint.rst @@ -91,7 +91,7 @@ The following PPL query demonstrated that where and stats command were pushed do { "name": "OpenSearchIndexScan", "description": { - "request": "OpenSearchQueryRequest(indexName=accounts, sourceBuilder={\"from\":0,\"size\":10000,\"timeout\":\"1m\",\"query\":{\"range\":{\"age\":{\"from\":10,\"to\":null,\"include_lower\":false,\"include_upper\":true,\"boost\":1.0}}},\"sort\":[{\"_doc\":{\"order\":\"asc\"}}],\"aggregations\":{\"avg(age)\":{\"avg\":{\"field\":\"age\"}}}}, searchDone=false, fieldTypeTolerance=false)" + "request": "OpenSearchQueryRequest(indexName=accounts, sourceBuilder={\"from\":0,\"size\":10000,\"timeout\":\"1m\",\"query\":{\"range\":{\"age\":{\"from\":10,\"to\":null,\"include_lower\":false,\"include_upper\":true,\"boost\":1.0}}},\"sort\":[{\"_doc\":{\"order\":\"asc\"}}],\"aggregations\":{\"avg(age)\":{\"avg\":{\"field\":\"age\"}}}}, searchDone=false)" }, "children": [] } 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 40ee963654..0bb9698f71 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 @@ -79,9 +79,7 @@ public void testSelectNestedFieldItself() { @Test public void testSelectObjectFieldOfArrayValuesItself() { JSONObject response = new JSONObject(query("SELECT accounts FROM %s")); - - // Only the first element of the list of is returned. - verifyDataRows(response, rows(new JSONObject("{\"id\":1}"))); + verifyDataRows(response, rows(new JSONArray("[{\"id\":1},{\"id\":2}]"))); } @Test 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 4ae683c229..18d93dbb2a 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 @@ -427,7 +427,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 e4b3da96fc..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,7 +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, false) + .put(Key.FIELD_TYPE_TOLERANCE, true) .build(); @Override diff --git a/integ-test/src/test/resources/expectedOutput/ppl/explain_filter_agg_push.json b/integ-test/src/test/resources/expectedOutput/ppl/explain_filter_agg_push.json index 8b17045306..8035822357 100644 --- a/integ-test/src/test/resources/expectedOutput/ppl/explain_filter_agg_push.json +++ b/integ-test/src/test/resources/expectedOutput/ppl/explain_filter_agg_push.json @@ -8,7 +8,7 @@ { "name": "OpenSearchIndexScan", "description": { - "request": "OpenSearchQueryRequest(indexName\u003dopensearch-sql_test_index_account, sourceBuilder\u003d{\"from\":0,\"size\":10000,\"timeout\":\"1m\",\"query\":{\"range\":{\"age\":{\"from\":30,\"to\":null,\"include_lower\":false,\"include_upper\":true,\"boost\":1.0}}},\"sort\":[{\"_doc\":{\"order\":\"asc\"}}],\"aggregations\":{\"composite_buckets\":{\"composite\":{\"size\":1000,\"sources\":[{\"state\":{\"terms\":{\"field\":\"state.keyword\",\"missing_bucket\":true,\"missing_order\":\"first\",\"order\":\"asc\"}}},{\"city\":{\"terms\":{\"field\":\"city.keyword\",\"missing_bucket\":true,\"missing_order\":\"first\",\"order\":\"asc\"}}}]},\"aggregations\":{\"avg_age\":{\"avg\":{\"field\":\"age\"}}}}}}, needClean\u003dtrue, searchDone\u003dfalse, pitId\u003dnull, cursorKeepAlive\u003dnull, searchAfter\u003dnull, searchResponse\u003dnull, fieldTypeTolerance\u003dfalse)" + "request": "OpenSearchQueryRequest(indexName\u003dopensearch-sql_test_index_account, sourceBuilder\u003d{\"from\":0,\"size\":10000,\"timeout\":\"1m\",\"query\":{\"range\":{\"age\":{\"from\":30,\"to\":null,\"include_lower\":false,\"include_upper\":true,\"boost\":1.0}}},\"sort\":[{\"_doc\":{\"order\":\"asc\"}}],\"aggregations\":{\"composite_buckets\":{\"composite\":{\"size\":1000,\"sources\":[{\"state\":{\"terms\":{\"field\":\"state.keyword\",\"missing_bucket\":true,\"missing_order\":\"first\",\"order\":\"asc\"}}},{\"city\":{\"terms\":{\"field\":\"city.keyword\",\"missing_bucket\":true,\"missing_order\":\"first\",\"order\":\"asc\"}}}]},\"aggregations\":{\"avg_age\":{\"avg\":{\"field\":\"age\"}}}}}}, needClean\u003dtrue, searchDone\u003dfalse, pitId\u003dnull, cursorKeepAlive\u003dnull, searchAfter\u003dnull, searchResponse\u003dnull)" }, "children": [] } diff --git a/integ-test/src/test/resources/expectedOutput/ppl/explain_filter_push.json b/integ-test/src/test/resources/expectedOutput/ppl/explain_filter_push.json index 68a9eb81a0..3e92a17b97 100644 --- a/integ-test/src/test/resources/expectedOutput/ppl/explain_filter_push.json +++ b/integ-test/src/test/resources/expectedOutput/ppl/explain_filter_push.json @@ -8,7 +8,7 @@ { "name": "OpenSearchIndexScan", "description": { - "request": "OpenSearchQueryRequest(indexName\u003dopensearch-sql_test_index_account, sourceBuilder\u003d{\"from\":0,\"size\":10000,\"timeout\":\"1m\",\"query\":{\"bool\":{\"filter\":[{\"bool\":{\"filter\":[{\"range\":{\"balance\":{\"from\":10000,\"to\":null,\"include_lower\":false,\"include_upper\":true,\"boost\":1.0}}},{\"range\":{\"age\":{\"from\":null,\"to\":40,\"include_lower\":true,\"include_upper\":false,\"boost\":1.0}}}],\"adjust_pure_negative\":true,\"boost\":1.0}},{\"range\":{\"age\":{\"from\":30,\"to\":null,\"include_lower\":false,\"include_upper\":true,\"boost\":1.0}}}],\"adjust_pure_negative\":true,\"boost\":1.0}},\"_source\":{\"includes\":[\"age\"],\"excludes\":[]},\"sort\":[{\"_doc\":{\"order\":\"asc\"}}]}, needClean\u003dtrue, searchDone\u003dfalse, pitId\u003dnull, cursorKeepAlive\u003dnull, searchAfter\u003dnull, searchResponse\u003dnull, fieldTypeTolerance\u003dfalse)" + "request": "OpenSearchQueryRequest(indexName\u003dopensearch-sql_test_index_account, sourceBuilder\u003d{\"from\":0,\"size\":10000,\"timeout\":\"1m\",\"query\":{\"bool\":{\"filter\":[{\"bool\":{\"filter\":[{\"range\":{\"balance\":{\"from\":10000,\"to\":null,\"include_lower\":false,\"include_upper\":true,\"boost\":1.0}}},{\"range\":{\"age\":{\"from\":null,\"to\":40,\"include_lower\":true,\"include_upper\":false,\"boost\":1.0}}}],\"adjust_pure_negative\":true,\"boost\":1.0}},{\"range\":{\"age\":{\"from\":30,\"to\":null,\"include_lower\":false,\"include_upper\":true,\"boost\":1.0}}}],\"adjust_pure_negative\":true,\"boost\":1.0}},\"_source\":{\"includes\":[\"age\"],\"excludes\":[]},\"sort\":[{\"_doc\":{\"order\":\"asc\"}}]}, needClean\u003dtrue, searchDone\u003dfalse, pitId\u003dnull, cursorKeepAlive\u003dnull, searchAfter\u003dnull, searchResponse\u003dnull)" }, "children": [] } diff --git a/integ-test/src/test/resources/expectedOutput/ppl/explain_limit_push.json b/integ-test/src/test/resources/expectedOutput/ppl/explain_limit_push.json index 7481620bbb..0a0b58f17d 100644 --- a/integ-test/src/test/resources/expectedOutput/ppl/explain_limit_push.json +++ b/integ-test/src/test/resources/expectedOutput/ppl/explain_limit_push.json @@ -16,7 +16,7 @@ { "name": "OpenSearchIndexScan", "description": { - "request": "OpenSearchQueryRequest(indexName=opensearch-sql_test_index_account, sourceBuilder={\"from\":0,\"size\":5,\"timeout\":\"1m\"}, needClean\u003dtrue, searchDone\u003dfalse, pitId\u003dnull, cursorKeepAlive\u003dnull, searchAfter\u003dnull, searchResponse\u003dnull, fieldTypeTolerance\u003dfalse)" + "request": "OpenSearchQueryRequest(indexName=opensearch-sql_test_index_account, sourceBuilder={\"from\":0,\"size\":5,\"timeout\":\"1m\"}, needClean\u003dtrue, searchDone\u003dfalse, pitId\u003dnull, cursorKeepAlive\u003dnull, searchAfter\u003dnull, searchResponse\u003dnull)" }, "children": [] } diff --git a/integ-test/src/test/resources/expectedOutput/ppl/explain_output.json b/integ-test/src/test/resources/expectedOutput/ppl/explain_output.json index ebae60b063..bd7310810e 100644 --- a/integ-test/src/test/resources/expectedOutput/ppl/explain_output.json +++ b/integ-test/src/test/resources/expectedOutput/ppl/explain_output.json @@ -31,7 +31,7 @@ { "name": "OpenSearchIndexScan", "description": { - "request": "OpenSearchQueryRequest(indexName\u003dopensearch-sql_test_index_account, sourceBuilder\u003d{\"from\":0,\"size\":10000,\"timeout\":\"1m\",\"query\":{\"range\":{\"age\":{\"from\":30,\"to\":null,\"include_lower\":false,\"include_upper\":true,\"boost\":1.0}}},\"sort\":[{\"_doc\":{\"order\":\"asc\"}}],\"aggregations\":{\"composite_buckets\":{\"composite\":{\"size\":1000,\"sources\":[{\"state\":{\"terms\":{\"field\":\"state.keyword\",\"missing_bucket\":true,\"missing_order\":\"first\",\"order\":\"asc\"}}},{\"city\":{\"terms\":{\"field\":\"city.keyword\",\"missing_bucket\":true,\"missing_order\":\"first\",\"order\":\"asc\"}}}]},\"aggregations\":{\"avg_age\":{\"avg\":{\"field\":\"age\"}}}}}}, needClean\u003dtrue, searchDone\u003dfalse, pitId\u003dnull, cursorKeepAlive\u003dnull, searchAfter\u003dnull, searchResponse\u003dnull, fieldTypeTolerance\u003dfalse)" + "request": "OpenSearchQueryRequest(indexName\u003dopensearch-sql_test_index_account, sourceBuilder\u003d{\"from\":0,\"size\":10000,\"timeout\":\"1m\",\"query\":{\"range\":{\"age\":{\"from\":30,\"to\":null,\"include_lower\":false,\"include_upper\":true,\"boost\":1.0}}},\"sort\":[{\"_doc\":{\"order\":\"asc\"}}],\"aggregations\":{\"composite_buckets\":{\"composite\":{\"size\":1000,\"sources\":[{\"state\":{\"terms\":{\"field\":\"state.keyword\",\"missing_bucket\":true,\"missing_order\":\"first\",\"order\":\"asc\"}}},{\"city\":{\"terms\":{\"field\":\"city.keyword\",\"missing_bucket\":true,\"missing_order\":\"first\",\"order\":\"asc\"}}}]},\"aggregations\":{\"avg_age\":{\"avg\":{\"field\":\"age\"}}}}}}, needClean\u003dtrue, searchDone\u003dfalse, pitId\u003dnull, cursorKeepAlive\u003dnull, searchAfter\u003dnull, searchResponse\u003dnull)" }, "children": [] } diff --git a/integ-test/src/test/resources/expectedOutput/ppl/explain_sort_push.json b/integ-test/src/test/resources/expectedOutput/ppl/explain_sort_push.json index 10e0b2080f..e2630e24f9 100644 --- a/integ-test/src/test/resources/expectedOutput/ppl/explain_sort_push.json +++ b/integ-test/src/test/resources/expectedOutput/ppl/explain_sort_push.json @@ -8,7 +8,7 @@ { "name": "OpenSearchIndexScan", "description": { - "request": "OpenSearchQueryRequest(indexName\u003dopensearch-sql_test_index_account, sourceBuilder\u003d{\"from\":0,\"size\":10000,\"timeout\":\"1m\",\"query\":{\"range\":{\"age\":{\"from\":30,\"to\":null,\"include_lower\":false,\"include_upper\":true,\"boost\":1.0}}},\"_source\":{\"includes\":[\"age\"],\"excludes\":[]},\"sort\":[{\"age\":{\"order\":\"asc\",\"missing\":\"_first\"}}]}, needClean\u003dtrue, searchDone\u003dfalse, pitId\u003dnull, cursorKeepAlive\u003dnull, searchAfter\u003dnull, searchResponse\u003dnull, fieldTypeTolerance\u003dfalse)" + "request": "OpenSearchQueryRequest(indexName\u003dopensearch-sql_test_index_account, sourceBuilder\u003d{\"from\":0,\"size\":10000,\"timeout\":\"1m\",\"query\":{\"range\":{\"age\":{\"from\":30,\"to\":null,\"include_lower\":false,\"include_upper\":true,\"boost\":1.0}}},\"_source\":{\"includes\":[\"age\"],\"excludes\":[]},\"sort\":[{\"age\":{\"order\":\"asc\",\"missing\":\"_first\"}}]}, needClean\u003dtrue, searchDone\u003dfalse, pitId\u003dnull, cursorKeepAlive\u003dnull, searchAfter\u003dnull, searchResponse\u003dnull)" }, "children": [] } 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 417aaddaee..401f21a83d 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 @@ -74,6 +74,8 @@ public class OpenSearchExprValueFactory { /** The Mapping of Field and ExprType. */ private final Map typeMapping; + private final boolean fieldTypeTolerance; + /** * Extend existing mapping by new data without overwrite. Called from aggregation only {@see * AggregationQueryBuilder#buildTypeMapping}. @@ -140,8 +142,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; } /** @@ -164,7 +168,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 1e8c5e0268..82ee56375a 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 @@ -73,8 +73,6 @@ public class OpenSearchQueryRequest implements OpenSearchRequest { private SearchResponse searchResponse = null; - private boolean fieldTypeTolerance; - /** Constructor of OpenSearchQueryRequest. */ public OpenSearchQueryRequest( String indexName, int size, OpenSearchExprValueFactory factory, List includes) { @@ -103,21 +101,6 @@ public OpenSearchQueryRequest( this.sourceBuilder = sourceBuilder; this.exprValueFactory = factory; this.includes = includes; - this.fieldTypeTolerance = false; - } - - /** Constructor of OpenSearchQueryRequest. */ - public OpenSearchQueryRequest( - IndexName indexName, - SearchSourceBuilder sourceBuilder, - OpenSearchExprValueFactory factory, - List includes, - boolean fieldTypeTolerance) { - this.indexName = indexName; - this.sourceBuilder = sourceBuilder; - this.exprValueFactory = factory; - this.includes = includes; - this.fieldTypeTolerance = fieldTypeTolerance; } /** Constructor of OpenSearchQueryRequest with PIT support. */ @@ -134,25 +117,6 @@ public OpenSearchQueryRequest( this.includes = includes; this.cursorKeepAlive = cursorKeepAlive; this.pitId = pitId; - this.fieldTypeTolerance = false; - } - - /** Constructor of OpenSearchQueryRequest with PIT support. */ - public OpenSearchQueryRequest( - IndexName indexName, - SearchSourceBuilder sourceBuilder, - OpenSearchExprValueFactory factory, - List includes, - TimeValue cursorKeepAlive, - String pitId, - boolean fieldTypeTolerance) { - this.indexName = indexName; - this.sourceBuilder = sourceBuilder; - this.exprValueFactory = factory; - this.includes = includes; - this.cursorKeepAlive = cursorKeepAlive; - this.pitId = pitId; - this.fieldTypeTolerance = fieldTypeTolerance; } /** @@ -162,7 +126,9 @@ public OpenSearchQueryRequest( * @param engine OpenSearchSqlEngine to get node-specific context. * @throws IOException thrown if reading from input {@code in} fails. */ - public OpenSearchQueryRequest(StreamInput in, OpenSearchStorageEngine engine) throws IOException { + public OpenSearchQueryRequest( + StreamInput in, OpenSearchStorageEngine engine, boolean fieldTypeTolerance) + throws IOException { // Deserialize the SearchSourceBuilder from the string representation String sourceBuilderString = in.readString(); @@ -187,7 +153,8 @@ 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(), fieldTypeTolerance); } @Override @@ -197,16 +164,14 @@ public OpenSearchResponse search( if (this.pitId == null) { // When SearchRequest doesn't contain PitId, fetch single page request if (searchDone) { - return new OpenSearchResponse( - SearchHits.empty(), exprValueFactory, includes, fieldTypeTolerance); + return new OpenSearchResponse(SearchHits.empty(), exprValueFactory, includes); } else { searchDone = true; return new OpenSearchResponse( searchAction.apply( new SearchRequest().indices(indexName.getIndexNames()).source(sourceBuilder)), exprValueFactory, - includes, - fieldTypeTolerance); + includes); } } else { // Search with PIT instead of scroll API @@ -217,9 +182,7 @@ public OpenSearchResponse search( public OpenSearchResponse searchWithPIT(Function searchAction) { OpenSearchResponse openSearchResponse; if (searchDone) { - openSearchResponse = - new OpenSearchResponse( - SearchHits.empty(), exprValueFactory, includes, fieldTypeTolerance); + openSearchResponse = new OpenSearchResponse(SearchHits.empty(), exprValueFactory, includes); } else { this.sourceBuilder.pointInTimeBuilder(new PointInTimeBuilder(this.pitId)); this.sourceBuilder.timeout(cursorKeepAlive); @@ -237,9 +200,7 @@ public OpenSearchResponse searchWithPIT(Function SearchRequest searchRequest = new SearchRequest().source(this.sourceBuilder); this.searchResponse = searchAction.apply(searchRequest); - openSearchResponse = - new OpenSearchResponse( - this.searchResponse, exprValueFactory, includes, fieldTypeTolerance); + openSearchResponse = new OpenSearchResponse(this.searchResponse, exprValueFactory, includes); needClean = openSearchResponse.isEmpty(); searchDone = openSearchResponse.isEmpty(); diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/request/OpenSearchRequestBuilder.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/request/OpenSearchRequestBuilder.java index 0a6a8a953b..bbba3cbd55 100644 --- a/opensearch/src/main/java/org/opensearch/sql/opensearch/request/OpenSearchRequestBuilder.java +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/request/OpenSearchRequestBuilder.java @@ -118,19 +118,12 @@ private OpenSearchRequest buildRequestWithPit( // Search with PIT request String pitId = createPit(indexName, cursorKeepAlive, client); return new OpenSearchQueryRequest( - indexName, - sourceBuilder, - exprValueFactory, - includes, - cursorKeepAlive, - pitId, - fieldTypeTolerance); + indexName, sourceBuilder, exprValueFactory, includes, cursorKeepAlive, pitId); } else { sourceBuilder.from(startFrom); sourceBuilder.size(requestedTotalSize); // Search with non-Pit request - return new OpenSearchQueryRequest( - indexName, sourceBuilder, exprValueFactory, includes, fieldTypeTolerance); + return new OpenSearchQueryRequest(indexName, sourceBuilder, exprValueFactory, includes); } } else { if (startFrom != 0) { @@ -140,13 +133,7 @@ private OpenSearchRequest buildRequestWithPit( // Search with PIT request String pitId = createPit(indexName, cursorKeepAlive, client); return new OpenSearchQueryRequest( - indexName, - sourceBuilder, - exprValueFactory, - includes, - cursorKeepAlive, - pitId, - fieldTypeTolerance); + indexName, sourceBuilder, exprValueFactory, includes, cursorKeepAlive, pitId); } } @@ -165,8 +152,7 @@ private OpenSearchRequest buildRequestWithScroll( } else { sourceBuilder.from(startFrom); sourceBuilder.size(requestedTotalSize); - return new OpenSearchQueryRequest( - indexName, sourceBuilder, exprValueFactory, includes, fieldTypeTolerance); + return new OpenSearchQueryRequest(indexName, sourceBuilder, exprValueFactory, includes); } } else { if (startFrom != 0) { 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 7b0fd55aae..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 @@ -102,7 +102,7 @@ public OpenSearchResponse search( openSearchResponse = searchAction.apply(initialSearchRequest); } - var response = new OpenSearchResponse(openSearchResponse, exprValueFactory, includes, false); + var response = new OpenSearchResponse(openSearchResponse, exprValueFactory, includes); needClean = response.isEmpty(); if (!needClean) { setScrollId(openSearchResponse.getScrollId()); @@ -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/response/OpenSearchResponse.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/response/OpenSearchResponse.java index b6ead66539..e43777a740 100644 --- a/opensearch/src/main/java/org/opensearch/sql/opensearch/response/OpenSearchResponse.java +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/response/OpenSearchResponse.java @@ -48,8 +48,6 @@ public class OpenSearchResponse implements Iterable { /** List of requested include fields. */ private final List includes; - private boolean fieldTypeTolerance; - /** OpenSearchExprValueFactory used to build ExprValue from search result. */ @EqualsAndHashCode.Exclude private final OpenSearchExprValueFactory exprValueFactory; @@ -57,26 +55,20 @@ public class OpenSearchResponse implements Iterable { public OpenSearchResponse( SearchResponse searchResponse, OpenSearchExprValueFactory exprValueFactory, - List includes, - boolean fieldTypeTolerance) { + List includes) { this.hits = searchResponse.getHits(); this.aggregations = searchResponse.getAggregations(); this.exprValueFactory = exprValueFactory; this.includes = includes; - this.fieldTypeTolerance = fieldTypeTolerance; } /** Constructor of OpenSearchResponse with SearchHits. */ public OpenSearchResponse( - SearchHits hits, - OpenSearchExprValueFactory exprValueFactory, - List includes, - boolean fieldTypeTolerance) { + SearchHits hits, OpenSearchExprValueFactory exprValueFactory, List includes) { this.hits = hits; this.aggregations = null; this.exprValueFactory = exprValueFactory; this.includes = includes; - this.fieldTypeTolerance = fieldTypeTolerance; } /** @@ -128,7 +120,7 @@ private void addParsedHitsToBuilder( exprValueFactory .construct( hit.getSourceAsString(), - fieldTypeTolerance || !(hit.getInnerHits() == null || hit.getInnerHits().isEmpty())) + !(hit.getInnerHits() == null || hit.getInnerHits().isEmpty())) .tupleValue()); } 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 04e4b3acbd..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 @@ -232,7 +232,7 @@ public class OpenSearchSettings extends Settings { public static final Setting FIELD_TYPE_TOLERANCE_SETTING = Setting.boolSetting( Key.FIELD_TYPE_TOLERANCE.getKeyValue(), - false, + true, Setting.Property.NodeScope, Setting.Property.Dynamic); 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..dff2bedf0f 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 @@ -168,6 +168,7 @@ public TableScanBuilder createScanBuilder() { requestBuilder -> new OpenSearchIndexScan( client, + settings.getSettingValue(Settings.Key.FIELD_TYPE_TOLERANCE), requestBuilder.getMaxResponseSize(), requestBuilder.build(indexName, getMaxResultWindow(), cursorKeepAlive, client)); return new OpenSearchIndexScanBuilder(builder, createScanOperator); @@ -177,7 +178,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..ad9f49f332 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,14 +47,18 @@ public class OpenSearchIndexScan extends TableScanOperator implements Serializab /** Search response for current batch. */ private Iterator iterator; - private Settings pluginSettings; + private boolean fieldTypeTolerance; /** Creates index scan based on a provided OpenSearchRequestBuilder. */ public OpenSearchIndexScan( - OpenSearchClient client, int maxResponseSize, OpenSearchRequest request) { + OpenSearchClient client, + boolean fieldTypeTolerance, + int maxResponseSize, + OpenSearchRequest request) { this.client = client; this.maxResponseSize = maxResponseSize; this.request = request; + this.fieldTypeTolerance = fieldTypeTolerance; } @Override @@ -129,9 +133,10 @@ public void readExternal(ObjectInput in) throws IOException { boolean pointInTimeEnabled = Boolean.parseBoolean( client.meta().get(Settings.Key.SQL_PAGINATION_API_SEARCH_AFTER.getKeyValue())); + fieldTypeTolerance = in.readBoolean(); try (BytesStreamInput bsi = new BytesStreamInput(requestStream)) { if (pointInTimeEnabled) { - request = new OpenSearchQueryRequest(bsi, engine); + request = new OpenSearchQueryRequest(bsi, engine, fieldTypeTolerance); } else { request = new OpenSearchScrollRequest(bsi, engine); } @@ -157,6 +162,7 @@ public void writeExternal(ObjectOutput out) throws IOException { out.writeInt(reqOut.size()); out.write(reqAsBytes, 0, reqOut.size()); + out.writeBoolean(fieldTypeTolerance); out.writeInt(maxResponseSize); } } 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/data/value/OpenSearchExprValueFactoryTest.java b/opensearch/src/test/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprValueFactoryTest.java index 6b4d825ab1..8d88666996 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 @@ -118,7 +118,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() { @@ -464,6 +467,13 @@ 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( @@ -473,15 +483,23 @@ public void constructNestedArraysOfStrings() { } @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"), + 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")); } @@ -577,6 +595,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( @@ -600,9 +632,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}," @@ -612,46 +659,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 ExprTimestampValue("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")); } @@ -763,20 +824,31 @@ 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 ExprTupleValue( - new LinkedHashMap() { - { - put("id", integerValue(1)); - put("state", stringValue("WA")); - } - }), + 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")); } @@ -798,7 +870,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)); @@ -815,7 +887,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), @@ -843,6 +916,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/executor/OpenSearchExecutionEngineTest.java b/opensearch/src/test/java/org/opensearch/sql/opensearch/executor/OpenSearchExecutionEngineTest.java index d5eb06cd5a..49679a4e0f 100644 --- a/opensearch/src/test/java/org/opensearch/sql/opensearch/executor/OpenSearchExecutionEngineTest.java +++ b/opensearch/src/test/java/org/opensearch/sql/opensearch/executor/OpenSearchExecutionEngineTest.java @@ -175,7 +175,7 @@ void explain_successfully() { Settings settings = mock(Settings.class); when(settings.getSettingValue(SQL_CURSOR_KEEP_ALIVE)).thenReturn(TimeValue.timeValueMinutes(1)); when(settings.getSettingValue(Settings.Key.SQL_PAGINATION_API_SEARCH_AFTER)).thenReturn(true); - when(settings.getSettingValue(Settings.Key.FIELD_TYPE_TOLERANCE)).thenReturn(false); + when(settings.getSettingValue(Settings.Key.FIELD_TYPE_TOLERANCE)).thenReturn(true); OpenSearchExprValueFactory exprValueFactory = mock(OpenSearchExprValueFactory.class); final var name = new OpenSearchRequest.IndexName("test"); @@ -186,6 +186,7 @@ void explain_successfully() { PhysicalPlan plan = new OpenSearchIndexScan( mock(OpenSearchClient.class), + true, maxResultWindow, requestBuilder.build( name, maxResultWindow, settings.getSettingValue(SQL_CURSOR_KEEP_ALIVE), client)); diff --git a/opensearch/src/test/java/org/opensearch/sql/opensearch/executor/protector/OpenSearchExecutionProtectorTest.java b/opensearch/src/test/java/org/opensearch/sql/opensearch/executor/protector/OpenSearchExecutionProtectorTest.java index 767da431aa..254052ec67 100644 --- a/opensearch/src/test/java/org/opensearch/sql/opensearch/executor/protector/OpenSearchExecutionProtectorTest.java +++ b/opensearch/src/test/java/org/opensearch/sql/opensearch/executor/protector/OpenSearchExecutionProtectorTest.java @@ -90,7 +90,7 @@ public void setup() { @Test void test_protect_indexScan() { when(settings.getSettingValue(Settings.Key.SQL_PAGINATION_API_SEARCH_AFTER)).thenReturn(true); - when(settings.getSettingValue(Settings.Key.FIELD_TYPE_TOLERANCE)).thenReturn(false); + when(settings.getSettingValue(Settings.Key.FIELD_TYPE_TOLERANCE)).thenReturn(true); String indexName = "test"; final int maxResultWindow = 10000; @@ -135,7 +135,10 @@ void test_protect_indexScan() { filter( resourceMonitor( new OpenSearchIndexScan( - client, maxResultWindow, request)), + client, + true, + maxResultWindow, + request)), filterExpr), aggregators, groupByExprs), @@ -162,7 +165,7 @@ void test_protect_indexScan() { PhysicalPlanDSL.agg( filter( new OpenSearchIndexScan( - client, maxResultWindow, request), + client, true, maxResultWindow, request), filterExpr), aggregators, groupByExprs), 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 0847e520cc..fbedca153c 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 @@ -459,7 +459,7 @@ void constructor_serialized() throws IOException { when(engine.getTable(null, "sample")).thenReturn(index); when(stream.readVInt()).thenReturn(2); when(stream.readGenericValue()).thenReturn("sampleSearchAfter"); - OpenSearchQueryRequest request = new OpenSearchQueryRequest(stream, engine); + OpenSearchQueryRequest request = new OpenSearchQueryRequest(stream, engine, true); assertNotNull(request); } 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 7b80c5a475..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; @@ -77,24 +79,24 @@ void isEmpty() { new TotalHits(2L, TotalHits.Relation.EQUAL_TO), 1.0F)); - var response = new OpenSearchResponse(searchResponse, factory, includes, false); + var response = new OpenSearchResponse(searchResponse, factory, includes); assertFalse(response.isEmpty()); when(searchResponse.getHits()).thenReturn(SearchHits.empty()); when(searchResponse.getAggregations()).thenReturn(null); - response = new OpenSearchResponse(searchResponse, factory, includes, false); + response = new OpenSearchResponse(searchResponse, factory, includes); assertTrue(response.isEmpty()); when(searchResponse.getHits()) .thenReturn(new SearchHits(null, new TotalHits(0, TotalHits.Relation.EQUAL_TO), 0)); - response = new OpenSearchResponse(searchResponse, factory, includes, false); + response = new OpenSearchResponse(searchResponse, factory, includes); assertTrue(response.isEmpty()); when(searchResponse.getHits()).thenReturn(SearchHits.empty()); when(searchResponse.getAggregations()).thenReturn(new Aggregations(emptyList())); - response = new OpenSearchResponse(searchResponse, factory, includes, false); + response = new OpenSearchResponse(searchResponse, factory, includes); assertFalse(response.isEmpty()); } @@ -109,12 +111,14 @@ void iterator() { when(searchHit1.getSourceAsString()).thenReturn("{\"id1\", 1}"); when(searchHit2.getSourceAsString()).thenReturn("{\"id1\", 2}"); + when(searchHit1.getInnerHits()).thenReturn(null); + when(searchHit2.getInnerHits()).thenReturn(null); when(factory.construct(any(), anyBoolean())) .thenReturn(exprTupleValue1) .thenReturn(exprTupleValue2); int i = 0; - for (ExprValue hit : new OpenSearchResponse(searchResponse, factory, List.of("id1"), false)) { + for (ExprValue hit : new OpenSearchResponse(searchResponse, factory, List.of("id1"))) { if (i == 0) { assertEquals(exprTupleValue1.tupleValue().get("id"), hit.tupleValue().get("id")); } else if (i == 1) { @@ -161,9 +165,10 @@ 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, false)) { + for (ExprValue hit : new OpenSearchResponse(searchResponse, factory, includes)) { if (i == 0) { assertEquals(exprTupleResponse, hit); } else { @@ -194,7 +199,7 @@ void iterator_metafields_withoutIncludes() { ExprTupleValue exprTupleResponse = ExprTupleValue.fromExprValueMap(ImmutableMap.of("id1", new ExprIntegerValue(1))); int i = 0; - for (ExprValue hit : new OpenSearchResponse(searchResponse, factory, includes, true)) { + for (ExprValue hit : new OpenSearchResponse(searchResponse, factory, includes)) { if (i == 0) { assertEquals(exprTupleResponse, hit); } else { @@ -234,7 +239,7 @@ void iterator_metafields_scoreNaN() { "_id", new ExprStringValue("testId"), "_sort", new ExprLongValue(123456L))); int i = 0; - for (ExprValue hit : new OpenSearchResponse(searchResponse, factory, includes, true)) { + for (ExprValue hit : new OpenSearchResponse(searchResponse, factory, includes)) { if (i == 0) { assertEquals(exprTupleResponse, hit); } else { @@ -246,6 +251,9 @@ 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( @@ -255,7 +263,7 @@ void iterator_with_inner_hits() { when(factory.construct(any(), anyBoolean())).thenReturn(exprTupleValue1); - for (ExprValue hit : new OpenSearchResponse(searchResponse, factory, includes, false)) { + for (ExprValue hit : new OpenSearchResponse(searchResponse, factory, includes)) { assertEquals(exprTupleValue1, hit); } } @@ -264,7 +272,7 @@ void iterator_with_inner_hits() { void response_is_aggregation_when_aggregation_not_empty() { when(searchResponse.getAggregations()).thenReturn(aggregations); - OpenSearchResponse response = new OpenSearchResponse(searchResponse, factory, includes, false); + OpenSearchResponse response = new OpenSearchResponse(searchResponse, factory, includes); assertTrue(response.isAggregationResponse()); } @@ -272,7 +280,7 @@ void response_is_aggregation_when_aggregation_not_empty() { void response_isnot_aggregation_when_aggregation_is_empty() { when(searchResponse.getAggregations()).thenReturn(null); - OpenSearchResponse response = new OpenSearchResponse(searchResponse, factory, includes, false); + OpenSearchResponse response = new OpenSearchResponse(searchResponse, factory, includes); assertFalse(response.isAggregationResponse()); } @@ -289,7 +297,7 @@ void aggregation_iterator() { .thenReturn(new ExprIntegerValue(2)); int i = 0; - for (ExprValue hit : new OpenSearchResponse(searchResponse, factory, includes, false)) { + for (ExprValue hit : new OpenSearchResponse(searchResponse, factory, includes)) { if (i == 0) { assertEquals(exprTupleValue1, hit); } else if (i == 1) { @@ -321,7 +329,7 @@ void highlight_iterator() { when(searchHit1.getHighlightFields()).thenReturn(highlightMap); when(factory.construct(any(), anyBoolean())).thenReturn(resultTuple); - for (ExprValue resultHit : new OpenSearchResponse(searchResponse, factory, includes, false)) { + for (ExprValue resultHit : new OpenSearchResponse(searchResponse, factory, includes)) { var expected = ExprValueUtils.collectionValue( Arrays.stream(searchHit.getHighlightFields().get("highlights").getFragments()) 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 fc95395b17..94573c1267 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,7 +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(false); + lenient().when(settings.getSettingValue(Settings.Key.FIELD_TYPE_TOLERANCE)).thenReturn(true); } @Test @@ -206,7 +207,10 @@ void implementRelationOperatorOnly() { new OpenSearchRequestBuilder(QUERY_SIZE_LIMIT, exprValueFactory, settings); assertEquals( new OpenSearchIndexScan( - client, 200, requestBuilder.build(INDEX_NAME, maxResultWindow, SCROLL_TIMEOUT, client)), + client, + true, + 200, + requestBuilder.build(INDEX_NAME, maxResultWindow, SCROLL_TIMEOUT, client)), index.implement(index.optimize(plan))); } @@ -220,7 +224,10 @@ void implementRelationOperatorWithOptimization() { new OpenSearchRequestBuilder(QUERY_SIZE_LIMIT, exprValueFactory, settings); assertEquals( new OpenSearchIndexScan( - client, 200, requestBuilder.build(INDEX_NAME, maxResultWindow, SCROLL_TIMEOUT, client)), + client, + true, + 200, + requestBuilder.build(INDEX_NAME, maxResultWindow, SCROLL_TIMEOUT, client)), index.implement(plan)); } @@ -260,6 +267,7 @@ void implementOtherLogicalOperators() { PhysicalPlanDSL.rename( new OpenSearchIndexScan( client, + true, QUERY_SIZE_LIMIT, requestBuilder.build( INDEX_NAME, maxResultWindow, SCROLL_TIMEOUT, client)), @@ -271,4 +279,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 28006894a7..a38cf9c66b 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,7 +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(false); + lenient().when(settings.getSettingValue(Settings.Key.FIELD_TYPE_TOLERANCE)).thenReturn(true); } @Mock private OpenSearchClient client; @@ -68,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() { @@ -77,6 +78,7 @@ void query_empty_result() { try (var indexScan = new OpenSearchIndexScan( client, + true, MAX_RESULT_WINDOW, builder.build(INDEX_NAME, MAX_RESULT_WINDOW, SCROLL_TIMEOUT, client))) { indexScan.open(); @@ -105,6 +107,7 @@ void dont_serialize_if_no_cursor() { try (var indexScan = new OpenSearchIndexScan( client, + true, MAX_RESULT_WINDOW, builder.build(INDEX_NAME, MAX_RESULT_WINDOW, SCROLL_TIMEOUT, client))) { indexScan.open(); 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 2502b39df3..79f0df5fc4 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,21 +78,22 @@ 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(false); + lenient().when(settings.getSettingValue(Settings.Key.FIELD_TYPE_TOLERANCE)).thenReturn(true); } @Test void explain() { var request = mock(OpenSearchRequest.class); when(request.toString()).thenReturn("explain works!"); - try (var indexScan = new OpenSearchIndexScan(client, QUERY_SIZE, request)) { + try (var indexScan = new OpenSearchIndexScan(client, true, QUERY_SIZE, request)) { assertEquals("explain works!", indexScan.explain()); } } @@ -102,7 +103,7 @@ void explain() { void throws_no_cursor_exception() { var request = mock(OpenSearchRequest.class); when(request.hasAnotherBatch()).thenReturn(false); - try (var indexScan = new OpenSearchIndexScan(client, QUERY_SIZE, request); + try (var indexScan = new OpenSearchIndexScan(client, true, QUERY_SIZE, request); var byteStream = new ByteArrayOutputStream(); var objectStream = new ObjectOutputStream(byteStream)) { assertThrows(NoCursorException.class, () -> objectStream.writeObject(indexScan)); @@ -138,7 +139,7 @@ void serialize(Integer numberOfIncludes) { when(hits.getHits()).thenReturn(new SearchHit[] {mock()}); request.search(null, (req) -> response); - try (var indexScan = new OpenSearchIndexScan(client, QUERY_SIZE, request)) { + try (var indexScan = new OpenSearchIndexScan(client, true, QUERY_SIZE, request)) { var planSerializer = new PlanSerializer(engine); var cursor = planSerializer.convertToCursor(indexScan); var newPlan = planSerializer.convertToPlan(cursor.toString()); @@ -178,7 +179,7 @@ void serialize_PIT(Integer numberOfIncludes) { when(hits.getHits()).thenReturn(new SearchHit[] {hit}); request.search((req) -> response, null); - try (var indexScan = new OpenSearchIndexScan(client, QUERY_SIZE, request)) { + try (var indexScan = new OpenSearchIndexScan(client, true, QUERY_SIZE, request)) { var planSerializer = new PlanSerializer(engine); var cursor = planSerializer.convertToCursor(indexScan); var newPlan = planSerializer.convertToPlan(cursor.toString()); @@ -200,7 +201,7 @@ void throws_io_exception_if_too_short() { ObjectInputStream objectInput = new ObjectInputStream(new ByteArrayInputStream(output.toByteArray())); - try (var indexScan = new OpenSearchIndexScan(client, QUERY_SIZE, request)) { + try (var indexScan = new OpenSearchIndexScan(client, true, QUERY_SIZE, request)) { assertThrows(IOException.class, () -> indexScan.readExternal(objectInput)); } } @@ -208,7 +209,7 @@ void throws_io_exception_if_too_short() { @Test void plan_for_serialization() { var request = mock(OpenSearchRequest.class); - try (var indexScan = new OpenSearchIndexScan(client, QUERY_SIZE, request)) { + try (var indexScan = new OpenSearchIndexScan(client, true, QUERY_SIZE, request)) { assertEquals(indexScan, indexScan.getPlanForSerialization()); } } @@ -221,6 +222,7 @@ void query_empty_result() { try (OpenSearchIndexScan indexScan = new OpenSearchIndexScan( client, + true, QUERY_SIZE, requestBuilder.build(name, MAX_RESULT_WINDOW, CURSOR_KEEP_ALIVE, client))) { indexScan.open(); @@ -240,7 +242,7 @@ void query_all_results_with_query() { final var requestBuilder = new OpenSearchRequestBuilder(QUERY_SIZE, exprValueFactory, settings); try (OpenSearchIndexScan indexScan = new OpenSearchIndexScan( - client, 10, requestBuilder.build(INDEX_NAME, 10000, CURSOR_KEEP_ALIVE, client))) { + client, true, 10, requestBuilder.build(INDEX_NAME, 10000, CURSOR_KEEP_ALIVE, client))) { indexScan.open(); assertAll( @@ -268,7 +270,7 @@ void query_all_results_with_scroll() { final var requestBuilder = new OpenSearchRequestBuilder(QUERY_SIZE, exprValueFactory, settings); try (OpenSearchIndexScan indexScan = new OpenSearchIndexScan( - client, 10, requestBuilder.build(INDEX_NAME, 10000, CURSOR_KEEP_ALIVE, client))) { + client, true, 10, requestBuilder.build(INDEX_NAME, 10000, CURSOR_KEEP_ALIVE, client))) { indexScan.open(); assertAll( @@ -299,6 +301,7 @@ void query_some_results_with_query() { try (OpenSearchIndexScan indexScan = new OpenSearchIndexScan( client, + true, limit, builder.build(INDEX_NAME, MAX_RESULT_WINDOW, CURSOR_KEEP_ALIVE, client))) { indexScan.open(); @@ -322,6 +325,7 @@ void query_some_results_with_scroll() { try (OpenSearchIndexScan indexScan = new OpenSearchIndexScan( client, + true, 3, requestuilder.build(INDEX_NAME, MAX_RESULT_WINDOW, CURSOR_KEEP_ALIVE, client))) { indexScan.open(); @@ -362,6 +366,7 @@ void query_results_limited_by_query_size() { try (OpenSearchIndexScan indexScan = new OpenSearchIndexScan( client, + true, defaultQuerySize, requestBuilder.build(INDEX_NAME, QUERY_SIZE, CURSOR_KEEP_ALIVE, client))) { indexScan.open(); @@ -465,6 +470,7 @@ PushDownAssertion shouldQueryHighlight(QueryBuilder query, HighlightBuilder high var indexScan = new OpenSearchIndexScan( client, + true, QUERY_SIZE, requestBuilder.build(EMPLOYEES_INDEX, 10000, CURSOR_KEEP_ALIVE, client)); indexScan.open(); @@ -485,6 +491,7 @@ PushDownAssertion shouldQuery(QueryBuilder expected) { var indexScan = new OpenSearchIndexScan( client, + true, 10000, requestBuilder.build(EMPLOYEES_INDEX, 10000, CURSOR_KEEP_ALIVE, client)); indexScan.open(); From d58b6dc842dc2e0f584eef6df7d6899586b47778 Mon Sep 17 00:00:00 2001 From: Norman Jordan Date: Thu, 24 Oct 2024 12:31:47 -0700 Subject: [PATCH 5/6] Code cleanup and adding more tests * Cleaned up how the fieldTypeTolerance setting is accessed * Added an integration test for when fieldTypeTolerance is disabled Signed-off-by: Norman Jordan --- .../sql/legacy/ObjectFieldSelectIT.java | 14 ++++ .../value/OpenSearchExprValueFactory.java | 1 + .../request/OpenSearchQueryRequest.java | 7 +- .../request/OpenSearchRequestBuilder.java | 1 - .../opensearch/storage/OpenSearchIndex.java | 1 - .../storage/scan/OpenSearchIndexScan.java | 12 +-- .../value/OpenSearchExprValueFactoryTest.java | 84 +++++++++++++++++++ .../OpenSearchExecutionEngineTest.java | 1 - .../OpenSearchExecutionProtectorTest.java | 7 +- .../request/OpenSearchQueryRequestTest.java | 2 +- .../storage/OpenSearchIndexTest.java | 11 +-- .../OpenSearchIndexScanPaginationTest.java | 2 - .../storage/scan/OpenSearchIndexScanTest.java | 22 ++--- 13 files changed, 117 insertions(+), 48 deletions(-) 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 0bb9698f71..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; /** @@ -82,6 +83,19 @@ public void testSelectObjectFieldOfArrayValuesItself() { verifyDataRows(response, rows(new JSONArray("[{\"id\":1},{\"id\":2}]"))); } + @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 public void testSelectObjectFieldOfArrayValuesInnerFields() { JSONObject response = new JSONObject(query("SELECT accounts.id FROM %s")); 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 401f21a83d..d9e21436b7 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 @@ -74,6 +74,7 @@ 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; /** 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 82ee56375a..3461660795 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 @@ -126,9 +126,7 @@ public OpenSearchQueryRequest( * @param engine OpenSearchSqlEngine to get node-specific context. * @throws IOException thrown if reading from input {@code in} fails. */ - public OpenSearchQueryRequest( - StreamInput in, OpenSearchStorageEngine engine, boolean fieldTypeTolerance) - throws IOException { + public OpenSearchQueryRequest(StreamInput in, OpenSearchStorageEngine engine) throws IOException { // Deserialize the SearchSourceBuilder from the string representation String sourceBuilderString = in.readString(); @@ -154,7 +152,8 @@ public OpenSearchQueryRequest( OpenSearchIndex index = (OpenSearchIndex) engine.getTable(null, indexName.toString()); exprValueFactory = - new OpenSearchExprValueFactory(index.getFieldOpenSearchTypes(), fieldTypeTolerance); + new OpenSearchExprValueFactory( + index.getFieldOpenSearchTypes(), index.isFieldTypeTolerance()); } @Override diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/request/OpenSearchRequestBuilder.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/request/OpenSearchRequestBuilder.java index bbba3cbd55..35a7508ffe 100644 --- a/opensearch/src/main/java/org/opensearch/sql/opensearch/request/OpenSearchRequestBuilder.java +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/request/OpenSearchRequestBuilder.java @@ -142,7 +142,6 @@ private OpenSearchRequest buildRequestWithScroll( int size = requestedTotalSize; FetchSourceContext fetchSource = this.sourceBuilder.fetchSource(); List includes = fetchSource != null ? Arrays.asList(fetchSource.includes()) : List.of(); - boolean fieldTypeTolerance = settings.getSettingValue(Settings.Key.FIELD_TYPE_TOLERANCE); if (pageSize == null) { if (startFrom + size > maxResultWindow) { 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 dff2bedf0f..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 @@ -168,7 +168,6 @@ public TableScanBuilder createScanBuilder() { requestBuilder -> new OpenSearchIndexScan( client, - settings.getSettingValue(Settings.Key.FIELD_TYPE_TOLERANCE), requestBuilder.getMaxResponseSize(), requestBuilder.build(indexName, getMaxResultWindow(), cursorKeepAlive, client)); return new OpenSearchIndexScanBuilder(builder, createScanOperator); 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 ad9f49f332..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,18 +47,12 @@ public class OpenSearchIndexScan extends TableScanOperator implements Serializab /** Search response for current batch. */ private Iterator iterator; - private boolean fieldTypeTolerance; - /** Creates index scan based on a provided OpenSearchRequestBuilder. */ public OpenSearchIndexScan( - OpenSearchClient client, - boolean fieldTypeTolerance, - int maxResponseSize, - OpenSearchRequest request) { + OpenSearchClient client, int maxResponseSize, OpenSearchRequest request) { this.client = client; this.maxResponseSize = maxResponseSize; this.request = request; - this.fieldTypeTolerance = fieldTypeTolerance; } @Override @@ -133,10 +127,9 @@ public void readExternal(ObjectInput in) throws IOException { boolean pointInTimeEnabled = Boolean.parseBoolean( client.meta().get(Settings.Key.SQL_PAGINATION_API_SEARCH_AFTER.getKeyValue())); - fieldTypeTolerance = in.readBoolean(); try (BytesStreamInput bsi = new BytesStreamInput(requestStream)) { if (pointInTimeEnabled) { - request = new OpenSearchQueryRequest(bsi, engine, fieldTypeTolerance); + request = new OpenSearchQueryRequest(bsi, engine); } else { request = new OpenSearchScrollRequest(bsi, engine); } @@ -162,7 +155,6 @@ public void writeExternal(ObjectOutput out) throws IOException { out.writeInt(reqOut.size()); out.write(reqAsBytes, 0, reqOut.size()); - out.writeBoolean(fieldTypeTolerance); out.writeInt(maxResponseSize); } } 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 8d88666996..5fd40ef6c4 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 @@ -853,6 +853,90 @@ public void constructFromOpenSearchArrayReturnAll() { .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() { + { + put("id", integerValue(1)); + put("state", stringValue("WA")); + } + }), + 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")); + } + @Test public void constructFromInvalidJsonThrowException() { IllegalStateException exception = diff --git a/opensearch/src/test/java/org/opensearch/sql/opensearch/executor/OpenSearchExecutionEngineTest.java b/opensearch/src/test/java/org/opensearch/sql/opensearch/executor/OpenSearchExecutionEngineTest.java index 49679a4e0f..4bac1390a7 100644 --- a/opensearch/src/test/java/org/opensearch/sql/opensearch/executor/OpenSearchExecutionEngineTest.java +++ b/opensearch/src/test/java/org/opensearch/sql/opensearch/executor/OpenSearchExecutionEngineTest.java @@ -186,7 +186,6 @@ void explain_successfully() { PhysicalPlan plan = new OpenSearchIndexScan( mock(OpenSearchClient.class), - true, maxResultWindow, requestBuilder.build( name, maxResultWindow, settings.getSettingValue(SQL_CURSOR_KEEP_ALIVE), client)); diff --git a/opensearch/src/test/java/org/opensearch/sql/opensearch/executor/protector/OpenSearchExecutionProtectorTest.java b/opensearch/src/test/java/org/opensearch/sql/opensearch/executor/protector/OpenSearchExecutionProtectorTest.java index 254052ec67..30ad0fee4b 100644 --- a/opensearch/src/test/java/org/opensearch/sql/opensearch/executor/protector/OpenSearchExecutionProtectorTest.java +++ b/opensearch/src/test/java/org/opensearch/sql/opensearch/executor/protector/OpenSearchExecutionProtectorTest.java @@ -135,10 +135,7 @@ void test_protect_indexScan() { filter( resourceMonitor( new OpenSearchIndexScan( - client, - true, - maxResultWindow, - request)), + client, maxResultWindow, request)), filterExpr), aggregators, groupByExprs), @@ -165,7 +162,7 @@ void test_protect_indexScan() { PhysicalPlanDSL.agg( filter( new OpenSearchIndexScan( - client, true, maxResultWindow, request), + client, maxResultWindow, request), filterExpr), aggregators, groupByExprs), 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 fbedca153c..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 @@ -459,7 +459,7 @@ void constructor_serialized() throws IOException { when(engine.getTable(null, "sample")).thenReturn(index); when(stream.readVInt()).thenReturn(2); when(stream.readGenericValue()).thenReturn("sampleSearchAfter"); - OpenSearchQueryRequest request = new OpenSearchQueryRequest(stream, engine, true); + OpenSearchQueryRequest request = new OpenSearchQueryRequest(stream, engine); assertNotNull(request); } 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 94573c1267..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 @@ -207,10 +207,7 @@ void implementRelationOperatorOnly() { new OpenSearchRequestBuilder(QUERY_SIZE_LIMIT, exprValueFactory, settings); assertEquals( new OpenSearchIndexScan( - client, - true, - 200, - requestBuilder.build(INDEX_NAME, maxResultWindow, SCROLL_TIMEOUT, client)), + client, 200, requestBuilder.build(INDEX_NAME, maxResultWindow, SCROLL_TIMEOUT, client)), index.implement(index.optimize(plan))); } @@ -224,10 +221,7 @@ void implementRelationOperatorWithOptimization() { new OpenSearchRequestBuilder(QUERY_SIZE_LIMIT, exprValueFactory, settings); assertEquals( new OpenSearchIndexScan( - client, - true, - 200, - requestBuilder.build(INDEX_NAME, maxResultWindow, SCROLL_TIMEOUT, client)), + client, 200, requestBuilder.build(INDEX_NAME, maxResultWindow, SCROLL_TIMEOUT, client)), index.implement(plan)); } @@ -267,7 +261,6 @@ void implementOtherLogicalOperators() { PhysicalPlanDSL.rename( new OpenSearchIndexScan( client, - true, QUERY_SIZE_LIMIT, requestBuilder.build( INDEX_NAME, maxResultWindow, SCROLL_TIMEOUT, client)), 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 a38cf9c66b..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 @@ -78,7 +78,6 @@ void query_empty_result() { try (var indexScan = new OpenSearchIndexScan( client, - true, MAX_RESULT_WINDOW, builder.build(INDEX_NAME, MAX_RESULT_WINDOW, SCROLL_TIMEOUT, client))) { indexScan.open(); @@ -107,7 +106,6 @@ void dont_serialize_if_no_cursor() { try (var indexScan = new OpenSearchIndexScan( client, - true, MAX_RESULT_WINDOW, builder.build(INDEX_NAME, MAX_RESULT_WINDOW, SCROLL_TIMEOUT, client))) { indexScan.open(); 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 79f0df5fc4..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 @@ -93,7 +93,7 @@ void setup() { void explain() { var request = mock(OpenSearchRequest.class); when(request.toString()).thenReturn("explain works!"); - try (var indexScan = new OpenSearchIndexScan(client, true, QUERY_SIZE, request)) { + try (var indexScan = new OpenSearchIndexScan(client, QUERY_SIZE, request)) { assertEquals("explain works!", indexScan.explain()); } } @@ -103,7 +103,7 @@ void explain() { void throws_no_cursor_exception() { var request = mock(OpenSearchRequest.class); when(request.hasAnotherBatch()).thenReturn(false); - try (var indexScan = new OpenSearchIndexScan(client, true, QUERY_SIZE, request); + try (var indexScan = new OpenSearchIndexScan(client, QUERY_SIZE, request); var byteStream = new ByteArrayOutputStream(); var objectStream = new ObjectOutputStream(byteStream)) { assertThrows(NoCursorException.class, () -> objectStream.writeObject(indexScan)); @@ -139,7 +139,7 @@ void serialize(Integer numberOfIncludes) { when(hits.getHits()).thenReturn(new SearchHit[] {mock()}); request.search(null, (req) -> response); - try (var indexScan = new OpenSearchIndexScan(client, true, QUERY_SIZE, request)) { + try (var indexScan = new OpenSearchIndexScan(client, QUERY_SIZE, request)) { var planSerializer = new PlanSerializer(engine); var cursor = planSerializer.convertToCursor(indexScan); var newPlan = planSerializer.convertToPlan(cursor.toString()); @@ -179,7 +179,7 @@ void serialize_PIT(Integer numberOfIncludes) { when(hits.getHits()).thenReturn(new SearchHit[] {hit}); request.search((req) -> response, null); - try (var indexScan = new OpenSearchIndexScan(client, true, QUERY_SIZE, request)) { + try (var indexScan = new OpenSearchIndexScan(client, QUERY_SIZE, request)) { var planSerializer = new PlanSerializer(engine); var cursor = planSerializer.convertToCursor(indexScan); var newPlan = planSerializer.convertToPlan(cursor.toString()); @@ -201,7 +201,7 @@ void throws_io_exception_if_too_short() { ObjectInputStream objectInput = new ObjectInputStream(new ByteArrayInputStream(output.toByteArray())); - try (var indexScan = new OpenSearchIndexScan(client, true, QUERY_SIZE, request)) { + try (var indexScan = new OpenSearchIndexScan(client, QUERY_SIZE, request)) { assertThrows(IOException.class, () -> indexScan.readExternal(objectInput)); } } @@ -209,7 +209,7 @@ void throws_io_exception_if_too_short() { @Test void plan_for_serialization() { var request = mock(OpenSearchRequest.class); - try (var indexScan = new OpenSearchIndexScan(client, true, QUERY_SIZE, request)) { + try (var indexScan = new OpenSearchIndexScan(client, QUERY_SIZE, request)) { assertEquals(indexScan, indexScan.getPlanForSerialization()); } } @@ -222,7 +222,6 @@ void query_empty_result() { try (OpenSearchIndexScan indexScan = new OpenSearchIndexScan( client, - true, QUERY_SIZE, requestBuilder.build(name, MAX_RESULT_WINDOW, CURSOR_KEEP_ALIVE, client))) { indexScan.open(); @@ -242,7 +241,7 @@ void query_all_results_with_query() { final var requestBuilder = new OpenSearchRequestBuilder(QUERY_SIZE, exprValueFactory, settings); try (OpenSearchIndexScan indexScan = new OpenSearchIndexScan( - client, true, 10, requestBuilder.build(INDEX_NAME, 10000, CURSOR_KEEP_ALIVE, client))) { + client, 10, requestBuilder.build(INDEX_NAME, 10000, CURSOR_KEEP_ALIVE, client))) { indexScan.open(); assertAll( @@ -270,7 +269,7 @@ void query_all_results_with_scroll() { final var requestBuilder = new OpenSearchRequestBuilder(QUERY_SIZE, exprValueFactory, settings); try (OpenSearchIndexScan indexScan = new OpenSearchIndexScan( - client, true, 10, requestBuilder.build(INDEX_NAME, 10000, CURSOR_KEEP_ALIVE, client))) { + client, 10, requestBuilder.build(INDEX_NAME, 10000, CURSOR_KEEP_ALIVE, client))) { indexScan.open(); assertAll( @@ -301,7 +300,6 @@ void query_some_results_with_query() { try (OpenSearchIndexScan indexScan = new OpenSearchIndexScan( client, - true, limit, builder.build(INDEX_NAME, MAX_RESULT_WINDOW, CURSOR_KEEP_ALIVE, client))) { indexScan.open(); @@ -325,7 +323,6 @@ void query_some_results_with_scroll() { try (OpenSearchIndexScan indexScan = new OpenSearchIndexScan( client, - true, 3, requestuilder.build(INDEX_NAME, MAX_RESULT_WINDOW, CURSOR_KEEP_ALIVE, client))) { indexScan.open(); @@ -366,7 +363,6 @@ void query_results_limited_by_query_size() { try (OpenSearchIndexScan indexScan = new OpenSearchIndexScan( client, - true, defaultQuerySize, requestBuilder.build(INDEX_NAME, QUERY_SIZE, CURSOR_KEEP_ALIVE, client))) { indexScan.open(); @@ -470,7 +466,6 @@ PushDownAssertion shouldQueryHighlight(QueryBuilder query, HighlightBuilder high var indexScan = new OpenSearchIndexScan( client, - true, QUERY_SIZE, requestBuilder.build(EMPLOYEES_INDEX, 10000, CURSOR_KEEP_ALIVE, client)); indexScan.open(); @@ -491,7 +486,6 @@ PushDownAssertion shouldQuery(QueryBuilder expected) { var indexScan = new OpenSearchIndexScan( client, - true, 10000, requestBuilder.build(EMPLOYEES_INDEX, 10000, CURSOR_KEEP_ALIVE, client)); indexScan.open(); From 69226c8f9c8dbfa79e77f66f9eff59705b884663 Mon Sep 17 00:00:00 2001 From: Norman Jordan Date: Thu, 24 Oct 2024 14:20:27 -0700 Subject: [PATCH 6/6] Removed some unused code Signed-off-by: Norman Jordan --- .../src/test/java/org/opensearch/sql/ppl/StandaloneIT.java | 2 +- .../sql/opensearch/request/OpenSearchRequestBuilder.java | 1 - .../sql/opensearch/executor/OpenSearchExecutionEngineTest.java | 1 - .../executor/protector/OpenSearchExecutionProtectorTest.java | 1 - 4 files changed, 1 insertion(+), 4 deletions(-) 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 b8ac6f072d..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,7 +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, false) + .put(Key.FIELD_TYPE_TOLERANCE, true) .build(); @Override diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/request/OpenSearchRequestBuilder.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/request/OpenSearchRequestBuilder.java index 35a7508ffe..6fa9b17697 100644 --- a/opensearch/src/main/java/org/opensearch/sql/opensearch/request/OpenSearchRequestBuilder.java +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/request/OpenSearchRequestBuilder.java @@ -110,7 +110,6 @@ private OpenSearchRequest buildRequestWithPit( int size = requestedTotalSize; FetchSourceContext fetchSource = this.sourceBuilder.fetchSource(); List includes = fetchSource != null ? Arrays.asList(fetchSource.includes()) : List.of(); - boolean fieldTypeTolerance = settings.getSettingValue(Settings.Key.FIELD_TYPE_TOLERANCE); if (pageSize == null) { if (startFrom + size > maxResultWindow) { diff --git a/opensearch/src/test/java/org/opensearch/sql/opensearch/executor/OpenSearchExecutionEngineTest.java b/opensearch/src/test/java/org/opensearch/sql/opensearch/executor/OpenSearchExecutionEngineTest.java index 4bac1390a7..e5cf94eb86 100644 --- a/opensearch/src/test/java/org/opensearch/sql/opensearch/executor/OpenSearchExecutionEngineTest.java +++ b/opensearch/src/test/java/org/opensearch/sql/opensearch/executor/OpenSearchExecutionEngineTest.java @@ -175,7 +175,6 @@ void explain_successfully() { Settings settings = mock(Settings.class); when(settings.getSettingValue(SQL_CURSOR_KEEP_ALIVE)).thenReturn(TimeValue.timeValueMinutes(1)); when(settings.getSettingValue(Settings.Key.SQL_PAGINATION_API_SEARCH_AFTER)).thenReturn(true); - when(settings.getSettingValue(Settings.Key.FIELD_TYPE_TOLERANCE)).thenReturn(true); OpenSearchExprValueFactory exprValueFactory = mock(OpenSearchExprValueFactory.class); final var name = new OpenSearchRequest.IndexName("test"); diff --git a/opensearch/src/test/java/org/opensearch/sql/opensearch/executor/protector/OpenSearchExecutionProtectorTest.java b/opensearch/src/test/java/org/opensearch/sql/opensearch/executor/protector/OpenSearchExecutionProtectorTest.java index 30ad0fee4b..da06c1eb66 100644 --- a/opensearch/src/test/java/org/opensearch/sql/opensearch/executor/protector/OpenSearchExecutionProtectorTest.java +++ b/opensearch/src/test/java/org/opensearch/sql/opensearch/executor/protector/OpenSearchExecutionProtectorTest.java @@ -90,7 +90,6 @@ public void setup() { @Test void test_protect_indexScan() { when(settings.getSettingValue(Settings.Key.SQL_PAGINATION_API_SEARCH_AFTER)).thenReturn(true); - when(settings.getSettingValue(Settings.Key.FIELD_TYPE_TOLERANCE)).thenReturn(true); String indexName = "test"; final int maxResultWindow = 10000;