Skip to content

Commit 75c01a7

Browse files
ioanatiaelasticmachine
authored andcommitted
Enable tests for out of range comparisons for float/half_float fields (elastic#113122)
* Enable tests for out of range comparisons for float/half_float fields * Address feedback comments * Implement suggestions --------- Co-authored-by: Elastic Machine <[email protected]>
1 parent 97c5956 commit 75c01a7

File tree

2 files changed

+62
-14
lines changed

2 files changed

+62
-14
lines changed

x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/RestEsqlTestCase.java

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -398,9 +398,8 @@ public void testOutOfRangeComparisons() throws IOException {
398398
"long",
399399
// TODO: https://github.com/elastic/elasticsearch/issues/102935
400400
// "unsigned_long",
401-
// TODO: https://github.com/elastic/elasticsearch/issues/100130
402-
// "half_float",
403-
// "float",
401+
"half_float",
402+
"float",
404403
"double",
405404
"scaled_float"
406405
);

x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/LocalPhysicalPlanOptimizerTests.java

Lines changed: 60 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
import org.elasticsearch.index.query.MatchQueryBuilder;
2222
import org.elasticsearch.index.query.QueryBuilder;
2323
import org.elasticsearch.index.query.QueryBuilders;
24+
import org.elasticsearch.index.query.RangeQueryBuilder;
2425
import org.elasticsearch.index.query.SearchExecutionContext;
2526
import org.elasticsearch.xpack.core.enrich.EnrichPolicy;
2627
import org.elasticsearch.xpack.esql.EsqlTestUtils;
@@ -147,6 +148,10 @@ private Analyzer makeAnalyzer(String mappingFileName, EnrichResolution enrichRes
147148
);
148149
}
149150

151+
private Analyzer makeAnalyzer(String mappingFileName) {
152+
return makeAnalyzer(mappingFileName, new EnrichResolution());
153+
}
154+
150155
/**
151156
* Expects
152157
* LimitExec[1000[INTEGER]]
@@ -449,7 +454,7 @@ public void testQueryStringFunctionWithFunctionsPushedToLucene() {
449454
from test
450455
| where qstr("last_name: Smith") and cidr_match(ip, "127.0.0.1/32")
451456
""";
452-
var analyzer = makeAnalyzer("mapping-all-types.json", new EnrichResolution());
457+
var analyzer = makeAnalyzer("mapping-all-types.json");
453458
var plan = plannerOptimizer.plan(queryText, IS_SV_STATS, analyzer);
454459

455460
var limit = as(plan, LimitExec.class);
@@ -610,7 +615,7 @@ public void testMatchFunctionWithFunctionsPushedToLucene() {
610615
from test
611616
| where match(text, "beta") and cidr_match(ip, "127.0.0.1/32")
612617
""";
613-
var analyzer = makeAnalyzer("mapping-all-types.json", new EnrichResolution());
618+
var analyzer = makeAnalyzer("mapping-all-types.json");
614619
var plan = plannerOptimizer.plan(queryText, IS_SV_STATS, analyzer);
615620

616621
var limit = as(plan, LimitExec.class);
@@ -892,8 +897,15 @@ public void testIsNotNull_TextField_Pushdown_WithCount() {
892897

893898
private record OutOfRangeTestCase(String fieldName, String tooLow, String tooHigh) {};
894899

900+
private static final String LT = "<";
901+
private static final String LTE = "<=";
902+
private static final String GT = ">";
903+
private static final String GTE = ">=";
904+
private static final String EQ = "==";
905+
private static final String NEQ = "!=";
906+
895907
public void testOutOfRangeFilterPushdown() {
896-
var allTypeMappingAnalyzer = makeAnalyzer("mapping-all-types.json", new EnrichResolution());
908+
var allTypeMappingAnalyzer = makeAnalyzer("mapping-all-types.json");
897909

898910
String largerThanInteger = String.valueOf(randomLongBetween(Integer.MAX_VALUE + 1L, Long.MAX_VALUE));
899911
String smallerThanInteger = String.valueOf(randomLongBetween(Long.MIN_VALUE, Integer.MIN_VALUE - 1L));
@@ -910,16 +922,8 @@ public void testOutOfRangeFilterPushdown() {
910922
new OutOfRangeTestCase("integer", smallerThanInteger, largerThanInteger),
911923
new OutOfRangeTestCase("long", smallerThanLong, largerThanLong)
912924
// TODO: add unsigned_long https://github.com/elastic/elasticsearch/issues/102935
913-
// TODO: add half_float, float https://github.com/elastic/elasticsearch/issues/100130
914925
);
915926

916-
final String LT = "<";
917-
final String LTE = "<=";
918-
final String GT = ">";
919-
final String GTE = ">=";
920-
final String EQ = "==";
921-
final String NEQ = "!=";
922-
923927
for (OutOfRangeTestCase testCase : cases) {
924928
List<String> trueForSingleValuesPredicates = List.of(
925929
LT + testCase.tooHigh,
@@ -972,6 +976,51 @@ public void testOutOfRangeFilterPushdown() {
972976
}
973977
}
974978

979+
public void testOutOfRangeFilterPushdownWithFloatAndHalfFloat() {
980+
var allTypeMappingAnalyzer = makeAnalyzer("mapping-all-types.json");
981+
982+
String smallerThanFloat = String.valueOf(randomDoubleBetween(-Double.MAX_VALUE, -Float.MAX_VALUE - 1d, true));
983+
String largerThanFloat = String.valueOf(randomDoubleBetween(Float.MAX_VALUE + 1d, Double.MAX_VALUE, true));
984+
985+
List<OutOfRangeTestCase> cases = List.of(
986+
new OutOfRangeTestCase("float", smallerThanFloat, largerThanFloat),
987+
new OutOfRangeTestCase("half_float", smallerThanFloat, largerThanFloat)
988+
);
989+
990+
for (OutOfRangeTestCase testCase : cases) {
991+
for (var value : List.of(testCase.tooHigh, testCase.tooLow)) {
992+
for (String predicate : List.of(LT, LTE, GT, GTE, EQ, NEQ)) {
993+
String comparison = testCase.fieldName + predicate + value;
994+
var query = "from test | where " + comparison;
995+
996+
Source expectedSource = new Source(1, 18, comparison);
997+
998+
logger.info("Query: " + query);
999+
EsQueryExec actualQueryExec = doTestOutOfRangeFilterPushdown(query, allTypeMappingAnalyzer);
1000+
1001+
assertThat(actualQueryExec.query(), is(instanceOf(SingleValueQuery.Builder.class)));
1002+
var actualLuceneQuery = (SingleValueQuery.Builder) actualQueryExec.query();
1003+
assertThat(actualLuceneQuery.field(), equalTo(testCase.fieldName));
1004+
assertThat(actualLuceneQuery.source(), equalTo(expectedSource));
1005+
1006+
QueryBuilder actualInnerLuceneQuery = actualLuceneQuery.next();
1007+
1008+
if (predicate.equals(EQ)) {
1009+
QueryBuilder expectedInnerQuery = QueryBuilders.termQuery(testCase.fieldName, Double.parseDouble(value));
1010+
assertThat(actualInnerLuceneQuery, equalTo(expectedInnerQuery));
1011+
} else if (predicate.equals(NEQ)) {
1012+
QueryBuilder expectedInnerQuery = QueryBuilders.boolQuery()
1013+
.mustNot(QueryBuilders.termQuery(testCase.fieldName, Double.parseDouble(value)));
1014+
assertThat(actualInnerLuceneQuery, equalTo(expectedInnerQuery));
1015+
} else { // one of LT, LTE, GT, GTE
1016+
assertTrue(actualInnerLuceneQuery instanceof RangeQueryBuilder);
1017+
assertThat(((RangeQueryBuilder) actualInnerLuceneQuery).fieldName(), equalTo(testCase.fieldName));
1018+
}
1019+
}
1020+
}
1021+
}
1022+
}
1023+
9751024
/**
9761025
* Expects e.g.
9771026
* LimitExec[1000[INTEGER]]

0 commit comments

Comments
 (0)