[Backport 2.19-dev] [BugFix] Not between should use range query (#5017)

* [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>

* Fix IT

Signed-off-by: Heng Qian <qianheng@amazon.com>

* Fix unlimited symbol

Signed-off-by: Heng Qian <qianheng@amazon.com>

* Fix unlimited symbol

Signed-off-by: Heng Qian <qianheng@amazon.com>

---------

Signed-off-by: Heng Qian <qianheng@amazon.com>
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: Heng Qian <qianheng@amazon.com>

Author: 3 people

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"));
+  }
 }

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) {

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)])

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;