Skip to content

Commit 4a2397d

Browse files
Fix flaky TPC-H Q1 test due to bugs in MatcherUtils.closeTo() (#5283)
* Fix the flaky tpch Q1 Signed-off-by: Lantao Jin <ltjin@amazon.com> * Change to ULP-aware to handle floating-point precision differences Signed-off-by: Lantao Jin <ltjin@amazon.com> --------- Signed-off-by: Lantao Jin <ltjin@amazon.com> (cherry picked from commit 246ed0d) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
1 parent 2dd8600 commit 4a2397d

File tree

1 file changed

+30
-9
lines changed

1 file changed

+30
-9
lines changed

integ-test/src/test/java/org/opensearch/sql/util/MatcherUtils.java

Lines changed: 30 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,12 @@
4242

4343
public class MatcherUtils {
4444

45+
/** Absolute tolerance floor for {@link #closeTo} numeric comparisons. */
46+
private static final double ABSOLUTE_TOLERANCE = 1e-10;
47+
48+
/** Number of ULPs tolerated by {@link #closeTo} to absorb platform-dependent rounding. */
49+
private static final int ULP_TOLERANCE_FACTOR = 4;
50+
4551
private static final Logger LOG = LogManager.getLogger();
4652
private static final ObjectMapper JSON_MAPPER = new ObjectMapper();
4753

@@ -302,29 +308,44 @@ protected boolean matchesSafely(JSONArray array) {
302308
}
303309

304310
public static TypeSafeMatcher<JSONArray> closeTo(Object... values) {
305-
final double error = 1e-10;
306311
return new TypeSafeMatcher<JSONArray>() {
307312
@Override
308313
protected boolean matchesSafely(JSONArray item) {
309314
List<Object> expectedValues = new ArrayList<>(Arrays.asList(values));
310315
List<Object> actualValues = new ArrayList<>();
311316
item.iterator().forEachRemaining(v -> actualValues.add((Object) v));
312-
return actualValues.stream()
313-
.allMatch(
314-
v ->
315-
v instanceof Number
316-
? valuesAreClose(
317-
(Number) v, (Number) expectedValues.get(actualValues.indexOf(v)))
318-
: v.equals(expectedValues.get(actualValues.indexOf(v))));
317+
if (actualValues.size() != expectedValues.size()) {
318+
return false;
319+
}
320+
for (int i = 0; i < actualValues.size(); i++) {
321+
Object actual = actualValues.get(i);
322+
Object expected = expectedValues.get(i);
323+
if (actual instanceof Number && expected instanceof Number) {
324+
if (!valuesAreClose((Number) actual, (Number) expected)) {
325+
return false;
326+
}
327+
} else if (!actual.equals(expected)) {
328+
return false;
329+
}
330+
}
331+
return true;
319332
}
320333

321334
@Override
322335
public void describeTo(Description description) {
323336
description.appendText(Arrays.toString(values));
324337
}
325338

339+
/**
340+
* ULP-aware comparison: tolerates up to {@link #ULP_TOLERANCE_FACTOR} ULPs or {@link
341+
* #ABSOLUTE_TOLERANCE}, whichever is larger.
342+
*/
326343
private boolean valuesAreClose(Number v1, Number v2) {
327-
return Math.abs(v1.doubleValue() - v2.doubleValue()) <= error;
344+
double d1 = v1.doubleValue();
345+
double d2 = v2.doubleValue();
346+
double diff = Math.abs(d1 - d2);
347+
double ulpTolerance = ULP_TOLERANCE_FACTOR * Math.max(Math.ulp(d1), Math.ulp(d2));
348+
return diff <= Math.max(ABSOLUTE_TOLERANCE, ulpTolerance);
328349
}
329350
};
330351
}

0 commit comments

Comments
 (0)