Skip to content

Commit e4290c3

Browse files
[BugFix] Not between should use range query (#5016)
Signed-off-by: Heng Qian <qianheng@amazon.com> (cherry picked from commit b39d803) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
1 parent 7f7495e commit e4290c3

3 files changed

Lines changed: 31 additions & 8 deletions

File tree

integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteExplainIT.java

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2307,4 +2307,15 @@ public void testNestedAggExplainWhenPushdownNotApplied() throws Exception {
23072307
+ " as avg_age by name"));
23082308
verifyErrorMessageContains(e, "Cannot execute nested aggregation on");
23092309
}
2310+
2311+
@Test
2312+
public void testNotBetweenPushDownExplain() throws Exception {
2313+
// test for issue https://github.com/opensearch-project/sql/issues/4903
2314+
enabledOnlyWhenPushdownIsEnabled();
2315+
String expected = loadExpectedPlan("explain_not_between_push.yaml");
2316+
assertYamlEqualsIgnoreId(
2317+
expected,
2318+
explainQueryYaml(
2319+
"source=opensearch-sql_test_index_bank | where age not between 30 and 39"));
2320+
}
23102321
}
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
calcite:
2+
logical: |
3+
LogicalSystemLimit(fetch=[10000], type=[QUERY_SIZE_LIMIT])
4+
LogicalProject(account_number=[$0], firstname=[$1], address=[$2], birthdate=[$3], gender=[$4], city=[$5], lastname=[$6], balance=[$7], employer=[$8], state=[$9], age=[$10], email=[$11], male=[$12])
5+
LogicalFilter(condition=[SEARCH($10, Sarg[(-∞..30), (39..+∞)])])
6+
CalciteLogicalIndexScan(table=[[OpenSearch, opensearch-sql_test_index_bank]])
7+
physical: |
8+
CalciteEnumerableIndexScan(table=[[OpenSearch, opensearch-sql_test_index_bank]], PushDownContext=[[PROJECT->[account_number, firstname, address, birthdate, gender, city, lastname, balance, employer, state, age, email, male], FILTER->SEARCH($10, Sarg[(-∞..30), (39..+∞)]), LIMIT->10000], OpenSearchRequestBuilder(sourceBuilder={"from":0,"size":10000,"timeout":"1m","query":{"bool":{"should":[{"range":{"age":{"from":null,"to":30.0,"include_lower":true,"include_upper":false,"boost":1.0}}},{"range":{"age":{"from":39.0,"to":null,"include_lower":false,"include_upper":true,"boost":1.0}}}],"adjust_pure_negative":true,"boost":1.0}},"_source":{"includes":["account_number","firstname","address","birthdate","gender","city","lastname","balance","employer","state","age","email","male"],"excludes":[]}}, requestedTotalSize=10000, pageSize=null, startFrom=0)])

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

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1438,14 +1438,18 @@ public QueryExpression between(Range<?> range, boolean isTimeStamp) {
14381438
Object upperBound =
14391439
range.hasUpperBound() ? convertEndpointValue(range.upperEndpoint(), isTimeStamp) : null;
14401440
RangeQueryBuilder rangeQueryBuilder = rangeQuery(getFieldReference());
1441-
rangeQueryBuilder =
1442-
range.lowerBoundType() == BoundType.CLOSED
1443-
? rangeQueryBuilder.gte(lowerBound)
1444-
: rangeQueryBuilder.gt(lowerBound);
1445-
rangeQueryBuilder =
1446-
range.upperBoundType() == BoundType.CLOSED
1447-
? rangeQueryBuilder.lte(upperBound)
1448-
: rangeQueryBuilder.lt(upperBound);
1441+
if (lowerBound != null) {
1442+
rangeQueryBuilder =
1443+
range.lowerBoundType() == BoundType.CLOSED
1444+
? rangeQueryBuilder.gte(lowerBound)
1445+
: rangeQueryBuilder.gt(lowerBound);
1446+
}
1447+
if (upperBound != null) {
1448+
rangeQueryBuilder =
1449+
range.upperBoundType() == BoundType.CLOSED
1450+
? rangeQueryBuilder.lte(upperBound)
1451+
: rangeQueryBuilder.lt(upperBound);
1452+
}
14491453
if (isTimeStamp) rangeQueryBuilder.format("date_time");
14501454
builder = rangeQueryBuilder;
14511455
return this;

0 commit comments

Comments
 (0)