forked from opensearch-project/sql
-
Notifications
You must be signed in to change notification settings - Fork 0
Support pagination in V2 engine, phase 1 #226
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
Merged
Merged
Changes from 40 commits
Commits
Show all changes
47 commits
Select commit
Hold shift + click to select a range
7e2dc80
Fixing integration tests broken during POC
d359751
Comment to clarify an exception.
9c3f7fe
Add support for paginated scroll request, first page.
1ee718b
Progress on paginated scroll request, subsequent page.
f3cade6
Move `ExpressionSerializer` from `opensearch` to `core`.
Yury-Fridlyand db342b8
Rename `Cursor` `asString` to `toString`.
Yury-Fridlyand c8f0935
Disable scroll cleaning.
Yury-Fridlyand fffc36d
Add full cursor serialization and deserialization.
Yury-Fridlyand d844977
Misc fixes.
Yury-Fridlyand 333432e
Further work on pagination.
Yury-Fridlyand 85c8825
Pagination fix for empty indices.
Yury-Fridlyand 484a8fe
Fix error reporting on wrong cursor.
Yury-Fridlyand cccce53
Minor comments and error reporting improvement.
Yury-Fridlyand 2895883
Add an end-to-end integration test.
Yury-Fridlyand dd6fcd6
Add `explain` request handlers.
Yury-Fridlyand 2a19e56
Add IT for explain.
Yury-Fridlyand a3ef2bf
Address issues flagged by checkstyle build step (#229)
2d29549
Pagination, phase 1: Add unit tests for `:core` module with coverage.…
Yury-Fridlyand 70ccfcb
Pagination, phase 1: Add unit tests for SQL module with coverage. (#239)
Yury-Fridlyand 803f50e
Pagination, phase 1: Add unit tests for `:opensearch` module with cov…
Yury-Fridlyand 27e1793
Fix the merges.
Yury-Fridlyand 304616d
Fix explain.
Yury-Fridlyand 1b5ab7e
Fix scroll cleaning.
Yury-Fridlyand f4ea4ad
Store `TotalHits` and use it to report `total` in response.
Yury-Fridlyand 7f0acdd
Add missing UT for `:protocol` module.
Yury-Fridlyand 2ce1626
Fix PPL UTs damaged in f4ea4ad8c.
Yury-Fridlyand b2e6e56
Minor checkstyle fixes.
Yury-Fridlyand c7ad219
Fallback to v1 engine for pagination (#245)
MaxKsyunz 981bc25
Add UT with coverage for `toCursor` serialization.
Yury-Fridlyand 960c039
Fix broken tests in `legacy`.
Yury-Fridlyand 4f0c176
Fix getting `total` from non-paged requests and from queries without …
Yury-Fridlyand bdd52a0
Fix scroll cleaning.
Yury-Fridlyand a16332f
Fix cursor request processing.
Yury-Fridlyand 9f9e873
Update ITs.
Yury-Fridlyand 3340e38
Fix (again) TotalHits feature.
Yury-Fridlyand 524f220
Fix typo in prometheus config.
Yury-Fridlyand 281f3cd
Recover commented logging.
Yury-Fridlyand ca76e1b
Move `test_pagination_blackbox` to a separate class and add logging.
Yury-Fridlyand c8fcd4e
Address some PR feedbacks: rename some classes and revert unnecessary…
Yury-Fridlyand ec5fb40
Minor commenting.
Yury-Fridlyand 4213388
Address PR comments.
Yury-Fridlyand ca20d16
Minor missing changes.
Yury-Fridlyand a58733e
Integration tests for fetch_size, max_result_window, and query.size_l…
75b8140
Remove `PaginatedQueryService`, extend `QueryService` to hold two pla…
Yury-Fridlyand ba81407
Move push down functions from request builders to a new interface.
Yury-Fridlyand cec012b
Some file moves.
Yury-Fridlyand 9032b3b
Minor clean-up according to PR review.
Yury-Fridlyand File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
45 changes: 45 additions & 0 deletions
45
core/src/main/java/org/opensearch/sql/ast/tree/Paginate.java
This file contains hidden or 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,45 @@ | ||
| /* | ||
| * Copyright OpenSearch Contributors | ||
| * SPDX-License-Identifier: Apache-2.0 | ||
| */ | ||
|
|
||
| package org.opensearch.sql.ast.tree; | ||
|
|
||
| import java.util.List; | ||
| import lombok.EqualsAndHashCode; | ||
| import lombok.Getter; | ||
| import lombok.RequiredArgsConstructor; | ||
| import lombok.ToString; | ||
| import org.opensearch.sql.ast.AbstractNodeVisitor; | ||
| import org.opensearch.sql.ast.Node; | ||
|
|
||
| @RequiredArgsConstructor | ||
| @EqualsAndHashCode(callSuper = false) | ||
| @ToString | ||
| public class Paginate extends UnresolvedPlan { | ||
Yury-Fridlyand marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| @Getter | ||
| private final int pageSize; | ||
| private UnresolvedPlan child; | ||
|
|
||
| public Paginate(int pageSize, UnresolvedPlan child) { | ||
| this.pageSize = pageSize; | ||
| this.child = child; | ||
| } | ||
|
|
||
| @Override | ||
| public List<? extends Node> getChild() { | ||
| return List.of(child); | ||
| } | ||
|
|
||
| @Override | ||
| public <T, C> T accept(AbstractNodeVisitor<T, C> nodeVisitor, C context) { | ||
| return nodeVisitor.visitPaginate(this, context); | ||
| } | ||
|
|
||
| @Override | ||
| public UnresolvedPlan attach(UnresolvedPlan child) { | ||
| assert this.child == null; | ||
| this.child = child; | ||
| return this; | ||
| } | ||
| } | ||
9 changes: 9 additions & 0 deletions
9
core/src/main/java/org/opensearch/sql/exception/UnsupportedCursorRequestException.java
This file contains hidden or 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| /* | ||
| * Copyright OpenSearch Contributors | ||
| * SPDX-License-Identifier: Apache-2.0 | ||
| */ | ||
|
|
||
| package org.opensearch.sql.exception; | ||
|
|
||
| public class UnsupportedCursorRequestException extends RuntimeException { | ||
Yury-Fridlyand marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
104 changes: 104 additions & 0 deletions
104
core/src/main/java/org/opensearch/sql/executor/CanPaginateVisitor.java
This file contains hidden or 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,104 @@ | ||
| /* | ||
| * Copyright OpenSearch Contributors | ||
| * SPDX-License-Identifier: Apache-2.0 | ||
| */ | ||
|
|
||
| package org.opensearch.sql.executor; | ||
|
|
||
| import java.util.concurrent.atomic.AtomicBoolean; | ||
| import org.opensearch.sql.ast.AbstractNodeVisitor; | ||
| import org.opensearch.sql.ast.Node; | ||
| import org.opensearch.sql.ast.expression.AllFields; | ||
| import org.opensearch.sql.ast.tree.Project; | ||
| import org.opensearch.sql.ast.tree.Relation; | ||
|
|
||
| /** | ||
| * Use this unresolved plan visitor to check if a plan can be serialized by PaginatedPlanCache. | ||
MaxKsyunz marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| * If plan.accept(new CanPaginateVisitor(...)) returns true, | ||
| * then PaginatedPlanCache.convertToCursor will succeed. | ||
| * Otherwise, it will fail. | ||
| * Currently, the conditions are: | ||
| * - only projection of a relation is supported. | ||
| * - projection only has * (a.k.a. allFields). | ||
| * - Relation only scans one table | ||
| * - The table is an open search index. | ||
| * See PaginatedPlanCache.canConvertToCursor for usage. | ||
| */ | ||
| public class CanPaginateVisitor extends AbstractNodeVisitor<Boolean, Object> { | ||
|
|
||
| @Override | ||
| public Boolean visitRelation(Relation node, Object context) { | ||
| if (!node.getChild().isEmpty()) { | ||
| // Relation instance should never have a child, but check just in case. | ||
| return Boolean.FALSE; | ||
| } | ||
|
|
||
| // TODO use storageEngine from the calling PaginatedPlanCache to determine if | ||
| // node.getTableQualifiedName is provided by the storage engine. Return false if it's | ||
| // not the case. | ||
Yury-Fridlyand marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| return Boolean.TRUE; | ||
| } | ||
|
|
||
| /* | ||
| private Boolean canPaginate(Node node, Object context) { | ||
Yury-Fridlyand marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| AtomicBoolean result = new AtomicBoolean(true); | ||
| node.getChild().forEach(n -> result.set(result.get() && n.accept(this, context))); | ||
| return result.get(); | ||
| } | ||
| For queries without `FROM` clause. | ||
| Required to overload `toCursor` function in `ValuesOperator` and modify cursor parsing. | ||
| @Override | ||
| public Boolean visitValues(Values node, Object context) { | ||
| return canPaginate(node, context); | ||
| } | ||
| For queries with LIMIT clause: | ||
| Required to overload `toCursor` function in `LimitOperator` and modify cursor parsing. | ||
| @Override | ||
| public Boolean visitLimit(Limit node, Object context) { | ||
| return canPaginate(node, context); | ||
| } | ||
| For queries with ORDER BY clause: | ||
| Required to overload `toCursor` function in `SortOperator` and modify cursor parsing. | ||
| @Override | ||
| public Boolean visitSort(Sort node, Object context) { | ||
| return canPaginate(node, context); | ||
| } | ||
| For queries with WHERE clause: | ||
| Required to overload `toCursor` function in `FilterOperator` and modify cursor parsing. | ||
| @Override | ||
| public Boolean visitFilter(Filter node, Object context) { | ||
| return canPaginate(node, context); | ||
| } | ||
| */ | ||
Yury-Fridlyand marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
|
||
| @Override | ||
| public Boolean visitChildren(Node node, Object context) { | ||
| return Boolean.FALSE; | ||
| } | ||
|
|
||
| @Override | ||
| public Boolean visitProject(Project node, Object context) { | ||
| // Allow queries with 'SELECT *' only. Those restriction could be removed, but consider | ||
| // in-memory aggregation performed by window function (see WindowOperator). | ||
| // SELECT max(age) OVER (PARTITION BY city) ... | ||
| var projections = node.getProjectList(); | ||
| if (projections.size() != 1) { | ||
| return Boolean.FALSE; | ||
| } | ||
|
|
||
| if (!(projections.get(0) instanceof AllFields)) { | ||
| return Boolean.FALSE; | ||
| } | ||
|
|
||
| var children = node.getChild(); | ||
| if (children.size() != 1) { | ||
| return Boolean.FALSE; | ||
| } | ||
|
|
||
| return children.get(0).accept(this, context); | ||
| } | ||
| } | ||
This file contains hidden or 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
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.