Updating termsQuery logic

Signed-off-by: Harsha Vamsi Kalluri <[email protected]>
opensearch-project · Jan 19, 2024 · 7ee0f4f · 7ee0f4f
1 parent 143ba67
commit 7ee0f4f
Show file tree

Hide file tree

Showing 5 changed files with 31 additions and 30 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
@@ -1299,4 +1299,4 @@ setup:
               },
             }
 
-  - match: { hits.total: 2 }
+  - match: { hits.total: 2 }
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,7 +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.FieldExistsQuery;
 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.TermInSetQuery;
@@ -289,23 +291,16 @@ public Query termQuery(Object value, QueryShardContext context) {
         @Override
         public Query termsQuery(List<?> values, QueryShardContext context) {
             failIfNotIndexedAndNoDocValues();
-            if (isSearchable() && hasDocValues()) {
-                Query query = new TermInSetQuery(name(), values.stream().map(this::indexedValueForSearch).toArray(BytesRef[]::new));
-                Query dvQuery = new TermInSetQuery(
-                    MultiTermQuery.DOC_VALUES_REWRITE,
-                    name(),
-                    values.stream().map(this::indexedValueForSearch).toArray(BytesRef[]::new)
-                );
-                return new IndexOrDocValuesQuery(query, dvQuery);
+            // if we do not get either True or False, we return no docs
+            if (!(values.contains(Values.TRUE)) || !(values.contains(Values.FALSE))){
+                return new MatchNoDocsQuery("Values do not contain True or False");
             }
-            if (hasDocValues()) {
-                return new TermInSetQuery(
-                    MultiTermQuery.DOC_VALUES_REWRITE,
-                    name(),
-                    values.stream().map(this::indexedValueForSearch).toArray(BytesRef[]::new)
-                );
+            // if we have either True or False, we delegate to termQuery
+            if((values.contains(Values.TRUE) && !(values.contains(Values.FALSE))) || (values.contains(Values.FALSE) && !values.contains(Values.TRUE))){
+                return termQuery(values.contains(Values.TRUE)? Values.TRUE : Values.FALSE, context);
             }
-            return new TermInSetQuery(name(), values.stream().map(this::indexedValueForSearch).toArray(BytesRef[]::new));
+            // if we have both True and False, we acknowledge that the field exists with a value
+            return new FieldExistsQuery(name());
         }
 
         @Override

diff --git a/server/src/main/java/org/opensearch/index/mapper/DateFieldMapper.java b/server/src/main/java/org/opensearch/index/mapper/DateFieldMapper.java
@@ -463,15 +463,15 @@ public Query rangeQuery(
             }
             DateMathParser parser = forcedDateParser == null ? dateMathParser : forcedDateParser;
             return dateRangeQuery(lowerTerm, upperTerm, includeLower, includeUpper, timeZone, parser, context, resolution, (l, u) -> {
-                if(isSearchable() && hasDocValues()){
-                Query query = LongPoint.newRangeQuery(name(), l, u);
-                Query dvQuery = SortedNumericDocValuesField.newSlowRangeQuery(name(), l, u);
-                query = new IndexOrDocValuesQuery(query, dvQuery);
+                if (isSearchable() && hasDocValues()) {
+                    Query query = LongPoint.newRangeQuery(name(), l, u);
+                    Query dvQuery = SortedNumericDocValuesField.newSlowRangeQuery(name(), l, u);
+                    query = new IndexOrDocValuesQuery(query, dvQuery);
 
-                if (context.indexSortedOnField(name())) {
-                    query = new IndexSortSortedNumericDocValuesRangeQuery(name(), l, u, query);
-                }
-                return query;
+                    if (context.indexSortedOnField(name())) {
+                        query = new IndexSortSortedNumericDocValuesRangeQuery(name(), l, u, query);
+                    }
+                    return query;
                 }
                 if (hasDocValues()) {
                     Query query = SortedNumericDocValuesField.newSlowRangeQuery(name(), l, u);

diff --git a/server/src/test/java/org/opensearch/index/mapper/BooleanFieldMapperTests.java b/server/src/test/java/org/opensearch/index/mapper/BooleanFieldMapperTests.java
@@ -33,12 +33,10 @@
 package org.opensearch.index.mapper;
 
 import org.apache.lucene.document.SortedNumericDocValuesField;
-import org.apache.lucene.document.SortedSetDocValuesField;
 import org.apache.lucene.index.DocValuesType;
 import org.apache.lucene.index.IndexableField;
 import org.apache.lucene.index.LeafReader;
 import org.apache.lucene.index.SortedNumericDocValues;
-import org.apache.lucene.index.SortedSetDocValues;
 import org.apache.lucene.index.Term;
 import org.apache.lucene.search.BoostQuery;
 import org.apache.lucene.search.IndexOrDocValuesQuery;
@@ -50,7 +48,6 @@
 import org.opensearch.index.mapper.ParseContext.Document;
 
 import java.io.IOException;
-import java.util.SortedSet;
 
 public class BooleanFieldMapperTests extends MapperTestCase {
 
@@ -211,7 +208,13 @@ public void testBoosts() throws Exception {
         }));
 
         MappedFieldType ft = mapperService.fieldType("field");
-        assertEquals(new IndexOrDocValuesQuery(new BoostQuery(new TermQuery(new Term("field", "T")), 2.0f), SortedNumericDocValuesField.newSlowExactQuery("field", 1)), ft.termQuery("true", null));
+        assertEquals(
+            new IndexOrDocValuesQuery(
+                new BoostQuery(new TermQuery(new Term("field", "T")), 2.0f),
+                SortedNumericDocValuesField.newSlowExactQuery("field", 1)
+            ),
+            ft.termQuery("true", null)
+        );
         assertParseMaximalWarnings();
     }
 }
diff --git a/server/src/test/java/org/opensearch/index/mapper/BooleanFieldTypeTests.java b/server/src/test/java/org/opensearch/index/mapper/BooleanFieldTypeTests.java
@@ -94,10 +94,13 @@ public void testTermsQuery() {
             ),
             ft.termsQuery(terms, null)
         );
-        assertEquals(new IndexOrDocValuesQuery(
+        assertEquals(
+            new IndexOrDocValuesQuery(
                 new TermInSetQuery("field", terms),
                 new TermInSetQuery(MultiTermQuery.DOC_VALUES_REWRITE, "field", terms)
-        ), ft.termsQuery(terms, null));
+            ),
+            ft.termsQuery(terms, null)
+        );
 
         MappedFieldType unsearchable = new BooleanFieldMapper.BooleanFieldType("field", false, false);
         IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> unsearchable.termsQuery(terms, null));
-Original file line number
+Diff line change
@@ Expand Up / @@ -1299,4 +1299,4 @@ setup: @@
                   },
                 }
-      - match: { hits.total: 2 }
+      - match: { hits.total: 2 }