Skip to content

Commit eeae4e9

Browse files
committed
Accumulate offsets when pushing down limit
Signed-off-by: Yuanchun Shen <yuanchu@amazon.com>
1 parent fecea40 commit eeae4e9

10 files changed

Lines changed: 49 additions & 8 deletions

File tree

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

Lines changed: 22 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -142,8 +142,8 @@ public void testLimitWithFilterPushdownExplain() throws Exception {
142142
public void testMultipleLimitExplain() throws Exception {
143143
String expected5Then10 =
144144
isCalciteEnabled()
145-
? loadFromFile("expectedOutput/calcite/explain_limit_5_10.json")
146-
: loadFromFile("expectedOutput/ppl/explain_limit_5_10.json");
145+
? loadFromFile("expectedOutput/calcite/explain_limit_5_10_push.json")
146+
: loadFromFile("expectedOutput/ppl/explain_limit_5_10_push.json");
147147
assertJsonEqualsIgnoreRelId(
148148
expected5Then10,
149149
explainQueryToString(
@@ -154,8 +154,8 @@ public void testMultipleLimitExplain() throws Exception {
154154

155155
String expected10Then5 =
156156
isCalciteEnabled()
157-
? loadFromFile("expectedOutput/calcite/explain_limit_10_5.json")
158-
: loadFromFile("expectedOutput/ppl/explain_limit_10_5.json");
157+
? loadFromFile("expectedOutput/calcite/explain_limit_10_5_push.json")
158+
: loadFromFile("expectedOutput/ppl/explain_limit_10_5_push.json");
159159
assertJsonEqualsIgnoreRelId(
160160
expected10Then5,
161161
explainQueryToString(
@@ -167,8 +167,8 @@ public void testMultipleLimitExplain() throws Exception {
167167
// The second limit should not be pushed down for limit-filter-limit queries
168168
String expected10ThenFilterThen5 =
169169
isCalciteEnabled()
170-
? loadFromFile("expectedOutput/calcite/explain_limit_10_filter_5.json")
171-
: loadFromFile("expectedOutput/ppl/explain_limit_10_filter_5.json");
170+
? loadFromFile("expectedOutput/calcite/explain_limit_10_filter_5_push.json")
171+
: loadFromFile("expectedOutput/ppl/explain_limit_10_filter_5_push.json");
172172
assertJsonEqualsIgnoreRelId(
173173
expected10ThenFilterThen5,
174174
explainQueryToString(
@@ -179,6 +179,22 @@ public void testMultipleLimitExplain() throws Exception {
179179
+ "| fields age"));
180180
}
181181

182+
@Test
183+
public void testLimitWithMultipleOffsetPushdownExplain() throws Exception {
184+
String expected =
185+
isCalciteEnabled()
186+
? loadFromFile("expectedOutput/calcite/explain_limit_offsets_push.json")
187+
: loadFromFile("expectedOutput/ppl/explain_limit_offsets_push.json");
188+
189+
assertJsonEqualsIgnoreRelId(
190+
expected,
191+
explainQueryToString(
192+
"source=opensearch-sql_test_index_account"
193+
+ "| head 10 from 1 "
194+
+ "| head 5 from 2 "
195+
+ "| fields age"));
196+
}
197+
182198
@Test
183199
public void testFillNullPushDownExplain() throws Exception {
184200
String expected = loadFromFile("expectedOutput/ppl/explain_fillnull_push.json");

integ-test/src/test/resources/expectedOutput/calcite/explain_limit_10_5.json renamed to integ-test/src/test/resources/expectedOutput/calcite/explain_limit_10_5_push.json

File renamed without changes.

integ-test/src/test/resources/expectedOutput/calcite/explain_limit_10_filter_5.json renamed to integ-test/src/test/resources/expectedOutput/calcite/explain_limit_10_filter_5_push.json

File renamed without changes.

integ-test/src/test/resources/expectedOutput/calcite/explain_limit_5_10.json renamed to integ-test/src/test/resources/expectedOutput/calcite/explain_limit_5_10_push.json

File renamed without changes.
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=[5])\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->5, PROJECT->[age]], OpenSearchRequestBuilder(sourceBuilder={\"from\":3,\"size\":5,\"timeout\":\"1m\",\"_source\":{\"includes\":[\"age\"],\"excludes\":[]}}, requestedTotalSize=5, pageSize=null, startFrom=3)])\n"
5+
}
6+
}

integ-test/src/test/resources/expectedOutput/ppl/explain_limit_10_5.json renamed to integ-test/src/test/resources/expectedOutput/ppl/explain_limit_10_5_push.json

File renamed without changes.

integ-test/src/test/resources/expectedOutput/ppl/explain_limit_10_filter_5.json renamed to integ-test/src/test/resources/expectedOutput/ppl/explain_limit_10_filter_5_push.json

File renamed without changes.

integ-test/src/test/resources/expectedOutput/ppl/explain_limit_5_10.json renamed to integ-test/src/test/resources/expectedOutput/ppl/explain_limit_5_10_push.json

File renamed without changes.
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\":5,\"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 & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -202,8 +202,10 @@ public void pushDownLimit(Integer limit, Integer offset) {
202202
// This also ensures that the limit won't exceed the initial default value. (set to
203203
// Settings.Key.QUERY_SIZE_LIMIT in OpenSearchIndex)
204204
requestedTotalSize = Math.min(limit, requestedTotalSize);
205-
startFrom = offset;
206-
sourceBuilder.from(offset).size(requestedTotalSize);
205+
// If there are multiple offset, we aggregate the offset
206+
// E.g. for `head 10 from 1 | head 5 from 2` equals to `head 5 from 3`
207+
startFrom += offset;
208+
sourceBuilder.from(startFrom).size(requestedTotalSize);
207209
}
208210

209211
public void pushDownTrackedScore(boolean trackScores) {

0 commit comments

Comments
 (0)