Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Support for Field Star in Nested Function #1773

Merged
merged 5 commits into from
Jun 28, 2023

Conversation

forestmvey
Copy link
Collaborator

Description

Add support for using the * at end of field parameter in the nested function. All nested fields under supplied path will be expanded as column identifiers. Grammar has been added to only support the AllTupleFields usage with the nested function since this change will effect field usage in all queries and needs further planning with V2 implementation.

Example Queries

SELECT nested(message.*) from nested_objects;

Issues Resolved

Portion of Issue: 1111

Changes From Legacy

  • Path parameter is redundant therefore not supported in parser and will fallback to legacy engine.

Edge Cases

Queries that produce repeating column identifiers like:

SELECT nested(field.*), nested(field.innerField) ...

Or

SELECT nested(field.*), nested(field.*) ...

Will result in error:

{
  "error": {
    "reason": "Invalid SQL query",
    "details": "Multiple entries with same key: nested(message.info)=\"a\" and nested(message.info)=\"a\"",
    "type": "IllegalArgumentException"
  },
  "status": 400
}

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
Copy link

codecov bot commented Jun 23, 2023

Codecov Report

Merging #1773 (51094dd) into main (1ec696d) will increase coverage by 0.01%.
The diff coverage is 100.00%.

@@             Coverage Diff              @@
##               main    #1773      +/-   ##
============================================
+ Coverage     97.30%   97.32%   +0.01%     
- Complexity     4442     4458      +16     
============================================
  Files           388      388              
  Lines         11000    11050      +50     
  Branches        785      790       +5     
============================================
+ Hits          10704    10754      +50     
  Misses          289      289              
  Partials          7        7              
Flag Coverage Δ
sql-engine 97.32% <100.00%> (+0.01%) ⬆️

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

Impacted Files Coverage Δ
...ain/java/org/opensearch/sql/analysis/Analyzer.java 100.00% <100.00%> (ø)
...rg/opensearch/sql/analysis/ExpressionAnalyzer.java 100.00% <100.00%> (ø)
...va/org/opensearch/sql/analysis/NestedAnalyzer.java 100.00% <100.00%> (ø)
...nsearch/sql/analysis/SelectExpressionAnalyzer.java 100.00% <100.00%> (ø)
...a/org/opensearch/sql/analysis/TypeEnvironment.java 100.00% <100.00%> (ø)
...rg/opensearch/sql/analysis/symbol/SymbolTable.java 100.00% <100.00%> (ø)
...pensearch/sql/sql/parser/AstExpressionBuilder.java 100.00% <100.00%> (ø)

@Yury-Fridlyand
Copy link
Collaborator

Please, fix DCO

Yury-Fridlyand
Yury-Fridlyand previously approved these changes Jun 27, 2023
GumpacG
GumpacG previously approved these changes Jun 27, 2023
Signed-off-by: forestmvey <forestv@bitquilltech.com>
GumpacG
GumpacG previously approved these changes Jun 28, 2023
Signed-off-by: forestmvey <forestv@bitquilltech.com>
@forestmvey forestmvey merged commit fa840e0 into opensearch-project:main Jun 28, 2023
13 of 15 checks passed
opensearch-trigger-bot bot pushed a commit that referenced this pull request Jun 28, 2023
* Add Support for Field Star in Nested Function.

Signed-off-by: forestmvey <forestv@bitquilltech.com>

* Removing toString for NestedAllTupleFields.

Signed-off-by: forestmvey <forestv@bitquilltech.com>

* Adding IT test for nested all fields in invalid clause of SQL statement.

Signed-off-by: forestmvey <forestv@bitquilltech.com>

* Use utility function for checking is nested in NestedAnalyzer.

Signed-off-by: forestmvey <forestv@bitquilltech.com>

* Formatting fixes.

Signed-off-by: forestmvey <forestv@bitquilltech.com>

---------

Signed-off-by: forestmvey <forestv@bitquilltech.com>
(cherry picked from commit fa840e0)
forestmvey added a commit that referenced this pull request Jun 28, 2023
* Add Support for Field Star in Nested Function.

Signed-off-by: forestmvey <forestv@bitquilltech.com>

* Removing toString for NestedAllTupleFields.

Signed-off-by: forestmvey <forestv@bitquilltech.com>

* Adding IT test for nested all fields in invalid clause of SQL statement.

Signed-off-by: forestmvey <forestv@bitquilltech.com>

* Use utility function for checking is nested in NestedAnalyzer.

Signed-off-by: forestmvey <forestv@bitquilltech.com>

* Formatting fixes.

Signed-off-by: forestmvey <forestv@bitquilltech.com>

---------

Signed-off-by: forestmvey <forestv@bitquilltech.com>
(cherry picked from commit fa840e0)
Signed-off-by: forestmvey <forestv@bitquilltech.com>
@forestmvey forestmvey deleted the integ-nested-all-fields branch June 28, 2023 21:38
forestmvey added a commit that referenced this pull request Jun 28, 2023
* Add Support for Field Star in Nested Function.

Signed-off-by: forestmvey <forestv@bitquilltech.com>

* Removing toString for NestedAllTupleFields.

Signed-off-by: forestmvey <forestv@bitquilltech.com>

* Adding IT test for nested all fields in invalid clause of SQL statement.

Signed-off-by: forestmvey <forestv@bitquilltech.com>

* Use utility function for checking is nested in NestedAnalyzer.

Signed-off-by: forestmvey <forestv@bitquilltech.com>

* Formatting fixes.

Signed-off-by: forestmvey <forestv@bitquilltech.com>

---------

Signed-off-by: forestmvey <forestv@bitquilltech.com>
(cherry picked from commit fa840e0)
Signed-off-by: forestmvey <forestv@bitquilltech.com>
@acarbonetto acarbonetto added v.2.9.0 v2.9.0 v2.9.0 and removed v.2.9.0 labels Jul 4, 2023
matthewryanwells pushed a commit to Bit-Quill/opensearch-project-sql that referenced this pull request Jul 7, 2023
* Add Support for Field Star in Nested Function.

Signed-off-by: forestmvey <forestv@bitquilltech.com>

* Removing toString for NestedAllTupleFields.

Signed-off-by: forestmvey <forestv@bitquilltech.com>

* Adding IT test for nested all fields in invalid clause of SQL statement.

Signed-off-by: forestmvey <forestv@bitquilltech.com>

* Use utility function for checking is nested in NestedAnalyzer.

Signed-off-by: forestmvey <forestv@bitquilltech.com>

* Formatting fixes.

Signed-off-by: forestmvey <forestv@bitquilltech.com>

---------

Signed-off-by: forestmvey <forestv@bitquilltech.com>
matthewryanwells pushed a commit to Bit-Quill/opensearch-project-sql that referenced this pull request Jul 7, 2023
* Add Support for Field Star in Nested Function.

Signed-off-by: forestmvey <forestv@bitquilltech.com>

* Removing toString for NestedAllTupleFields.

Signed-off-by: forestmvey <forestv@bitquilltech.com>

* Adding IT test for nested all fields in invalid clause of SQL statement.

Signed-off-by: forestmvey <forestv@bitquilltech.com>

* Use utility function for checking is nested in NestedAnalyzer.

Signed-off-by: forestmvey <forestv@bitquilltech.com>

* Formatting fixes.

Signed-off-by: forestmvey <forestv@bitquilltech.com>

---------

Signed-off-by: forestmvey <forestv@bitquilltech.com>
Signed-off-by: Matthew Wells <matthew.wells@improving.com>
forestmvey added a commit that referenced this pull request Jul 11, 2023
* Add Support for Field Star in Nested Function.

Signed-off-by: forestmvey <forestv@bitquilltech.com>

* Removing toString for NestedAllTupleFields.

Signed-off-by: forestmvey <forestv@bitquilltech.com>

* Adding IT test for nested all fields in invalid clause of SQL statement.

Signed-off-by: forestmvey <forestv@bitquilltech.com>

* Use utility function for checking is nested in NestedAnalyzer.

Signed-off-by: forestmvey <forestv@bitquilltech.com>

* Formatting fixes.

Signed-off-by: forestmvey <forestv@bitquilltech.com>

---------

Signed-off-by: forestmvey <forestv@bitquilltech.com>
(cherry picked from commit fa840e0)
Signed-off-by: forestmvey <forestv@bitquilltech.com>
forestmvey added a commit that referenced this pull request Jul 11, 2023
* Add Support for Field Star in Nested Function.



* Removing toString for NestedAllTupleFields.



* Adding IT test for nested all fields in invalid clause of SQL statement.



* Use utility function for checking is nested in NestedAnalyzer.



* Formatting fixes.



---------


(cherry picked from commit fa840e0)

Signed-off-by: forestmvey <forestv@bitquilltech.com>
Co-authored-by: Forest Vey <forestv@bitquilltech.com>
opensearch-trigger-bot bot pushed a commit that referenced this pull request Jul 11, 2023
* Add Support for Field Star in Nested Function.

Signed-off-by: forestmvey <forestv@bitquilltech.com>

* Removing toString for NestedAllTupleFields.

Signed-off-by: forestmvey <forestv@bitquilltech.com>

* Adding IT test for nested all fields in invalid clause of SQL statement.

Signed-off-by: forestmvey <forestv@bitquilltech.com>

* Use utility function for checking is nested in NestedAnalyzer.

Signed-off-by: forestmvey <forestv@bitquilltech.com>

* Formatting fixes.

Signed-off-by: forestmvey <forestv@bitquilltech.com>

---------

Signed-off-by: forestmvey <forestv@bitquilltech.com>
(cherry picked from commit fa840e0)
forestmvey added a commit that referenced this pull request Jul 11, 2023
* Add Support for Field Star in Nested Function.

Signed-off-by: forestmvey <forestv@bitquilltech.com>

* Removing toString for NestedAllTupleFields.

Signed-off-by: forestmvey <forestv@bitquilltech.com>

* Adding IT test for nested all fields in invalid clause of SQL statement.

Signed-off-by: forestmvey <forestv@bitquilltech.com>

* Use utility function for checking is nested in NestedAnalyzer.

Signed-off-by: forestmvey <forestv@bitquilltech.com>

* Formatting fixes.

Signed-off-by: forestmvey <forestv@bitquilltech.com>

---------

Signed-off-by: forestmvey <forestv@bitquilltech.com>
(cherry picked from commit fa840e0)
Signed-off-by: forestmvey <forestv@bitquilltech.com>
forestmvey added a commit that referenced this pull request Jul 11, 2023
* Add Support for Field Star in Nested Function.

Signed-off-by: forestmvey <forestv@bitquilltech.com>

* Removing toString for NestedAllTupleFields.

Signed-off-by: forestmvey <forestv@bitquilltech.com>

* Adding IT test for nested all fields in invalid clause of SQL statement.

Signed-off-by: forestmvey <forestv@bitquilltech.com>

* Use utility function for checking is nested in NestedAnalyzer.

Signed-off-by: forestmvey <forestv@bitquilltech.com>

* Formatting fixes.

Signed-off-by: forestmvey <forestv@bitquilltech.com>

---------

Signed-off-by: forestmvey <forestv@bitquilltech.com>
(cherry picked from commit fa840e0)
Signed-off-by: forestmvey <forestv@bitquilltech.com>
Yury-Fridlyand pushed a commit that referenced this pull request Jul 11, 2023
* Add Support for Field Star in Nested Function.



* Removing toString for NestedAllTupleFields.



* Adding IT test for nested all fields in invalid clause of SQL statement.



* Use utility function for checking is nested in NestedAnalyzer.



* Formatting fixes.



---------


(cherry picked from commit fa840e0)

Signed-off-by: forestmvey <forestv@bitquilltech.com>
Co-authored-by: Forest Vey <forestv@bitquilltech.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants