From 78c247c68f416c662eae7db8372a96dc5cd208c0 Mon Sep 17 00:00:00 2001 From: Harsha Vamsi Kalluri Date: Mon, 29 Jan 2024 11:15:57 -0800 Subject: [PATCH] Adding more tests for indexedValueForSearch Signed-off-by: Harsha Vamsi Kalluri --- .../index/mapper/BooleanFieldMapper.java | 39 +++++++++---------- .../index/mapper/BooleanFieldMapperTests.java | 35 +++++++++++++++++ .../index/mapper/BooleanFieldTypeTests.java | 4 +- 3 files changed, 56 insertions(+), 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 index 1f467c66f700d..3cf454283da64 100644 --- 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 index bb716f265b235..c0415c92b6997 100644 --- 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 index e9a98feefb13d..aaabeb06c3d2b 100644 --- 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 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 newTerms = new ArrayList<>(); newTerms.add(new BytesRef("true"));