diff --git a/integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteExplainIT.java b/integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteExplainIT.java index 395ac5888d..9616dcbd7b 100644 --- a/integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteExplainIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteExplainIT.java @@ -2307,4 +2307,15 @@ public void testNestedAggExplainWhenPushdownNotApplied() throws Exception { + " as avg_age by name")); verifyErrorMessageContains(e, "Cannot execute nested aggregation on"); } + + @Test + public void testNotBetweenPushDownExplain() throws Exception { + // test for issue https://github.com/opensearch-project/sql/issues/4903 + enabledOnlyWhenPushdownIsEnabled(); + String expected = loadExpectedPlan("explain_not_between_push.yaml"); + assertYamlEqualsIgnoreId( + expected, + explainQueryYaml( + "source=opensearch-sql_test_index_bank | where age not between 30 and 39")); + } } diff --git a/integ-test/src/test/java/org/opensearch/sql/util/MatcherUtils.java b/integ-test/src/test/java/org/opensearch/sql/util/MatcherUtils.java index 063c138bcb..3fc7634c57 100644 --- a/integ-test/src/test/java/org/opensearch/sql/util/MatcherUtils.java +++ b/integ-test/src/test/java/org/opensearch/sql/util/MatcherUtils.java @@ -459,7 +459,9 @@ private static String cleanUpYaml(String s) { .replaceAll("pitId=[^,]+,", "pitId=*,") .replaceAll("\\$t?\\d+", "\\$FIELD_INDEX") .replaceAll(" needClean=true,", "") - .replaceAll(" searchDone=false,", ""); + .replaceAll(" searchDone=false,", "") + .replaceAll("\\([^)]*\\.\\.\\d+\\)", "?") + .replaceAll("\\(\\d+\\.\\.[^)]*\\)", "?"); } private static String jsonToYaml(String json) { diff --git a/integ-test/src/test/resources/expectedOutput/calcite/explain_not_between_push.yaml b/integ-test/src/test/resources/expectedOutput/calcite/explain_not_between_push.yaml new file mode 100644 index 0000000000..beeec6867f --- /dev/null +++ b/integ-test/src/test/resources/expectedOutput/calcite/explain_not_between_push.yaml @@ -0,0 +1,8 @@ +calcite: + logical: | + LogicalSystemLimit(fetch=[10000], type=[QUERY_SIZE_LIMIT]) + 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]) + LogicalFilter(condition=[SEARCH($10, Sarg[(-∞..30), (39..+∞)])]) + CalciteLogicalIndexScan(table=[[OpenSearch, opensearch-sql_test_index_bank]]) + physical: | + 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)]) diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/request/PredicateAnalyzer.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/request/PredicateAnalyzer.java index 812cdbfd72..8999b2c20a 100644 --- a/opensearch/src/main/java/org/opensearch/sql/opensearch/request/PredicateAnalyzer.java +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/request/PredicateAnalyzer.java @@ -1438,14 +1438,18 @@ public QueryExpression between(Range range, boolean isTimeStamp) { Object upperBound = range.hasUpperBound() ? convertEndpointValue(range.upperEndpoint(), isTimeStamp) : null; RangeQueryBuilder rangeQueryBuilder = rangeQuery(getFieldReference()); - rangeQueryBuilder = - range.lowerBoundType() == BoundType.CLOSED - ? rangeQueryBuilder.gte(lowerBound) - : rangeQueryBuilder.gt(lowerBound); - rangeQueryBuilder = - range.upperBoundType() == BoundType.CLOSED - ? rangeQueryBuilder.lte(upperBound) - : rangeQueryBuilder.lt(upperBound); + if (lowerBound != null) { + rangeQueryBuilder = + range.lowerBoundType() == BoundType.CLOSED + ? rangeQueryBuilder.gte(lowerBound) + : rangeQueryBuilder.gt(lowerBound); + } + if (upperBound != null) { + rangeQueryBuilder = + range.upperBoundType() == BoundType.CLOSED + ? rangeQueryBuilder.lte(upperBound) + : rangeQueryBuilder.lt(upperBound); + } if (isTimeStamp) rangeQueryBuilder.format("date_time"); builder = rangeQueryBuilder; return this;