Skip to content

Commit e543a0b

Browse files
Null values need to be converted to correct type for numeric matchers (#133506)
A recent change to the random testing numeric matchers use specific Java types for the related matcher rather than Integer for all integer types and Double for all double types. The same needs to be done for values which are provide as null_values, so that the null_value replacement and regular values can be tested correctly for equality.
1 parent 6d0711a commit e543a0b

File tree

3 files changed

+169
-72
lines changed

3 files changed

+169
-72
lines changed

muted-tests.yml

Lines changed: 0 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -612,60 +612,18 @@ tests:
612612
- class: org.elasticsearch.xpack.esql.action.CrossClusterAsyncQueryStopIT
613613
method: testStopQueryLocal
614614
issue: https://github.com/elastic/elasticsearch/issues/133481
615-
- class: org.elasticsearch.xpack.logsdb.qa.StandardVersusStandardReindexedIntoLogsDbChallengeRestIT
616-
method: testEsqlSource
617-
issue: https://github.com/elastic/elasticsearch/issues/132601
618-
- class: org.elasticsearch.xpack.logsdb.qa.BulkChallengeRestIT
619-
method: testEsqlSource
620-
issue: https://github.com/elastic/elasticsearch/issues/132600
621-
- class: org.elasticsearch.xpack.logsdb.qa.StoredSourceLogsDbVersusReindexedLogsDbChallengeRestIT
622-
method: testEsqlSource
623-
issue: https://github.com/elastic/elasticsearch/issues/132602
624-
- class: org.elasticsearch.xpack.logsdb.qa.BulkChallengeRestIT
625-
method: testMatchAllQuery
626-
issue: https://github.com/elastic/elasticsearch/issues/133497
627-
- class: org.elasticsearch.xpack.logsdb.qa.StandardVersusStandardReindexedIntoLogsDbChallengeRestIT
628-
method: testMatchAllQuery
629-
issue: https://github.com/elastic/elasticsearch/issues/133498
630-
- class: org.elasticsearch.xpack.logsdb.qa.StoredSourceLogsDbVersusReindexedLogsDbChallengeRestIT
631-
method: testMatchAllQuery
632-
issue: https://github.com/elastic/elasticsearch/issues/133499
633-
- class: org.elasticsearch.xpack.logsdb.qa.BulkChallengeRestIT
634-
method: testRandomQueries
635-
issue: https://github.com/elastic/elasticsearch/issues/133503
636-
- class: org.elasticsearch.xpack.logsdb.qa.StandardVersusStandardReindexedIntoLogsDbChallengeRestIT
637-
method: testRandomQueries
638-
issue: https://github.com/elastic/elasticsearch/issues/133504
639-
- class: org.elasticsearch.xpack.logsdb.qa.StoredSourceLogsDbVersusReindexedLogsDbChallengeRestIT
640-
method: testRandomQueries
641-
issue: https://github.com/elastic/elasticsearch/issues/133505
642615
- class: org.elasticsearch.xpack.esql.qa.mixed.MixedClusterEsqlSpecIT
643616
method: test {csv-spec:spatial.ConvertFromStringParseError}
644617
issue: https://github.com/elastic/elasticsearch/issues/133507
645618
- class: org.elasticsearch.test.rest.yaml.RcsCcsCommonYamlTestSuiteIT
646619
method: test {p0=search.vectors/90_sparse_vector/Indexing and searching multi-value sparse vectors in >=8.15}
647620
issue: https://github.com/elastic/elasticsearch/issues/133508
648-
- class: org.elasticsearch.xpack.logsdb.qa.BulkChallengeRestIT
649-
method: testTermsQuery
650-
issue: https://github.com/elastic/elasticsearch/issues/133516
651-
- class: org.elasticsearch.xpack.logsdb.qa.StandardVersusStandardReindexedIntoLogsDbChallengeRestIT
652-
method: testTermsQuery
653-
issue: https://github.com/elastic/elasticsearch/issues/133517
654-
- class: org.elasticsearch.xpack.logsdb.qa.StoredSourceLogsDbVersusReindexedLogsDbChallengeRestIT
655-
method: testTermsQuery
656-
issue: https://github.com/elastic/elasticsearch/issues/133518
657-
- class: org.elasticsearch.xpack.logsdb.qa.LogsDbVersusReindexedLogsDbChallengeRestIT
658-
method: testEsqlSource
659-
issue: https://github.com/elastic/elasticsearch/issues/133520
660621
- class: org.elasticsearch.xpack.esql.action.EsqlActionBreakerIT
661622
method: testTopNPushedToLucene
662623
issue: https://github.com/elastic/elasticsearch/issues/133556
663624
- class: org.elasticsearch.xpack.esql.action.EsqlActionBreakerIT
664625
method: testFilterNestedFields
665626
issue: https://github.com/elastic/elasticsearch/issues/133557
666-
- class: org.elasticsearch.xpack.logsdb.qa.LogsDbVersusReindexedLogsDbChallengeRestIT
667-
method: testMatchAllQuery
668-
issue: https://github.com/elastic/elasticsearch/issues/133560
669627
- class: org.elasticsearch.test.rest.yaml.RcsCcsCommonYamlTestSuiteIT
670628
method: test {p0=search/10_source_filtering/no filtering}
671629
issue: https://github.com/elastic/elasticsearch/issues/133561

test/framework/src/main/java/org/elasticsearch/datageneration/matchers/source/FieldSpecificMatcher.java

Lines changed: 121 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -51,13 +51,13 @@ static Map<String, FieldSpecificMatcher> matchers(
5151
return new HashMap<>() {
5252
{
5353
put("keyword", new KeywordMatcher(actualMappings, actualSettings, expectedMappings, expectedSettings));
54-
put("long", new NumberMatcher("long", actualMappings, actualSettings, expectedMappings, expectedSettings));
54+
put("long", new LongMatcher(actualMappings, actualSettings, expectedMappings, expectedSettings));
5555
put("unsigned_long", new UnsignedLongMatcher(actualMappings, actualSettings, expectedMappings, expectedSettings));
56-
put("integer", new NumberMatcher("integer", actualMappings, actualSettings, expectedMappings, expectedSettings));
57-
put("short", new NumberMatcher("short", actualMappings, actualSettings, expectedMappings, expectedSettings));
58-
put("byte", new NumberMatcher("byte", actualMappings, actualSettings, expectedMappings, expectedSettings));
59-
put("double", new NumberMatcher("double", actualMappings, actualSettings, expectedMappings, expectedSettings));
60-
put("float", new NumberMatcher("float", actualMappings, actualSettings, expectedMappings, expectedSettings));
56+
put("integer", new IntegerMatcher(actualMappings, actualSettings, expectedMappings, expectedSettings));
57+
put("short", new ShortMatcher(actualMappings, actualSettings, expectedMappings, expectedSettings));
58+
put("byte", new ByteMatcher(actualMappings, actualSettings, expectedMappings, expectedSettings));
59+
put("double", new DoubleMatcher(actualMappings, actualSettings, expectedMappings, expectedSettings));
60+
put("float", new FloatMatcher(actualMappings, actualSettings, expectedMappings, expectedSettings));
6161
put("half_float", new HalfFloatMatcher(actualMappings, actualSettings, expectedMappings, expectedSettings));
6262
put("scaled_float", new ScaledFloatMatcher(actualMappings, actualSettings, expectedMappings, expectedSettings));
6363
put("counted_keyword", new CountedKeywordMatcher(actualMappings, actualSettings, expectedMappings, expectedSettings));
@@ -327,9 +327,15 @@ Object convert(Object value, Object nullValue) {
327327
yield nullValueBigInt;
328328
}
329329

330-
yield s;
330+
try {
331+
yield new BigInteger(s);
332+
} catch (NumberFormatException e) {
333+
// malformed
334+
yield value;
335+
}
331336
}
332337
case Long l -> BigInteger.valueOf(l);
338+
case Integer i -> BigInteger.valueOf(i);
333339
default -> value;
334340
};
335341

@@ -356,31 +362,30 @@ Object convert(Object value, Object nullValue) {
356362
}
357363
}
358364

359-
class NumberMatcher extends GenericMappingAwareMatcher {
365+
abstract class NumberMatcher extends GenericMappingAwareMatcher {
360366

361-
private final FieldType fieldType;
362367
private final NumberFieldMapper.NumberType numberType;
363368

364-
NumberMatcher(
365-
String fieldType,
369+
private NumberMatcher(
370+
FieldType fieldType,
366371
XContentBuilder actualMappings,
367372
Settings.Builder actualSettings,
368373
XContentBuilder expectedMappings,
369374
Settings.Builder expectedSettings
370375
) {
371-
super(fieldType, actualMappings, actualSettings, expectedMappings, expectedSettings);
372-
this.fieldType = FieldType.tryParse(fieldType);
373-
this.numberType = NumberFieldMapper.NumberType.valueOf(this.fieldType.name());
376+
super(fieldType.toString(), actualMappings, actualSettings, expectedMappings, expectedSettings);
377+
this.numberType = NumberFieldMapper.NumberType.valueOf(fieldType.name());
374378
}
375379

376380
@Override
377381
Object convert(Object value, Object nullValue) {
378382
if (value == null) {
379-
return nullValue;
383+
return cast(nullValue);
380384
}
385+
381386
// Special case for number coercion from strings
382387
if (value instanceof String s && s.isEmpty()) {
383-
return nullValue;
388+
return cast(nullValue);
384389
}
385390

386391
// Attempt to coerce string values into numbers
@@ -394,25 +399,111 @@ Object convert(Object value, Object nullValue) {
394399
}
395400
}
396401

397-
// When a number mapping is coerced, the expected value will come from the above parser and will have the correct java type.
398-
// Whereas, if it fits, the actual value will be in an Integer or a Double. To correctly treat expected and actual values as
399-
// equal the actual value must be cast to the appropriate type.
400-
if (value instanceof Integer v) {
401-
return switch (fieldType) {
402-
case LONG -> v.longValue();
403-
case SHORT -> v.shortValue();
404-
case BYTE -> v.byteValue();
405-
default -> value;
406-
};
407-
}
408-
if (value instanceof Double v) {
409-
return fieldType == FieldType.FLOAT ? v.floatValue() : value;
410-
}
402+
return cast(value);
403+
}
404+
405+
// When a number mapping is coerced, the expected value will come from the above parser and will have the correct java type.
406+
// Whereas, if it fits, the actual value will be in an Integer or a Double. To correctly treat expected and actual values as
407+
// equal the actual value must be cast to the appropriate type.
408+
abstract Object cast(Object value);
409+
}
410+
411+
class LongMatcher extends NumberMatcher {
412+
LongMatcher(
413+
XContentBuilder actualMappings,
414+
Settings.Builder actualSettings,
415+
XContentBuilder expectedMappings,
416+
Settings.Builder expectedSettings
417+
) {
418+
super(FieldType.LONG, actualMappings, actualSettings, expectedMappings, expectedSettings);
419+
}
420+
421+
@Override
422+
protected Object cast(Object value) {
423+
return value instanceof Integer v ? v.longValue() : value;
424+
}
425+
}
426+
427+
class IntegerMatcher extends NumberMatcher {
428+
IntegerMatcher(
429+
XContentBuilder actualMappings,
430+
Settings.Builder actualSettings,
431+
XContentBuilder expectedMappings,
432+
Settings.Builder expectedSettings
433+
) {
434+
super(FieldType.INTEGER, actualMappings, actualSettings, expectedMappings, expectedSettings);
435+
}
436+
437+
@Override
438+
protected Object cast(Object value) {
439+
return value;
440+
}
441+
}
442+
443+
class ShortMatcher extends NumberMatcher {
444+
ShortMatcher(
445+
XContentBuilder actualMappings,
446+
Settings.Builder actualSettings,
447+
XContentBuilder expectedMappings,
448+
Settings.Builder expectedSettings
449+
) {
450+
super(FieldType.SHORT, actualMappings, actualSettings, expectedMappings, expectedSettings);
451+
}
452+
453+
@Override
454+
protected Object cast(Object value) {
455+
return value instanceof Integer v ? v.shortValue() : value;
456+
}
457+
}
458+
459+
class ByteMatcher extends NumberMatcher {
460+
ByteMatcher(
461+
XContentBuilder actualMappings,
462+
Settings.Builder actualSettings,
463+
XContentBuilder expectedMappings,
464+
Settings.Builder expectedSettings
465+
) {
466+
super(FieldType.BYTE, actualMappings, actualSettings, expectedMappings, expectedSettings);
467+
}
468+
469+
@Override
470+
protected Object cast(Object value) {
471+
return value instanceof Integer v ? v.byteValue() : value;
472+
}
473+
}
411474

475+
class DoubleMatcher extends NumberMatcher {
476+
DoubleMatcher(
477+
XContentBuilder actualMappings,
478+
Settings.Builder actualSettings,
479+
XContentBuilder expectedMappings,
480+
Settings.Builder expectedSettings
481+
) {
482+
super(FieldType.DOUBLE, actualMappings, actualSettings, expectedMappings, expectedSettings);
483+
}
484+
485+
@Override
486+
protected Object cast(Object value) {
412487
return value;
413488
}
414489
}
415490

491+
class FloatMatcher extends NumberMatcher {
492+
FloatMatcher(
493+
XContentBuilder actualMappings,
494+
Settings.Builder actualSettings,
495+
XContentBuilder expectedMappings,
496+
Settings.Builder expectedSettings
497+
) {
498+
super(FieldType.FLOAT, actualMappings, actualSettings, expectedMappings, expectedSettings);
499+
}
500+
501+
@Override
502+
protected Object cast(Object value) {
503+
return value instanceof Integer v ? v.floatValue() : value;
504+
}
505+
}
506+
416507
class BooleanMatcher extends GenericMappingAwareMatcher {
417508
BooleanMatcher(
418509
XContentBuilder actualMappings,

test/framework/src/test/java/org/elasticsearch/logsdb/datageneration/SourceMatcherTests.java

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -160,4 +160,52 @@ public void testCoercedNumberField() throws IOException {
160160
var sut = new SourceMatcher(Map.of(), mapping, Settings.builder(), mapping, Settings.builder(), actual, expected, false);
161161
assertTrue(sut.match().isMatch());
162162
}
163+
164+
public void testNullValueIntegerNumericMatcherCastsType() throws IOException {
165+
String type = randomFrom("byte", "short", "integer", "long");
166+
int nullValue = 51;
167+
List<Map<String, Object>> actual = List.of(Map.of("field", List.of(5, nullValue, "")));
168+
List<Map<String, Object>> expected = List.of(Map.of("field", List.of(5, nullValue, nullValue)));
169+
170+
var mapping = XContentBuilder.builder(XContentType.JSON.xContent());
171+
mapping.startObject();
172+
mapping.startObject("_doc");
173+
{
174+
mapping.startObject("field");
175+
{
176+
mapping.field("type", type);
177+
mapping.field("null_value", nullValue);
178+
}
179+
mapping.endObject();
180+
}
181+
mapping.endObject();
182+
mapping.endObject();
183+
184+
var sut = new SourceMatcher(Map.of(), mapping, Settings.builder(), mapping, Settings.builder(), actual, expected, false);
185+
assertTrue(sut.match().isMatch());
186+
}
187+
188+
public void testNullValueFloatingPointNumericMatcherCastsType() throws IOException {
189+
String type = randomFrom("float", "double");
190+
double nullValue = 51.123;
191+
List<Map<String, Object>> actual = List.of(Map.of("field", List.of(5, nullValue, "")));
192+
List<Map<String, Object>> expected = List.of(Map.of("field", List.of(5, nullValue, nullValue)));
193+
194+
var mapping = XContentBuilder.builder(XContentType.JSON.xContent());
195+
mapping.startObject();
196+
mapping.startObject("_doc");
197+
{
198+
mapping.startObject("field");
199+
{
200+
mapping.field("type", type);
201+
mapping.field("null_value", nullValue);
202+
}
203+
mapping.endObject();
204+
}
205+
mapping.endObject();
206+
mapping.endObject();
207+
208+
var sut = new SourceMatcher(Map.of(), mapping, Settings.builder(), mapping, Settings.builder(), actual, expected, false);
209+
assertTrue(sut.match().isMatch());
210+
}
163211
}

0 commit comments

Comments
 (0)