Skip to content

Updating exact long and scaled_float queries to make use of docvalues only when available #130088

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

Updating exact long and scaled_float queries to make use of docvalues only when available #130088

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion ...er-extras/src/main/java/org/elasticsearch/index/mapper/extras/ScaledFloatFieldMapper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,7 @@ public boolean isSearchable() {
public Query termQuery(Object value, SearchExecutionContext context) {
failIfNotIndexedNorDocValuesFallback(context);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it has no doc values, it should fail here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that it needs to have both index: false and doc_values: false to fail here, and the default for numeric fields seems to be index: true

long scaledValue = Math.round(scale(value));
return NumberFieldMapper.NumberType.LONG.termQuery(name(), scaledValue, isIndexed());
return NumberFieldMapper.NumberType.LONG.termQuery(name(), scaledValue, isIndexed(), hasDocValues());
}

@Override
Expand Down
121 changes: 121 additions & 0 deletions rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/search/issue129995.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,121 @@
---
setup:
- do:
indices.create:
index: test
body:
mappings:
properties:
id:
type: keyword
long_no_dv:
type: long
doc_values: false
long_dv:
type: long
doc_values: true
scaled_float_no_dv:
type: scaled_float
doc_values: false
scaling_factor: 1.0
scaled_float_dv:
type: scaled_float
doc_values: true
scaling_factor: 1.0

- do:
index:
index: test
body:
id: "1"
long_no_dv: 1

- do:
index:
index: test
body:
id: "2"
long_dv: 2


- do:
index:
index: test
body:
id: "3"
scaled_float_no_dv: 3.0


- do:
index:
index: test
body:
id: "4"
scaled_float_dv: 4.0


- do:
indices.refresh:
index: [test]

---
"Test exact queries on long fields with doc_values":

- do:
search:
rest_total_hits_as_int: true
index: test
body:
query:
match:
long_dv:
query: 2

- match: { hits.total: 1 }
- match: { hits.hits.0._source.id: "2" }

---
"Test exact queries on scaled_float fields with doc_values":
- do:
search:
rest_total_hits_as_int: true
index: test
body:
query:
match:
scaled_float_dv:
query: 4.0

- match: { hits.total: 1 }
- match: { hits.hits.0._source.id: "4" }

---
"Test exact queries on long fields without doc_values":
- do:
search:
rest_total_hits_as_int: true
index: test
body:
query:
match:
long_no_dv:
query: 1

- match: { hits.total: 1 }
- match: { hits.hits.0._source.id: "1" }


---
"Test exact queries on sacled_float fields without doc_values":
- do:
search:
rest_total_hits_as_int: true
index: test
body:
query:
match:
scaled_float_no_dv:
query: 3.0

- match: { hits.total: 1 }
- match: { hits.hits.0._source.id: "3" }
32 changes: 18 additions & 14 deletions server/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -325,7 +325,7 @@ public Float parse(XContentParser parser, boolean coerce) throws IOException {
}

@Override
public Query termQuery(String field, Object value, boolean isIndexed) {
public Query termQuery(String field, Object value, boolean isIndexed, boolean hasDocValues) {
float v = parseToFloat(value);
if (Float.isFinite(HalfFloatPoint.sortableShortToHalfFloat(HalfFloatPoint.halfFloatToSortableShort(v))) == false) {
return Queries.newMatchNoDocsQuery("Value [" + value + "] is out of range");
Expand Down Expand Up @@ -510,7 +510,7 @@ public Float parse(XContentParser parser, boolean coerce) throws IOException {
}

@Override
public Query termQuery(String field, Object value, boolean isIndexed) {
public Query termQuery(String field, Object value, boolean isIndexed, boolean hasDocValues) {
float v = parseToFloat(value);
if (Float.isFinite(v) == false) {
return new MatchNoDocsQuery("Value [" + value + "] is out of range");
Expand Down Expand Up @@ -678,7 +678,7 @@ public FieldValues<Number> compile(String fieldName, Script script, ScriptCompil
}

@Override
public Query termQuery(String field, Object value, boolean isIndexed) {
public Query termQuery(String field, Object value, boolean isIndexed, boolean hasDocValues) {
double v = objectToDouble(value);
if (Double.isFinite(v) == false) {
return Queries.newMatchNoDocsQuery("Value [" + value + "] has a decimal part");
Expand Down Expand Up @@ -833,12 +833,12 @@ public Short parse(XContentParser parser, boolean coerce) throws IOException {
}

@Override
public Query termQuery(String field, Object value, boolean isIndexed) {
public Query termQuery(String field, Object value, boolean isIndexed, boolean hasDocValues) {
if (isOutOfRange(value)) {
return new MatchNoDocsQuery("Value [" + value + "] is out of range");
}

return INTEGER.termQuery(field, value, isIndexed);
return INTEGER.termQuery(field, value, isIndexed, hasDocValues);
}

@Override
Expand Down Expand Up @@ -947,11 +947,11 @@ public Short parse(XContentParser parser, boolean coerce) throws IOException {
}

@Override
public Query termQuery(String field, Object value, boolean isIndexed) {
public Query termQuery(String field, Object value, boolean isIndexed, boolean hasDocValues) {
if (isOutOfRange(value)) {
return Queries.newMatchNoDocsQuery("Value [" + value + "] is out of range");
}
return INTEGER.termQuery(field, value, isIndexed);
return INTEGER.termQuery(field, value, isIndexed, hasDocValues);
}

@Override
Expand Down Expand Up @@ -1063,7 +1063,7 @@ public Integer parse(XContentParser parser, boolean coerce) throws IOException {
}

@Override
public Query termQuery(String field, Object value, boolean isIndexed) {
public Query termQuery(String field, Object value, boolean isIndexed, boolean hasDocValues) {
if (hasDecimalPart(value)) {
return Queries.newMatchNoDocsQuery("Value [" + value + "] has a decimal part");
}
Expand Down Expand Up @@ -1237,7 +1237,7 @@ public FieldValues<Number> compile(String fieldName, Script script, ScriptCompil
}

@Override
public Query termQuery(String field, Object value, boolean isIndexed) {
public Query termQuery(String field, Object value, boolean isIndexed, boolean hasDocValues) {
if (hasDecimalPart(value)) {
return Queries.newMatchNoDocsQuery("Value [" + value + "] has a decimal part");
}
Expand All @@ -1246,10 +1246,14 @@ public Query termQuery(String field, Object value, boolean isIndexed) {
}

long v = parse(value, true);
if (isIndexed) {
return NumericField.newExactLongQuery(field, v);
if (hasDocValues) {
if (isIndexed) {
return NumericField.newExactLongQuery(field, v);
} else {
return SortedNumericDocValuesField.newSlowExactQuery(field, v);
}
} else {
return SortedNumericDocValuesField.newSlowExactQuery(field, v);
return LongPoint.newExactQuery(field, v);
}
}

Expand Down Expand Up @@ -1396,7 +1400,7 @@ public final TypeParser parser() {
return parser;
}

public abstract Query termQuery(String field, Object value, boolean isIndexed);
public abstract Query termQuery(String field, Object value, boolean isIndexed, boolean hasDocValues);

public abstract Query termsQuery(String field, Collection<?> values);

Expand Down Expand Up @@ -1724,7 +1728,7 @@ public boolean isSearchable() {
@Override
public Query termQuery(Object value, SearchExecutionContext context) {
failIfNotIndexedNorDocValuesFallback(context);
return type.termQuery(name(), value, isIndexed());
return type.termQuery(name(), value, isIndexed(), hasDocValues());
}

@Override
Expand Down
18 changes: 14 additions & 4 deletions server/src/test/java/org/elasticsearch/index/mapper/NumberFieldTypeTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,7 @@ public void testTermQueryWithOutOfRangeValues() {
);

for (OutOfRangeTermQueryTestCase testCase : testCases) {
assertTrue(testCase.type.termQuery("field", testCase.value, randomBoolean()) instanceof MatchNoDocsQuery);
assertTrue(testCase.type.termQuery("field", testCase.value, randomBoolean(), randomBoolean()) instanceof MatchNoDocsQuery);
}
}

Expand Down Expand Up @@ -625,6 +625,7 @@ public void testHalfFloatRange() throws IOException {

public void testNegativeZero() {
final boolean isIndexed = randomBoolean();
final boolean hasDocValues = randomBoolean();
assertEquals(
NumberType.DOUBLE.rangeQuery("field", null, -0d, true, true, false, MOCK_CONTEXT, isIndexed),
NumberType.DOUBLE.rangeQuery("field", null, +0d, true, false, false, MOCK_CONTEXT, isIndexed)
Expand All @@ -638,9 +639,18 @@ public void testNegativeZero() {
NumberType.HALF_FLOAT.rangeQuery("field", null, +0f, true, false, false, MOCK_CONTEXT, isIndexed)
);

assertNotEquals(NumberType.DOUBLE.termQuery("field", -0d, isIndexed), NumberType.DOUBLE.termQuery("field", +0d, isIndexed));
assertNotEquals(NumberType.FLOAT.termQuery("field", -0f, isIndexed), NumberType.FLOAT.termQuery("field", +0f, isIndexed));
assertNotEquals(NumberType.HALF_FLOAT.termQuery("field", -0f, isIndexed), NumberType.HALF_FLOAT.termQuery("field", +0f, isIndexed));
assertNotEquals(
NumberType.DOUBLE.termQuery("field", -0d, isIndexed, hasDocValues),
NumberType.DOUBLE.termQuery("field", +0d, isIndexed, hasDocValues)
);
assertNotEquals(
NumberType.FLOAT.termQuery("field", -0f, isIndexed, hasDocValues),
NumberType.FLOAT.termQuery("field", +0f, isIndexed, hasDocValues)
);
assertNotEquals(
NumberType.HALF_FLOAT.termQuery("field", -0f, isIndexed, hasDocValues),
NumberType.HALF_FLOAT.termQuery("field", +0f, isIndexed, hasDocValues)
);
}

// Make sure we construct the IndexOrDocValuesQuery objects with queries that match
Expand Down