Skip to content

Commit

Permalink
Updating numeric term and terms queries to use IODVQ
Browse files Browse the repository at this point in the history 
Signed-off-by: Harsha Vamsi Kalluri <[email protected]>
  • Loading branch information
@harshavamsi
harshavamsi committed Nov 15, 2023
1 parent 57ec29e commit d97865e
Show file tree
Hide file tree
Showing 4 changed files with 46 additions and 37 deletions.
8 changes: 4 additions & 4 deletions modules/mapper-extras/src/main/java/org/opensearch/index/mapper/ScaledFloatFieldMapper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}
Expand All @@ -210,7 +210,7 @@ public Query termQuery(Object value, QueryShardContext context) {

@Override
public Query termsQuery(List<?> values, QueryShardContext context) {
failIfNotIndexed();
failIfNotIndexedAndNoDocValues();
List<Long> scaledValues = new ArrayList<>(values.size());
for (Object value : values) {
long scaledValue = Math.round(scale(value));
Expand All @@ -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);
Expand Down
8 changes: 6 additions & 2 deletions ...es/mapper-extras/src/test/java/org/opensearch/index/mapper/ScaledFloatFieldTypeTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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() {
Expand All @@ -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 {
Expand Down
37 changes: 19 additions & 18 deletions server/src/main/java/org/opensearch/index/mapper/NumberFieldMapper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand All @@ -219,7 +221,7 @@ public Query termsQuery(String field, List<Object> 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
Expand Down Expand Up @@ -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;
Expand All @@ -333,7 +334,7 @@ public Query termsQuery(String field, List<Object> 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
Expand Down Expand Up @@ -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;
Expand All @@ -436,7 +436,7 @@ public Query termsQuery(String field, List<Object> 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
Expand Down Expand Up @@ -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);
}
Expand All @@ -688,7 +688,7 @@ public Query termsQuery(String field, List<Object> values) {
if (upTo != v.length) {
v = Arrays.copyOf(v, upTo);
}
return IntPoint.newSetQuery(field, v);
return IntField.newSetQuery(field, v);
}

@Override
Expand Down Expand Up @@ -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);
}
Expand All @@ -807,7 +807,7 @@ public Query termsQuery(String field, List<Object> values) {
if (upTo != v.length) {
v = Arrays.copyOf(v, upTo);
}
return LongPoint.newSetQuery(field, v);
return LongField.newSetQuery(field, v);
}

@Override
Expand Down Expand Up @@ -897,6 +897,7 @@ public Query termsQuery(String field, List<Object> values) {
v = Arrays.copyOf(v, upTo);
}

// TODO: replace this with IODVQ
return BigIntegerPoint.newSetQuery(field, v);
}

Expand Down
30 changes: 17 additions & 13 deletions server/src/test/java/org/opensearch/index/mapper/NumberFieldTypeTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}

Expand All @@ -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() {
Expand Down Expand Up @@ -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() {
Expand All @@ -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() {
Expand All @@ -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() {
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit d97865e

Please sign in to comment.