Skip to content

Commit 3d0dc26

Browse files
committed
Fix ITs
Signed-off-by: Kai Huang <ahkcs@amazon.com>
1 parent 300c320 commit 3d0dc26

3 files changed

Lines changed: 37 additions & 12 deletions

File tree

core/src/main/java/org/opensearch/sql/executor/QueryService.java

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
import org.opensearch.sql.analysis.AnalysisContext;
3434
import org.opensearch.sql.analysis.Analyzer;
3535
import org.opensearch.sql.ast.statement.Explain;
36+
import org.opensearch.sql.ast.tree.Paginate;
3637
import org.opensearch.sql.ast.tree.UnresolvedPlan;
3738
import org.opensearch.sql.calcite.CalcitePlanContext;
3839
import org.opensearch.sql.calcite.CalciteRelNodeVisitor;
@@ -95,7 +96,12 @@ public void execute(
9596
if (shouldUseCalcite(queryType)) {
9697
executeWithCalcite(plan, queryType, listener, pageSize, offset);
9798
} else {
98-
executeWithLegacy(plan, queryType, listener, Optional.empty());
99+
// For legacy path (SQL), wrap with Paginate when pagination is enabled
100+
if (pageSize > 0) {
101+
executeWithLegacy(new Paginate(pageSize, plan), queryType, listener, Optional.empty());
102+
} else {
103+
executeWithLegacy(plan, queryType, listener, Optional.empty());
104+
}
99105
}
100106
}
101107

core/src/main/java/org/opensearch/sql/executor/execution/QueryPlanFactory.java

Lines changed: 26 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
import org.opensearch.sql.ast.tree.FetchCursor;
1818
import org.opensearch.sql.ast.tree.UnresolvedPlan;
1919
import org.opensearch.sql.common.response.ResponseListener;
20+
import org.opensearch.sql.exception.UnsupportedCursorRequestException;
2021
import org.opensearch.sql.executor.ExecutionEngine;
2122
import org.opensearch.sql.executor.QueryId;
2223
import org.opensearch.sql.executor.QueryService;
@@ -106,15 +107,31 @@ public AbstractPlan visitQuery(
106107
context) {
107108
requireNonNull(context.getLeft(), "[BUG] query listener must be not null");
108109
if (node.getFetchSize() > 0) {
109-
// Pagination enabled - use pageSize and offset
110-
return new QueryPlan(
111-
QueryId.queryId(),
112-
node.getQueryType(),
113-
node.getPlan(),
114-
node.getFetchSize(),
115-
node.getPaginationOffset(),
116-
queryService,
117-
context.getLeft());
110+
if (node.getQueryType() == QueryType.PPL) {
111+
// PPL pagination - use pageSize and offset (Calcite path)
112+
return new QueryPlan(
113+
QueryId.queryId(),
114+
node.getQueryType(),
115+
node.getPlan(),
116+
node.getFetchSize(),
117+
node.getPaginationOffset(),
118+
queryService,
119+
context.getLeft());
120+
} else {
121+
// SQL pagination - use legacy cursor-based pagination
122+
if (canConvertToCursor(node.getPlan())) {
123+
return new QueryPlan(
124+
QueryId.queryId(),
125+
node.getQueryType(),
126+
node.getPlan(),
127+
node.getFetchSize(),
128+
queryService,
129+
context.getLeft());
130+
} else {
131+
// This should be picked up by the legacy engine.
132+
throw new UnsupportedCursorRequestException();
133+
}
134+
}
118135
} else {
119136
return new QueryPlan(
120137
QueryId.queryId(), node.getQueryType(), node.getPlan(), queryService, context.getLeft());

core/src/test/java/org/opensearch/sql/executor/execution/QueryPlanFactoryTest.java

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -99,15 +99,17 @@ public void no_consumer_response_channel() {
9999
@Test
100100
public void create_query_with_fetch_size_should_create_query_plan() {
101101
factory = new QueryPlanFactory(queryService);
102-
Statement query = new Query(plan, 10, queryType);
102+
// Use PPL query type for new pagination feature
103+
Statement query = new Query(plan, 10, QueryType.PPL);
103104
AbstractPlan queryExecution = factory.create(query, queryListener, explainListener);
104105
assertTrue(queryExecution instanceof QueryPlan);
105106
}
106107

107108
@Test
108109
public void create_query_with_fetch_size_and_offset_should_create_query_plan() {
109110
factory = new QueryPlanFactory(queryService);
110-
Statement query = new Query(plan, 10, queryType, 50);
111+
// Use PPL query type for new pagination with offset feature
112+
Statement query = new Query(plan, 10, QueryType.PPL, 50);
111113
AbstractPlan queryExecution = factory.create(query, queryListener, explainListener);
112114
assertTrue(queryExecution instanceof QueryPlan);
113115
}

0 commit comments

Comments
 (0)