diff --git a/core/src/main/java/org/opensearch/sql/analysis/NestedAnalyzer.java b/core/src/main/java/org/opensearch/sql/analysis/NestedAnalyzer.java index 02814caf92..497a0cefe0 100644 --- a/core/src/main/java/org/opensearch/sql/analysis/NestedAnalyzer.java +++ b/core/src/main/java/org/opensearch/sql/analysis/NestedAnalyzer.java @@ -16,6 +16,7 @@ import org.opensearch.sql.ast.expression.UnresolvedExpression; import org.opensearch.sql.expression.NamedExpression; import org.opensearch.sql.expression.ReferenceExpression; +import org.opensearch.sql.expression.function.BuiltinFunctionName; import org.opensearch.sql.planner.logical.LogicalNested; import org.opensearch.sql.planner.logical.LogicalPlan; @@ -41,7 +42,7 @@ public LogicalPlan visitAlias(Alias node, AnalysisContext context) { @Override public LogicalPlan visitFunction(Function node, AnalysisContext context) { - if (node.getFuncName().equalsIgnoreCase("nested")) { + if (node.getFuncName().equalsIgnoreCase(BuiltinFunctionName.NESTED.name())) { List expressions = node.getFuncArgs(); ReferenceExpression nestedField = diff --git a/core/src/main/java/org/opensearch/sql/expression/nested/NestedFunction.java b/core/src/main/java/org/opensearch/sql/expression/nested/NestedFunction.java index 480f86c5f4..26be064712 100644 --- a/core/src/main/java/org/opensearch/sql/expression/nested/NestedFunction.java +++ b/core/src/main/java/org/opensearch/sql/expression/nested/NestedFunction.java @@ -33,6 +33,6 @@ public ExprValue valueOf(Environment valueEnv) { @Override public ExprType type() { - return this.arguments.get(0).type(); + return this.arguments.get(0).type(); } } diff --git a/core/src/main/java/org/opensearch/sql/planner/physical/UnnestOperator.java b/core/src/main/java/org/opensearch/sql/planner/physical/UnnestOperator.java index 637fc01cb4..a7112d5f97 100644 --- a/core/src/main/java/org/opensearch/sql/planner/physical/UnnestOperator.java +++ b/core/src/main/java/org/opensearch/sql/planner/physical/UnnestOperator.java @@ -108,6 +108,10 @@ public ExprValue next() { result = flatten(nonNestedField, inputValue, result, false); } + if (result.isEmpty()) { + return new ExprTupleValue(new LinkedHashMap<>()); + } + flattenedResult = result.listIterator(); } return new ExprTupleValue(new LinkedHashMap<>(flattenedResult.next())); @@ -132,16 +136,25 @@ public void generateNonNestedFieldsMap(ExprValue inputMap) { ExprValue currentObj = inputField.getValue(); while (!nestingComplete) { if (currentObj instanceof ExprTupleValue) { - var it = currentObj.tupleValue().entrySet().iterator().next(); - currentObj = it.getValue(); - nonNestedField += "." + it.getKey(); + var it = currentObj.tupleValue().entrySet().iterator(); + if (it.hasNext()) { + var next = it.next(); + currentObj = next.getValue(); + nonNestedField += "." + next.getKey(); + } else { + nestingComplete = true; + nonNestedField = null; + } } else if (currentObj instanceof ExprCollectionValue) { currentObj = currentObj.collectionValue().get(0); } else { nestingComplete = true; } } - this.nonNestedFields.add(nonNestedField); + + if (nonNestedField != null) { + this.nonNestedFields.add(nonNestedField); + } } } } @@ -262,7 +275,7 @@ private void getNested( } else { currentObj = null; } - } else { + } else if (currentObj instanceof ExprCollectionValue) { ExprValue arrayObj = currentObj; if (supportArrays) { for (int x = 0; x < arrayObj.collectionValue().size(); x++) { @@ -275,6 +288,8 @@ private void getNested( getNested(field, nestedField, row, ret, currentObj, supportArrays); currentObj = null; } + } else { + currentObj = null; } // Return final nested result diff --git a/core/src/test/java/org/opensearch/sql/planner/physical/UnnestOperatorTest.java b/core/src/test/java/org/opensearch/sql/planner/physical/UnnestOperatorTest.java index 3feec01b5c..462eed867d 100644 --- a/core/src/test/java/org/opensearch/sql/planner/physical/UnnestOperatorTest.java +++ b/core/src/test/java/org/opensearch/sql/planner/physical/UnnestOperatorTest.java @@ -7,6 +7,7 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.contains; +import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.Mockito.when; import static org.opensearch.sql.data.model.ExprValueUtils.collectionValue; import static org.opensearch.sql.data.model.ExprValueUtils.tupleValue; @@ -72,6 +73,24 @@ class UnnestOperatorTest extends PhysicalPlanTestBase { ) ); + private final ExprValue missingTupleData = tupleValue( + ImmutableMap.of( + "tuple", + tupleValue( + ImmutableMap.of() + ) + ) + ); + + private final ExprValue missingArrayData = tupleValue( + ImmutableMap.of( + "missing", + collectionValue( + List.of("value") + ) + ) + ); + @Test public void nested_one_nested_field() { when(inputPlan.hasNext()).thenReturn(true, false); @@ -165,4 +184,36 @@ public void non_nested_field_tests() { ) ); } + + @Test + public void nested_missing_tuple_field() { + when(inputPlan.hasNext()).thenReturn(true, false); + when(inputPlan.next()) + .thenReturn(missingTupleData); + Set fields = Set.of("message.val"); + Map> groupedFieldsByPath = + Map.of("message", List.of("message.val")); + assertTrue( + execute(new UnnestOperator(inputPlan, fields, groupedFieldsByPath)) + .get(0) + .tupleValue() + .size() == 0 + ); + } + + @Test + public void nested_missing_array_field() { + when(inputPlan.hasNext()).thenReturn(true, false); + when(inputPlan.next()) + .thenReturn(missingArrayData); + Set fields = Set.of("missing.data"); + Map> groupedFieldsByPath = + Map.of("message", List.of("message.data")); + assertTrue( + execute(new UnnestOperator(inputPlan, fields, groupedFieldsByPath)) + .get(0) + .tupleValue() + .size() == 0 + ); + } } diff --git a/docs/user/dql/functions.rst b/docs/user/dql/functions.rst index fed96e6745..17f7959f35 100644 --- a/docs/user/dql/functions.rst +++ b/docs/user/dql/functions.rst @@ -4300,17 +4300,16 @@ Another example to show how to set custom values for the optional parameters:: +-------------------------------------------+ NESTED ------------- +------ Description >>>>>>>>>>> -``nested(field | [field, path] | [path, condition])`` +``nested(field | [field, path])`` The ``nested`` function maps to the ``nested`` query used in search engine. It returns nested field types in documents that match the provided specified field(s). -The ``field`` and ``path`` parameters are valid parameters for a nested function used in the ``SELECT`` clause. -Example with only ``field`` and ``path`` parameters:: +Example with ``field`` and ``path`` parameters:: os> SELECT nested(message.info, message) FROM nested; fetched rows / total rows = 2/2 diff --git a/integ-test/src/test/java/org/opensearch/sql/legacy/SQLIntegTestCase.java b/integ-test/src/test/java/org/opensearch/sql/legacy/SQLIntegTestCase.java index a56888dd7a..cb86ed6a11 100644 --- a/integ-test/src/test/java/org/opensearch/sql/legacy/SQLIntegTestCase.java +++ b/integ-test/src/test/java/org/opensearch/sql/legacy/SQLIntegTestCase.java @@ -601,10 +601,14 @@ public enum Index { "datasource", getMappingFile("datasources_index_mappings.json"), "src/test/resources/datasources.json"), - MULTI_NESTED(TestsConstants.TEST_INDEX_MULTI_NESTED, + MULTI_NESTED(TestsConstants.TEST_INDEX_MULTI_NESTED_TYPE, "multi_nested", getMappingFile("multi_nested.json"), - "src/test/resources/multi_nested_objects.json"); + "src/test/resources/multi_nested_objects.json"), + NESTED_WITH_NULLS(TestsConstants.TEST_INDEX_NESTED_WITH_NULLS, + "multi_nested", + getNestedTypeIndexMapping(), + "src/test/resources/nested_with_nulls.json"); private final String name; private final String type; diff --git a/integ-test/src/test/java/org/opensearch/sql/legacy/TestsConstants.java b/integ-test/src/test/java/org/opensearch/sql/legacy/TestsConstants.java index 6650d508d6..c3af98b794 100644 --- a/integ-test/src/test/java/org/opensearch/sql/legacy/TestsConstants.java +++ b/integ-test/src/test/java/org/opensearch/sql/legacy/TestsConstants.java @@ -58,7 +58,7 @@ public class TestsConstants { public final static String TEST_INDEX_CALCS = TEST_INDEX + "_calcs"; public final static String TEST_INDEX_WILDCARD = TEST_INDEX + "_wildcard"; public final static String TEST_INDEX_MULTI_NESTED_TYPE = TEST_INDEX + "_multi_nested"; - public final static String TEST_INDEX_MULTI_NESTED= TEST_INDEX + "_multi_nested"; + public final static String TEST_INDEX_NESTED_WITH_NULLS = TEST_INDEX + "_nested_with_nulls"; public final static String DATASOURCES = ".ql-datasources"; public final static String DATE_FORMAT = "yyyy-MM-dd'T'HH:mm:ss.SSS'Z'"; diff --git a/integ-test/src/test/java/org/opensearch/sql/sql/NestedIT.java b/integ-test/src/test/java/org/opensearch/sql/sql/NestedIT.java index 576983711a..da02b95997 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 @@ -5,10 +5,10 @@ package org.opensearch.sql.sql; -import static org.opensearch.sql.legacy.TestsConstants.TEST_INDEX_EMPLOYEE_NESTED; import static org.opensearch.sql.legacy.TestsConstants.TEST_INDEX_MULTI_NESTED_TYPE; import static org.opensearch.sql.legacy.TestsConstants.TEST_INDEX_NESTED_TYPE; import static org.opensearch.sql.legacy.TestsConstants.TEST_INDEX_NESTED_TYPE_WITHOUT_ARRAYS; +import static org.opensearch.sql.legacy.TestsConstants.TEST_INDEX_NESTED_WITH_NULLS; import static org.opensearch.sql.util.MatcherUtils.rows; import static org.opensearch.sql.util.MatcherUtils.schema; import static org.opensearch.sql.util.MatcherUtils.verifyDataRows; @@ -20,6 +20,7 @@ import org.json.JSONObject; import org.junit.Test; import org.junit.jupiter.api.Disabled; +import org.opensearch.sql.data.model.ExprNullValue; import org.opensearch.sql.legacy.SQLIntegTestCase; public class NestedIT extends SQLIntegTestCase { @@ -29,6 +30,7 @@ public void init() throws IOException { loadIndex(Index.NESTED); loadIndex(Index.NESTED_WITHOUT_ARRAYS); loadIndex(Index.EMPLOYEE_NESTED); + loadIndex(Index.NESTED_WITH_NULLS); } @Test @@ -115,6 +117,26 @@ public void nested_function_with_array_of_multi_nested_field_test() { rows("yy")); } + @Test + public void nested_function_with_null_and_missing_fields_test() { + String query = "SELECT nested(message.info), nested(comment.data) FROM " + + TEST_INDEX_NESTED_WITH_NULLS; + JSONObject result = executeJdbcRequest(query); + + assertEquals(10, result.getInt("total")); + verifyDataRows(result, + rows(null, "hh"), + rows("b", "aa"), + rows("c", "aa"), + rows("c", "ab"), + rows("a", "ab"), + rows("zz", new JSONArray(List.of("aa", "bb"))), + rows("zz", new JSONArray(List.of("aa", "bb"))), + rows(null, "ee"), + rows("a", "ab"), + rows("rr", new JSONArray(List.of("asdf", "sdfg")))); + } + @Test public void nested_function_multiple_fields_with_matched_and_mismatched_paths_test() { String query = diff --git a/integ-test/src/test/resources/nested_with_nulls.json b/integ-test/src/test/resources/nested_with_nulls.json new file mode 100644 index 0000000000..b02a8ab110 --- /dev/null +++ b/integ-test/src/test/resources/nested_with_nulls.json @@ -0,0 +1,24 @@ +{"index":{"_id":"1"}} +{"message":{"author":"e","dayOfWeek":5},"comment":{"data":"hh","likes":5},"myNum":7,"someField":"a"} +{"index":{"_id":"2"}} +{"message":{"info":"b","author":"f","dayOfWeek":2},"comment":{"data":"aa","likes":2},"myNum":2,"someField":"a"} +{"index":{"_id":"3"}} +{"message":{"info":"c","author":"g","dayOfWeek":1},"comment":{"data":"aa","likes":3},"myNum":3,"someField":"a"} +{"index":{"_id":"4"}} +{"message":[{"info":"c","author":"h","dayOfWeek":4},{"info":"a","author":"i","dayOfWeek":5}],"comment":{"data":"ab","likes":1},"myNum":4,"someField":"b"} +{"index":{"_id":"5"}} +{"message": [{"info":"zz","author":"zz","dayOfWeek":6}],"comment":{"data":["aa","bb"],"likes":10},"myNum":[3,4],"someField":"a"} +{"index":{"_id":"7"}} +{"message":[{"info":"zz", "author":"z\"z", "dayOfWeek":6}], "comment":{"data":["aa","bb"], "likes":10}, "myNum":[3,4], "someField":"a"} +{"index":{"_id":"8"}} +{"message":{"info":null,"author":"e","dayOfWeek":7},"comment":{"data":"ee","likes":6},"myNum":6,"someField":"a"} +{"index":{"_id":"9"}} +{"message":{"info":"a","author":"e","dayOfWeek":1},"comment":{"data":"ab","likes":3},"myNum":1,"someField":"b"} +{"index":{"_id":"10"}} +{"message":[{"info":"rr", "author":"this \"value\" contains quotes", "dayOfWeek":3}], "comment":{"data":["asdf","sdfg"], "likes":56}, "myNum":[1,2,4], "someField":"ert"} +{"index":{"_id":"11"}} +{"comment":{"data":"jj","likes":1},"myNum":8,"someField":"a"} +{"index":{"_id":"12"}} +{"message":null,"comment":{"data":"kk","likes":0},"myNum":9,"someField":"a"} +{"index":{"_id":"13"}} +{}