From 872d8535619f0d23c4b14c47629f8da9d86b9375 Mon Sep 17 00:00:00 2001 From: Guian Gumpac Date: Sat, 21 Jan 2023 09:37:52 -0800 Subject: [PATCH 1/7] Added test for all rows of message.info Signed-off-by: Guian Gumpac --- .../java/org/opensearch/sql/sql/NestedIT.java | 23 ++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) 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 e8d6dcc8aa..7aa5178a94 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,6 +5,9 @@ package org.opensearch.sql.sql; +import static org.opensearch.sql.util.MatcherUtils.rows; +import static org.opensearch.sql.util.MatcherUtils.verifyDataRows; + import org.json.JSONObject; import org.junit.Test; import org.opensearch.sql.legacy.SQLIntegTestCase; @@ -19,10 +22,24 @@ public void init() throws IOException { loadIndex(Index.NESTED); } + // Incorrect expected result +// @Test +// public void nested_string_subfield_test() { +// String query = "SELECT nested(message.dayOfWeek) FROM " + TEST_INDEX_NESTED_TYPE; +// JSONObject result = executeJdbcRequest(query); +// assertEquals(5, result.getInt("total")); +// } + @Test - public void nested_string_subfield_test() { - String query = "SELECT nested(message.dayOfWeek) FROM " + TEST_INDEX_NESTED_TYPE; + public void nested_function_with_array_of_nested_field_test() { + String query = "SELECT nested(message.info) FROM " + TEST_INDEX_NESTED_TYPE; JSONObject result = executeJdbcRequest(query); - assertEquals(5, result.getInt("total")); + verifyDataRows(result, + rows("a"), + rows("b"), + rows("c"), + rows("c"), + rows("a"), + rows("zz")); } } From 0293c102dd017a9a093dfb6e52421d3a4a33980c Mon Sep 17 00:00:00 2001 From: Guian Gumpac Date: Tue, 24 Jan 2023 10:27:07 -0800 Subject: [PATCH 2/7] Initial solution to return all values Signed-off-by: Guian Gumpac --- .../sql/expression/ReferenceExpression.java | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/core/src/main/java/org/opensearch/sql/expression/ReferenceExpression.java b/core/src/main/java/org/opensearch/sql/expression/ReferenceExpression.java index 94bb4e067d..807ec04a72 100644 --- a/core/src/main/java/org/opensearch/sql/expression/ReferenceExpression.java +++ b/core/src/main/java/org/opensearch/sql/expression/ReferenceExpression.java @@ -8,6 +8,7 @@ import static org.opensearch.sql.utils.ExpressionUtils.PATH_SEP; +import java.util.ArrayList; import java.util.Arrays; import java.util.List; import lombok.EqualsAndHashCode; @@ -15,6 +16,8 @@ import lombok.RequiredArgsConstructor; import org.opensearch.sql.data.model.ExprTupleValue; import org.opensearch.sql.data.model.ExprValue; +import org.opensearch.sql.data.model.ExprValueUtils; +import org.opensearch.sql.data.type.ExprCoreType; import org.opensearch.sql.data.type.ExprType; import org.opensearch.sql.expression.env.Environment; @@ -100,6 +103,14 @@ public ExprValue resolve(ExprTupleValue value) { } private ExprValue resolve(ExprValue value, List paths) { + if (value.type().equals(ExprCoreType.ARRAY)){ + ExprValue result = ExprValueUtils.collectionValue(new ArrayList<>()); + + for (ExprValue val: value.collectionValue()){ + result.collectionValue().add(resolve(val, paths)); + } + return result; + } final ExprValue wholePathValue = value.keyValue(String.join(PATH_SEP, paths)); if (!wholePathValue.isMissing() || paths.size() == 1) { return wholePathValue; From cea553e7cb68de14acb9998372c725b4f99a39ea Mon Sep 17 00:00:00 2001 From: Guian Gumpac Date: Wed, 25 Jan 2023 13:27:24 -0800 Subject: [PATCH 3/7] Fix for arrays of objects but broke arrays of simple type values Signed-off-by: Guian Gumpac --- .../sql/planner/physical/ProjectOperator.java | 17 ++++++++++++++++- .../executor/OpenSearchExecutionEngine.java | 9 ++++++++- 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/core/src/main/java/org/opensearch/sql/planner/physical/ProjectOperator.java b/core/src/main/java/org/opensearch/sql/planner/physical/ProjectOperator.java index 496e4e6ddb..57cba826ee 100644 --- a/core/src/main/java/org/opensearch/sql/planner/physical/ProjectOperator.java +++ b/core/src/main/java/org/opensearch/sql/planner/physical/ProjectOperator.java @@ -8,6 +8,8 @@ import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableMap.Builder; + +import java.util.ArrayList; import java.util.Collections; import java.util.List; import java.util.Optional; @@ -19,6 +21,7 @@ import org.opensearch.sql.data.model.ExprTupleValue; import org.opensearch.sql.data.model.ExprValue; import org.opensearch.sql.data.model.ExprValueUtils; +import org.opensearch.sql.data.type.ExprCoreType; import org.opensearch.sql.executor.ExecutionEngine; import org.opensearch.sql.expression.NamedExpression; import org.opensearch.sql.expression.parse.ParseExpression; @@ -56,6 +59,7 @@ public boolean hasNext() { public ExprValue next() { ExprValue inputValue = input.next(); ImmutableMap.Builder mapBuilder = new Builder<>(); + ExprValue result = ExprValueUtils.collectionValue(new ArrayList<>()); // ParseExpression will always override NamedExpression when identifier conflicts // TODO needs a better implementation, see https://github.com/opensearch-project/sql/issues/458 @@ -65,7 +69,16 @@ public ExprValue next() { .filter(parseExpr -> parseExpr.getNameOrAlias().equals(expr.getNameOrAlias())) .findFirst(); if (optionalParseExpression.isEmpty()) { - mapBuilder.put(expr.getNameOrAlias(), exprValue); + if (exprValue.type().equals(ExprCoreType.ARRAY)) { + for (ExprValue exprVal : exprValue.collectionValue()) { + mapBuilder.put(expr.getNameOrAlias(), exprVal); + result.collectionValue().add(ExprTupleValue.fromExprValueMap(mapBuilder.build())); + mapBuilder = new Builder<>(); + } + } + else { + mapBuilder.put(expr.getNameOrAlias(), exprValue); + } continue; } @@ -85,6 +98,8 @@ public ExprValue next() { mapBuilder.put(parseExpression.getNameOrAlias(), parsedValue); } } + if (result.collectionValue().size() > 0) + return result; return ExprTupleValue.fromExprValueMap(mapBuilder.build()); } diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/executor/OpenSearchExecutionEngine.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/executor/OpenSearchExecutionEngine.java index 9a136a3bec..5dc10234e0 100644 --- a/opensearch/src/main/java/org/opensearch/sql/opensearch/executor/OpenSearchExecutionEngine.java +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/executor/OpenSearchExecutionEngine.java @@ -12,6 +12,7 @@ import lombok.RequiredArgsConstructor; import org.opensearch.sql.common.response.ResponseListener; import org.opensearch.sql.data.model.ExprValue; +import org.opensearch.sql.data.type.ExprCoreType; import org.opensearch.sql.executor.ExecutionContext; import org.opensearch.sql.executor.ExecutionEngine; import org.opensearch.sql.executor.Explain; @@ -46,7 +47,13 @@ public void execute(PhysicalPlan physicalPlan, ExecutionContext context, plan.open(); while (plan.hasNext()) { - result.add(plan.next()); + ExprValue nextPlan = plan.next(); + + if (nextPlan.type().equals(ExprCoreType.ARRAY)) + for (ExprValue value: nextPlan.collectionValue()) + result.add(value); + else + result.add(nextPlan); } QueryResponse response = new QueryResponse(physicalPlan.schema(), result); From b8c5a54d312efdc3e5ddb9ff826f188ba6d8fece Mon Sep 17 00:00:00 2001 From: Guian Gumpac Date: Sat, 28 Jan 2023 21:43:37 -0800 Subject: [PATCH 4/7] Revert "Fix for arrays of objects but broke arrays of simple type values" This reverts commit cea553e7cb68de14acb9998372c725b4f99a39ea. Signed-off-by: Guian Gumpac --- .../sql/planner/physical/ProjectOperator.java | 17 +---------------- .../executor/OpenSearchExecutionEngine.java | 9 +-------- 2 files changed, 2 insertions(+), 24 deletions(-) diff --git a/core/src/main/java/org/opensearch/sql/planner/physical/ProjectOperator.java b/core/src/main/java/org/opensearch/sql/planner/physical/ProjectOperator.java index 57cba826ee..496e4e6ddb 100644 --- a/core/src/main/java/org/opensearch/sql/planner/physical/ProjectOperator.java +++ b/core/src/main/java/org/opensearch/sql/planner/physical/ProjectOperator.java @@ -8,8 +8,6 @@ import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableMap.Builder; - -import java.util.ArrayList; import java.util.Collections; import java.util.List; import java.util.Optional; @@ -21,7 +19,6 @@ import org.opensearch.sql.data.model.ExprTupleValue; import org.opensearch.sql.data.model.ExprValue; import org.opensearch.sql.data.model.ExprValueUtils; -import org.opensearch.sql.data.type.ExprCoreType; import org.opensearch.sql.executor.ExecutionEngine; import org.opensearch.sql.expression.NamedExpression; import org.opensearch.sql.expression.parse.ParseExpression; @@ -59,7 +56,6 @@ public boolean hasNext() { public ExprValue next() { ExprValue inputValue = input.next(); ImmutableMap.Builder mapBuilder = new Builder<>(); - ExprValue result = ExprValueUtils.collectionValue(new ArrayList<>()); // ParseExpression will always override NamedExpression when identifier conflicts // TODO needs a better implementation, see https://github.com/opensearch-project/sql/issues/458 @@ -69,16 +65,7 @@ public ExprValue next() { .filter(parseExpr -> parseExpr.getNameOrAlias().equals(expr.getNameOrAlias())) .findFirst(); if (optionalParseExpression.isEmpty()) { - if (exprValue.type().equals(ExprCoreType.ARRAY)) { - for (ExprValue exprVal : exprValue.collectionValue()) { - mapBuilder.put(expr.getNameOrAlias(), exprVal); - result.collectionValue().add(ExprTupleValue.fromExprValueMap(mapBuilder.build())); - mapBuilder = new Builder<>(); - } - } - else { - mapBuilder.put(expr.getNameOrAlias(), exprValue); - } + mapBuilder.put(expr.getNameOrAlias(), exprValue); continue; } @@ -98,8 +85,6 @@ public ExprValue next() { mapBuilder.put(parseExpression.getNameOrAlias(), parsedValue); } } - if (result.collectionValue().size() > 0) - return result; return ExprTupleValue.fromExprValueMap(mapBuilder.build()); } diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/executor/OpenSearchExecutionEngine.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/executor/OpenSearchExecutionEngine.java index 5dc10234e0..9a136a3bec 100644 --- a/opensearch/src/main/java/org/opensearch/sql/opensearch/executor/OpenSearchExecutionEngine.java +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/executor/OpenSearchExecutionEngine.java @@ -12,7 +12,6 @@ import lombok.RequiredArgsConstructor; import org.opensearch.sql.common.response.ResponseListener; import org.opensearch.sql.data.model.ExprValue; -import org.opensearch.sql.data.type.ExprCoreType; import org.opensearch.sql.executor.ExecutionContext; import org.opensearch.sql.executor.ExecutionEngine; import org.opensearch.sql.executor.Explain; @@ -47,13 +46,7 @@ public void execute(PhysicalPlan physicalPlan, ExecutionContext context, plan.open(); while (plan.hasNext()) { - ExprValue nextPlan = plan.next(); - - if (nextPlan.type().equals(ExprCoreType.ARRAY)) - for (ExprValue value: nextPlan.collectionValue()) - result.add(value); - else - result.add(nextPlan); + result.add(plan.next()); } QueryResponse response = new QueryResponse(physicalPlan.schema(), result); From 4f5a7ab613a5d5743b0c71d042fcd4b8c0938302 Mon Sep 17 00:00:00 2001 From: Guian Gumpac Date: Mon, 30 Jan 2023 13:09:40 -0800 Subject: [PATCH 5/7] Added tests for objects and partiQL cases. Removed legacy object tests with undesired expected results Signed-off-by: Guian Gumpac --- .../sql/expression/ReferenceExpression.java | 3 ++ .../sql/legacy/ObjectFieldSelectIT.java | 16 ------ .../sql/legacy/SQLIntegTestCase.java | 10 +++- .../opensearch/sql/legacy/TestsConstants.java | 4 +- .../java/org/opensearch/sql/sql/NestedIT.java | 41 +++++++++------ .../opensearch/sql/sql/NestedPartiQLIT.java | 50 +++++++++++++++++++ .../java/org/opensearch/sql/sql/ObjectIT.java | 50 +++++++++++++++++++ .../test/resources/multi_nested_objects.json | 6 +-- 8 files changed, 144 insertions(+), 36 deletions(-) create mode 100644 integ-test/src/test/java/org/opensearch/sql/sql/NestedPartiQLIT.java create mode 100644 integ-test/src/test/java/org/opensearch/sql/sql/ObjectIT.java diff --git a/core/src/main/java/org/opensearch/sql/expression/ReferenceExpression.java b/core/src/main/java/org/opensearch/sql/expression/ReferenceExpression.java index 807ec04a72..55b1ce8182 100644 --- a/core/src/main/java/org/opensearch/sql/expression/ReferenceExpression.java +++ b/core/src/main/java/org/opensearch/sql/expression/ReferenceExpression.java @@ -103,6 +103,8 @@ public ExprValue resolve(ExprTupleValue value) { } private ExprValue resolve(ExprValue value, List paths) { + // This case is to allow returning all values in an array to be in one row + // and fixes https://github.com/opensearch-project/sql/issues/1305 if (value.type().equals(ExprCoreType.ARRAY)){ ExprValue result = ExprValueUtils.collectionValue(new ArrayList<>()); @@ -111,6 +113,7 @@ private ExprValue resolve(ExprValue value, List paths) { } return result; } + final ExprValue wholePathValue = value.keyValue(String.join(PATH_SEP, paths)); if (!wholePathValue.isMissing() || paths.size() == 1) { return wholePathValue; 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 bddaa22772..dc1c8f21c4 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 @@ -85,22 +85,6 @@ 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}"))); - } - - @Test - public void testSelectObjectFieldOfArrayValuesInnerFields() { - JSONObject response = new JSONObject(query("SELECT accounts.id FROM %s")); - - // Only the first element of the list of is returned. - verifyDataRows(response, rows(1)); - } - private String query(String sql) { return executeQuery( StringUtils.format(sql, TEST_INDEX_DEEP_NESTED), 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 53d14f67ab..1f48184bb1 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 @@ -587,7 +587,15 @@ public enum Index { "src/test/resources/calcs.json"), MULTI_NESTED(TestsConstants.TEST_INDEX_MULTI_NESTED, "multi_nested", - getMappingFile("indexDefinitions/multi_nested.json"), + getMappingFile("multi_nested.json"), + "src/test/resources/multi_nested_objects.json"), + NESTED_OBJECT(TestsConstants.TEST_INDEX_NESTED_OBJECT, + "nested_object", + null, + "src/test/resources/nested_objects.json"), + MULTI_NESTED_OBJECT(TestsConstants.TEST_INDEX_MULTI_NESTED_OBJECT, + "multi_nested_object", + null, "src/test/resources/multi_nested_objects.json"); private final String name; 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 11c4306be3..beaefd5469 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 @@ -53,7 +53,9 @@ public class TestsConstants { public final static String TEST_INDEX_BEER = TEST_INDEX + "_beer"; public final static String TEST_INDEX_NULL_MISSING = TEST_INDEX + "_null_missing"; public final static String TEST_INDEX_CALCS = TEST_INDEX + "_calcs"; - public final static String TEST_INDEX_MULTI_NESTED= TEST_INDEX + "_multi_nested"; + public final static String TEST_INDEX_MULTI_NESTED = TEST_INDEX + "_multi_nested"; + public final static String TEST_INDEX_NESTED_OBJECT = TEST_INDEX + "_nested_object"; + public final static String TEST_INDEX_MULTI_NESTED_OBJECT = TEST_INDEX + "_multi_nested_object"; public final static String DATE_FORMAT = "yyyy-MM-dd'T'HH:mm:ss.SSS'Z'"; public final static String TS_DATE_FORMAT = "yyyy-MM-dd HH:mm:ss.SSS"; 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 7aa5178a94..cb5c24fcc5 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,14 +5,17 @@ package org.opensearch.sql.sql; +import static org.opensearch.sql.legacy.TestsConstants.TEST_INDEX_MULTI_NESTED; import static org.opensearch.sql.util.MatcherUtils.rows; import static org.opensearch.sql.util.MatcherUtils.verifyDataRows; +import org.json.JSONArray; import org.json.JSONObject; import org.junit.Test; import org.opensearch.sql.legacy.SQLIntegTestCase; import java.io.IOException; +import java.util.List; import static org.opensearch.sql.legacy.TestsConstants.TEST_INDEX_NESTED_TYPE; @@ -20,26 +23,34 @@ public class NestedIT extends SQLIntegTestCase { @Override public void init() throws IOException { loadIndex(Index.NESTED); + loadIndex(Index.MULTI_NESTED); } - // Incorrect expected result -// @Test -// public void nested_string_subfield_test() { -// String query = "SELECT nested(message.dayOfWeek) FROM " + TEST_INDEX_NESTED_TYPE; -// JSONObject result = executeJdbcRequest(query); -// assertEquals(5, result.getInt("total")); -// } - @Test public void nested_function_with_array_of_nested_field_test() { - String query = "SELECT nested(message.info) FROM " + TEST_INDEX_NESTED_TYPE; + String query = "SELECT nested(message.info), nested(comment.data) FROM " + TEST_INDEX_NESTED_TYPE; + JSONObject result = executeJdbcRequest(query); + + assertEquals(5, result.getInt("total")); + verifyDataRows(result, + rows("a", "ab"), + rows("b", "aa"), + rows("c", "aa"), + rows(new JSONArray(List.of("c","a")), "ab"), + rows(new JSONArray(List.of("zz")), new JSONArray(List.of("aa", "bb")))); + } + + @Test + public void nested_function_with_array_of_multi_nested_field_test() { + String query = "SELECT nested(message.author.name) FROM " + TEST_INDEX_MULTI_NESTED; JSONObject result = executeJdbcRequest(query); + + assertEquals(5, result.getInt("total")); verifyDataRows(result, - rows("a"), - rows("b"), - rows("c"), - rows("c"), - rows("a"), - rows("zz")); + rows("e"), + rows("f"), + rows("g"), + rows(new JSONArray(List.of("h", "p"))), + rows(new JSONArray(List.of("yy")))); } } diff --git a/integ-test/src/test/java/org/opensearch/sql/sql/NestedPartiQLIT.java b/integ-test/src/test/java/org/opensearch/sql/sql/NestedPartiQLIT.java new file mode 100644 index 0000000000..d8c9e0ec65 --- /dev/null +++ b/integ-test/src/test/java/org/opensearch/sql/sql/NestedPartiQLIT.java @@ -0,0 +1,50 @@ +package org.opensearch.sql.sql; + +import org.json.JSONArray; +import org.json.JSONObject; +import org.junit.Test; +import org.opensearch.sql.legacy.SQLIntegTestCase; + +import java.io.IOException; +import java.util.List; + +import static org.opensearch.sql.legacy.TestsConstants.TEST_INDEX_MULTI_NESTED; +import static org.opensearch.sql.legacy.TestsConstants.TEST_INDEX_NESTED_TYPE; +import static org.opensearch.sql.util.MatcherUtils.rows; +import static org.opensearch.sql.util.MatcherUtils.verifyDataRows; + +public class NestedPartiQLIT extends SQLIntegTestCase { + @Override + public void init() throws IOException { + loadIndex(Index.NESTED); + loadIndex(Index.MULTI_NESTED); + } + + @Test + public void partiQL_with_array_of_nested_field_test() { + String query = "SELECT message.info, comment.data FROM " + TEST_INDEX_NESTED_TYPE; + JSONObject result = executeJdbcRequest(query); + + assertEquals(5, result.getInt("total")); + verifyDataRows(result, + rows("a", "ab"), + rows("b", "aa"), + rows("c", "aa"), + rows(new JSONArray(List.of("c","a")), "ab"), + rows(new JSONArray(List.of("zz")), new JSONArray(List.of("aa", "bb")))); + } + + @Test + public void partiQL_with_array_of_multi_nested_field_test() { + String query = "SELECT message.author.name, message.info FROM " + TEST_INDEX_MULTI_NESTED; + JSONObject result = executeJdbcRequest(query); + + assertEquals(5, result.getInt("total")); + verifyDataRows(result, + rows("e", "a"), + rows("f", "b"), + rows("g", "c"), + rows(new JSONArray(List.of("h", "p")), new JSONArray(List.of("d","i"))), + rows(new JSONArray(List.of("yy")), new JSONArray(List.of("zz")))); + } +} diff --git a/integ-test/src/test/java/org/opensearch/sql/sql/ObjectIT.java b/integ-test/src/test/java/org/opensearch/sql/sql/ObjectIT.java new file mode 100644 index 0000000000..105bd0dab0 --- /dev/null +++ b/integ-test/src/test/java/org/opensearch/sql/sql/ObjectIT.java @@ -0,0 +1,50 @@ +package org.opensearch.sql.sql; + +import org.json.JSONArray; +import org.json.JSONObject; +import org.junit.Test; +import org.opensearch.sql.legacy.SQLIntegTestCase; + +import java.io.IOException; +import java.util.List; + +import static org.opensearch.sql.legacy.TestsConstants.TEST_INDEX_MULTI_NESTED_OBJECT; +import static org.opensearch.sql.legacy.TestsConstants.TEST_INDEX_NESTED_OBJECT; +import static org.opensearch.sql.util.MatcherUtils.rows; +import static org.opensearch.sql.util.MatcherUtils.verifyDataRows; + +public class ObjectIT extends SQLIntegTestCase { + @Override + public void init() throws IOException { + loadIndex(Index.NESTED_OBJECT); + loadIndex(Index.MULTI_NESTED_OBJECT); + } + + @Test + public void object_array_of_objects_test() { + String query = "SELECT message.info, comment.data FROM " + TEST_INDEX_NESTED_OBJECT; + JSONObject result = executeJdbcRequest(query); + + assertEquals(5, result.getInt("total")); + verifyDataRows(result, + rows("a", "ab"), + rows("b", "aa"), + rows("c", "aa"), + rows(new JSONArray(List.of("c","a")), "ab"), + rows(new JSONArray(List.of("zz")), new JSONArray(List.of("aa", "bb")))); + } + + @Test + public void object_with_array_of_multi_nested_objects_test() { + String query = "SELECT message.author.name, message.info FROM " + TEST_INDEX_MULTI_NESTED_OBJECT; + JSONObject result = executeJdbcRequest(query); + + assertEquals(5, result.getInt("total")); + verifyDataRows(result, + rows("e", "a"), + rows("f", "b"), + rows("g", "c"), + rows(new JSONArray(List.of("h", "p")), new JSONArray(List.of("d","i"))), + rows(new JSONArray(List.of("yy")), new JSONArray(List.of("zz")))); + } +} diff --git a/integ-test/src/test/resources/multi_nested_objects.json b/integ-test/src/test/resources/multi_nested_objects.json index bdee031357..f5838fe90b 100644 --- a/integ-test/src/test/resources/multi_nested_objects.json +++ b/integ-test/src/test/resources/multi_nested_objects.json @@ -1,9 +1,9 @@ {"index":{"_id":"1"}} -{"message":[{"info":"a","author":{"name": "e", "address": {"street": "bc", "number": 1}},"dayOfWeek":1}]} +{"message":{"info":"a","author":{"name": "e", "address": {"street": "bc", "number": 1}},"dayOfWeek":1}} {"index":{"_id":"2"}} -{"message":[{"info":"b","author":{"name": "f", "address": {"street": "ab", "number": 2}},"dayOfWeek":2}]} +{"message":{"info":"b","author":{"name": "f", "address": {"street": "ab", "number": 2}},"dayOfWeek":2}} {"index":{"_id":"3"}} -{"message":[{"info":"c","author":{"name": "g", "address": {"street": "sk", "number": 3}},"dayOfWeek":1}]} +{"message":{"info":"c","author":{"name": "g", "address": {"street": "sk", "number": 3}},"dayOfWeek":1}} {"index":{"_id":"4"}} {"message":[{"info":"d","author":{"name": "h", "address": {"street": "mb", "number": 4}},"dayOfWeek":4},{"info":"i","author":{"name": "p", "address": {"street": "on", "number": 5}},"dayOfWeek":5}]} {"index":{"_id":"5"}} From ff901da23fe331bfdad88f5d11cbb807c32c1b0c Mon Sep 17 00:00:00 2001 From: Guian Gumpac Date: Tue, 31 Jan 2023 07:34:45 -0800 Subject: [PATCH 6/7] Removed comment with github issue Signed-off-by: Guian Gumpac --- .../java/org/opensearch/sql/expression/ReferenceExpression.java | 1 - 1 file changed, 1 deletion(-) diff --git a/core/src/main/java/org/opensearch/sql/expression/ReferenceExpression.java b/core/src/main/java/org/opensearch/sql/expression/ReferenceExpression.java index 55b1ce8182..adf240d809 100644 --- a/core/src/main/java/org/opensearch/sql/expression/ReferenceExpression.java +++ b/core/src/main/java/org/opensearch/sql/expression/ReferenceExpression.java @@ -104,7 +104,6 @@ public ExprValue resolve(ExprTupleValue value) { private ExprValue resolve(ExprValue value, List paths) { // This case is to allow returning all values in an array to be in one row - // and fixes https://github.com/opensearch-project/sql/issues/1305 if (value.type().equals(ExprCoreType.ARRAY)){ ExprValue result = ExprValueUtils.collectionValue(new ArrayList<>()); From a48587d233b8753268598a65661aaa9c82a61854 Mon Sep 17 00:00:00 2001 From: Guian Gumpac Date: Tue, 31 Jan 2023 14:27:44 -0800 Subject: [PATCH 7/7] Addressed PR comment Signed-off-by: Guian Gumpac --- .../sql/expression/ReferenceExpression.java | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/core/src/main/java/org/opensearch/sql/expression/ReferenceExpression.java b/core/src/main/java/org/opensearch/sql/expression/ReferenceExpression.java index adf240d809..8c4f3b5681 100644 --- a/core/src/main/java/org/opensearch/sql/expression/ReferenceExpression.java +++ b/core/src/main/java/org/opensearch/sql/expression/ReferenceExpression.java @@ -8,15 +8,16 @@ import static org.opensearch.sql.utils.ExpressionUtils.PATH_SEP; -import java.util.ArrayList; import java.util.Arrays; import java.util.List; +import java.util.stream.Collectors; + import lombok.EqualsAndHashCode; import lombok.Getter; import lombok.RequiredArgsConstructor; +import org.opensearch.sql.data.model.ExprCollectionValue; import org.opensearch.sql.data.model.ExprTupleValue; import org.opensearch.sql.data.model.ExprValue; -import org.opensearch.sql.data.model.ExprValueUtils; import org.opensearch.sql.data.type.ExprCoreType; import org.opensearch.sql.data.type.ExprType; import org.opensearch.sql.expression.env.Environment; @@ -105,12 +106,8 @@ public ExprValue resolve(ExprTupleValue value) { private ExprValue resolve(ExprValue value, List paths) { // This case is to allow returning all values in an array to be in one row if (value.type().equals(ExprCoreType.ARRAY)){ - ExprValue result = ExprValueUtils.collectionValue(new ArrayList<>()); - - for (ExprValue val: value.collectionValue()){ - result.collectionValue().add(resolve(val, paths)); - } - return result; + return new ExprCollectionValue(value.collectionValue().stream() + .map(val -> resolve(val, paths)).collect(Collectors.toList())); } final ExprValue wholePathValue = value.keyValue(String.join(PATH_SEP, paths));