Skip to content

Commit 79c4303

Browse files
committed
Disable limit-then-filter pushdown without Calcite
Signed-off-by: Yuanchun Shen <yuanchu@amazon.com>
1 parent ed2d080 commit 79c4303

4 files changed

Lines changed: 38 additions & 10 deletions

File tree

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

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -111,9 +111,6 @@ public void testLimitPushDownExplain() throws Exception {
111111

112112
@Test
113113
public void testLimitWithFilterPushdownExplain() throws Exception {
114-
// TODO: Fix limit-then-filter pushdown without Calcite
115-
// Currently both limit-then-filter and filter-then-limit have the
116-
// limit-then-filter effect
117114
String expectedFilterThenLimit =
118115
isCalciteEnabled()
119116
? loadFromFile("expectedOutput/calcite/explain_filter_then_limit_push.json")
@@ -126,6 +123,8 @@ public void testLimitWithFilterPushdownExplain() throws Exception {
126123
+ "| head 5 "
127124
+ "| fields age"));
128125

126+
// The filter in limit-then-filter queries should not be pushed since the current DSL will
127+
// execute it as filter-then-limit
129128
String expectedLimitThenFilter =
130129
isCalciteEnabled()
131130
? loadFromFile("expectedOutput/calcite/explain_limit_then_filter_push.json")
@@ -165,7 +164,7 @@ public void testMultipleLimitExplain() throws Exception {
165164
+ "| head 5 "
166165
+ "| fields age"));
167166

168-
// TODO: Fix limit-filter-limit pushdown without Calcite
167+
// The second limit should not be pushed down for limit-filter-limit queries
169168
String expected10ThenFilterThen5 =
170169
isCalciteEnabled()
171170
? loadFromFile("expectedOutput/calcite/explain_limit_10_filter_5.json")

integ-test/src/test/resources/expectedOutput/ppl/explain_limit_10_filter_5.json

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,28 @@
66
},
77
"children": [
88
{
9-
"name": "OpenSearchIndexScan",
9+
"name": "LimitOperator",
1010
"description": {
11-
"request": "OpenSearchQueryRequest(indexName=opensearch-sql_test_index_account, sourceBuilder={\"from\":0,\"size\":5,\"timeout\":\"1m\",\"query\":{\"range\":{\"age\":{\"from\":30,\"to\":null,\"include_lower\":false,\"include_upper\":true,\"boost\":1.0}}},\"_source\":{\"includes\":[\"age\"],\"excludes\":[]},\"sort\":[{\"_doc\":{\"order\":\"asc\"}}]}, needClean=true, searchDone=false, pitId=null, cursorKeepAlive=null, searchAfter=null, searchResponse=null)"
11+
"limit": 5,
12+
"offset": 0
1213
},
13-
"children": []
14+
"children": [
15+
{
16+
"name": "FilterOperator",
17+
"description": {
18+
"conditions": ">(age, 30)"
19+
},
20+
"children": [
21+
{
22+
"name": "OpenSearchIndexScan",
23+
"description": {
24+
"request": "OpenSearchQueryRequest(indexName=opensearch-sql_test_index_account, sourceBuilder={\"from\":0,\"size\":10,\"timeout\":\"1m\"}, needClean=true, searchDone=false, pitId=null, cursorKeepAlive=null, searchAfter=null, searchResponse=null)"
25+
},
26+
"children": []
27+
}
28+
]
29+
}
30+
]
1431
}
1532
]
1633
}

integ-test/src/test/resources/expectedOutput/ppl/explain_limit_then_filter_push.json

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,19 @@
66
},
77
"children": [
88
{
9-
"name": "OpenSearchIndexScan",
9+
"name": "FilterOperator",
1010
"description": {
11-
"request": "OpenSearchQueryRequest(indexName=opensearch-sql_test_index_account, sourceBuilder={\"from\":0,\"size\":5,\"timeout\":\"1m\",\"query\":{\"range\":{\"age\":{\"from\":30,\"to\":null,\"include_lower\":false,\"include_upper\":true,\"boost\":1.0}}},\"_source\":{\"includes\":[\"age\"],\"excludes\":[]},\"sort\":[{\"_doc\":{\"order\":\"asc\"}}]}, needClean=true, searchDone=false, pitId=null, cursorKeepAlive=null, searchAfter=null, searchResponse=null)"
11+
"conditions": ">(age, 30)"
1212
},
13-
"children": []
13+
"children": [
14+
{
15+
"name": "OpenSearchIndexScan",
16+
"description": {
17+
"request": "OpenSearchQueryRequest(indexName=opensearch-sql_test_index_account, sourceBuilder={\"from\":0,\"size\":5,\"timeout\":\"1m\"}, needClean=true, searchDone=false, pitId=null, cursorKeepAlive=null, searchAfter=null, searchResponse=null)"
18+
},
19+
"children": []
20+
}
21+
]
1422
}
1523
]
1624
}

opensearch/src/main/java/org/opensearch/sql/opensearch/storage/scan/OpenSearchIndexScanBuilder.java

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,10 @@ public TableScanOperator build() {
6060

6161
@Override
6262
public boolean pushDownFilter(LogicalFilter filter) {
63+
// Current DSL can not handle limit-then-filter correctly
64+
if (isLimitPushedDown) {
65+
return false;
66+
}
6367
return delegate.pushDownFilter(filter);
6468
}
6569

0 commit comments

Comments
 (0)