Backend API support for pagination#4948
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the ✨ Finishing touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Can you add some tests in CalciteExplainIT?
Is this query
POST /_plugins/_ppl
{
"query": "source=logs | where status='error' | sort timestamp",
"pageSize": 100,
"offset": 1000
}
equals to
POST /_plugins/_ppl
{
"query": "source=logs | where status='error' | sort timestamp | head 100 from 1000"
}
?
| * <p>Unlike regular LogicalSort which may be merged with other sorts by Calcite optimizer, this | ||
| * class ensures pagination is applied as a final post-processing step on the query result. | ||
| */ | ||
| public class LogicalPaginationLimit extends Sort { |
There was a problem hiding this comment.
Please reuse LogicalSystemLimit with type=pagination
| RelNode optimized = optimize(relNode, context); | ||
| RelNode calcitePlan = convertToCalcitePlan(optimized); | ||
| executionEngine.execute(calcitePlan, context, listener); | ||
| AccessController.doPrivileged( |
There was a problem hiding this comment.
why add the doPrivileged back?
There was a problem hiding this comment.
Updated to remove doPrivileged
Added test cases in CalciteExplainIT |
ahkcs
force-pushed
the
feat/pagination_support
branch
from
39352de to
9a809b7
Compare
Signed-off-by: Kai Huang <ahkcs@amazon.com> # Conflicts: # core/src/main/java/org/opensearch/sql/executor/QueryService.java
Signed-off-by: Kai Huang <ahkcs@amazon.com>
Signed-off-by: Kai Huang <ahkcs@amazon.com> # Conflicts: # integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteExplainIT.java
ahkcs
force-pushed
the
feat/pagination_support
branch
from
69f3cdb to
ad1c15b
Compare
LantaoJin
left a comment
LantaoJin
left a comment
There was a problem hiding this comment.
Is this PR ready for review now?
| */ | ||
| @Test | ||
| public void testExplainPaginationApi() throws IOException { | ||
| enabledOnlyWhenPushdownIsEnabled(); |
There was a problem hiding this comment.
Remove this line since the PPL pagination feature should work when the DSL pushdown disabled. We should check the calcite plan without pushdown.
| */ | ||
| @Test | ||
| public void testExplainPaginationApiWithHeadFrom() throws IOException { | ||
| enabledOnlyWhenPushdownIsEnabled(); |
Not yet, it's currently a POC PR, still need to finalize design |
2 participants
Description
This PR adds offset-based pagination support for PPL queries executed through the Calcite engine. Users can now paginate through large result sets by specifying pageSize and offset parameters in the request body
API Design
Request Format
Parameters
Example: Paginating Through Results
Check List
--signoffor-s.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.