Update terms logic to be more succinct

Signed-off-by: Harsha Vamsi Kalluri <harshavamsi096@gmail.com>
opensearch-project · Apr 2, 2024 · cd76bc1 · cd76bc1
1 parent 0e4d13b
commit cd76bc1
Show file tree

Hide file tree

Showing 2 changed files with 23 additions and 17 deletions.
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,6 @@
 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;
@@ -62,8 +61,10 @@
 import java.time.ZoneId;
 import java.util.Arrays;
 import java.util.Collections;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 import java.util.function.Supplier;
 
 /**
@@ -284,26 +285,25 @@ public Query termQuery(Object value, QueryShardContext context) {
         @Override
         public Query termsQuery(List<?> values, QueryShardContext context) {
             failIfNotIndexedAndNoDocValues();
-            boolean seenTrue = false;
-            boolean seenFalse = false;
-            for (Object value : values) {
+            int distinct = 0;
+            Set<?> distinctValues = new HashSet<>(values);
+            for (Object value : distinctValues) {
                 if (Values.TRUE.equals(indexedValueForSearch(value))) {
-                    seenTrue = true;
+                    distinct |= 2;
                 } else if (Values.FALSE.equals(indexedValueForSearch(value))) {
-                    seenFalse = true;
-                } else {
-                    return new MatchNoDocsQuery("Values did not contain True or False");
+                    distinct |= 1;
                 }
-            }
-            if (seenTrue) {
-                if (seenFalse) {
-                    return new FieldExistsQuery(name());
+                if (distinct == 3) {
+                    return this.existsQuery(context);
                 }
-                return termQuery("true", context);
             }
-            if (seenFalse) {
-                return termQuery("false", context);
+            switch (distinct) {
+                case 1:
+                    return termQuery("false", context);
+                case 2:
+                    return termQuery("true", context);
             }
+
             return new MatchNoDocsQuery("Values did not contain True or False");
         }
 

diff --git a/server/src/test/java/org/opensearch/index/mapper/BooleanFieldTypeTests.java b/server/src/test/java/org/opensearch/index/mapper/BooleanFieldTypeTests.java
@@ -34,7 +34,7 @@
 import org.apache.lucene.document.SortedNumericDocValuesField;
 import org.apache.lucene.index.Term;
 import org.apache.lucene.search.BoostQuery;
-import org.apache.lucene.search.FieldExistsQuery;
+import org.apache.lucene.search.DocValuesFieldExistsQuery;
 import org.apache.lucene.search.TermQuery;
 import org.apache.lucene.util.BytesRef;
 
@@ -84,12 +84,18 @@ public void testTermsQuery() {
         List<BytesRef> terms = new ArrayList<>();
         terms.add(new BytesRef("true"));
         terms.add(new BytesRef("false"));
-        assertEquals(new FieldExistsQuery("field"), ft.termsQuery(terms, null));
+        assertEquals(new DocValuesFieldExistsQuery("field"), ft.termsQuery(terms, null));
 
         List<BytesRef> newTerms = new ArrayList<>();
         newTerms.add(new BytesRef("true"));
         assertEquals(new TermQuery(new Term("field", "T")), ft.termsQuery(newTerms, null));
 
+        List<BytesRef> incorrectTerms = new ArrayList<>();
+        incorrectTerms.add(new BytesRef("true"));
+        incorrectTerms.add(new BytesRef("random"));
+        IllegalArgumentException ex = expectThrows(IllegalArgumentException.class, () -> ft.termsQuery(incorrectTerms, null));
+        assertEquals("Can't parse boolean value [random], expected [true] or [false]", ex.getMessage());
+
         MappedFieldType doc_only_ft = new BooleanFieldMapper.BooleanFieldType("field", false, true);
 
         assertEquals(SortedNumericDocValuesField.newSlowExactQuery("field", 1), doc_only_ft.termsQuery(newTerms, null));