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.
craigtaverner · Jan 3, 2024 · 1b41302 · 1b41302
1 parent 22f631f
commit 1b41302
Show file tree

Hide file tree

Showing 3 changed files with 11 additions and 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
@@ -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
@@ -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
@@ -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,15 +606,15 @@ protected SyntheticSourceSupport syntheticSourceSupport(boolean ignoreMalformed)
             public SyntheticSourceExample example(int maxVals) {
                 if (randomBoolean()) {
                     Tuple<Object, GeoPoint> 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<Tuple<Object, GeoPoint>> values = randomList(1, maxVals, this::generateValue);
                 List<Object> in = values.stream().map(Tuple::v1).toList();
                 // The results are currently sorted in order of encoded values, so we need to sort the expected values too
                 List<GeoPoint> outList = values.stream().map(v -> encode(v.v2())).sorted().map(this::decode).toList();
                 Object out = outList.size() == 1 ? outList.get(0) : outList;
 
-                List<Long> outBlockList = outList.stream().map(this::encode).toList();
+                List<BytesRef> 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();
             }