Adding more tests for indexedValueForSearch

Signed-off-by: Harsha Vamsi Kalluri <[email protected]>
harshavamsi · Jan 29, 2024 · 78c247c · 78c247c
1 parent e5b003e
commit 78c247c
Show file tree

Hide file tree

Showing 3 changed files with 56 additions and 22 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,8 +39,8 @@
 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.MatchAllDocsQuery;
 import org.apache.lucene.search.MatchNoDocsQuery;
 import org.apache.lucene.search.MultiTermQuery;
 import org.apache.lucene.search.Query;
@@ -101,16 +101,6 @@ public static class Values {
         public static final BytesRef FALSE = new BytesRef("F");
     }
 
-    /**
-     * ExpandedValues for Booleans that can be used for this field mapper
-     *
-     * @opensearch.internal
-     */
-    public static class ExpandedValues {
-        public static final BytesRef TRUE = new BytesRef("true");
-        public static final BytesRef FALSE = new BytesRef("false");
-    }
-
     private static BooleanFieldMapper toType(FieldMapper in) {
         return (BooleanFieldMapper) in;
     }
@@ -237,8 +227,12 @@ public BytesRef indexedValueForSearch(Object value) {
             switch (sValue) {
                 case "true":
                     return Values.TRUE;
+                case "T":
+                    return Values.TRUE;
                 case "false":
                     return Values.FALSE;
+                case "F":
+                    return Values.FALSE;
                 default:
                     throw new IllegalArgumentException("Can't parse boolean value [" + sValue + "], expected [true] or [false]");
             }
@@ -300,17 +294,22 @@ public Query termQuery(Object value, QueryShardContext context) {
         @Override
         public Query termsQuery(List<?> values, QueryShardContext context) {
             failIfNotIndexedAndNoDocValues();
-            // if we do not get either True or False, we return no docs
-            if (!(values.contains(ExpandedValues.TRUE)) && !(values.contains(ExpandedValues.FALSE))) {
-                return new MatchNoDocsQuery("Values do not contain True or False");
+            boolean seenTrue = false;
+            boolean seenFalse = false;
+            for (Object value : values) {
+                if (Values.TRUE.equals(indexedValueForSearch(value))) seenTrue = true;
+                else seenFalse = true;
+            }
+            if (seenTrue) {
+                if (seenFalse) {
+                    return new MatchAllDocsQuery();
+                }
+                return termQuery(Values.TRUE, context);
             }
-            // if we have either True or False, we delegate to termQuery
-            if ((values.contains(ExpandedValues.TRUE) && !(values.contains(ExpandedValues.FALSE)))
-                || (values.contains(ExpandedValues.FALSE) && !values.contains(ExpandedValues.TRUE))) {
-                return termQuery(values.contains(ExpandedValues.TRUE) ? ExpandedValues.TRUE : ExpandedValues.FALSE, context);
+            if (seenFalse) {
+                return termQuery(Values.FALSE, context);
             }
-            // if we have both True and False, we acknowledge that the field exists with a value
-            return new FieldExistsQuery(name());
+            return new MatchNoDocsQuery("Values did not contain True or False");
         }
 
         @Override

diff --git a/server/src/test/java/org/opensearch/index/mapper/BooleanFieldMapperTests.java b/server/src/test/java/org/opensearch/index/mapper/BooleanFieldMapperTests.java
@@ -217,4 +217,39 @@ public void testBoosts() throws Exception {
         );
         assertParseMaximalWarnings();
     }
+
+    public void testIndexedValueForSearch() throws Exception {
+        assertEquals(new BooleanFieldMapper.BooleanFieldType("bool").indexedValueForSearch(null), BooleanFieldMapper.Values.FALSE);
+
+        assertEquals(new BooleanFieldMapper.BooleanFieldType("bool").indexedValueForSearch(false), BooleanFieldMapper.Values.FALSE);
+
+        assertEquals(new BooleanFieldMapper.BooleanFieldType("bool").indexedValueForSearch(true), BooleanFieldMapper.Values.TRUE);
+
+        assertEquals(
+            new BooleanFieldMapper.BooleanFieldType("bool").indexedValueForSearch(new BytesRef("true")),
+            BooleanFieldMapper.Values.TRUE
+        );
+
+        assertEquals(
+            new BooleanFieldMapper.BooleanFieldType("bool").indexedValueForSearch(new BytesRef("false")),
+            BooleanFieldMapper.Values.FALSE
+        );
+
+        assertEquals(
+            new BooleanFieldMapper.BooleanFieldType("bool").indexedValueForSearch(new BytesRef("T")),
+            BooleanFieldMapper.Values.TRUE
+        );
+
+        assertEquals(
+            new BooleanFieldMapper.BooleanFieldType("bool").indexedValueForSearch(new BytesRef("F")),
+            BooleanFieldMapper.Values.FALSE
+        );
+
+        IllegalArgumentException e = expectThrows(
+            IllegalArgumentException.class,
+            () -> new BooleanFieldMapper.BooleanFieldType("bool").indexedValueForSearch(new BytesRef("random"))
+        );
+
+        assertEquals("Can't parse boolean value [random], expected [true] or [false]", e.getMessage());
+    }
 }
diff --git a/server/src/test/java/org/opensearch/index/mapper/BooleanFieldTypeTests.java b/server/src/test/java/org/opensearch/index/mapper/BooleanFieldTypeTests.java
@@ -33,8 +33,8 @@
 
 import org.apache.lucene.document.SortedNumericDocValuesField;
 import org.apache.lucene.index.Term;
-import org.apache.lucene.search.FieldExistsQuery;
 import org.apache.lucene.search.IndexOrDocValuesQuery;
+import org.apache.lucene.search.MatchAllDocsQuery;
 import org.apache.lucene.search.MatchNoDocsQuery;
 import org.apache.lucene.search.TermQuery;
 import org.apache.lucene.util.BytesRef;
@@ -83,7 +83,7 @@ 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 MatchAllDocsQuery(), ft.termsQuery(terms, null));
 
         List<BytesRef> newTerms = new ArrayList<>();
         newTerms.add(new BytesRef("true"));