From 317095f7fd27c0de637fecb15a82c9b97ae74fb6 Mon Sep 17 00:00:00 2001 From: Harsha Vamsi Kalluri Date: Thu, 18 Jan 2024 21:21:17 -0800 Subject: [PATCH 01/24] Making fields searchable Signed-off-by: Harsha Vamsi Kalluri --- .../index/mapper/BooleanFieldMapper.java | 81 ++++++++++++++++++- .../index/mapper/DateFieldMapper.java | 19 +++-- .../index/mapper/BooleanFieldMapperTests.java | 7 +- .../index/mapper/BooleanFieldTypeTests.java | 48 ++++++++++- .../index/mapper/DateFieldTypeTests.java | 8 +- 5 files changed, 148 insertions(+), 15 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 3c7925809415a..cedd4de5343bb 100644 --- a/server/src/main/java/org/opensearch/index/mapper/BooleanFieldMapper.java +++ b/server/src/main/java/org/opensearch/index/mapper/BooleanFieldMapper.java @@ -37,7 +37,13 @@ import org.apache.lucene.document.SortedNumericDocValuesField; import org.apache.lucene.document.StoredField; 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.MultiTermQuery; import org.apache.lucene.search.Query; +import org.apache.lucene.search.TermInSetQuery; +import org.apache.lucene.search.TermQuery; import org.apache.lucene.search.TermRangeQuery; import org.apache.lucene.util.BytesRef; import org.opensearch.common.Booleans; @@ -175,6 +181,10 @@ public BooleanFieldType(String name, boolean searchable) { this(name, searchable, false, true, false, Collections.emptyMap()); } + public BooleanFieldType(String name, boolean searchable, boolean hasDocValues) { + this(name, searchable, false, hasDocValues, false, Collections.emptyMap()); + } + @Override public String typeName() { return CONTENT_TYPE; @@ -257,9 +267,78 @@ public DocValueFormat docValueFormat(@Nullable String format, ZoneId timeZone) { return DocValueFormat.BOOLEAN; } + @Override + public Query termQuery(Object value, QueryShardContext context) { + failIfNotIndexedAndNoDocValues(); + Query query = new TermQuery(new Term(name(), indexedValueForSearch(value))); + if (boost() != 1f) { + query = new BoostQuery(query, boost()); + } + if (isSearchable() && hasDocValues()) { + return new IndexOrDocValuesQuery( + query, + SortedNumericDocValuesField.newSlowExactQuery(name(), Values.TRUE.bytesEquals(indexedValueForSearch(value)) ? 1 : 0) + ); + } + if (hasDocValues()) { + return SortedNumericDocValuesField.newSlowExactQuery(name(), Values.TRUE.bytesEquals(indexedValueForSearch(value)) ? 1 : 0); + } + return query; + } + + @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 (hasDocValues()) { + return new TermInSetQuery( + MultiTermQuery.DOC_VALUES_REWRITE, + name(), + values.stream().map(this::indexedValueForSearch).toArray(BytesRef[]::new) + ); + } + return new TermInSetQuery(name(), values.stream().map(this::indexedValueForSearch).toArray(BytesRef[]::new)); + } + @Override public Query rangeQuery(Object lowerTerm, Object upperTerm, boolean includeLower, boolean includeUpper, QueryShardContext context) { - failIfNotIndexed(); + 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 (hasDocValues()) { + return new TermRangeQuery( + name(), + lowerTerm == null ? null : indexedValueForSearch(lowerTerm), + upperTerm == null ? null : indexedValueForSearch(upperTerm), + includeLower, + includeUpper, + MultiTermQuery.DOC_VALUES_REWRITE + ); + } return new TermRangeQuery( name(), lowerTerm == null ? null : indexedValueForSearch(lowerTerm), diff --git a/server/src/main/java/org/opensearch/index/mapper/DateFieldMapper.java b/server/src/main/java/org/opensearch/index/mapper/DateFieldMapper.java index d98e6ea6af83d..d03476738c9cc 100644 --- a/server/src/main/java/org/opensearch/index/mapper/DateFieldMapper.java +++ b/server/src/main/java/org/opensearch/index/mapper/DateFieldMapper.java @@ -457,22 +457,30 @@ public Query rangeQuery( @Nullable DateMathParser forcedDateParser, QueryShardContext context ) { - failIfNotIndexed(); + failIfNotIndexedAndNoDocValues(); if (relation == ShapeRelation.DISJOINT) { throw new IllegalArgumentException("Field [" + name() + "] of type [" + typeName() + "] does not support DISJOINT ranges"); } 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); - if (hasDocValues()) { - Query dvQuery = SortedNumericDocValuesField.newSlowRangeQuery(name(), l, u); - query = new IndexOrDocValuesQuery(query, dvQuery); + 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 (hasDocValues()) { + Query query = SortedNumericDocValuesField.newSlowRangeQuery(name(), l, u); if (context.indexSortedOnField(name())) { query = new IndexSortSortedNumericDocValuesRangeQuery(name(), l, u, query); } + return query; } - return query; + return LongPoint.newRangeQuery(name(), l, u); }); } @@ -543,6 +551,7 @@ public static long parseToLong( @Override public Query distanceFeatureQuery(Object origin, String pivot, float boost, QueryShardContext context) { + failIfNotIndexedAndNoDocValues(); long originLong = parseToLong(origin, true, null, null, context::nowInMillis); TimeValue pivotTime = TimeValue.parseTimeValue(pivot, "distance_feature.pivot"); return resolution.distanceFeatureQuery(name(), boost, originLong, pivotTime); 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 8dec03a353d16..6981b1dd544be 100644 --- a/server/src/test/java/org/opensearch/index/mapper/BooleanFieldMapperTests.java +++ b/server/src/test/java/org/opensearch/index/mapper/BooleanFieldMapperTests.java @@ -32,12 +32,16 @@ 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; import org.apache.lucene.search.TermQuery; import org.apache.lucene.util.BytesRef; import org.opensearch.common.xcontent.XContentFactory; @@ -46,6 +50,7 @@ import org.opensearch.index.mapper.ParseContext.Document; import java.io.IOException; +import java.util.SortedSet; public class BooleanFieldMapperTests extends MapperTestCase { @@ -206,7 +211,7 @@ public void testBoosts() throws Exception { })); MappedFieldType ft = mapperService.fieldType("field"); - assertEquals(new BoostQuery(new TermQuery(new Term("field", "T")), 2.0f), 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 index 14092706411cb..0f843b86988bd 100644 --- a/server/src/test/java/org/opensearch/index/mapper/BooleanFieldTypeTests.java +++ b/server/src/test/java/org/opensearch/index/mapper/BooleanFieldTypeTests.java @@ -31,11 +31,18 @@ package org.opensearch.index.mapper; +import org.apache.lucene.document.SortedNumericDocValuesField; import org.apache.lucene.index.Term; +import org.apache.lucene.search.IndexOrDocValuesQuery; +import org.apache.lucene.search.MultiTermQuery; +import org.apache.lucene.search.TermInSetQuery; import org.apache.lucene.search.TermQuery; +import org.apache.lucene.util.BytesRef; import java.io.IOException; +import java.util.ArrayList; import java.util.Collections; +import java.util.List; public class BooleanFieldTypeTests extends FieldTypeTestCase { @@ -56,12 +63,45 @@ public void testValueForSearch() { public void testTermQuery() { MappedFieldType ft = new BooleanFieldMapper.BooleanFieldType("field"); - assertEquals(new TermQuery(new Term("field", "T")), ft.termQuery("true", null)); - assertEquals(new TermQuery(new Term("field", "F")), ft.termQuery("false", null)); + assertEquals( + new IndexOrDocValuesQuery(new TermQuery(new Term("field", "T")), SortedNumericDocValuesField.newSlowExactQuery("field", 1)), + ft.termQuery("true", null) + ); + assertEquals( + new IndexOrDocValuesQuery(new TermQuery(new Term("field", "F")), SortedNumericDocValuesField.newSlowExactQuery("field", 0)), + ft.termQuery("false", null) + ); - MappedFieldType unsearchable = new BooleanFieldMapper.BooleanFieldType("field", false); + MappedFieldType unsearchable = new BooleanFieldMapper.BooleanFieldType("field", false, false); IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> unsearchable.termQuery("true", null)); - assertEquals("Cannot search on field [field] since it is not indexed.", e.getMessage()); + assertEquals("Cannot search on field [field] since it is both not indexed, and does not have doc_values enabled.", e.getMessage()); + } + + public void testTermsQuery() { + MappedFieldType ft = new BooleanFieldMapper.BooleanFieldType("field"); + BooleanFieldMapper.BooleanFieldType booleanFieldType = new BooleanFieldMapper.BooleanFieldType("field"); + List terms = new ArrayList<>(); + terms.add(new BytesRef("true")); + terms.add(new BytesRef("false")); + assertEquals( + new IndexOrDocValuesQuery( + new TermInSetQuery("field", terms.stream().map(booleanFieldType::indexedValueForSearch).toArray(BytesRef[]::new)), + new TermInSetQuery( + MultiTermQuery.DOC_VALUES_REWRITE, + "field", + terms.stream().map(booleanFieldType::indexedValueForSearch).toArray(BytesRef[]::new) + ) + ), + ft.termsQuery(terms, null) + ); + assertEquals(new IndexOrDocValuesQuery( + new TermInSetQuery("field", terms), + new TermInSetQuery(MultiTermQuery.DOC_VALUES_REWRITE, "field", terms) + ), ft.termsQuery(terms, null)); + + MappedFieldType unsearchable = new BooleanFieldMapper.BooleanFieldType("field", false, false); + IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> unsearchable.termsQuery(terms, null)); + assertEquals("Cannot search on field [field] since it is both not indexed, and does not have doc_values enabled.", e.getMessage()); } public void testFetchSourceValue() throws IOException { diff --git a/server/src/test/java/org/opensearch/index/mapper/DateFieldTypeTests.java b/server/src/test/java/org/opensearch/index/mapper/DateFieldTypeTests.java index ab53ae81ab0ce..db5e1e419de93 100644 --- a/server/src/test/java/org/opensearch/index/mapper/DateFieldTypeTests.java +++ b/server/src/test/java/org/opensearch/index/mapper/DateFieldTypeTests.java @@ -216,14 +216,14 @@ public void testTermQuery() { "field", false, false, - true, + false, DateFieldMapper.getDefaultDateTimeFormatter(), Resolution.MILLISECONDS, null, Collections.emptyMap() ); IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> unsearchable.termQuery(date, context)); - assertEquals("Cannot search on field [field] since it is not indexed.", e.getMessage()); + assertEquals("Cannot search on field [field] since it is both not indexed, and does not have doc_values enabled.", e.getMessage()); } public void testRangeQuery() throws IOException { @@ -279,7 +279,7 @@ public void testRangeQuery() throws IOException { "field", false, false, - true, + false, DateFieldMapper.getDefaultDateTimeFormatter(), Resolution.MILLISECONDS, null, @@ -289,7 +289,7 @@ public void testRangeQuery() throws IOException { IllegalArgumentException.class, () -> unsearchable.rangeQuery(date1, date2, true, true, null, null, null, context) ); - assertEquals("Cannot search on field [field] since it is not indexed.", e.getMessage()); + assertEquals("Cannot search on field [field] since it is both not indexed, and does not have doc_values enabled.", e.getMessage()); } public void testRangeQueryWithIndexSort() { From 8b0b2092ca2a8774798f5a7c4667dba82e749c21 Mon Sep 17 00:00:00 2001 From: Harsha Vamsi Kalluri Date: Fri, 19 Jan 2024 09:37:32 -0800 Subject: [PATCH 02/24] Adding tests for boolean fields Signed-off-by: Harsha Vamsi Kalluri --- .../test/search/340_doc_values_field.yml | 150 ++++++++++++++++-- 1 file changed, 141 insertions(+), 9 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 index d5ece1719dc48..52fa7db0e01e1 100644 --- 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 @@ -46,6 +46,10 @@ type: ip index: true doc_values: true + boolean: + type: boolean + index: true + doc_values: true - do: bulk: @@ -53,11 +57,11 @@ refresh: true body: - '{"index": {"_index": "test-iodvq", "_id": "1" }}' - - '{ "some_keyword": "ingesting some random keyword data", "byte": 120, "double": 100.0, "float": "800.0", "half_float": "400.0", "integer": 1290, "long": 13456, "short": 150, "unsigned_long": 10223372036854775800, "ip_field": "192.168.0.1" }' + - '{ "some_keyword": "ingesting some random keyword data", "byte": 120, "double": 100.0, "float": "800.0", "half_float": "400.0", "integer": 1290, "long": 13456, "short": 150, "unsigned_long": 10223372036854775800, "ip_field": "192.168.0.1", "boolean": true, "date_nanos": "2018-10-29T12:12:12.123456789Z", "date": "2018-10-29T12:12:12.987Z" }' - '{ "index": { "_index": "test-iodvq", "_id": "2" }}' - - '{ "some_keyword": "400", "byte": 121, "double": 101.0, "float": "801.0", "half_float": "401.0", "integer": 1291, "long": 13457, "short": 151, "unsigned_long": 10223372036854775801, "ip_field": "192.168.0.2" }' + - '{ "some_keyword": "400", "byte": 121, "double": 101.0, "float": "801.0", "half_float": "401.0", "integer": 1291, "long": 13457, "short": 151, "unsigned_long": 10223372036854775801, "ip_field": "192.168.0.2", "boolean": true, "date_nanos": "2020-10-29T12:12:12.987654321Z", "date": "2020-10-29T12:12:12.987Z" }' - '{ "index": { "_index": "test-iodvq", "_id": "3" } }' - - '{ "some_keyword": "5", "byte": 122, "double": 102.0, "float": "802.0", "half_float": "402.0", "integer": 1292, "long": 13458, "short": 152, "unsigned_long": 10223372036854775802, "ip_field": "192.168.0.3" }' + - '{ "some_keyword": "5", "byte": 122, "double": 102.0, "float": "802.0", "half_float": "402.0", "integer": 1292, "long": 13458, "short": 152, "unsigned_long": 10223372036854775802, "ip_field": "192.168.0.3", "boolean": false, "date_nanos": "2024-10-29T12:12:12.987654321Z", "date": "2024-10-29T12:12:12.987Z" }' - do: search: @@ -183,6 +187,17 @@ - match: {hits.total: 1} + - do: + search: + rest_total_hits_as_int: true + index: test-iodvq + body: + query: + term: + boolean: true + + - match: { hits.total: 2 } + - do: search: rest_total_hits_as_int: true @@ -415,6 +430,34 @@ - match: { hits.total: 2 } + - do: + search: + rest_total_hits_as_int: true + index: test-iodvq + body: + query: + range: { + date_nanos: { + gte: "2018-10-29T12:12:12.123456789Z" + }, + } + + - match: { hits.total: 3 } + + - do: + search: + rest_total_hits_as_int: true + index: test-iodvq + body: + query: + range: { + date: { + gte: "2018-10-29T12:12:12.987Z", + lte: "2020-10-29T12:12:12.987Z" + }, + } + + - match: { hits.total: 2 } --- "search on fields with only index enabled": - do: @@ -463,6 +506,10 @@ type: ip index: true doc_values: false + boolean: + type: boolean + index: true + doc_values: false - do: bulk: @@ -470,11 +517,11 @@ refresh: true body: - '{"index": {"_index": "test-index", "_id": "1" }}' - - '{ "some_keyword": "ingesting some random keyword data", "byte": 120, "double": 100.0, "float": "800.0", "half_float": "400.0", "integer": 1290, "long": 13456, "short": 150, "unsigned_long": 10223372036854775800, "ip_field": "192.168.0.1" }' + - '{ "some_keyword": "ingesting some random keyword data", "byte": 120, "double": 100.0, "float": "800.0", "half_float": "400.0", "integer": 1290, "long": 13456, "short": 150, "unsigned_long": 10223372036854775800, "ip_field": "192.168.0.1", "boolean": true, "date_nanos": "2018-10-29T12:12:12.123456789Z", "date": "2018-10-29T12:12:12.987Z" }' - '{ "index": { "_index": "test-index", "_id": "2" }}' - - '{ "some_keyword": "400", "byte": 121, "double": 101.0, "float": "801.0", "half_float": "401.0", "integer": 1291, "long": 13457, "short": 151, "unsigned_long": 10223372036854775801, "ip_field": "192.168.0.2" }' + - '{ "some_keyword": "400", "byte": 121, "double": 101.0, "float": "801.0", "half_float": "401.0", "integer": 1291, "long": 13457, "short": 151, "unsigned_long": 10223372036854775801, "ip_field": "192.168.0.2", "boolean": true, "date_nanos": "2020-10-29T12:12:12.123456789Z", "date": "2020-10-29T12:12:12.987Z" }' - '{ "index": { "_index": "test-index", "_id": "3" } }' - - '{ "some_keyword": "5", "byte": 122, "double": 102.0, "float": "802.0", "half_float": "402.0", "integer": 1292, "long": 13458, "short": 152, "unsigned_long": 10223372036854775802, "ip_field": "192.168.0.3" }' + - '{ "some_keyword": "5", "byte": 122, "double": 102.0, "float": "802.0", "half_float": "402.0", "integer": 1292, "long": 13458, "short": 152, "unsigned_long": 10223372036854775802, "ip_field": "192.168.0.3", "boolean": false, "date_nanos": "2024-10-29T12:12:12.123456789Z", "date": "2024-10-29T12:12:12.987Z" }' - do: search: @@ -600,6 +647,18 @@ - match: {hits.total: 1} + - do: + search: + rest_total_hits_as_int: true + index: test-index + body: + query: + term: + boolean: true + + - match: { hits.total: 2 } + + - do: search: rest_total_hits_as_int: true @@ -831,6 +890,35 @@ lte: "192.168.0.2" - match: { hits.total: 2 } + + - do: + search: + rest_total_hits_as_int: true + index: test-index + body: + query: + range: { + date_nanos: { + gte: "2018-10-29T12:12:12.123456789Z" + }, + } + + - match: { hits.total: 3 } + + - do: + search: + rest_total_hits_as_int: true + index: test-index + body: + query: + range: { + date: { + gte: "2018-10-29T12:12:12.987Z", + lte: "2020-10-29T12:12:12.987Z" + }, + } + + - match: { hits.total: 2 } --- "search on fields with only doc_values enabled": - skip: @@ -883,6 +971,10 @@ type: ip index: false doc_values: true + boolean: + type: boolean + index: false + doc_values: true - do: bulk: @@ -890,11 +982,11 @@ refresh: true body: - '{"index": {"_index": "test-doc-values", "_id": "1" }}' - - '{ "some_keyword": "ingesting some random keyword data", "byte": 120, "double": 100.0, "float": "800.0", "half_float": "400.0", "integer": 1290, "long": 13456, "short": 150, "unsigned_long": 10223372036854775800, "ip_field": "192.168.0.1" }' + - '{ "some_keyword": "ingesting some random keyword data", "byte": 120, "double": 100.0, "float": "800.0", "half_float": "400.0", "integer": 1290, "long": 13456, "short": 150, "unsigned_long": 10223372036854775800, "ip_field": "192.168.0.1", "boolean": true, "date_nanos": "2018-10-29T12:12:12.123456789Z", "date": "2018-10-29T12:12:12.987Z" }' - '{ "index": { "_index": "test-doc-values", "_id": "2" }}' - - '{ "some_keyword": "400", "byte": 121, "double": 101.0, "float": "801.0", "half_float": "401.0", "integer": 1291, "long": 13457, "short": 151, "unsigned_long": 10223372036854775801, "ip_field": "192.168.0.2" }' + - '{ "some_keyword": "400", "byte": 121, "double": 101.0, "float": "801.0", "half_float": "401.0", "integer": 1291, "long": 13457, "short": 151, "unsigned_long": 10223372036854775801, "ip_field": "192.168.0.2", "boolean": true, "date_nanos": "2020-10-29T12:12:12.123456789Z", "date": "2020-10-29T12:12:12.987Z" }' - '{ "index": { "_index": "test-doc-values", "_id": "3" } }' - - '{ "some_keyword": "5", "byte": 122, "double": 102.0, "float": "802.0", "half_float": "402.0", "integer": 1292, "long": 13458, "short": 152, "unsigned_long": 10223372036854775802, "ip_field": "192.168.0.3" }' + - '{ "some_keyword": "5", "byte": 122, "double": 102.0, "float": "802.0", "half_float": "402.0", "integer": 1292, "long": 13458, "short": 152, "unsigned_long": 10223372036854775802, "ip_field": "192.168.0.3", "boolean": false, "date_nanos": "2024-10-29T12:12:12.123456789Z", "date": "2024-10-29T12:12:12.987Z" }' - do: search: @@ -1019,6 +1111,17 @@ - match: { hits.total: 1 } + - do: + search: + rest_total_hits_as_int: true + index: test-doc-values + body: + query: + term: + boolean: false + + - match: { hits.total: 1 } + - do: search: rest_total_hits_as_int: true @@ -1239,3 +1342,32 @@ lte: "192.168.0.2" - match: { hits.total: 2 } + + - do: + search: + rest_total_hits_as_int: true + index: test-doc-values + body: + query: + range: { + date_nanos: { + gte: "2018-10-29T12:12:12.123456789Z" + }, + } + + - match: { hits.total: 3 } + + - do: + search: + rest_total_hits_as_int: true + index: test-doc-values + body: + query: + range: { + date: { + gte: "2018-10-29T12:12:12.987Z", + lte: "2020-10-29T12:12:12.987Z" + }, + } + + - match: { hits.total: 2 } From bdb46d8548ec4b0178926bcf7bdeafd1d9b71f42 Mon Sep 17 00:00:00 2001 From: Harsha Vamsi Kalluri Date: Fri, 19 Jan 2024 10:39:56 -0800 Subject: [PATCH 03/24] Added tests for date fields Signed-off-by: Harsha Vamsi Kalluri --- CHANGELOG.md | 1 + .../test/search/340_doc_values_field.yml | 25 +++++++++++++++++++ 2 files changed, 26 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1036b33c67395..a59e550e6faec 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -59,6 +59,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Bump `lycheeverse/lychee-action` from 1.9.3 to 1.10.0 ([#13447](https://github.com/opensearch-project/OpenSearch/pull/13447)) ### Changed +- Add ability for Boolean and date field queries to run when only doc_values are enabled ([#11650](https://github.com/opensearch-project/OpenSearch/pull/11650)) - [BWC and API enforcement] Enforcing the presence of API annotations at build time ([#12872](https://github.com/opensearch-project/OpenSearch/pull/12872)) - Improve built-in secure transports support ([#12907](https://github.com/opensearch-project/OpenSearch/pull/12907)) - Update links to documentation in rest-api-spec ([#13043](https://github.com/opensearch-project/OpenSearch/pull/13043)) 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 index 52fa7db0e01e1..bedf3caac0499 100644 --- 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 @@ -50,6 +50,14 @@ type: boolean index: true doc_values: true + date: + type: date + index: true + doc_values: true + date_nanos: + type: date_nanos + index: true + doc_values: true - do: bulk: @@ -458,6 +466,7 @@ } - match: { hits.total: 2 } + --- "search on fields with only index enabled": - do: @@ -510,6 +519,14 @@ type: boolean index: true doc_values: false + date_nanos: + type: date_nanos + index: true + doc_values: false + date: + type: date + index: true + doc_values: false - do: bulk: @@ -975,6 +992,14 @@ type: boolean index: false doc_values: true + date_nanos: + type: date_nanos + index: false + doc_values: true + date: + type: date + index: false + doc_values: true - do: bulk: From 52b4ceccec8bcffe253beb9606fc8cb67487be12 Mon Sep 17 00:00:00 2001 From: Harsha Vamsi Kalluri Date: Fri, 19 Jan 2024 11:44:36 -0800 Subject: [PATCH 04/24] Updating termsQuery logic Signed-off-by: Harsha Vamsi Kalluri --- .../index/mapper/BooleanFieldMapper.java | 25 ++++++++----------- .../index/mapper/DateFieldMapper.java | 16 ++++++------ .../index/mapper/BooleanFieldMapperTests.java | 11 +++++--- .../index/mapper/BooleanFieldTypeTests.java | 7 ++++-- 4 files changed, 30 insertions(+), 29 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 cedd4de5343bb..7789ece2d0263 100644 --- 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 index d03476738c9cc..9673a2d392de1 100644 --- 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 index 6981b1dd544be..bb716f265b235 100644 --- 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 index 0f843b86988bd..722997ce2dc74 100644 --- 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)); From 6602d8f51d65fb2bf925cf772f76360911f38a5d Mon Sep 17 00:00:00 2001 From: Harsha Vamsi Kalluri Date: Fri, 19 Jan 2024 12:15:38 -0800 Subject: [PATCH 05/24] Spotless Signed-off-by: Harsha Vamsi Kalluri --- .../org/opensearch/index/mapper/BooleanFieldMapper.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 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 7789ece2d0263..7d4eb2db0a581 100644 --- a/server/src/main/java/org/opensearch/index/mapper/BooleanFieldMapper.java +++ b/server/src/main/java/org/opensearch/index/mapper/BooleanFieldMapper.java @@ -44,7 +44,6 @@ import org.apache.lucene.search.MatchNoDocsQuery; import org.apache.lucene.search.MultiTermQuery; import org.apache.lucene.search.Query; -import org.apache.lucene.search.TermInSetQuery; import org.apache.lucene.search.TermQuery; import org.apache.lucene.search.TermRangeQuery; import org.apache.lucene.util.BytesRef; @@ -292,12 +291,13 @@ public Query termQuery(Object value, QueryShardContext context) { public Query termsQuery(List values, QueryShardContext context) { failIfNotIndexedAndNoDocValues(); // if we do not get either True or False, we return no docs - if (!(values.contains(Values.TRUE)) || !(values.contains(Values.FALSE))){ + if (!(values.contains(Values.TRUE)) || !(values.contains(Values.FALSE))) { return new MatchNoDocsQuery("Values do not contain True or False"); } // 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); + 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); } // if we have both True and False, we acknowledge that the field exists with a value return new FieldExistsQuery(name()); From a0dbf49e92221549da70c7fda02036e3c395cf70 Mon Sep 17 00:00:00 2001 From: Harsha Vamsi Kalluri Date: Fri, 19 Jan 2024 13:52:16 -0800 Subject: [PATCH 06/24] Updating terms test Signed-off-by: Harsha Vamsi Kalluri --- .../index/mapper/BooleanFieldMapper.java | 13 ++++++--- .../index/mapper/BooleanFieldTypeTests.java | 27 ++++++++++--------- 2 files changed, 24 insertions(+), 16 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 7d4eb2db0a581..d55914bbbd9cc 100644 --- a/server/src/main/java/org/opensearch/index/mapper/BooleanFieldMapper.java +++ b/server/src/main/java/org/opensearch/index/mapper/BooleanFieldMapper.java @@ -101,6 +101,11 @@ public static class Values { public static final BytesRef FALSE = new BytesRef("F"); } + 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; } @@ -291,13 +296,13 @@ public Query termQuery(Object value, QueryShardContext context) { public Query termsQuery(List values, QueryShardContext context) { failIfNotIndexedAndNoDocValues(); // if we do not get either True or False, we return no docs - if (!(values.contains(Values.TRUE)) || !(values.contains(Values.FALSE))) { + if (!(values.contains(ExpandedValues.TRUE)) && !(values.contains(ExpandedValues.FALSE))) { return new MatchNoDocsQuery("Values do not contain True or False"); } // 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); + 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 we have both True and False, we acknowledge that the field exists with a value return new FieldExistsQuery(name()); 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 722997ce2dc74..d12c6c6779f14 100644 --- a/server/src/test/java/org/opensearch/index/mapper/BooleanFieldTypeTests.java +++ b/server/src/test/java/org/opensearch/index/mapper/BooleanFieldTypeTests.java @@ -33,7 +33,9 @@ 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.MatchNoDocsQuery; import org.apache.lucene.search.MultiTermQuery; import org.apache.lucene.search.TermInSetQuery; import org.apache.lucene.search.TermQuery; @@ -84,22 +86,23 @@ public void testTermsQuery() { terms.add(new BytesRef("true")); terms.add(new BytesRef("false")); assertEquals( - new IndexOrDocValuesQuery( - new TermInSetQuery("field", terms.stream().map(booleanFieldType::indexedValueForSearch).toArray(BytesRef[]::new)), - new TermInSetQuery( - MultiTermQuery.DOC_VALUES_REWRITE, - "field", - terms.stream().map(booleanFieldType::indexedValueForSearch).toArray(BytesRef[]::new) - ) - ), + new FieldExistsQuery("field"), ft.termsQuery(terms, null) ); + + List newTerms = new ArrayList<>(); + newTerms.add(new BytesRef("true")); assertEquals( - new IndexOrDocValuesQuery( - new TermInSetQuery("field", terms), - new TermInSetQuery(MultiTermQuery.DOC_VALUES_REWRITE, "field", terms) + new IndexOrDocValuesQuery( + new TermQuery(new Term("field", "T")), + SortedNumericDocValuesField.newSlowExactQuery("field", 1) ), - ft.termsQuery(terms, null) + ft.termsQuery(newTerms, null) + ); + + assertEquals( + new MatchNoDocsQuery("Values do not contain True or False"), + ft.termsQuery(new ArrayList<>(), null) ); MappedFieldType unsearchable = new BooleanFieldMapper.BooleanFieldType("field", false, false); From fad7ba34406e950b7705870f01e53dfca6e3e150 Mon Sep 17 00:00:00 2001 From: Harsha Vamsi Kalluri Date: Mon, 22 Jan 2024 13:01:50 -0800 Subject: [PATCH 07/24] Spotless Signed-off-by: Harsha Vamsi Kalluri --- .../index/mapper/BooleanFieldTypeTests.java | 17 +++-------------- 1 file changed, 3 insertions(+), 14 deletions(-) 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 d12c6c6779f14..e9a98feefb13d 100644 --- a/server/src/test/java/org/opensearch/index/mapper/BooleanFieldTypeTests.java +++ b/server/src/test/java/org/opensearch/index/mapper/BooleanFieldTypeTests.java @@ -36,8 +36,6 @@ 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.TermInSetQuery; import org.apache.lucene.search.TermQuery; import org.apache.lucene.util.BytesRef; @@ -85,25 +83,16 @@ 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 FieldExistsQuery("field"), ft.termsQuery(terms, null)); List newTerms = new ArrayList<>(); newTerms.add(new BytesRef("true")); assertEquals( - new IndexOrDocValuesQuery( - new TermQuery(new Term("field", "T")), - SortedNumericDocValuesField.newSlowExactQuery("field", 1) - ), + new IndexOrDocValuesQuery(new TermQuery(new Term("field", "T")), SortedNumericDocValuesField.newSlowExactQuery("field", 1)), ft.termsQuery(newTerms, null) ); - assertEquals( - new MatchNoDocsQuery("Values do not contain True or False"), - ft.termsQuery(new ArrayList<>(), null) - ); + assertEquals(new MatchNoDocsQuery("Values do not contain True or False"), ft.termsQuery(new ArrayList<>(), null)); MappedFieldType unsearchable = new BooleanFieldMapper.BooleanFieldType("field", false, false); IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> unsearchable.termsQuery(terms, null)); From 762ffccef03d42b9fc20a7ce33b1a13e20456634 Mon Sep 17 00:00:00 2001 From: Harsha Vamsi Kalluri Date: Mon, 22 Jan 2024 15:52:12 -0800 Subject: [PATCH 08/24] Ensure that the points are intersecting for doc_values Signed-off-by: Harsha Vamsi Kalluri --- .../java/org/opensearch/index/mapper/DateFieldMapper.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/server/src/main/java/org/opensearch/index/mapper/DateFieldMapper.java b/server/src/main/java/org/opensearch/index/mapper/DateFieldMapper.java index 9673a2d392de1..6ed06024f0051 100644 --- a/server/src/main/java/org/opensearch/index/mapper/DateFieldMapper.java +++ b/server/src/main/java/org/opensearch/index/mapper/DateFieldMapper.java @@ -34,6 +34,7 @@ import org.apache.lucene.document.LongPoint; import org.apache.lucene.document.SortedNumericDocValuesField; +import org.apache.lucene.document.SortedSetDocValuesField; import org.apache.lucene.document.StoredField; import org.apache.lucene.index.IndexReader; import org.apache.lucene.index.PointValues; @@ -568,6 +569,9 @@ public Relation isFieldWithinQuery( DateMathParser dateParser, QueryRewriteContext context ) throws IOException { + if (isSearchable() == false && hasDocValues()) { + return Relation.INTERSECTS; + } if (dateParser == null) { dateParser = this.dateMathParser; } From ad8baaeae6cee617cf82b062b477c6960908fbc7 Mon Sep 17 00:00:00 2001 From: Harsha Vamsi Kalluri Date: Mon, 22 Jan 2024 17:30:15 -0800 Subject: [PATCH 09/24] Spotless Signed-off-by: Harsha Vamsi Kalluri --- .../main/java/org/opensearch/index/mapper/DateFieldMapper.java | 1 - 1 file changed, 1 deletion(-) diff --git a/server/src/main/java/org/opensearch/index/mapper/DateFieldMapper.java b/server/src/main/java/org/opensearch/index/mapper/DateFieldMapper.java index 6ed06024f0051..52df502a3f1b7 100644 --- a/server/src/main/java/org/opensearch/index/mapper/DateFieldMapper.java +++ b/server/src/main/java/org/opensearch/index/mapper/DateFieldMapper.java @@ -34,7 +34,6 @@ import org.apache.lucene.document.LongPoint; import org.apache.lucene.document.SortedNumericDocValuesField; -import org.apache.lucene.document.SortedSetDocValuesField; import org.apache.lucene.document.StoredField; import org.apache.lucene.index.IndexReader; import org.apache.lucene.index.PointValues; From 90749d854ef3ea46987bbccb99d9e466360ee8d6 Mon Sep 17 00:00:00 2001 From: Harsha Vamsi Kalluri Date: Wed, 24 Jan 2024 10:23:13 -0800 Subject: [PATCH 10/24] Adding missing javadocs Signed-off-by: Harsha Vamsi Kalluri --- .../java/org/opensearch/index/mapper/BooleanFieldMapper.java | 5 +++++ 1 file changed, 5 insertions(+) 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 d55914bbbd9cc..1f467c66f700d 100644 --- a/server/src/main/java/org/opensearch/index/mapper/BooleanFieldMapper.java +++ b/server/src/main/java/org/opensearch/index/mapper/BooleanFieldMapper.java @@ -101,6 +101,11 @@ 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"); From 834a8e54f08dc0e5502fa9f78eec2580874aad3e Mon Sep 17 00:00:00 2001 From: Harsha Vamsi Kalluri Date: Mon, 29 Jan 2024 11:15:57 -0800 Subject: [PATCH 11/24] 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")); From 1c57ee6b3e95406629cb2be86fd6e8fbd76c1aca Mon Sep 17 00:00:00 2001 From: Harsha Vamsi Kalluri Date: Mon, 29 Jan 2024 11:57:22 -0800 Subject: [PATCH 12/24] Adding MatchAllDocsQuery to asserts Signed-off-by: Harsha Vamsi Kalluri --- .../java/org/opensearch/index/query/TermsQueryBuilderTests.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/server/src/test/java/org/opensearch/index/query/TermsQueryBuilderTests.java b/server/src/test/java/org/opensearch/index/query/TermsQueryBuilderTests.java index 32bf290627b63..3173bc0d0461e 100644 --- a/server/src/test/java/org/opensearch/index/query/TermsQueryBuilderTests.java +++ b/server/src/test/java/org/opensearch/index/query/TermsQueryBuilderTests.java @@ -35,6 +35,7 @@ import org.apache.lucene.search.BooleanQuery; import org.apache.lucene.search.ConstantScoreQuery; import org.apache.lucene.search.IndexOrDocValuesQuery; +import org.apache.lucene.search.MatchAllDocsQuery; import org.apache.lucene.search.MatchNoDocsQuery; import org.apache.lucene.search.PointInSetQuery; import org.apache.lucene.search.Query; @@ -137,6 +138,7 @@ protected void doAssertLuceneQuery(TermsQueryBuilder queryBuilder, Query query, .or(instanceOf(ConstantScoreQuery.class)) .or(instanceOf(MatchNoDocsQuery.class)) .or(instanceOf(IndexOrDocValuesQuery.class)) + .or(instanceOf(MatchAllDocsQuery.class)) ); if (query instanceof ConstantScoreQuery) { assertThat(((ConstantScoreQuery) query).getQuery(), instanceOf(BooleanQuery.class)); From acffbe1c89c64e117fd7b0c66a3a97263cffb7e4 Mon Sep 17 00:00:00 2001 From: Harsha Vamsi Kalluri Date: Fri, 8 Mar 2024 11:10:36 -0800 Subject: [PATCH 13/24] Fix changelog Signed-off-by: Harsha Vamsi Kalluri --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index a59e550e6faec..0c8de6dfaaa12 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -67,6 +67,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Refactoring globMatch using simpleMatchWithNormalizedStrings from Regex ([#13104](https://github.com/opensearch-project/OpenSearch/pull/13104)) - [BWC and API enforcement] Reconsider the breaking changes check policy to detect breaking changes against released versions ([#13292](https://github.com/opensearch-project/OpenSearch/pull/13292)) - Switch to macos-13 runner for precommit and assemble github actions due to macos-latest is now arm64 ([#13412](https://github.com/opensearch-project/OpenSearch/pull/13412)) +- Add ability for Boolean and date field queries to run when only doc_values are enabled ([#11650](https://github.com/opensearch-project/OpenSearch/pull/11650)) ### Deprecated From 2db68c9655276fb408e6bc9c00e7295e54b5c7f2 Mon Sep 17 00:00:00 2001 From: Harsha Vamsi Kalluri Date: Mon, 11 Mar 2024 11:29:45 -0700 Subject: [PATCH 14/24] Simplify loop criteria + remove IndexOrDocValuesQuery Signed-off-by: Harsha Vamsi Kalluri --- .../index/mapper/BooleanFieldMapper.java | 30 ++++++++----------- .../index/mapper/BooleanFieldTypeTests.java | 26 ++++++++-------- 2 files changed, 26 insertions(+), 30 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 3cf454283da64..b3112a6002d84 100644 --- a/server/src/main/java/org/opensearch/index/mapper/BooleanFieldMapper.java +++ b/server/src/main/java/org/opensearch/index/mapper/BooleanFieldMapper.java @@ -227,12 +227,8 @@ 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]"); } @@ -275,19 +271,13 @@ public DocValueFormat docValueFormat(@Nullable String format, ZoneId timeZone) { @Override public Query termQuery(Object value, QueryShardContext context) { failIfNotIndexedAndNoDocValues(); + if (!isSearchable()) { + return SortedNumericDocValuesField.newSlowExactQuery(name(), Values.TRUE.bytesEquals(indexedValueForSearch(value)) ? 1 : 0); + } Query query = new TermQuery(new Term(name(), indexedValueForSearch(value))); if (boost() != 1f) { query = new BoostQuery(query, boost()); } - if (isSearchable() && hasDocValues()) { - return new IndexOrDocValuesQuery( - query, - SortedNumericDocValuesField.newSlowExactQuery(name(), Values.TRUE.bytesEquals(indexedValueForSearch(value)) ? 1 : 0) - ); - } - if (hasDocValues()) { - return SortedNumericDocValuesField.newSlowExactQuery(name(), Values.TRUE.bytesEquals(indexedValueForSearch(value)) ? 1 : 0); - } return query; } @@ -297,19 +287,25 @@ public Query termsQuery(List values, QueryShardContext context) { boolean seenTrue = false; boolean seenFalse = false; for (Object value : values) { - if (Values.TRUE.equals(indexedValueForSearch(value))) seenTrue = true; - else seenFalse = true; + if (Values.TRUE.equals(indexedValueForSearch(value))) { + seenTrue = true; + } else if (Values.FALSE.equals(indexedValueForSearch(value))) { + seenFalse = true; + } else { + return new MatchNoDocsQuery("Values did not contain True or False"); + } } if (seenTrue) { if (seenFalse) { return new MatchAllDocsQuery(); } - return termQuery(Values.TRUE, context); + return termQuery("true", context); } if (seenFalse) { - return termQuery(Values.FALSE, context); + return termQuery("false", context); } return new MatchNoDocsQuery("Values did not contain True or False"); + } @Override 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 aaabeb06c3d2b..d5eda3b7042b3 100644 --- a/server/src/test/java/org/opensearch/index/mapper/BooleanFieldTypeTests.java +++ b/server/src/test/java/org/opensearch/index/mapper/BooleanFieldTypeTests.java @@ -33,7 +33,7 @@ import org.apache.lucene.document.SortedNumericDocValuesField; import org.apache.lucene.index.Term; -import org.apache.lucene.search.IndexOrDocValuesQuery; +import org.apache.lucene.search.BoostQuery; import org.apache.lucene.search.MatchAllDocsQuery; import org.apache.lucene.search.MatchNoDocsQuery; import org.apache.lucene.search.TermQuery; @@ -63,14 +63,17 @@ public void testValueForSearch() { public void testTermQuery() { MappedFieldType ft = new BooleanFieldMapper.BooleanFieldType("field"); - assertEquals( - new IndexOrDocValuesQuery(new TermQuery(new Term("field", "T")), SortedNumericDocValuesField.newSlowExactQuery("field", 1)), - ft.termQuery("true", null) - ); - assertEquals( - new IndexOrDocValuesQuery(new TermQuery(new Term("field", "F")), SortedNumericDocValuesField.newSlowExactQuery("field", 0)), - ft.termQuery("false", null) - ); + assertEquals(new TermQuery(new Term("field", "T")), ft.termQuery("true", null)); + assertEquals(new TermQuery(new Term("field", "F")), ft.termQuery("false", null)); + + MappedFieldType doc_ft = new BooleanFieldMapper.BooleanFieldType("field", false, true); + assertEquals(SortedNumericDocValuesField.newSlowExactQuery("field", 1), doc_ft.termQuery("true", null)); + assertEquals(SortedNumericDocValuesField.newSlowExactQuery("field", 0), doc_ft.termQuery("false", null)); + + MappedFieldType boost_ft = new BooleanFieldMapper.BooleanFieldType("field"); + boost_ft.setBoost(2f); + assertEquals(new BoostQuery(new TermQuery(new Term("field", "T")), 2f), boost_ft.termQuery("true", null)); + assertEquals(new BoostQuery(new TermQuery(new Term("field", "F")), 2f), boost_ft.termQuery("false", null)); MappedFieldType unsearchable = new BooleanFieldMapper.BooleanFieldType("field", false, false); IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> unsearchable.termQuery("true", null)); @@ -87,10 +90,7 @@ public void testTermsQuery() { List newTerms = new ArrayList<>(); newTerms.add(new BytesRef("true")); - assertEquals( - new IndexOrDocValuesQuery(new TermQuery(new Term("field", "T")), SortedNumericDocValuesField.newSlowExactQuery("field", 1)), - ft.termsQuery(newTerms, null) - ); + assertEquals(new TermQuery(new Term("field", "T")), ft.termsQuery(newTerms, null)); assertEquals(new MatchNoDocsQuery("Values do not contain True or False"), ft.termsQuery(new ArrayList<>(), null)); From 7c9a200a2526771ea75e1abc2f4dd8ced747566c Mon Sep 17 00:00:00 2001 From: Harsha Vamsi Kalluri Date: Mon, 11 Mar 2024 11:37:02 -0700 Subject: [PATCH 15/24] Add some comments Signed-off-by: Harsha Vamsi Kalluri --- .../main/java/org/opensearch/index/mapper/DateFieldMapper.java | 1 + 1 file changed, 1 insertion(+) diff --git a/server/src/main/java/org/opensearch/index/mapper/DateFieldMapper.java b/server/src/main/java/org/opensearch/index/mapper/DateFieldMapper.java index 52df502a3f1b7..b7ee3bb8ca3e3 100644 --- a/server/src/main/java/org/opensearch/index/mapper/DateFieldMapper.java +++ b/server/src/main/java/org/opensearch/index/mapper/DateFieldMapper.java @@ -568,6 +568,7 @@ public Relation isFieldWithinQuery( DateMathParser dateParser, QueryRewriteContext context ) throws IOException { + // if we have only doc_values enabled we do not look at the BKD so we return an INTERSECTS by default if (isSearchable() == false && hasDocValues()) { return Relation.INTERSECTS; } From 76ff2c20d14fbd8cf674a6a41a69c5222c65f082 Mon Sep 17 00:00:00 2001 From: Harsha Vamsi Kalluri Date: Mon, 11 Mar 2024 13:36:39 -0700 Subject: [PATCH 16/24] Fix indendation Signed-off-by: Harsha Vamsi Kalluri --- .../rest-api-spec/test/search/340_doc_values_field.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 index bedf3caac0499..eed57ab3d93ce 100644 --- 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 @@ -671,7 +671,7 @@ body: query: term: - boolean: true + boolean: true - match: { hits.total: 2 } From c9453ff81d06fd10d4f7edd545f3873d51249572 Mon Sep 17 00:00:00 2001 From: Harsha Vamsi Kalluri Date: Wed, 20 Mar 2024 09:15:33 -0700 Subject: [PATCH 17/24] Fixing boolean field tests Signed-off-by: Harsha Vamsi Kalluri --- .../test/search/340_doc_values_field.yml | 47 ++++++++++++++++--- .../index/mapper/BooleanFieldMapper.java | 30 ++++-------- .../index/mapper/BooleanFieldMapperTests.java | 20 +------- .../index/mapper/BooleanFieldTypeTests.java | 16 ++++--- 4 files changed, 61 insertions(+), 52 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 index eed57ab3d93ce..e249f2bb2a6ee 100644 --- 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 @@ -305,6 +305,17 @@ - match: { hits.total: 2 } + - do: + search: + rest_total_hits_as_int: true + index: test-iodvq + body: + query: + terms: + boolean: [true, false] + + - match: { hits.total: 3 } + - do: search: rest_total_hits_as_int: true @@ -665,13 +676,13 @@ - match: {hits.total: 1} - do: - search: - rest_total_hits_as_int: true - index: test-index - body: - query: - term: - boolean: true + search: + rest_total_hits_as_int: true + index: test-index + body: + query: + term: + boolean: true - match: { hits.total: 2 } @@ -775,6 +786,17 @@ - match: { hits.total: 2 } + - do: + search: + rest_total_hits_as_int: true + index: test-index + body: + query: + terms: + boolean: [true, false] + + - match: { hits.total: 3 } + - do: search: rest_total_hits_as_int: true @@ -1235,6 +1257,17 @@ - match: { hits.total: 2 } + - do: + search: + rest_total_hits_as_int: true + index: test-doc-values + body: + query: + terms: + boolean: [true, false] + + - match: { hits.total: 3 } + - do: search: rest_total_hits_as_int: true 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 b3112a6002d84..3ea70800d0763 100644 --- a/server/src/main/java/org/opensearch/index/mapper/BooleanFieldMapper.java +++ b/server/src/main/java/org/opensearch/index/mapper/BooleanFieldMapper.java @@ -40,10 +40,9 @@ import org.apache.lucene.index.Term; import org.apache.lucene.search.BoostQuery; 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; +import org.apache.lucene.search.TermInSetQuery; import org.apache.lucene.search.TermQuery; import org.apache.lucene.search.TermRangeQuery; import org.apache.lucene.util.BytesRef; @@ -284,27 +283,18 @@ 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) { - if (Values.TRUE.equals(indexedValueForSearch(value))) { - seenTrue = true; - } else if (Values.FALSE.equals(indexedValueForSearch(value))) { - seenFalse = true; - } else { - return new MatchNoDocsQuery("Values did not contain True or False"); - } - } - if (seenTrue) { - if (seenFalse) { - return new MatchAllDocsQuery(); + if (!isSearchable()) { + long[] v = new long[values.size()]; + for (int i = 0; i < v.length; i++) { + v[i] = Values.TRUE.bytesEquals(indexedValueForSearch(values.get(i))) ? 1 : 0; } - return termQuery("true", context); + return SortedNumericDocValuesField.newSlowSetQuery(name(), v); } - if (seenFalse) { - return termQuery("false", context); + BytesRef[] bytesRefs = new BytesRef[values.size()]; + for (int i = 0; i < bytesRefs.length; i++) { + bytesRefs[i] = indexedValueForSearch(values.get(i)); } - return new MatchNoDocsQuery("Values did not contain True or False"); + return new TermInSetQuery(name(), List.of(bytesRefs)); } 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 c0415c92b6997..5392bd6c358d3 100644 --- a/server/src/test/java/org/opensearch/index/mapper/BooleanFieldMapperTests.java +++ b/server/src/test/java/org/opensearch/index/mapper/BooleanFieldMapperTests.java @@ -32,14 +32,12 @@ package org.opensearch.index.mapper; -import org.apache.lucene.document.SortedNumericDocValuesField; 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.Term; import org.apache.lucene.search.BoostQuery; -import org.apache.lucene.search.IndexOrDocValuesQuery; import org.apache.lucene.search.TermQuery; import org.apache.lucene.util.BytesRef; import org.opensearch.common.xcontent.XContentFactory; @@ -208,13 +206,7 @@ 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 BoostQuery(new TermQuery(new Term("field", "T")), 2.0f), ft.termQuery("true", null)); assertParseMaximalWarnings(); } @@ -235,16 +227,6 @@ public void testIndexedValueForSearch() throws Exception { 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")) 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 d5eda3b7042b3..6d077b1b188b4 100644 --- a/server/src/test/java/org/opensearch/index/mapper/BooleanFieldTypeTests.java +++ b/server/src/test/java/org/opensearch/index/mapper/BooleanFieldTypeTests.java @@ -34,8 +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.MatchAllDocsQuery; -import org.apache.lucene.search.MatchNoDocsQuery; +import org.apache.lucene.search.TermInSetQuery; import org.apache.lucene.search.TermQuery; import org.apache.lucene.util.BytesRef; @@ -43,6 +42,7 @@ import java.util.ArrayList; import java.util.Collections; import java.util.List; +import java.util.stream.Stream; public class BooleanFieldTypeTests extends FieldTypeTestCase { @@ -82,17 +82,21 @@ public void testTermQuery() { public void testTermsQuery() { MappedFieldType ft = new BooleanFieldMapper.BooleanFieldType("field"); - BooleanFieldMapper.BooleanFieldType booleanFieldType = new BooleanFieldMapper.BooleanFieldType("field"); List terms = new ArrayList<>(); terms.add(new BytesRef("true")); terms.add(new BytesRef("false")); - assertEquals(new MatchAllDocsQuery(), ft.termsQuery(terms, null)); + assertEquals(new TermInSetQuery("field", List.of(new BytesRef("T"), newBytesRef("F"))), ft.termsQuery(terms, null)); List newTerms = new ArrayList<>(); newTerms.add(new BytesRef("true")); - assertEquals(new TermQuery(new Term("field", "T")), ft.termsQuery(newTerms, null)); + assertEquals(new TermInSetQuery("field", List.of(new BytesRef("T"))), ft.termsQuery(newTerms, null)); - assertEquals(new MatchNoDocsQuery("Values do not contain True or False"), ft.termsQuery(new ArrayList<>(), null)); + MappedFieldType doc_only_ft = new BooleanFieldMapper.BooleanFieldType("field", false, true); + + assertEquals( + SortedNumericDocValuesField.newSlowSetQuery("field", Stream.of(1).mapToLong(l -> l).toArray()), + doc_only_ft.termsQuery(newTerms, null) + ); MappedFieldType unsearchable = new BooleanFieldMapper.BooleanFieldType("field", false, false); IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> unsearchable.termsQuery(terms, null)); From 396ac85fa067f730d6060ffb8629243fcdb5f4fc Mon Sep 17 00:00:00 2001 From: Harsha Vamsi Kalluri Date: Tue, 2 Apr 2024 11:19:08 -0700 Subject: [PATCH 18/24] Revert to correct logic for termsQuery Signed-off-by: Harsha Vamsi Kalluri --- .../index/mapper/BooleanFieldMapper.java | 31 ++++++++++++------- .../index/mapper/BooleanFieldTypeTests.java | 12 +++---- 2 files changed, 24 insertions(+), 19 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 3ea70800d0763..7c4587f719f36 100644 --- a/server/src/main/java/org/opensearch/index/mapper/BooleanFieldMapper.java +++ b/server/src/main/java/org/opensearch/index/mapper/BooleanFieldMapper.java @@ -39,10 +39,11 @@ 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; import org.apache.lucene.search.TermQuery; import org.apache.lucene.search.TermRangeQuery; import org.apache.lucene.util.BytesRef; @@ -283,19 +284,27 @@ public Query termQuery(Object value, QueryShardContext context) { @Override public Query termsQuery(List values, QueryShardContext context) { failIfNotIndexedAndNoDocValues(); - if (!isSearchable()) { - long[] v = new long[values.size()]; - for (int i = 0; i < v.length; i++) { - v[i] = Values.TRUE.bytesEquals(indexedValueForSearch(values.get(i))) ? 1 : 0; + boolean seenTrue = false; + boolean seenFalse = false; + for (Object value : values) { + if (Values.TRUE.equals(indexedValueForSearch(value))) { + seenTrue = true; + } else if (Values.FALSE.equals(indexedValueForSearch(value))) { + seenFalse = true; + } else { + return new MatchNoDocsQuery("Values did not contain True or False"); + } + } + if (seenTrue) { + if (seenFalse) { + return new FieldExistsQuery(name()); } - return SortedNumericDocValuesField.newSlowSetQuery(name(), v); + return termQuery("true", context); } - BytesRef[] bytesRefs = new BytesRef[values.size()]; - for (int i = 0; i < bytesRefs.length; i++) { - bytesRefs[i] = indexedValueForSearch(values.get(i)); + if (seenFalse) { + return termQuery("false", context); } - return new TermInSetQuery(name(), List.of(bytesRefs)); - + return new MatchNoDocsQuery("Values did not contain True or False"); } @Override 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 6d077b1b188b4..33c30beef6c7d 100644 --- 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.TermInSetQuery; +import org.apache.lucene.search.FieldExistsQuery; import org.apache.lucene.search.TermQuery; import org.apache.lucene.util.BytesRef; @@ -42,7 +42,6 @@ import java.util.ArrayList; import java.util.Collections; import java.util.List; -import java.util.stream.Stream; public class BooleanFieldTypeTests extends FieldTypeTestCase { @@ -85,18 +84,15 @@ public void testTermsQuery() { List terms = new ArrayList<>(); terms.add(new BytesRef("true")); terms.add(new BytesRef("false")); - assertEquals(new TermInSetQuery("field", List.of(new BytesRef("T"), newBytesRef("F"))), ft.termsQuery(terms, null)); + assertEquals(new FieldExistsQuery("field"), ft.termsQuery(terms, null)); List newTerms = new ArrayList<>(); newTerms.add(new BytesRef("true")); - assertEquals(new TermInSetQuery("field", List.of(new BytesRef("T"))), ft.termsQuery(newTerms, null)); + assertEquals(new TermQuery(new Term("field", "T")), ft.termsQuery(newTerms, null)); MappedFieldType doc_only_ft = new BooleanFieldMapper.BooleanFieldType("field", false, true); - assertEquals( - SortedNumericDocValuesField.newSlowSetQuery("field", Stream.of(1).mapToLong(l -> l).toArray()), - doc_only_ft.termsQuery(newTerms, null) - ); + assertEquals(SortedNumericDocValuesField.newSlowExactQuery("field", 1), doc_only_ft.termsQuery(newTerms, null)); MappedFieldType unsearchable = new BooleanFieldMapper.BooleanFieldType("field", false, false); IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> unsearchable.termsQuery(terms, null)); From aff9c5bb75484ebab4a62fc486e1e6f296b03374 Mon Sep 17 00:00:00 2001 From: Harsha Vamsi Kalluri Date: Tue, 2 Apr 2024 13:23:25 -0700 Subject: [PATCH 19/24] Update terms logic to be more succinct Signed-off-by: Harsha Vamsi Kalluri --- .../index/mapper/BooleanFieldMapper.java | 30 +++++++++---------- .../index/mapper/BooleanFieldTypeTests.java | 10 +++++-- 2 files changed, 23 insertions(+), 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 index 7c4587f719f36..9016b1014d627 100644 --- 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 index 33c30beef6c7d..7f649034d6a5b 100644 --- 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 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 newTerms = new ArrayList<>(); newTerms.add(new BytesRef("true")); assertEquals(new TermQuery(new Term("field", "T")), ft.termsQuery(newTerms, null)); + List 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)); From 9ab4abf6c545ce487fabe6a576524c7cfbf82cb6 Mon Sep 17 00:00:00 2001 From: Harsha Vamsi Kalluri Date: Tue, 2 Apr 2024 13:27:05 -0700 Subject: [PATCH 20/24] Fix terms test Signed-off-by: Harsha Vamsi Kalluri --- .../java/org/opensearch/index/query/TermsQueryBuilderTests.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/server/src/test/java/org/opensearch/index/query/TermsQueryBuilderTests.java b/server/src/test/java/org/opensearch/index/query/TermsQueryBuilderTests.java index 3173bc0d0461e..61f7dc5facebe 100644 --- a/server/src/test/java/org/opensearch/index/query/TermsQueryBuilderTests.java +++ b/server/src/test/java/org/opensearch/index/query/TermsQueryBuilderTests.java @@ -34,6 +34,7 @@ import org.apache.lucene.search.BooleanQuery; import org.apache.lucene.search.ConstantScoreQuery; +import org.apache.lucene.search.FieldExistsQuery; import org.apache.lucene.search.IndexOrDocValuesQuery; import org.apache.lucene.search.MatchAllDocsQuery; import org.apache.lucene.search.MatchNoDocsQuery; @@ -139,6 +140,7 @@ protected void doAssertLuceneQuery(TermsQueryBuilder queryBuilder, Query query, .or(instanceOf(MatchNoDocsQuery.class)) .or(instanceOf(IndexOrDocValuesQuery.class)) .or(instanceOf(MatchAllDocsQuery.class)) + .or(instanceOf(FieldExistsQuery.class)) ); if (query instanceof ConstantScoreQuery) { assertThat(((ConstantScoreQuery) query).getQuery(), instanceOf(BooleanQuery.class)); From cd360bbdeda0208d462d169bdf2bfe7d0613db1f Mon Sep 17 00:00:00 2001 From: Harsha Vamsi Kalluri Date: Thu, 4 Apr 2024 16:03:57 -0700 Subject: [PATCH 21/24] Adding tests for boolean range query + fix range query to use term query inside Signed-off-by: Harsha Vamsi Kalluri --- .../test/search/340_doc_values_field.yml | 173 ++++++++++++++++++ .../index/mapper/BooleanFieldMapper.java | 65 +++---- .../index/mapper/BooleanFieldTypeTests.java | 20 ++ .../index/query/TermsQueryBuilderTests.java | 1 + 4 files changed, 223 insertions(+), 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 index e249f2bb2a6ee..a133060f07c6f 100644 --- 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 index 9016b1014d627..b9446a4542467 100644 --- 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 index 7f649034d6a5b..66bfcb0f834d4 100644 --- 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 index 61f7dc5facebe..355de73e100b1 100644 --- 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; From d09786758cc4951ff852f9ee763295ba9508c7e9 Mon Sep 17 00:00:00 2001 From: Harsha Vamsi Kalluri Date: Mon, 29 Apr 2024 10:52:51 -0700 Subject: [PATCH 22/24] Checking if upper and lower terms are valid Signed-off-by: Harsha Vamsi Kalluri --- .../org/opensearch/index/mapper/BooleanFieldMapper.java | 7 +++++-- .../org/opensearch/index/mapper/BooleanFieldTypeTests.java | 4 ++++ 2 files changed, 9 insertions(+), 2 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 b9446a4542467..b4cf585c1329d 100644 --- a/server/src/main/java/org/opensearch/index/mapper/BooleanFieldMapper.java +++ b/server/src/main/java/org/opensearch/index/mapper/BooleanFieldMapper.java @@ -318,14 +318,17 @@ public Query rangeQuery(Object lowerTerm, Object upperTerm, boolean includeLower } + lowerTerm = indexedValueForSearch(lowerTerm); + upperTerm = indexedValueForSearch(upperTerm); + if (lowerTerm == upperTerm) { if (!includeLower || !includeUpper) { return new MatchNoDocsQuery(); } - return termQuery(lowerTerm, context); + return termQuery(lowerTerm.equals(Values.TRUE), context); } - if ((boolean) lowerTerm) { + if (lowerTerm.equals(Values.TRUE)) { return new MatchNoDocsQuery(); } if (!includeLower && !includeUpper) { 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 66bfcb0f834d4..aab63fc30efd7 100644 --- a/server/src/test/java/org/opensearch/index/mapper/BooleanFieldTypeTests.java +++ b/server/src/test/java/org/opensearch/index/mapper/BooleanFieldTypeTests.java @@ -123,6 +123,10 @@ public void testRangeQuery() { 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)); + + IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> ft.rangeQuery("random", null, true, true, null)); + + assertEquals("Can't parse boolean value [random], expected [true] or [false]", e.getMessage()); } public void testFetchSourceValue() throws IOException { From 7444538b4b922f511caebe4676dd189c42eb864d Mon Sep 17 00:00:00 2001 From: Harsha Vamsi Kalluri Date: Mon, 29 Apr 2024 11:13:44 -0700 Subject: [PATCH 23/24] Checking if upper and lower terms are valid Signed-off-by: Harsha Vamsi Kalluri --- .../java/org/opensearch/index/query/TermsQueryBuilderTests.java | 1 - 1 file changed, 1 deletion(-) diff --git a/server/src/test/java/org/opensearch/index/query/TermsQueryBuilderTests.java b/server/src/test/java/org/opensearch/index/query/TermsQueryBuilderTests.java index 355de73e100b1..61f7dc5facebe 100644 --- a/server/src/test/java/org/opensearch/index/query/TermsQueryBuilderTests.java +++ b/server/src/test/java/org/opensearch/index/query/TermsQueryBuilderTests.java @@ -41,7 +41,6 @@ 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; From fc94f6ca3d48e2702776ee9f911d77c98350e905 Mon Sep 17 00:00:00 2001 From: Harsha Vamsi Kalluri Date: Mon, 29 Apr 2024 12:08:41 -0700 Subject: [PATCH 24/24] Fixing changelog Signed-off-by: Harsha Vamsi Kalluri --- CHANGELOG.md | 1 - 1 file changed, 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0c8de6dfaaa12..95c1ec308d664 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -59,7 +59,6 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Bump `lycheeverse/lychee-action` from 1.9.3 to 1.10.0 ([#13447](https://github.com/opensearch-project/OpenSearch/pull/13447)) ### Changed -- Add ability for Boolean and date field queries to run when only doc_values are enabled ([#11650](https://github.com/opensearch-project/OpenSearch/pull/11650)) - [BWC and API enforcement] Enforcing the presence of API annotations at build time ([#12872](https://github.com/opensearch-project/OpenSearch/pull/12872)) - Improve built-in secure transports support ([#12907](https://github.com/opensearch-project/OpenSearch/pull/12907)) - Update links to documentation in rest-api-spec ([#13043](https://github.com/opensearch-project/OpenSearch/pull/13043))