From d97865e04b49df2671427e37838602bdc5b436ac Mon Sep 17 00:00:00 2001 From: Harsha Vamsi Kalluri Date: Wed, 15 Nov 2023 14:57:43 -0800 Subject: [PATCH] Updating numeric term and terms queries to use IODVQ Signed-off-by: Harsha Vamsi Kalluri --- .../index/mapper/ScaledFloatFieldMapper.java | 8 ++-- .../mapper/ScaledFloatFieldTypeTests.java | 8 +++- .../index/mapper/NumberFieldMapper.java | 37 ++++++++++--------- .../index/mapper/NumberFieldTypeTests.java | 30 ++++++++------- 4 files changed, 46 insertions(+), 37 deletions(-) diff --git a/modules/mapper-extras/src/main/java/org/opensearch/index/mapper/ScaledFloatFieldMapper.java b/modules/mapper-extras/src/main/java/org/opensearch/index/mapper/ScaledFloatFieldMapper.java index 7be241017f683..c21c1aa723e13 100644 --- a/modules/mapper-extras/src/main/java/org/opensearch/index/mapper/ScaledFloatFieldMapper.java +++ b/modules/mapper-extras/src/main/java/org/opensearch/index/mapper/ScaledFloatFieldMapper.java @@ -199,9 +199,9 @@ public String typeName() { @Override public Query termQuery(Object value, QueryShardContext context) { - failIfNotIndexed(); + failIfNotIndexedAndNoDocValues(); long scaledValue = Math.round(scale(value)); - Query query = NumberFieldMapper.NumberType.LONG.termQuery(name(), scaledValue); + Query query = NumberFieldMapper.NumberType.LONG.termQuery(name(), scaledValue, hasDocValues()); if (boost() != 1f) { query = new BoostQuery(query, boost()); } @@ -210,7 +210,7 @@ public Query termQuery(Object value, QueryShardContext context) { @Override public Query termsQuery(List values, QueryShardContext context) { - failIfNotIndexed(); + failIfNotIndexedAndNoDocValues(); List scaledValues = new ArrayList<>(values.size()); for (Object value : values) { long scaledValue = Math.round(scale(value)); @@ -225,7 +225,7 @@ public Query termsQuery(List values, QueryShardContext context) { @Override public Query rangeQuery(Object lowerTerm, Object upperTerm, boolean includeLower, boolean includeUpper, QueryShardContext context) { - failIfNotIndexed(); + failIfNotIndexedAndNoDocValues(); Long lo = null; if (lowerTerm != null) { double dValue = scale(lowerTerm); diff --git a/modules/mapper-extras/src/test/java/org/opensearch/index/mapper/ScaledFloatFieldTypeTests.java b/modules/mapper-extras/src/test/java/org/opensearch/index/mapper/ScaledFloatFieldTypeTests.java index be12c49321b87..53ab651afde8e 100644 --- a/modules/mapper-extras/src/test/java/org/opensearch/index/mapper/ScaledFloatFieldTypeTests.java +++ b/modules/mapper-extras/src/test/java/org/opensearch/index/mapper/ScaledFloatFieldTypeTests.java @@ -34,11 +34,13 @@ import org.apache.lucene.document.Document; import org.apache.lucene.document.DoublePoint; +import org.apache.lucene.document.LongField; import org.apache.lucene.document.LongPoint; import org.apache.lucene.document.SortedNumericDocValuesField; import org.apache.lucene.index.DirectoryReader; import org.apache.lucene.index.IndexWriter; import org.apache.lucene.index.IndexWriterConfig; +import org.apache.lucene.search.IndexOrDocValuesQuery; import org.apache.lucene.search.IndexSearcher; import org.apache.lucene.search.Query; import org.apache.lucene.store.Directory; @@ -63,7 +65,9 @@ public void testTermQuery() { ); double value = (randomDouble() * 2 - 1) * 10000; long scaledValue = Math.round(value * ft.getScalingFactor()); - assertEquals(LongPoint.newExactQuery("scaled_float", scaledValue), ft.termQuery(value, null)); + Query dvQuery = SortedNumericDocValuesField.newSlowExactQuery("scaled_float", scaledValue); + Query query = new IndexOrDocValuesQuery(LongPoint.newExactQuery("scaled_float", scaledValue), dvQuery); + assertEquals(query, ft.termQuery(value, null)); } public void testTermsQuery() { @@ -75,7 +79,7 @@ public void testTermsQuery() { long scaledValue1 = Math.round(value1 * ft.getScalingFactor()); double value2 = (randomDouble() * 2 - 1) * 10000; long scaledValue2 = Math.round(value2 * ft.getScalingFactor()); - assertEquals(LongPoint.newSetQuery("scaled_float", scaledValue1, scaledValue2), ft.termsQuery(Arrays.asList(value1, value2), null)); + assertEquals(LongField.newSetQuery("scaled_float", scaledValue1, scaledValue2), ft.termsQuery(Arrays.asList(value1, value2), null)); } public void testRangeQuery() throws IOException { diff --git a/server/src/main/java/org/opensearch/index/mapper/NumberFieldMapper.java b/server/src/main/java/org/opensearch/index/mapper/NumberFieldMapper.java index 91ca2de3fe639..030f7d97eeb9f 100644 --- a/server/src/main/java/org/opensearch/index/mapper/NumberFieldMapper.java +++ b/server/src/main/java/org/opensearch/index/mapper/NumberFieldMapper.java @@ -35,10 +35,14 @@ import com.fasterxml.jackson.core.JsonParseException; import com.fasterxml.jackson.core.exc.InputCoercionException; +import org.apache.lucene.document.DoubleField; import org.apache.lucene.document.DoublePoint; import org.apache.lucene.document.Field; +import org.apache.lucene.document.FloatField; import org.apache.lucene.document.FloatPoint; +import org.apache.lucene.document.IntField; import org.apache.lucene.document.IntPoint; +import org.apache.lucene.document.LongField; import org.apache.lucene.document.LongPoint; import org.apache.lucene.document.SortedNumericDocValuesField; import org.apache.lucene.document.StoredField; @@ -204,10 +208,8 @@ public Float parse(XContentParser parser, boolean coerce) throws IOException { public Query termQuery(String field, Object value, Boolean hasDocValues) { float v = parse(value, false); Query query = HalfFloatPoint.newExactQuery(field, v); - if(hasDocValues){ - Query dvQuery = SortedNumericDocValuesField.newSlowExactQuery( - field, HalfFloatPoint.halfFloatToSortableShort(v) - ); + if (hasDocValues) { + Query dvQuery = SortedNumericDocValuesField.newSlowExactQuery(field, HalfFloatPoint.halfFloatToSortableShort(v)); query = new IndexOrDocValuesQuery(query, dvQuery); } return query; @@ -219,7 +221,7 @@ public Query termsQuery(String field, List values) { for (int i = 0; i < values.size(); ++i) { v[i] = parse(values.get(i), false); } - return HalfFloatPoint.newSetQuery(field, v); + return FloatField.newSetQuery(field, v); } @Override @@ -318,10 +320,9 @@ public Float parse(XContentParser parser, boolean coerce) throws IOException { @Override public Query termQuery(String field, Object value, Boolean hasDocValues) { float v = parse(value, false); - Query query = FloatPoint.newExactQuery(field, v); - if(hasDocValues){ - Query dvQuery = SortedNumericDocValuesField.newSlowExactQuery(field, - NumericUtils.floatToSortableInt(v)); + Query query = FloatPoint.newExactQuery(field, v); + if (hasDocValues) { + Query dvQuery = SortedNumericDocValuesField.newSlowExactQuery(field, NumericUtils.floatToSortableInt(v)); query = new IndexOrDocValuesQuery(query, dvQuery); } return query; @@ -333,7 +334,7 @@ public Query termsQuery(String field, List values) { for (int i = 0; i < values.size(); ++i) { v[i] = parse(values.get(i), false); } - return FloatPoint.newSetQuery(field, v); + return FloatField.newSetQuery(field, v); } @Override @@ -422,9 +423,8 @@ public Double parse(XContentParser parser, boolean coerce) throws IOException { public Query termQuery(String field, Object value, Boolean hasDocValues) { double v = parse(value, false); Query query = DoublePoint.newExactQuery(field, v); - if(hasDocValues){ - Query dvQuery = SortedNumericDocValuesField.newSlowExactQuery(field, - NumericUtils.doubleToSortableLong(v)); + if (hasDocValues) { + Query dvQuery = SortedNumericDocValuesField.newSlowExactQuery(field, NumericUtils.doubleToSortableLong(v)); query = new IndexOrDocValuesQuery(query, dvQuery); } return query; @@ -436,7 +436,7 @@ public Query termsQuery(String field, List values) { for (int i = 0; i < values.size(); ++i) { v[i] = parse(values.get(i), false); } - return DoublePoint.newSetQuery(field, v); + return DoubleField.newSetQuery(field, v); } @Override @@ -663,7 +663,7 @@ public Query termQuery(String field, Object value, Boolean hasDocValues) { } int v = parse(value, true); Query query = IntPoint.newExactQuery(field, v); - if(hasDocValues){ + if (hasDocValues) { Query dvQuery = SortedNumericDocValuesField.newSlowExactQuery(field, v); query = new IndexOrDocValuesQuery(query, dvQuery); } @@ -688,7 +688,7 @@ public Query termsQuery(String field, List values) { if (upTo != v.length) { v = Arrays.copyOf(v, upTo); } - return IntPoint.newSetQuery(field, v); + return IntField.newSetQuery(field, v); } @Override @@ -782,7 +782,7 @@ public Query termQuery(String field, Object value, Boolean hasDocValues) { } long v = parse(value, true); Query query = LongPoint.newExactQuery(field, v); - if(hasDocValues){ + if (hasDocValues) { Query dvQuery = SortedNumericDocValuesField.newSlowExactQuery(field, v); query = new IndexOrDocValuesQuery(query, dvQuery); } @@ -807,7 +807,7 @@ public Query termsQuery(String field, List values) { if (upTo != v.length) { v = Arrays.copyOf(v, upTo); } - return LongPoint.newSetQuery(field, v); + return LongField.newSetQuery(field, v); } @Override @@ -897,6 +897,7 @@ public Query termsQuery(String field, List values) { v = Arrays.copyOf(v, upTo); } + // TODO: replace this with IODVQ return BigIntegerPoint.newSetQuery(field, v); } diff --git a/server/src/test/java/org/opensearch/index/mapper/NumberFieldTypeTests.java b/server/src/test/java/org/opensearch/index/mapper/NumberFieldTypeTests.java index 3c30bb81a9a32..bdcf867a24d89 100644 --- a/server/src/test/java/org/opensearch/index/mapper/NumberFieldTypeTests.java +++ b/server/src/test/java/org/opensearch/index/mapper/NumberFieldTypeTests.java @@ -37,7 +37,9 @@ import org.apache.lucene.document.Document; import org.apache.lucene.document.DoublePoint; import org.apache.lucene.document.FloatPoint; +import org.apache.lucene.document.IntField; import org.apache.lucene.document.IntPoint; +import org.apache.lucene.document.LongField; import org.apache.lucene.document.LongPoint; import org.apache.lucene.document.SortedNumericDocValuesField; import org.apache.lucene.index.DirectoryReader; @@ -118,15 +120,15 @@ public void testIsFieldWithinQuery() throws IOException { public void testIntegerTermsQueryWithDecimalPart() { MappedFieldType ft = new NumberFieldMapper.NumberFieldType("field", NumberType.INTEGER); - assertEquals(IntPoint.newSetQuery("field", 1), ft.termsQuery(Arrays.asList(1, 2.1), null)); - assertEquals(IntPoint.newSetQuery("field", 1), ft.termsQuery(Arrays.asList(1.0, 2.1), null)); + assertEquals(IntField.newSetQuery("field", 1), ft.termsQuery(Arrays.asList(1, 2.1), null)); + assertEquals(IntField.newSetQuery("field", 1), ft.termsQuery(Arrays.asList(1.0, 2.1), null)); assertTrue(ft.termsQuery(Arrays.asList(1.1, 2.1), null) instanceof MatchNoDocsQuery); } public void testLongTermsQueryWithDecimalPart() { MappedFieldType ft = new NumberFieldMapper.NumberFieldType("field", NumberType.LONG); - assertEquals(LongPoint.newSetQuery("field", 1), ft.termsQuery(Arrays.asList(1, 2.1), null)); - assertEquals(LongPoint.newSetQuery("field", 1), ft.termsQuery(Arrays.asList(1.0, 2.1), null)); + assertEquals(LongField.newSetQuery("field", 1), ft.termsQuery(Arrays.asList(1, 2.1), null)); + assertEquals(LongField.newSetQuery("field", 1), ft.termsQuery(Arrays.asList(1.0, 2.1), null)); assertTrue(ft.termsQuery(Arrays.asList(1.1, 2.1), null) instanceof MatchNoDocsQuery); } @@ -151,16 +153,18 @@ public void testLongTermQueryWithDecimalPart() { } private static MappedFieldType unsearchable() { - return new NumberFieldType("field", NumberType.LONG, false, false, true, true, null, Collections.emptyMap()); + return new NumberFieldType("field", NumberType.LONG, false, false, false, true, null, Collections.emptyMap()); } public void testTermQuery() { MappedFieldType ft = new NumberFieldMapper.NumberFieldType("field", NumberFieldMapper.NumberType.LONG); - assertEquals(LongPoint.newExactQuery("field", 42), ft.termQuery("42", null)); + Query dvQuery = SortedNumericDocValuesField.newSlowExactQuery("field", 42); + Query query = new IndexOrDocValuesQuery(LongPoint.newExactQuery("field", 42), dvQuery); + assertEquals(query, ft.termQuery("42", null)); MappedFieldType unsearchable = unsearchable(); IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> unsearchable.termQuery("42", 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 testRangeQueryWithNegativeBounds() { @@ -380,7 +384,7 @@ public void testLongRangeQuery() { IllegalArgumentException.class, () -> unsearchable.rangeQuery("1", "3", true, true, null, null, null, MOCK_QSC) ); - 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 testUnsignedLongRangeQuery() { @@ -396,7 +400,7 @@ public void testUnsignedLongRangeQuery() { IllegalArgumentException.class, () -> unsearchable.rangeQuery("1", "3", true, true, null, null, null, MOCK_QSC) ); - 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 testDoubleRangeQuery() { @@ -416,7 +420,7 @@ public void testDoubleRangeQuery() { IllegalArgumentException.class, () -> unsearchable.rangeQuery("1", "3", true, true, null, null, null, MOCK_QSC) ); - 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 testConversions() { @@ -570,9 +574,9 @@ public void testNegativeZero() { NumberType.HALF_FLOAT.rangeQuery("field", null, +0f, true, false, false, MOCK_QSC) ); - assertFalse(NumberType.DOUBLE.termQuery("field", -0d).equals(NumberType.DOUBLE.termQuery("field", +0d))); - assertFalse(NumberType.FLOAT.termQuery("field", -0f).equals(NumberType.FLOAT.termQuery("field", +0f))); - assertFalse(NumberType.HALF_FLOAT.termQuery("field", -0f).equals(NumberType.HALF_FLOAT.termQuery("field", +0f))); + assertFalse(NumberType.DOUBLE.termQuery("field", -0d, true).equals(NumberType.DOUBLE.termQuery("field", +0d, true))); + assertFalse(NumberType.FLOAT.termQuery("field", -0f, true).equals(NumberType.FLOAT.termQuery("field", +0f, true))); + assertFalse(NumberType.HALF_FLOAT.termQuery("field", -0f, true).equals(NumberType.HALF_FLOAT.termQuery("field", +0f, true))); } // Make sure we construct the IndexOrDocValuesQuery objects with queries that match