Skip to content

Commit 3bf5de9

Browse files
[8.19] Fixes flaky ST_CENTROID_AGG tests (elastic#114892) (elastic#128037)
* Fixes flaky ST_CENTROID_AGG tests (elastic#114892) Even with Kahan summation, we were occasionally getting floating point differences at the 14th decimal point, well beyond anything a GIS use case would care about. * Unmute after cherry-pick
1 parent 373e9b1 commit 3bf5de9

File tree

2 files changed

+50
-12
lines changed

2 files changed

+50
-12
lines changed

muted-tests.yml

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -52,12 +52,6 @@ tests:
5252
- class: org.elasticsearch.xpack.ml.integration.MlJobIT
5353
method: testMultiIndexDelete
5454
issue: https://github.com/elastic/elasticsearch/issues/112381
55-
- class: org.elasticsearch.xpack.esql.expression.function.aggregate.SpatialCentroidTests
56-
method: "testAggregateIntermediate {TestCase=<geo_point> #2}"
57-
issue: https://github.com/elastic/elasticsearch/issues/112461
58-
- class: org.elasticsearch.xpack.esql.expression.function.aggregate.SpatialCentroidTests
59-
method: testAggregateIntermediate {TestCase=<geo_point>}
60-
issue: https://github.com/elastic/elasticsearch/issues/112463
6155
- class: org.elasticsearch.xpack.inference.external.http.RequestBasedTaskRunnerTests
6256
method: testLoopOneAtATime
6357
issue: https://github.com/elastic/elasticsearch/issues/112471

x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/aggregate/SpatialCentroidTests.java

Lines changed: 50 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -23,13 +23,15 @@
2323
import org.elasticsearch.xpack.esql.expression.function.MultiRowTestCaseSupplier;
2424
import org.elasticsearch.xpack.esql.expression.function.MultiRowTestCaseSupplier.IncludingAltitude;
2525
import org.elasticsearch.xpack.esql.expression.function.TestCaseSupplier;
26+
import org.hamcrest.BaseMatcher;
27+
import org.hamcrest.Description;
28+
import org.hamcrest.Matcher;
2629

27-
import java.nio.ByteOrder;
2830
import java.util.List;
2931
import java.util.function.Supplier;
3032
import java.util.stream.Stream;
3133

32-
import static org.hamcrest.Matchers.equalTo;
34+
import static org.hamcrest.Matchers.closeTo;
3335

3436
@FunctionName("st_centroid_agg")
3537
public class SpatialCentroidTests extends AbstractAggregationTestCase {
@@ -74,16 +76,58 @@ private static TestCaseSupplier makeSupplier(TestCaseSupplier.TypedDataSupplier
7476
count++;
7577
}
7678

77-
var expected = new BytesRef(
78-
WellKnownBinary.toWKB(new Point(xSum.value() / count, ySum.value() / count), ByteOrder.LITTLE_ENDIAN)
79-
);
79+
var expectedX = xSum.value() / count;
80+
var expectedY = ySum.value() / count;
8081

8182
return new TestCaseSupplier.TestCase(
8283
List.of(fieldTypedData),
8384
"SpatialCentroid[field=Attribute[channel=0]]",
8485
fieldTypedData.type(),
85-
equalTo(expected)
86+
centroidMatches(expectedX, expectedY, 1e-14)
8687
);
8788
});
8889
}
90+
91+
@SuppressWarnings("SameParameterValue")
92+
private static Matcher<BytesRef> centroidMatches(double x, double y, double error) {
93+
return new TestCentroidMatcher(x, y, error);
94+
}
95+
96+
private static class TestCentroidMatcher extends BaseMatcher<BytesRef> {
97+
private final double x;
98+
private final double y;
99+
private final Matcher<Double> mx;
100+
private final Matcher<Double> my;
101+
102+
private TestCentroidMatcher(double x, double y, double error) {
103+
this.x = x;
104+
this.y = y;
105+
this.mx = closeTo(x, error);
106+
this.my = closeTo(y, error);
107+
}
108+
109+
@Override
110+
public boolean matches(Object item) {
111+
if (item instanceof BytesRef wkb) {
112+
var point = (Point) WellKnownBinary.fromWKB(GeometryValidator.NOOP, false, wkb.bytes, wkb.offset, wkb.length);
113+
return mx.matches(point.getX()) && my.matches(point.getY());
114+
}
115+
return false;
116+
}
117+
118+
@Override
119+
public void describeMismatch(Object item, Description description) {
120+
if (item instanceof BytesRef wkb) {
121+
var point = (Point) WellKnownBinary.fromWKB(GeometryValidator.NOOP, false, wkb.bytes, wkb.offset, wkb.length);
122+
description.appendText("was ").appendValue(point);
123+
} else {
124+
description.appendText("was ").appendValue(item);
125+
}
126+
}
127+
128+
@Override
129+
public void describeTo(Description description) {
130+
description.appendValue(" POINT (" + x + " " + y + ")");
131+
}
132+
}
89133
}

0 commit comments

Comments
 (0)