Add Nested Function Support In SELECT Clause #1490

forestmvey · 2023-03-30T21:21:56Z

Description

Syntax: nested( [field] | [field,path] )

Nested function allows the query of nested types of an index. The path parameter is determined dynamically if not provided by the user, and the output query should be identical assuming the user input the correct parameter values. The condition parameter is not supported when the nested function is used in the SELECT clause. Nested types structure is flattened making the full path of an object the key, and the object it refers to the value. Wildcard support will be added in a follow up PR along with Support for push down with relevance based search functions.

   Sample input=
   keys = ['comments.likes']
   row = comments: {
   likes: 2
   }
   output =
   flattenedRow = {comment.likes: 2}

Example Queries

Simple nested query, array of `message.info` is flattened into two rows.

SELECT nested(message.info) FROM nested_objects;

After flattening notice `comment.data` has repeating `ab`

SELECT nested(message.info), nested(comment.data) FROM nested_objects;

someField is of type object

SELECT nested(message.info), someField FROM nested_objects;

To Do

nested function nested in other function call (Not currently supported in legacy)

Changes from Legacy Functionality

Nested function does not support condition parameter in SELECT clause.
Updated partiql.rst to handle object arrays same as V2 engine.
Legacy implementation uses the FIELD as the column identifier. V2 engine uses function name.

Issues Resolved

Check List

New functionality includes testing.
- All tests pass, including unit test, integration test and doctest
New functionality has been documented.
- New functionality has javadoc added
- New functionality has user manual doc added
Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

codecov-commenter · 2023-03-30T21:43:14Z

Codecov Report

Merging #1490 (fcb3558) into main (e805151) will decrease coverage by 1.31%.
The diff coverage is 100.00%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@             Coverage Diff              @@
##               main    #1490      +/-   ##
============================================
- Coverage     98.49%   97.18%   -1.31%     
- Complexity     3928     4100     +172     
============================================
  Files           347      371      +24     
  Lines          9771    10448     +677     
  Branches        645      703      +58     
============================================
+ Hits           9624    10154     +530     
- Misses          142      287     +145     
- Partials          5        7       +2

Flag	Coverage Δ
sql-engine	`97.18% <100.00%> (-1.31%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
...rg/opensearch/sql/analysis/ExpressionAnalyzer.java	`100.00% <ø> (ø)`
...ch/sql/planner/optimizer/LogicalPlanOptimizer.java	`100.00% <ø> (ø)`
...pensearch/sql/sql/parser/AstExpressionBuilder.java	`100.00% <ø> (ø)`
...ain/java/org/opensearch/sql/analysis/Analyzer.java	`100.00% <100.00%> (ø)`
...va/org/opensearch/sql/analysis/NestedAnalyzer.java	`100.00% <100.00%> (ø)`
...main/java/org/opensearch/sql/executor/Explain.java	`100.00% <100.00%> (ø)`
...c/main/java/org/opensearch/sql/expression/DSL.java	`100.00% <100.00%> (ø)`
...opensearch/sql/expression/ReferenceExpression.java	`100.00% <100.00%> (ø)`
...h/sql/expression/function/BuiltinFunctionName.java	`100.00% <100.00%> (ø)`
...h/sql/expression/function/OpenSearchFunctions.java	`100.00% <100.00%> (ø)`
... and 16 more

... and 27 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

sql/src/main/java/org/opensearch/sql/sql/parser/AstExpressionBuilder.java

dai-chen · 2023-04-03T23:28:42Z

core/src/main/java/org/opensearch/sql/analysis/Analyzer.java

+    for (UnresolvedExpression expr : node.getProjectList()) {
+      NestedAnalyzer nestedAnalyzer = new NestedAnalyzer(
+          namedExpressions, expressionAnalyzer, child
+      );
+      child = nestedAnalyzer.analyze(expr, context);
+    }


How about we generate a single LogicalNested here rather than get them merged later in MergeNestedAndNested?

I will update to create the single LogicalNested here rather than merging further down query execution.

Thanks. I'm just thinking if we have all the project items and always merge multiple LogicalNested later, we may be able to save that optimizer rule.

Checked the latest revision and wondering if the for loop is needed? Because we generate single nested operator with all namedExpressions right?

Yes the for loop is still needed. The namedExpressions are passed in each time for project push down, but we are fulfilling the LogicalNested nested fields by each nested Function in the projectList. Both namedExpressions and nested functions are required.

using for-loop inside NesteAnalyzer, and change interface?

child = nestedAnalyzer.analyze(node.getProjectList(), context);

It would be my preference to keep the for-loop as is so we don't have to complicate adding additional arguments to an already formed LogicalPlan. If we move the for-loop into the NestedAnalyzer we will have to do some not-so-nice logic in NestedAnalyzer:Analyze to aggregate the arguments from the analyzed LogicalPlan's.
NestedAnalyzer.java#L36-L74
If you would prefer this implementation I can make the necessary revisions!

core/src/main/java/org/opensearch/sql/expression/nested/NestedFunction.java

core/src/main/java/org/opensearch/sql/planner/physical/UnnestOperator.java

sql/src/main/antlr/OpenSearchSQLParser.g4

dai-chen · 2023-04-04T16:36:15Z

Meanwhile could you double check if the logical and physical operator is generic and extensible enough:

Later can we expose this capability to PPL? I see there are SPL commands for JSON, MultiValue and flatten too.
Does the physical operator has anything specific to OpenSearch? Does it work for any Struct or Struct Array field no matter where it comes from? ex. Prometheus, CloudWatch etc.

forestmvey · 2023-04-05T15:27:28Z

Meanwhile could you double check if the logical and physical operator is generic and extensible enough:

Later can we expose this capability to PPL? I see there are SPL commands for JSON, MultiValue and flatten too.

Does the physical operator has anything specific to OpenSearch? Does it work for any Struct or Struct Array field no matter where it comes from? ex. Prometheus, CloudWatch etc.

I think the physical operator should work well with exposing functionality in PPL. The functionality is generic and should be able to handle Struct and Arrays fields from other datasources. More investigation will be needed later but the code is generic that porting shouldn't be too difficult.

core/src/main/java/org/opensearch/sql/planner/logical/LogicalNested.java

core/src/main/java/org/opensearch/sql/planner/logical/LogicalPlanNodeVisitor.java

sql/src/main/antlr/OpenSearchSQLParser.g4

opensearch/src/main/java/org/opensearch/sql/opensearch/response/OpenSearchResponse.java

docs/user/dql/functions.rst

core/src/main/java/org/opensearch/sql/expression/function/OpenSearchFunctions.java

dai-chen · 2023-04-05T20:37:02Z

core/src/main/java/org/opensearch/sql/analysis/Analyzer.java

+    for (UnresolvedExpression expr : node.getProjectList()) {
+      NestedAnalyzer nestedAnalyzer = new NestedAnalyzer(
+          namedExpressions, expressionAnalyzer, child
+      );
+      child = nestedAnalyzer.analyze(expr, context);
+    }


Checked the latest revision and wondering if the for loop is needed? Because we generate single nested operator with all namedExpressions right?

core/src/main/java/org/opensearch/sql/planner/physical/UnnestOperator.java

penghuo · 2023-04-06T17:29:43Z

docs/user/dql/functions.rst

+``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).
+If the user does not provide the ``path`` parameter it will be generated dynamically. The ``field`` ``user.office.cubicle`` would dynamically generate the path


what is user.office.cubicle? is it an example sub field?

Yes this is just an example field.

i see, i feel it miss context info, could you add, for example, The field user.office.cubicle would dynamically generate the path

Thanks, I've updated the comment to be more descriptive.

sql/src/main/antlr/OpenSearchSQLParser.g4

penghuo · 2023-04-06T17:40:36Z

core/src/main/java/org/opensearch/sql/analysis/Analyzer.java

+    for (UnresolvedExpression expr : node.getProjectList()) {
+      NestedAnalyzer nestedAnalyzer = new NestedAnalyzer(
+          namedExpressions, expressionAnalyzer, child
+      );
+      child = nestedAnalyzer.analyze(expr, context);
+    }


using for-loop inside NesteAnalyzer, and change interface?

child = nestedAnalyzer.analyze(node.getProjectList(), context);

core/src/test/java/org/opensearch/sql/analysis/AnalyzerTest.java

core/src/main/java/org/opensearch/sql/planner/logical/LogicalNested.java

core/src/test/java/org/opensearch/sql/planner/physical/NestedOperatorTest.java

penghuo · 2023-04-06T18:21:55Z

integ-test/src/test/java/org/opensearch/sql/sql/NestedIT.java

+import org.junit.jupiter.api.Disabled;
+import org.opensearch.sql.legacy.SQLIntegTestCase;
+
+public class NestedIT extends SQLIntegTestCase {


question: Are nested functions limited to being used only in the select clause, or can they also be used in the where or group by clause? what is the expecuted result if nested function been used in where / group by clause?

This PR is only to support nested in the SELECT clause. Follow up PR's will be made for GROUP BY and WHERE clauses.

Maybe: rename to NestedInSelectIT

opensearch-trigger-bot · 2023-04-12T21:22:56Z

The backport to 2.x failed:

The process '/usr/bin/git' failed with exit code 1

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-2.x 2.x
# Navigate to the new working tree
cd .worktrees/backport-2.x
# Create a new branch
git switch --create backport/backport-1490-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 fbc72a41ef66d33c4650171260ddf8c8702903ca
# Push it to GitHub
git push --set-upstream origin backport/backport-1490-to-2.x
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-2.x

Then, create a pull request where the base branch is 2.x and the compare/head branch is backport/backport-1490-to-2.x.

opensearch-trigger-bot · 2023-04-12T21:22:57Z

The backport to 2.7 failed:

The process '/usr/bin/git' failed with exit code 1

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-2.7 2.7
# Navigate to the new working tree
cd .worktrees/backport-2.7
# Create a new branch
git switch --create backport/backport-1490-to-2.7
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 fbc72a41ef66d33c4650171260ddf8c8702903ca
# Push it to GitHub
git push --set-upstream origin backport/backport-1490-to-2.7
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-2.7

Then, create a pull request where the base branch is 2.7 and the compare/head branch is backport/backport-1490-to-2.7.

Users can now query nested fields in an index. --------- Signed-off-by: forestmvey <forestv@bitquilltech.com> (cherry picked from commit fbc72a4)

Users can now query nested fields in an index. --------- Signed-off-by: forestmvey <forestv@bitquilltech.com> (cherry picked from commit fbc72a4) Signed-off-by: forestmvey <forestv@bitquilltech.com>

Users can now query nested fields in an index. --------- (cherry picked from commit fbc72a4) Signed-off-by: forestmvey <forestv@bitquilltech.com>

Users can now query nested fields in an index. --------- Signed-off-by: forestmvey <forestv@bitquilltech.com> (cherry picked from commit fbc72a4)

…roject#1490) Users can now query nested fields in an index. --------- Signed-off-by: forestmvey <forestv@bitquilltech.com>

Users can now query nested fields in an index. --------- Signed-off-by: forestmvey <forestv@bitquilltech.com> (cherry picked from commit fbc72a4)

forestmvey requested review from anirudha, penghuo, ps48, kavithacm, derek-ho, joshuali925, dai-chen, YANG-DB, rupal-bq, mengweieric, vmmusings, Yury-Fridlyand and MaxKsyunz as code owners March 30, 2023 21:21

dai-chen reviewed Apr 4, 2023

View reviewed changes

dai-chen added the enhancement New feature or request label Apr 4, 2023

forestmvey force-pushed the integ-nested-select-clause branch from e8114bd to 8526ef1 Compare April 5, 2023 15:04

acarbonetto previously approved these changes Apr 5, 2023

View reviewed changes

core/src/main/java/org/opensearch/sql/planner/logical/LogicalNested.java Show resolved Hide resolved

forestmvey dismissed acarbonetto’s stale review via 3ff9fdd April 5, 2023 19:15

dai-chen reviewed Apr 5, 2023

View reviewed changes

GabeFernandez310 previously approved these changes Apr 6, 2023

View reviewed changes

MaxKsyunz previously approved these changes Apr 6, 2023

View reviewed changes

core/src/main/java/org/opensearch/sql/planner/physical/UnnestOperator.java Outdated Show resolved Hide resolved

GumpacG previously approved these changes Apr 6, 2023

View reviewed changes

MaxKsyunz added backport 2.x backport 2.7 labels Apr 6, 2023

penghuo reviewed Apr 6, 2023

View reviewed changes

forestmvey dismissed stale reviews from GumpacG and MaxKsyunz via c04d962 April 6, 2023 19:16

forestmvey requested review from Yury-Fridlyand, dai-chen and penghuo April 11, 2023 16:01

acarbonetto approved these changes Apr 11, 2023

View reviewed changes

penghuo approved these changes Apr 11, 2023

View reviewed changes

forestmvey requested a review from MaxKsyunz April 11, 2023 21:06

MaxKsyunz approved these changes Apr 12, 2023

View reviewed changes

Yury-Fridlyand approved these changes Apr 12, 2023

View reviewed changes

MaxKsyunz merged commit fbc72a4 into opensearch-project:main Apr 12, 2023

forestmvey mentioned this pull request Apr 13, 2023

[Backport 2.x] Add Nested Support in Select Clause (#1490) #1517

Merged

6 tasks

forestmvey mentioned this pull request Apr 13, 2023

[Backport 2.7] Add Nested Support in Select Clause (#1490) #1518

Merged

6 tasks

Yury-Fridlyand pushed a commit that referenced this pull request Apr 13, 2023

Support nested function in SELECT clause (#1490) (#1518)

ae15c3b

Users can now query nested fields in an index. --------- (cherry picked from commit fbc72a4) Signed-off-by: forestmvey <forestv@bitquilltech.com>

Yury-Fridlyand pushed a commit that referenced this pull request Apr 13, 2023

Support nested function in SELECT clause (#1490) (#1517)

d16cd3d

Users can now query nested fields in an index. --------- Signed-off-by: forestmvey <forestv@bitquilltech.com> (cherry picked from commit fbc72a4)

This was referenced May 2, 2023

Add Nested Documentation for 2.7 Related Features Bit-Quill/opensearch-project-sql#262

Merged

[FEATURE] P0 Nested Function Support #1111

Closed

forestmvey mentioned this pull request May 9, 2023

Add Nested Documentation for 2.7 Related Features #1620

Merged

6 tasks

forestmvey mentioned this pull request May 18, 2023

Adding Nested Function Documentation Bit-Quill/documentation-website#2

Merged

6 tasks

forestmvey mentioned this pull request Jun 13, 2023

Adding Nested Function Documentation opensearch-project/documentation-website#4308

Closed

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Nested Function Support In SELECT Clause #1490

Add Nested Function Support In SELECT Clause #1490

forestmvey commented Mar 30, 2023 •

edited

Loading

codecov-commenter commented Mar 30, 2023 •

edited

Loading

dai-chen Apr 3, 2023

forestmvey Apr 4, 2023

dai-chen Apr 4, 2023

dai-chen Apr 5, 2023

forestmvey Apr 5, 2023

penghuo Apr 6, 2023

forestmvey Apr 6, 2023

dai-chen commented Apr 4, 2023 •

edited

Loading

forestmvey commented Apr 5, 2023

dai-chen Apr 5, 2023

penghuo Apr 6, 2023

forestmvey Apr 6, 2023

penghuo Apr 10, 2023

forestmvey Apr 10, 2023

penghuo Apr 6, 2023

penghuo Apr 6, 2023

forestmvey Apr 6, 2023

Yury-Fridlyand Apr 7, 2023

opensearch-trigger-bot bot commented Apr 12, 2023

opensearch-trigger-bot bot commented Apr 12, 2023

Add Nested Function Support In SELECT Clause #1490

Add Nested Function Support In SELECT Clause #1490

Conversation

forestmvey commented Mar 30, 2023 • edited Loading

Description

Example Queries

Simple nested query, array of message.info is flattened into two rows.

After flattening notice comment.data has repeating ab

someField is of type object

To Do

Changes from Legacy Functionality

Issues Resolved

Check List

codecov-commenter commented Mar 30, 2023 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dai-chen commented Apr 4, 2023 • edited Loading

forestmvey commented Apr 5, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

opensearch-trigger-bot bot commented Apr 12, 2023

opensearch-trigger-bot bot commented Apr 12, 2023

forestmvey commented Mar 30, 2023 •

edited

Loading

Simple nested query, array of `message.info` is flattened into two rows.

After flattening notice `comment.data` has repeating `ab`

codecov-commenter commented Mar 30, 2023 •

edited

Loading

dai-chen commented Apr 4, 2023 •

edited

Loading