Remove paginate operator during logical plan optimization. #1528
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: MaxKsyunz <maxk@bitquilltech.com>
MaxKsyunz
requested review from
anirudha,
penghuo,
ps48,
kavithacm,
derek-ho,
joshuali925,
dai-chen,
YANG-DB,
rupal-bq,
mengweieric,
vmmusings and
Yury-Fridlyand
as code owners
Codecov Report
📣 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 @@
## feature/pagination/P1 #1528 +/- ##
========================================================
Coverage 97.25% 97.25%
Complexity 4229 4229
========================================================
Files 389 389
Lines 10729 10729
Branches 720 720
========================================================
Hits 10435 10435
Misses 287 287
Partials 7 7
Flags with carried forward coverage won't be shown. Click here to find out more.
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
Yury-Fridlyand
approved these changes
Apr 17, 2023
core/src/test/java/org/opensearch/sql/planner/optimizer/LogicalPlanOptimizerTest.java
Outdated
Show resolved
Hide resolved
Signed-off-by: MaxKsyunz <maxk@bitquilltech.com>
6 tasks
Signed-off-by: MaxKsyunz <maxk@bitquilltech.com>
Yury-Fridlyand
approved these changes
Apr 17, 2023
Yury-Fridlyand
added a commit
that referenced
this pull request
Apr 27, 2023
* Support pagination in V2 engine, phase 1 (#226) * Fixing integration tests broken during POC Signed-off-by: MaxKsyunz <maxk@bitquilltech.com> * Comment to clarify an exception. Signed-off-by: MaxKsyunz <maxk@bitquilltech.com> * Add support for paginated scroll request, first page. Implement PaginatedPlanCache.convertToPlan for second page to work. Signed-off-by: MaxKsyunz <maxk@bitquilltech.com> * Progress on paginated scroll request, subsequent page. Signed-off-by: MaxKsyunz <maxk@bitquilltech.com> * Move `ExpressionSerializer` from `opensearch` to `core`. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Rename `Cursor` `asString` to `toString`. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Disable scroll cleaning. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Add full cursor serialization and deserialization. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Misc fixes. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Further work on pagination. * Added push down page size from `LogicalPaginate` to `LogicalRelation`. * Improved cursor encoding and decoding. * Added cursor compression. * Fixed issuing `SearchScrollRequest`. * Fixed returning last empty page. * Minor code grooming/commenting. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Pagination fix for empty indices. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Fix error reporting on wrong cursor. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Minor comments and error reporting improvement. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Add an end-to-end integration test. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Add `explain` request handlers. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Add IT for explain. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Address issues flagged by checkstyle build step (#229) Signed-off-by: MaxKsyunz <maxk@bitquilltech.com> * Pagination, phase 1: Add unit tests for `:core` module with coverage. (#230) * Add unit tests for `:core` module with coverage. Uncovered: `toCursor`, because it is will be changed soon. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Pagination, phase 1: Add unit tests for SQL module with coverage. (#239) * Add unit tests for SQL module with coverage. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Update sql/src/main/java/org/opensearch/sql/sql/domain/SQLQueryRequest.java Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> Co-authored-by: GabeFernandez310 <Gabriel.Fernandez@improving.com> --------- Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> Co-authored-by: GabeFernandez310 <Gabriel.Fernandez@improving.com> * Pagination, phase 1: Add unit tests for `:opensearch` module with coverage. (#233) * Add UT for `:opensearch` module with full coverage, except `toCursor`. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Fix checkstyle. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> --------- Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Fix the merges. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Fix explain. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Fix scroll cleaning. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Store `TotalHits` and use it to report `total` in response. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Add missing UT for `:protocol` module. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Fix PPL UTs damaged in f4ea4ad. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Minor checkstyle fixes. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Fallback to v1 engine for pagination (#245) * Pagination fallback integration tests. Signed-off-by: MaxKsyunz <maxk@bitquilltech.com> * Add UT with coverage for `toCursor` serialization. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Fix broken tests in `legacy`. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Fix getting `total` from non-paged requests and from queries without `FROM` clause. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Fix scroll cleaning. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Fix cursor request processing. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Update ITs. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Fix (again) TotalHits feature. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Fix typo in prometheus config. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Recover commented logging. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Move `test_pagination_blackbox` to a separate class and add logging. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Address some PR feedbacks: rename some classes and revert unnecessary whitespace changed. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Minor commenting. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Address PR comments. * Add javadocs * Renames * Cleaning up some comments * Remove unused code * Speed up IT Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Minor missing changes. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Integration tests for fetch_size, max_result_window, and query.size_limit (#248) Signed-off-by: MaxKsyunz <maxk@bitquilltech.com> * Remove `PaginatedQueryService`, extend `QueryService` to hold two planners and use them. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Move push down functions from request builders to a new interface. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Some file moves. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Minor clean-up according to PR review. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> --------- Signed-off-by: MaxKsyunz <maxk@bitquilltech.com> Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> Co-authored-by: MaxKsyunz <maxk@bitquilltech.com> Co-authored-by: GabeFernandez310 <Gabriel.Fernandez@improving.com> Co-authored-by: Max Ksyunz <max.ksyunz@improving.com> * Make scroll timeout configurable. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Fix IT to set cursor keep alive parameter. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Remove `QueryId.None`. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Rename according to PR feedback. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Remove default implementations of `PushDownRequestBuilder`. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Merge paginated plan optimizer into the regular optimizer. (#1516) Merge paginated plan optimizer into the regular optimizer. --------- Signed-off-by: MaxKsyunz <maxk@bitquilltech.com> Co-authored-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Complete rework on serialization and deserialization. (#1498) Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Resolve merge conflicts and fix tests. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Minor cleanup. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Minor cleanup - missing changes for the previous commit. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Remove paginate operator (#1528) * Remove PaginateOperator class since it is no longer used. --------- Signed-off-by: MaxKsyunz <maxk@bitquilltech.com> * Remove `PaginatedPlan` - move logic to `QueryPlan`. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Remove default implementations from `SerializablePlan`. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Add a doc. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Update design graphs. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * More fixes for merge from upstream/main. Signed-off-by: MaxKsyunz <maxk@bitquilltech.com> --------- Signed-off-by: MaxKsyunz <maxk@bitquilltech.com> Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> Co-authored-by: MaxKsyunz <maxk@bitquilltech.com> Co-authored-by: GabeFernandez310 <Gabriel.Fernandez@improving.com> Co-authored-by: Max Ksyunz <max.ksyunz@improving.com>
acarbonetto
pushed a commit
to Bit-Quill/opensearch-project-sql
that referenced
this pull request
Apr 28, 2023
* Support pagination in V2 engine, phase 1 (#226) * Fixing integration tests broken during POC Signed-off-by: MaxKsyunz <maxk@bitquilltech.com> * Comment to clarify an exception. Signed-off-by: MaxKsyunz <maxk@bitquilltech.com> * Add support for paginated scroll request, first page. Implement PaginatedPlanCache.convertToPlan for second page to work. Signed-off-by: MaxKsyunz <maxk@bitquilltech.com> * Progress on paginated scroll request, subsequent page. Signed-off-by: MaxKsyunz <maxk@bitquilltech.com> * Move `ExpressionSerializer` from `opensearch` to `core`. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Rename `Cursor` `asString` to `toString`. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Disable scroll cleaning. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Add full cursor serialization and deserialization. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Misc fixes. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Further work on pagination. * Added push down page size from `LogicalPaginate` to `LogicalRelation`. * Improved cursor encoding and decoding. * Added cursor compression. * Fixed issuing `SearchScrollRequest`. * Fixed returning last empty page. * Minor code grooming/commenting. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Pagination fix for empty indices. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Fix error reporting on wrong cursor. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Minor comments and error reporting improvement. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Add an end-to-end integration test. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Add `explain` request handlers. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Add IT for explain. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Address issues flagged by checkstyle build step (#229) Signed-off-by: MaxKsyunz <maxk@bitquilltech.com> * Pagination, phase 1: Add unit tests for `:core` module with coverage. (#230) * Add unit tests for `:core` module with coverage. Uncovered: `toCursor`, because it is will be changed soon. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Pagination, phase 1: Add unit tests for SQL module with coverage. (#239) * Add unit tests for SQL module with coverage. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Update sql/src/main/java/org/opensearch/sql/sql/domain/SQLQueryRequest.java Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> Co-authored-by: GabeFernandez310 <Gabriel.Fernandez@improving.com> --------- Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> Co-authored-by: GabeFernandez310 <Gabriel.Fernandez@improving.com> * Pagination, phase 1: Add unit tests for `:opensearch` module with coverage. (#233) * Add UT for `:opensearch` module with full coverage, except `toCursor`. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Fix checkstyle. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> --------- Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Fix the merges. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Fix explain. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Fix scroll cleaning. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Store `TotalHits` and use it to report `total` in response. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Add missing UT for `:protocol` module. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Fix PPL UTs damaged in f4ea4ad. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Minor checkstyle fixes. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Fallback to v1 engine for pagination (#245) * Pagination fallback integration tests. Signed-off-by: MaxKsyunz <maxk@bitquilltech.com> * Add UT with coverage for `toCursor` serialization. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Fix broken tests in `legacy`. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Fix getting `total` from non-paged requests and from queries without `FROM` clause. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Fix scroll cleaning. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Fix cursor request processing. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Update ITs. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Fix (again) TotalHits feature. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Fix typo in prometheus config. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Recover commented logging. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Move `test_pagination_blackbox` to a separate class and add logging. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Address some PR feedbacks: rename some classes and revert unnecessary whitespace changed. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Minor commenting. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Address PR comments. * Add javadocs * Renames * Cleaning up some comments * Remove unused code * Speed up IT Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Minor missing changes. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Integration tests for fetch_size, max_result_window, and query.size_limit (#248) Signed-off-by: MaxKsyunz <maxk@bitquilltech.com> * Remove `PaginatedQueryService`, extend `QueryService` to hold two planners and use them. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Move push down functions from request builders to a new interface. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Some file moves. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Minor clean-up according to PR review. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> --------- Signed-off-by: MaxKsyunz <maxk@bitquilltech.com> Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> Co-authored-by: MaxKsyunz <maxk@bitquilltech.com> Co-authored-by: GabeFernandez310 <Gabriel.Fernandez@improving.com> Co-authored-by: Max Ksyunz <max.ksyunz@improving.com> * Make scroll timeout configurable. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Fix IT to set cursor keep alive parameter. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Remove `QueryId.None`. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Rename according to PR feedback. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Remove default implementations of `PushDownRequestBuilder`. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Merge paginated plan optimizer into the regular optimizer. (opensearch-project#1516) Merge paginated plan optimizer into the regular optimizer. --------- Signed-off-by: MaxKsyunz <maxk@bitquilltech.com> Co-authored-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Complete rework on serialization and deserialization. (opensearch-project#1498) Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Resolve merge conflicts and fix tests. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Minor cleanup. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Minor cleanup - missing changes for the previous commit. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Remove paginate operator (opensearch-project#1528) * Remove PaginateOperator class since it is no longer used. --------- Signed-off-by: MaxKsyunz <maxk@bitquilltech.com> * Remove `PaginatedPlan` - move logic to `QueryPlan`. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Remove default implementations from `SerializablePlan`. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Add a doc. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * Update design graphs. Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> * More fixes for merge from upstream/main. Signed-off-by: MaxKsyunz <maxk@bitquilltech.com> --------- Signed-off-by: MaxKsyunz <maxk@bitquilltech.com> Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com> Co-authored-by: MaxKsyunz <maxk@bitquilltech.com> Co-authored-by: GabeFernandez310 <Gabriel.Fernandez@improving.com> Co-authored-by: Max Ksyunz <max.ksyunz@improving.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The operator is pushed down during logical plan optimization.
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.