Pagination Phase 2: Support `WHERE` clause, column list in `SELECT` clause and for functions and expressions in the query. #1500

Yury-Fridlyand · 2023-04-06T05:14:07Z

Pagination in V2: Phase 2. See Phase 1 in #1497.

Doc: https://github.com/Bit-Quill/opensearch-project-sql/blob/61767f2b200b2a7f8c8b2df32de209b3c30caa61/docs/dev/Pagination-v2.md

curl -XPOST http://localhost:9200/_plugins/_sql -H 'Content-Type: application/json' -d '{"query": "select highlight(\"Body\") from beer.stackexchange where simple_query_string([\"Tags\" ^ 1.5, \"Title\", \"Body\" 4.2], \"taste\") and Tags like \"% % %\" and Title like \"%\";", "fetch_size": 20 }'

You can use attached script for testing as well. Command line: ./cursor_test.sh <table> <page size>. Requires jq. Rename the script before use.
cursor_test.sh.txt

./cursor_test.sh 'SELECT date0, DATE_ADD(CAST(date0 AS date), INTERVAL 1 HOUR), time0, DATE_ADD(datetime(cast(time0 AS string)), INTERVAL 1 HOUR), time1, DATE_ADD(CAST(time1 AS time), INTERVAL 1 HOUR), datetime0, DATE_ADD(CAST(datetime0 AS timestamp), INTERVAL 1 HOUR) FROM calcs where int3 + num1 < 20;' 2
Got 2 rows
Got 2 rows
Got 2 rows
Got 2 rows
Got 2 rows
Fetched 10 rows in 6 requests in .346s

Description

Add support for more complex queries in paged requests.
Supported things:

WHERE clause
HAVING clause
column names in SELECT clause
functions in SELECT and WHERE:
- build-in functions
- SQL specific functions, like IN, BETWEEN, CASE .. WHEN ..
- OpenSearch functions (relevancy search and highlight)
literals
metafields like _id, _score, etc

Unsupported things:

System queries SHOW and DESCRIBE
NESTED function
aggregation functions like min, max
In-memory aggregation with window function OVER

Issues Resolved

Full support of SELECT clause and support WHERE clause, including functions and expressions.

DEMO

Pagination.with.WHERE.clause.mp4

TODOs:

Demo
More integration tests

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-04-06T05:29:36Z

Codecov Report

Merging #1500 (bd27566) into main (691012d) will increase coverage by 0.04%.
The diff coverage is 100.00%.

@@             Coverage Diff              @@
##               main    #1500      +/-   ##
============================================
+ Coverage     97.27%   97.32%   +0.04%     
- Complexity     4330     4418      +88     
============================================
  Files           388      388              
  Lines         10807    10976     +169     
  Branches        761      775      +14     
============================================
+ Hits          10513    10682     +169     
  Misses          287      287              
  Partials          7        7

Flag	Coverage Δ
sql-engine	`97.32% <100.00%> (+0.04%)`	⬆️

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

Impacted Files	Coverage Δ
...sql/opensearch/request/OpenSearchQueryRequest.java	`100.00% <ø> (ø)`
...arch/sql/opensearch/request/OpenSearchRequest.java	`100.00% <ø> (ø)`
...ch/sql/executor/pagination/CanPaginateVisitor.java	`100.00% <100.00%> (ø)`
...ql/opensearch/request/OpenSearchScrollRequest.java	`100.00% <100.00%> (ø)`

... and 3 files with indirect coverage changes

GumpacG · 2023-05-02T14:56:36Z

core/src/main/java/org/opensearch/sql/executor/pagination/CanPaginateVisitor.java

-    // SELECT max(age) OVER (PARTITION BY city) ...
-    var projections = node.getProjectList();
-    if (projections.size() != 1) {
+  public Boolean visitSort(Sort node, Object context) {


Would it be better to override the defaultResult instead of returning Boolean.FALSE for all unsupported nodes?

Or on the flip side, you could make the defaultResult Boolean.TRUE so that you don't have make duplicated canPaginate(node, context) calls. Doing this can eliminate canPaginate and would use the default visitor of the AbstractNodeVisitor as it traverses through it's children anyways.

core/src/test/java/org/opensearch/sql/planner/DefaultImplementorTest.java

GumpacG · 2023-06-07T19:54:03Z

Queries with aliases seem to be falling back to legacy.
For example,
SELECT int0 as 'a' FROM calcs

Yury-Fridlyand · 2023-06-08T02:51:28Z

Queries with aliases seem to be falling back to legacy. For example, SELECT int0 as 'a' FROM calcs

@GumpacG, There is a syntax issue. Correct one is SELECT int0 as `a` FROM calcs or SELECT int0 as a FROM calcs.

Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com>

forestmvey · 2023-06-08T15:00:42Z

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

@@ -17,6 +17,7 @@
 import java.io.IOException;
 import org.json.JSONObject;
 import org.junit.Assert;
+import org.junit.Ignore;


Probably missed this.

forestmvey · 2023-06-08T15:29:48Z

core/src/main/java/org/opensearch/sql/executor/pagination/CanPaginateVisitor.java

+
+  // Queries with LIMIT clause are not supported
+  @Override
+  public Boolean visitLimit(Limit node, Object context) {


What's the point of having these explicit cases for what isn't supported if it will return false without the override. Perhaps we should log something stating the feature is unsupported. Maybe add a TODO comment for future work.

Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com>

Yury-Fridlyand · 2023-06-14T20:54:02Z

CI fails on BWC, ignoring it for now.

…lause and for functions and expressions in the query. (#1500) * Add support for `WHERE` clause, column list in `SELECT` clause and for functions and expressions in the query. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Fix merge issue and address PR feedback by updating comments. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * More comments. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Add extra check for unset `initialSearchRequest`. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> --------- Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> (cherry picked from commit da386e5)

…lause and for functions and expressions in the query. (#1500) (#1741) * Add support for `WHERE` clause, column list in `SELECT` clause and for functions and expressions in the query. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Fix merge issue and address PR feedback by updating comments. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * More comments. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Add extra check for unset `initialSearchRequest`. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> --------- Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> (cherry picked from commit da386e5) Co-authored-by: Yury-Fridlyand <yury.fridlyand@improving.com>

opensearch-project deleted a comment from github-advanced-security bot Apr 6, 2023

Yury-Fridlyand marked this pull request as ready for review April 6, 2023 05:45

Yury-Fridlyand requested review from anirudha, penghuo, ps48, kavithacm, derek-ho, joshuali925, dai-chen, YANG-DB, rupal-bq, mengweieric, vamsi-amazon and MaxKsyunz as code owners April 6, 2023 05:45

acarbonetto mentioned this pull request Apr 10, 2023

SQL ingestion and ops - standup #791

Open

GabeFernandez310 previously approved these changes Apr 13, 2023

View reviewed changes

matthewryanwells previously approved these changes Apr 14, 2023

View reviewed changes

Yury-Fridlyand force-pushed the feature/pagination/P2 branch from 117e889 to 899d083 Compare April 15, 2023 01:52

Yury-Fridlyand force-pushed the feature/pagination/where branch from 4b995b7 to 4f6da54 Compare April 15, 2023 03:01

MaxKsyunz mentioned this pull request Apr 17, 2023

Support pagination in V2 engine, phase 1 #1497

Merged

6 tasks

Yury-Fridlyand changed the base branch from feature/pagination/P2 to feature/pagination/integ April 27, 2023 21:06

Yury-Fridlyand requested review from pjfitzgibbons, Swiddis and seankao-az as code owners April 27, 2023 21:06

Yury-Fridlyand added the pagination Pagination feature, ref #656 label May 1, 2023

Yury-Fridlyand mentioned this pull request May 2, 2023

Pagination Phase 2: Support ORDER BY clauses and queries without FROM. #1599

Merged

6 tasks

GumpacG reviewed May 2, 2023

View reviewed changes

core/src/test/java/org/opensearch/sql/planner/DefaultImplementorTest.java Outdated Show resolved Hide resolved

Yury-Fridlyand mentioned this pull request May 3, 2023

[BUG] Pagination in V2 doesn't track mapping change #1569

Open

Yury-Fridlyand mentioned this pull request Jun 7, 2023

[FEATURE] Pagination with NESTED function in v2 #1718

Open

Yury-Fridlyand added 2 commits June 7, 2023 19:53

Fix merge issue and address PR feedback by updating comments.

e00f8f1

Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com>

More comments.

45d8ed6

Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com>

Yury-Fridlyand requested review from MaxKsyunz, GumpacG and penghuo June 8, 2023 03:03

forestmvey reviewed Jun 8, 2023

View reviewed changes

forestmvey previously approved these changes Jun 8, 2023

View reviewed changes

GumpacG previously approved these changes Jun 8, 2023

View reviewed changes

Add extra check for unset initialSearchRequest.

bd27566

Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com>

Yury-Fridlyand dismissed stale reviews from GumpacG and forestmvey via bd27566 June 14, 2023 00:13

Yury-Fridlyand requested review from forestmvey and GumpacG June 14, 2023 00:52

Yury-Fridlyand changed the title ~~Add support for WHERE clause, column list in SELECT clause and for functions and expressions in the query.~~ Pagination Phase 2: Support WHERE clause, column list in SELECT clause and for functions and expressions in the query. Jun 14, 2023

MaxKsyunz approved these changes Jun 14, 2023

View reviewed changes

GumpacG approved these changes Jun 14, 2023

View reviewed changes

forestmvey approved these changes Jun 14, 2023

View reviewed changes

Yury-Fridlyand merged commit da386e5 into main Jun 14, 2023
17 of 22 checks passed

Yury-Fridlyand deleted the feature/pagination/where branch June 14, 2023 22:41

opensearch-trigger-bot bot mentioned this pull request Jun 14, 2023

[Backport 2.x] Pagination Phase 2: Support WHERE clause, column list in SELECT clause and for functions and expressions in the query. #1741

Merged

This was referenced Jun 17, 2023

[FEATURE] - Support pagination for PPL and SQL query #656

Closed

[FEATURE] Implement pagination in V2 #1759

Open

Yury-Fridlyand added this to In Release 2.9 in SQL/PPL Epic Roadmap Jul 11, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Pagination Phase 2: Support `WHERE` clause, column list in `SELECT` clause and for functions and expressions in the query. #1500

Pagination Phase 2: Support `WHERE` clause, column list in `SELECT` clause and for functions and expressions in the query. #1500

Yury-Fridlyand commented Apr 6, 2023 •

edited

codecov-commenter commented Apr 6, 2023 •

edited by codecov bot

GumpacG May 2, 2023

GumpacG May 2, 2023

GumpacG commented Jun 7, 2023

Yury-Fridlyand commented Jun 8, 2023

forestmvey Jun 8, 2023

forestmvey Jun 8, 2023 •

edited

Yury-Fridlyand commented Jun 14, 2023

Pagination Phase 2: Support WHERE clause, column list in SELECT clause and for functions and expressions in the query. #1500

Pagination Phase 2: Support WHERE clause, column list in SELECT clause and for functions and expressions in the query. #1500

Conversation

Yury-Fridlyand commented Apr 6, 2023 • edited

Description

Issues Resolved

DEMO

TODOs:

Check List

codecov-commenter commented Apr 6, 2023 • edited by codecov bot

Codecov Report

GumpacG May 2, 2023

Choose a reason for hiding this comment

GumpacG May 2, 2023

Choose a reason for hiding this comment

GumpacG commented Jun 7, 2023

Yury-Fridlyand commented Jun 8, 2023

forestmvey Jun 8, 2023

Choose a reason for hiding this comment

forestmvey Jun 8, 2023 • edited

Choose a reason for hiding this comment

Yury-Fridlyand commented Jun 14, 2023

Pagination Phase 2: Support `WHERE` clause, column list in `SELECT` clause and for functions and expressions in the query. #1500

Pagination Phase 2: Support `WHERE` clause, column list in `SELECT` clause and for functions and expressions in the query. #1500

Yury-Fridlyand commented Apr 6, 2023 •

edited

codecov-commenter commented Apr 6, 2023 •

edited by codecov bot

forestmvey Jun 8, 2023 •

edited