Skip to content

Commit 998f552

Browse files
committed
Fix multiple limit pushdown with shirinking caused by offset
Signed-off-by: Yuanchun Shen <yuanchu@amazon.com>
1 parent 2100019 commit 998f552

4 files changed

Lines changed: 39 additions & 1 deletion

File tree

integ-test/src/test/java/org/opensearch/sql/ppl/ExplainIT.java

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -164,6 +164,18 @@ public void testMultipleLimitExplain() throws Exception {
164164
+ "| head 5 "
165165
+ "| fields age"));
166166

167+
String expected10from1then10from2 =
168+
isCalciteEnabled()
169+
? loadFromFile("expectedOutput/calcite/explain_limit_10from1_10from2_push.json")
170+
: loadFromFile("expectedOutput/ppl/explain_limit_10from1_10from2_push.json");
171+
assertJsonEqualsIgnoreId(
172+
expected10from1then10from2,
173+
explainQueryToString(
174+
"source=opensearch-sql_test_index_account"
175+
+ "| head 10 from 1 "
176+
+ "| head 10 from 2 "
177+
+ "| fields age"));
178+
167179
// The second limit should not be pushed down for limit-filter-limit queries
168180
String expected10ThenFilterThen5 =
169181
isCalciteEnabled()
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
{
2+
"calcite": {
3+
"logical": "LogicalProject(age=[$8])\n LogicalSort(offset=[2], fetch=[10])\n LogicalSort(offset=[1], fetch=[10])\n CalciteLogicalIndexScan(table=[[OpenSearch, opensearch-sql_test_index_account]])\n",
4+
"physical": "CalciteEnumerableIndexScan(table=[[OpenSearch, opensearch-sql_test_index_account]], PushDownContext=[[LIMIT->10, LIMIT->10, PROJECT->[age]], OpenSearchRequestBuilder(sourceBuilder={\"from\":3,\"size\":8,\"timeout\":\"1m\",\"_source\":{\"includes\":[\"age\"],\"excludes\":[]}}, requestedTotalSize=8, pageSize=null, startFrom=3)])\n"
5+
}
6+
}
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
{
2+
"root": {
3+
"name": "ProjectOperator",
4+
"description": {
5+
"fields": "[age]"
6+
},
7+
"children": [
8+
{
9+
"name": "OpenSearchIndexScan",
10+
"description": {
11+
"request": "OpenSearchQueryRequest(indexName=opensearch-sql_test_index_account, sourceBuilder={\"from\":3,\"size\":8,\"timeout\":\"1m\",\"_source\":{\"includes\":[\"age\"],\"excludes\":[]}}, needClean=true, searchDone=false, pitId=null, cursorKeepAlive=null, searchAfter=null, searchResponse=null)"
12+
},
13+
"children": []
14+
}
15+
]
16+
}
17+
}

opensearch/src/main/java/org/opensearch/sql/opensearch/request/OpenSearchRequestBuilder.java

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -215,7 +215,10 @@ public void pushDownLimit(Integer limit, Integer offset) {
215215
// E.g. for `source=t | head 10 | head 5`, we take 5
216216
// This also ensures that the limit won't exceed the initial default value. (set to
217217
// Settings.Key.QUERY_SIZE_LIMIT in OpenSearchIndex)
218-
requestedTotalSize = Math.min(limit, requestedTotalSize);
218+
// Besides, there may be cases when the existing requestedTotalSize does not satisfy the
219+
// new limit and offset. E.g. for `head 11 | head 10 from 2`, the new requested total size
220+
// is 9. We need to adjust it accordingly.
221+
requestedTotalSize = Math.min(limit, requestedTotalSize - offset);
219222
// If there are multiple offset, we aggregate the offset
220223
// E.g. for `head 10 from 1 | head 5 from 2` equals to `head 5 from 3`
221224
startFrom += offset;

0 commit comments

Comments
 (0)