Skip to content

Commit

Permalink
[8.x] ESQL: Fix AbstractShapeGeometryFieldMapperTests (#119265) (#119284
Browse files Browse the repository at this point in the history
)

* ESQL: Fix AbstractShapeGeometryFieldMapperTests (#119265)

Fixed a bug in AbstractShapeGeometryFieldMapperTests when the directory reader would contain multiple leaves.

Resolves #119201.

* Unmute test
  • Loading branch information
GalLalouche authored Dec 26, 2024
1 parent cdd35a8 commit 0e0b4fb
Show file tree
Hide file tree
Showing 4 changed files with 35 additions and 12 deletions.
6 changes: 6 additions & 0 deletions docs/changelog/119265.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
pr: 119265
summary: Fix `AbstractShapeGeometryFieldMapperTests`
area: "ES|QL"
type: bug
issues:
- 119201
3 changes: 0 additions & 3 deletions muted-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -427,9 +427,6 @@ tests:
- class: org.elasticsearch.smoketest.SmokeTestMultiNodeClientYamlTestSuiteIT
issue: https://github.com/elastic/elasticsearch/issues/119191
method: test {yaml=indices.create/20_synthetic_source/create index with use_synthetic_source}
- class: org.elasticsearch.index.mapper.AbstractShapeGeometryFieldMapperTests
method: testCartesianBoundsBlockLoader
issue: https://github.com/elastic/elasticsearch/issues/119201
- class: org.elasticsearch.xpack.ml.integration.InferenceIngestInputConfigIT
method: testIngestWithInputFields
issue: https://github.com/elastic/elasticsearch/issues/118092
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,10 @@ public void read(int docId, BlockLoader.StoredFields storedFields, BlockLoader.B

private void read(BinaryDocValues binaryDocValues, int doc, GeometryDocValueReader reader, BytesRefBuilder builder)
throws IOException {
binaryDocValues.advanceExact(doc);
if (binaryDocValues.advanceExact(doc) == false) {
builder.appendNull();
return;
}
reader.reset(binaryDocValues.binaryValue());
var extent = reader.getExtent();
// This is rather silly: an extent is already encoded as ints, but we convert it to Rectangle to
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@

import org.apache.lucene.document.Document;
import org.apache.lucene.index.DirectoryReader;
import org.apache.lucene.index.LeafReaderContext;
import org.apache.lucene.index.LeafReader;
import org.apache.lucene.store.Directory;
import org.apache.lucene.tests.index.RandomIndexWriter;
import org.apache.lucene.util.BytesRef;
Expand All @@ -30,6 +30,7 @@
import org.elasticsearch.test.hamcrest.WellKnownBinaryBytesRefMatcher;

import java.io.IOException;
import java.util.ArrayList;
import java.util.Optional;
import java.util.function.Function;
import java.util.function.Supplier;
Expand Down Expand Up @@ -61,7 +62,7 @@ private void testBoundsBlockLoaderAux(
Function<String, ShapeIndexer> indexerFactory,
Function<Geometry, Optional<Rectangle>> visitor
) throws IOException {
var geometries = IntStream.range(0, 20).mapToObj(i -> generator.get()).toList();
var geometries = IntStream.range(0, 50).mapToObj(i -> generator.get()).toList();
var loader = new AbstractShapeGeometryFieldMapper.AbstractShapeGeometryFieldType.BoundsBlockLoader("field", encoder);
try (Directory directory = newDirectory()) {
try (var iw = new RandomIndexWriter(random(), directory)) {
Expand All @@ -73,23 +74,39 @@ private void testBoundsBlockLoaderAux(
iw.addDocument(doc);
}
}
var indices = IntStream.range(0, geometries.size() / 2).map(x -> x * 2).toArray();
// We specifically check just the even indices, to verify the loader can skip documents correctly.
var evenIndices = evenArray(geometries.size());
try (DirectoryReader reader = DirectoryReader.open(directory)) {
LeafReaderContext ctx = reader.leaves().get(0);
TestBlock block = (TestBlock) loader.reader(ctx).read(TestBlock.factory(ctx.reader().numDocs()), TestBlock.docs(indices));
for (int i = 0; i < indices.length; i++) {
var idx = indices[i];
var byteRefResults = new ArrayList<BytesRef>();
for (var leaf : reader.leaves()) {
LeafReader leafReader = leaf.reader();
int numDocs = leafReader.numDocs();
try (
TestBlock block = (TestBlock) loader.reader(leaf)
.read(TestBlock.factory(leafReader.numDocs()), TestBlock.docs(evenArray(numDocs)))
) {
for (int i = 0; i < block.size(); i++) {
byteRefResults.add((BytesRef) block.get(i));
}
}
}
for (int i = 0; i < evenIndices.length; i++) {
var idx = evenIndices[i];
var geometry = geometries.get(idx);
var geoString = geometry.toString();
var geometryString = geoString.length() > 200 ? geoString.substring(0, 200) + "..." : geoString;
Rectangle r = visitor.apply(geometry).get();
assertThat(
Strings.format("geometries[%d] ('%s') wasn't extracted correctly", idx, geometryString),
(BytesRef) block.get(i),
byteRefResults.get(i),
WellKnownBinaryBytesRefMatcher.encodes(RectangleMatcher.closeToFloat(r, 1e-3, encoder))
);
}
}
}
}

private static int[] evenArray(int maxIndex) {
return IntStream.range(0, maxIndex / 2).map(x -> x * 2).toArray();
}
}

0 comments on commit 0e0b4fb

Please sign in to comment.