From 1b41302d851e105816810b6119ac3883189b74f8 Mon Sep 17 00:00:00 2001 From: Craig Taverner Date: Wed, 3 Jan 2024 12:08:15 +0100 Subject: [PATCH] Remove intermediate WKT from GeoPointFieldMapper Previously we were reading source, mapping to WKT and then later mapping to WKB. Now just map directly to WKB. --- .../mapper/AbstractGeometryFieldMapper.java | 2 +- .../index/mapper/BlockSourceReader.java | 19 ++----------------- .../mapper/GeoPointFieldMapperTests.java | 10 ++++++++-- 3 files changed, 11 insertions(+), 20 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/index/mapper/AbstractGeometryFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/AbstractGeometryFieldMapper.java index 42994158540e8..80d2ef13d8034 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/AbstractGeometryFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/AbstractGeometryFieldMapper.java @@ -144,7 +144,7 @@ protected Object parseSourceValue(Object value) { public BlockLoader blockLoader(BlockLoaderContext blContext) { // Currently we can only load from source in ESQL return new BlockSourceReader.GeometriesBlockLoader( - valueFetcher(blContext.sourcePaths(name()), nullValue, GeometryFormatterFactory.WKT) + valueFetcher(blContext.sourcePaths(name()), nullValue, GeometryFormatterFactory.WKB) ); } } diff --git a/server/src/main/java/org/elasticsearch/index/mapper/BlockSourceReader.java b/server/src/main/java/org/elasticsearch/index/mapper/BlockSourceReader.java index 6cf3f4e8c665f..5982baf5f423a 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/BlockSourceReader.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/BlockSourceReader.java @@ -13,16 +13,12 @@ import org.apache.lucene.util.BytesRef; import org.apache.lucene.util.UnicodeUtil; import org.elasticsearch.common.geo.SpatialPoint; -import org.elasticsearch.geometry.Geometry; import org.elasticsearch.geometry.Point; -import org.elasticsearch.geometry.utils.GeometryValidator; import org.elasticsearch.geometry.utils.WellKnownBinary; -import org.elasticsearch.geometry.utils.WellKnownText; import org.elasticsearch.search.fetch.StoredFieldsSpec; import java.io.IOException; import java.nio.ByteOrder; -import java.text.ParseException; import java.util.ArrayList; import java.util.List; @@ -186,19 +182,8 @@ protected void append(BlockLoader.Builder builder, Object v) { if (v instanceof SpatialPoint point) { BytesRef wkb = new BytesRef(WellKnownBinary.toWKB(new Point(point.getX(), point.getY()), ByteOrder.LITTLE_ENDIAN)); ((BlockLoader.BytesRefBuilder) builder).appendBytesRef(wkb); - } else if (v instanceof String wkt) { - try { - // TODO: figure out why this is not already happening in the GeoPointFieldMapper - Geometry geometry = WellKnownText.fromWKT(GeometryValidator.NOOP, false, wkt); - if (geometry instanceof Point point) { - BytesRef wkb = new BytesRef(WellKnownBinary.toWKB(point, ByteOrder.LITTLE_ENDIAN)); - ((BlockLoader.BytesRefBuilder) builder).appendBytesRef(wkb); - } else { - throw new IllegalArgumentException("Cannot convert geometry into point:: " + geometry.type()); - } - } catch (IOException | ParseException e) { - throw new IllegalArgumentException("Failed to parse point geometry: " + e.getMessage(), e); - } + } else if (v instanceof byte[] wkb) { + ((BlockLoader.BytesRefBuilder) builder).appendBytesRef(new BytesRef(wkb)); } else { throw new IllegalArgumentException("Unsupported source type for point: " + v.getClass().getSimpleName()); } diff --git a/server/src/test/java/org/elasticsearch/index/mapper/GeoPointFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/GeoPointFieldMapperTests.java index b2d071caff036..61d25a621fc21 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/GeoPointFieldMapperTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/GeoPointFieldMapperTests.java @@ -20,6 +20,7 @@ import org.elasticsearch.core.Tuple; import org.elasticsearch.geo.GeometryTestUtils; import org.elasticsearch.geometry.Point; +import org.elasticsearch.geometry.utils.WellKnownBinary; import org.elasticsearch.geometry.utils.WellKnownText; import org.elasticsearch.index.IndexMode; import org.elasticsearch.index.IndexSettings; @@ -31,6 +32,7 @@ import org.junit.AssumptionViolatedException; import java.io.IOException; +import java.nio.ByteOrder; import java.util.ArrayList; import java.util.List; import java.util.Map; @@ -604,7 +606,7 @@ protected SyntheticSourceSupport syntheticSourceSupport(boolean ignoreMalformed) public SyntheticSourceExample example(int maxVals) { if (randomBoolean()) { Tuple v = generateValue(); - return new SyntheticSourceExample(v.v1(), decode(encode(v.v2())), encode(v.v2()), this::mapping); + return new SyntheticSourceExample(v.v1(), decode(encode(v.v2())), asWKB(v.v2()), this::mapping); } List> values = randomList(1, maxVals, this::generateValue); List in = values.stream().map(Tuple::v1).toList(); @@ -612,7 +614,7 @@ public SyntheticSourceExample example(int maxVals) { List outList = values.stream().map(v -> encode(v.v2())).sorted().map(this::decode).toList(); Object out = outList.size() == 1 ? outList.get(0) : outList; - List outBlockList = outList.stream().map(this::encode).toList(); + List outBlockList = outList.stream().map(this::asWKB).toList(); Object outBlock = outBlockList.size() == 1 ? outBlockList.get(0) : outBlockList; return new SyntheticSourceExample(in, out, outBlock, this::mapping); } @@ -647,6 +649,10 @@ private Object randomGeoPointInput(GeoPoint point) { }; } + private BytesRef asWKB(GeoPoint point) { + return new BytesRef(WellKnownBinary.toWKB(new Point(point.getX(), point.getY()), ByteOrder.LITTLE_ENDIAN)); + } + private long encode(GeoPoint point) { return new LatLonDocValuesField("f", point.lat(), point.lon()).numericValue().longValue(); }