Adding tests for boolean range query + fix range query to use term qu…

…ery inside Signed-off-by: Harsha Vamsi Kalluri <harshavamsi096@gmail.com>
opensearch-project · Apr 4, 2024 · 1914641 · 1914641
1 parent 47e5bb8
commit 1914641
Show file tree

Hide file tree

Showing 4 changed files with 223 additions and 36 deletions.
diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/search/340_doc_values_field.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/search/340_doc_values_field.yml
@@ -478,6 +478,63 @@
 
   - match: { hits.total: 2 }
 
+  - do:
+      search:
+        rest_total_hits_as_int: true
+        index: test-iodvq
+        body:
+          query:
+            range: {
+              boolean: {
+                gte: true
+              },
+            }
+
+  - match: { hits.total: 2 }
+
+  - do:
+      search:
+        rest_total_hits_as_int: true
+        index: test-iodvq
+        body:
+          query:
+            range: {
+              boolean: {
+                lte: true
+              },
+            }
+
+  - match: { hits.total: 3 }
+
+  - do:
+      search:
+        rest_total_hits_as_int: true
+        index: test-iodvq
+        body:
+          query:
+            range: {
+              boolean: {
+                lte: true,
+                gte: false
+              },
+            }
+
+  - match: { hits.total: 3 }
+
+  - do:
+      search:
+        rest_total_hits_as_int: true
+        index: test-iodvq
+        body:
+          query:
+            range: {
+              boolean: {
+                lte: false,
+                gte: true
+              },
+            }
+
+  - match: { hits.total: 0 }
 ---
 "search on fields with only index enabled":
   - do:
@@ -958,6 +1015,64 @@
             }
 
   - match: { hits.total: 2 }
+
+  - do:
+      search:
+        rest_total_hits_as_int: true
+        index: test-index
+        body:
+          query:
+            range: {
+              boolean: {
+                gte: true
+              },
+            }
+
+  - match: { hits.total: 2 }
+
+  - do:
+      search:
+        rest_total_hits_as_int: true
+        index: test-index
+        body:
+          query:
+            range: {
+              boolean: {
+                lte: true
+              },
+            }
+
+  - match: { hits.total: 3 }
+
+  - do:
+      search:
+        rest_total_hits_as_int: true
+        index: test-index
+        body:
+          query:
+            range: {
+              boolean: {
+                lte: true,
+                gte: false
+              },
+            }
+
+  - match: { hits.total: 3 }
+
+  - do:
+      search:
+        rest_total_hits_as_int: true
+        index: test-index
+        body:
+          query:
+            range: {
+              boolean: {
+                lte: false,
+                gte: true
+              },
+            }
+
+  - match: { hits.total: 0 }
 ---
 "search on fields with only doc_values enabled":
   - skip:
@@ -1429,3 +1544,61 @@
             }
 
   - match: { hits.total: 2 }
+
+  - do:
+      search:
+        rest_total_hits_as_int: true
+        index: test-doc-values
+        body:
+          query:
+            range: {
+              boolean: {
+                gte: true
+              },
+            }
+
+  - match: { hits.total: 2 }
+
+  - do:
+      search:
+        rest_total_hits_as_int: true
+        index: test-doc-values
+        body:
+          query:
+            range: {
+              boolean: {
+                lte: true
+              },
+            }
+
+  - match: { hits.total: 3 }
+
+  - do:
+      search:
+        rest_total_hits_as_int: true
+        index: test-doc-values
+        body:
+          query:
+            range: {
+              boolean: {
+                lte: true,
+                gte: false
+              },
+            }
+
+  - match: { hits.total: 3 }
+
+  - do:
+      search:
+        rest_total_hits_as_int: true
+        index: test-doc-values
+        body:
+          query:
+            range: {
+              boolean: {
+                lte: false,
+                gte: true
+              },
+            }
+
+  - match: { hits.total: 0 }
diff --git a/server/src/main/java/org/opensearch/index/mapper/BooleanFieldMapper.java b/server/src/main/java/org/opensearch/index/mapper/BooleanFieldMapper.java
@@ -39,12 +39,9 @@
 import org.apache.lucene.index.IndexOptions;
 import org.apache.lucene.index.Term;
 import org.apache.lucene.search.BoostQuery;
-import org.apache.lucene.search.IndexOrDocValuesQuery;
 import org.apache.lucene.search.MatchNoDocsQuery;
-import org.apache.lucene.search.MultiTermQuery;
 import org.apache.lucene.search.Query;
 import org.apache.lucene.search.TermQuery;
-import org.apache.lucene.search.TermRangeQuery;
 import org.apache.lucene.util.BytesRef;
 import org.opensearch.common.Booleans;
 import org.opensearch.common.Nullable;
@@ -310,41 +307,37 @@ public Query termsQuery(List<?> values, QueryShardContext context) {
         @Override
         public Query rangeQuery(Object lowerTerm, Object upperTerm, boolean includeLower, boolean includeUpper, QueryShardContext context) {
             failIfNotIndexedAndNoDocValues();
-            if (isSearchable() && hasDocValues()) {
-                Query query = new TermRangeQuery(
-                    name(),
-                    lowerTerm == null ? null : indexedValueForSearch(lowerTerm),
-                    upperTerm == null ? null : indexedValueForSearch(upperTerm),
-                    includeLower,
-                    includeUpper
-                );
-                Query dvQuery = new TermRangeQuery(
-                    name(),
-                    lowerTerm == null ? null : indexedValueForSearch(lowerTerm),
-                    upperTerm == null ? null : indexedValueForSearch(upperTerm),
-                    includeLower,
-                    includeUpper,
-                    MultiTermQuery.DOC_VALUES_REWRITE
-                );
-                return new IndexOrDocValuesQuery(query, dvQuery);
+            if (lowerTerm == null) {
+                lowerTerm = false;
+                includeLower = true;
+
             }
-            if (hasDocValues()) {
-                return new TermRangeQuery(
-                    name(),
-                    lowerTerm == null ? null : indexedValueForSearch(lowerTerm),
-                    upperTerm == null ? null : indexedValueForSearch(upperTerm),
-                    includeLower,
-                    includeUpper,
-                    MultiTermQuery.DOC_VALUES_REWRITE
-                );
+            if (upperTerm == null) {
+                upperTerm = true;
+                includeUpper = true;
+
             }
-            return new TermRangeQuery(
-                name(),
-                lowerTerm == null ? null : indexedValueForSearch(lowerTerm),
-                upperTerm == null ? null : indexedValueForSearch(upperTerm),
-                includeLower,
-                includeUpper
-            );
+
+            if (lowerTerm == upperTerm) {
+                if (!includeLower || !includeUpper) {
+                    return new MatchNoDocsQuery();
+                }
+                return termQuery(lowerTerm, context);
+            }
+
+            if ((boolean) lowerTerm) {
+                return new MatchNoDocsQuery();
+            }
+            if (!includeLower && !includeUpper) {
+                return new MatchNoDocsQuery();
+            } else if (!includeLower) {
+                return termQuery(true, context);
+            } else if (!includeUpper) {
+                return termQuery(false, context);
+            } else {
+                return this.existsQuery(context);
+            }
+
         }
     }
 

diff --git a/server/src/test/java/org/opensearch/index/mapper/BooleanFieldTypeTests.java b/server/src/test/java/org/opensearch/index/mapper/BooleanFieldTypeTests.java
@@ -35,6 +35,7 @@
 import org.apache.lucene.index.Term;
 import org.apache.lucene.search.BoostQuery;
 import org.apache.lucene.search.DocValuesFieldExistsQuery;
+import org.apache.lucene.search.MatchNoDocsQuery;
 import org.apache.lucene.search.TermQuery;
 import org.apache.lucene.util.BytesRef;
 
@@ -105,6 +106,25 @@ public void testTermsQuery() {
         assertEquals("Cannot search on field [field] since it is both not indexed, and does not have doc_values enabled.", e.getMessage());
     }
 
+    public void testRangeQuery() {
+        BooleanFieldMapper.BooleanFieldType ft = new BooleanFieldMapper.BooleanFieldType("field");
+        assertEquals(new DocValuesFieldExistsQuery("field"), ft.rangeQuery(false, true, true, true, null));
+
+        assertEquals(new TermQuery(new Term("field", "T")), ft.rangeQuery(false, true, false, true, null));
+
+        assertEquals(new TermQuery(new Term("field", "F")), ft.rangeQuery(false, true, true, false, null));
+
+        assertEquals(new MatchNoDocsQuery(), ft.rangeQuery(false, true, false, false, null));
+
+        assertEquals(new MatchNoDocsQuery(), ft.rangeQuery(false, true, false, false, null));
+
+        assertEquals(new TermQuery(new Term("field", "F")), ft.rangeQuery(false, false, true, true, null));
+
+        assertEquals(new TermQuery(new Term("field", "F")), ft.rangeQuery(null, false, true, true, null));
+
+        assertEquals(new DocValuesFieldExistsQuery("field"), ft.rangeQuery(false, null, true, true, null));
+    }
+
     public void testFetchSourceValue() throws IOException {
 
         MappedFieldType fieldType = new BooleanFieldMapper.BooleanFieldType("field");

diff --git a/server/src/test/java/org/opensearch/index/query/TermsQueryBuilderTests.java b/server/src/test/java/org/opensearch/index/query/TermsQueryBuilderTests.java
@@ -41,6 +41,7 @@
 import org.apache.lucene.search.PointInSetQuery;
 import org.apache.lucene.search.Query;
 import org.apache.lucene.search.TermInSetQuery;
+import org.apache.lucene.search.TermQuery;
 import org.apache.lucene.util.BytesRef;
 import org.opensearch.OpenSearchException;
 import org.opensearch.action.get.GetRequest;