ESQL Support loading points from source into WKB blocks

docs/changelog/103698.yaml

@@ -0,0 +1,5 @@
+pr: 103698
+summary: Reading points from source to reduce precision loss
+area: ES|QL
+type: enhancement
+issues: []

server/src/main/java/org/elasticsearch/common/geo/GeoPoint.java

@@ -25,12 +25,11 @@
 import java.io.IOException;
 import java.util.Locale;
 
-public final class GeoPoint implements SpatialPoint, ToXContentFragment {
+public final class GeoPoint extends SpatialPoint implements ToXContentFragment {
 
-    private double lat;
-    private double lon;
-
-    public GeoPoint() {}
+    public GeoPoint() {
+        super(0, 0);
+    }
 
     /**
      * Create a new Geopoint from a string. This String must either be a geohash
@@ -39,31 +38,31 @@ public GeoPoint() {}
      * @param value String to create the point from
      */
     public GeoPoint(String value) {
+        this();  // initialize to defaults (since super-class does not have empty constructor)
         this.resetFromString(value);
     }
 
     public GeoPoint(double lat, double lon) {
-        this.lat = lat;
-        this.lon = lon;
+        super(lon, lat);
     }
 
     public GeoPoint(SpatialPoint template) {
-        this(template.getY(), template.getX());
+        super(template);
     }
 
     public GeoPoint reset(double lat, double lon) {
-        this.lat = lat;
-        this.lon = lon;
+        this.y = lat;
+        this.x = lon;
         return this;
     }
 
     public GeoPoint resetLat(double lat) {
-        this.lat = lat;
+        this.y = lat;
         return this;
     }
 
     public GeoPoint resetLon(double lon) {
-        this.lon = lon;
+        this.x = lon;
         return this;
     }
 
@@ -134,8 +133,8 @@ GeoPoint parseGeoHash(String geohash, EffectivePoint effectivePoint) {
     }
 
     public GeoPoint resetFromIndexHash(long hash) {
-        lon = Geohash.decodeLongitude(hash);
-        lat = Geohash.decodeLatitude(hash);
+        this.x = Geohash.decodeLongitude(hash);
+        this.y = Geohash.decodeLatitude(hash);
         return this;
     }
 
@@ -155,43 +154,33 @@ public GeoPoint resetFromGeoHash(long geohashLong) {
     }
 
     public double lat() {
-        return this.lat;
+        return this.y;
     }
 
     public double getLat() {
-        return this.lat;
+        return this.y;
     }
 
     public double lon() {
-        return this.lon;
+        return this.x;
     }
 
     public double getLon() {
-        return this.lon;
-    }
-
-    @Override
-    public double getX() {
-        return this.lon;
-    }
-
-    @Override
-    public double getY() {
-        return this.lat;
+        return this.x;
     }
 
     public String geohash() {
-        return Geohash.stringEncode(lon, lat);
+        return Geohash.stringEncode(x, y);
     }
 
     public String getGeohash() {
-        return Geohash.stringEncode(lon, lat);
+        return Geohash.stringEncode(x, y);
     }
 
     /** Return the point in Lucene encoded format used to stored points as doc values */
     public long getEncoded() {
-        final int latitudeEncoded = GeoEncodingUtils.encodeLatitude(this.lat);
-        final int longitudeEncoded = GeoEncodingUtils.encodeLongitude(this.lon);
+        final int latitudeEncoded = GeoEncodingUtils.encodeLatitude(this.y);
+        final int longitudeEncoded = GeoEncodingUtils.encodeLongitude(this.x);
         return (((long) latitudeEncoded) << 32) | (longitudeEncoded & 0xFFFFFFFFL);
     }
 
@@ -205,33 +194,10 @@ private GeoPoint resetFromEncoded(int encodedLat, int encodedLon) {
         return reset(GeoEncodingUtils.decodeLatitude(encodedLat), GeoEncodingUtils.decodeLongitude(encodedLon));
     }
 
-    @Override
-    public boolean equals(Object o) {
-        if (this == o) return true;
-        if (o == null || getClass() != o.getClass()) return false;
-
-        GeoPoint geoPoint = (GeoPoint) o;
-
-        if (Double.compare(geoPoint.lat, lat) != 0) return false;
-        if (Double.compare(geoPoint.lon, lon) != 0) return false;
-
-        return true;
-    }
-
-    @Override
-    public int hashCode() {
-        int result;
-        long temp;
-        temp = lat != +0.0d ? Double.doubleToLongBits(lat) : 0L;
-        result = Long.hashCode(temp);
-        temp = lon != +0.0d ? Double.doubleToLongBits(lon) : 0L;
-        result = 31 * result + Long.hashCode(temp);
-        return result;
-    }
-
     @Override
     public String toString() {
-        return lat + ", " + lon;
+        // TODO: Should prefer to use super.toString() which uses WKT, but that has consequences
+        return y + ", " + x;
     }
 
     public static GeoPoint fromGeohash(String geohash) {
@@ -244,7 +210,7 @@ public static GeoPoint fromGeohash(long geohashLong) {
 
     @Override
     public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
-        return builder.latlon(lat, lon);
+        return builder.latlon(y, x);
     }
 
     public static double assertZValue(final boolean ignoreZValue, double zValue) {

server/src/main/java/org/elasticsearch/common/geo/SpatialPoint.java

@@ -12,8 +12,65 @@
  * To facilitate maximizing the use of common code between GeoPoint and projected CRS
  * we introduced this ElasticPoint as an interface of commonality.
  */
-public interface SpatialPoint {
-    double getX();
+public class SpatialPoint implements Comparable<SpatialPoint> {
 
-    double getY();
+    protected double x;
+    protected double y;
+
+    public SpatialPoint(double x, double y) {
+        this.x = x;
+        this.y = y;
+    }
+
+    public SpatialPoint(SpatialPoint template) {
+        this(template.getX(), template.getY());
+    }
+
+    public final double getX() {
+        return x;
+    }
+
+    public final double getY() {
+        return y;
+    }
+
+    @Override
+    public int hashCode() {
+        return 31 * 31 * getClass().getSimpleName().hashCode() + 31 * Double.hashCode(x) + Double.hashCode(y);
+    }
+
+    @Override
+    public boolean equals(Object obj) {
+        if (this == obj) return true;
+        if (obj == null || getClass() != obj.getClass()) return false;
+        SpatialPoint point = (SpatialPoint) obj;
+        return (Double.compare(point.x, x) == 0) && Double.compare(point.y, y) == 0;
+    }
+
+    @Override
+    public String toString() {
+        return toWKT();
+    }
+
+    public String toWKT() {
+        // Code designed to mimic WellKnownText.toWKT, with much less stack depth and object creation
+        return "POINT (" + x + " " + y + ")";
+    }
+
+    @Override
+    public int compareTo(SpatialPoint other) {
+        if (this.getClass().equals(other.getClass())) {
+            double xd = this.getX() - other.getX();
+            double yd = this.getY() - other.getY();
+            return (xd == 0) ? comparison(yd) : comparison(xd);
+        } else {
+            // TODO: Rather separate based on CRS, but since we don't have that yet, we use class name
+            // The sort order here is unimportant and does not (yet) introduce BWC issues, so we are free to change it later with CRS
+            return this.getClass().getSimpleName().compareTo(other.getClass().getSimpleName());
+        }
+    }
+
+    private int comparison(double delta) {
+        return delta == 0 ? 0 : delta < 0 ? -1 : 1;
+    }
 }

server/src/main/java/org/elasticsearch/index/mapper/AbstractGeometryFieldMapper.java

@@ -87,16 +87,19 @@ private static XContentParser wrapObject(Object sourceMap) throws IOException {
     public abstract static class AbstractGeometryFieldType<T> extends MappedFieldType {
 
         protected final Parser<T> geometryParser;
+        private final T nullValue;
 
         protected AbstractGeometryFieldType(
             String name,
             boolean indexed,
             boolean stored,
             boolean hasDocValues,
             Parser<T> geometryParser,
+            T nullValue,
             Map<String, String> meta
         ) {
             super(name, indexed, stored, hasDocValues, TextSearchInfo.NONE, meta);
+            this.nullValue = nullValue;
             this.geometryParser = geometryParser;
         }
 
@@ -136,6 +139,14 @@ protected Object parseSourceValue(Object value) {
                 }
             };
         }
+
+        @Override
+        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)
+            );
+        }
     }
 
     private final Explicit<Boolean> ignoreMalformed;

server/src/main/java/org/elasticsearch/index/mapper/AbstractShapeGeometryFieldMapper.java

@@ -55,13 +55,19 @@ protected AbstractShapeGeometryFieldType(
             Orientation orientation,
             Map<String, String> meta
         ) {
-            super(name, isSearchable, isStored, hasDocValues, parser, meta);
+            super(name, isSearchable, isStored, hasDocValues, parser, null, meta);
             this.orientation = orientation;
         }
 
         public Orientation orientation() {
             return this.orientation;
         }
+
+        @Override
+        public BlockLoader blockLoader(BlockLoaderContext blContext) {
+            // TODO: Support shapes in ESQL
+            return null;
+        }
     }
 
     protected Explicit<Boolean> coerce;

server/src/main/java/org/elasticsearch/index/mapper/BlockLoader.java

@@ -12,6 +12,7 @@
 import org.apache.lucene.index.SortedDocValues;
 import org.apache.lucene.index.SortedSetDocValues;
 import org.apache.lucene.util.BytesRef;
+import org.elasticsearch.common.geo.SpatialPoint;
 import org.elasticsearch.core.Releasable;
 import org.elasticsearch.search.fetch.StoredFieldsSpec;
 import org.elasticsearch.search.lookup.Source;
@@ -348,6 +349,11 @@ interface BlockFactory {
          */
         BytesRefBuilder bytesRefs(int expectedCount);
 
+        /**
+         * Build a builder to load {@link SpatialPoint}s backed by WKB in BytesRefBlock.
+         */
+        BytesRefBuilder geometries(int expectedCount);
+
         /**
          * Build a builder to load doubles as loaded from doc values.
          * Doc values load doubles deduplicated and in sorted order.

server/src/main/java/org/elasticsearch/index/mapper/BlockSourceReader.java

@@ -12,9 +12,17 @@
 import org.apache.lucene.index.SortedSetDocValues;
 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;
 
@@ -131,6 +139,24 @@ public RowStrideReader rowStrideReader(LeafReaderContext context) {
         }
     }
 
+    public static class GeometriesBlockLoader extends SourceBlockLoader {
+        private final ValueFetcher fetcher;
+
+        public GeometriesBlockLoader(ValueFetcher fetcher) {
+            this.fetcher = fetcher;
+        }
+
+        @Override
+        public Builder builder(BlockFactory factory, int expectedCount) {
+            return factory.geometries(expectedCount);
+        }
+
+        @Override
+        public RowStrideReader rowStrideReader(LeafReaderContext context) {
+            return new Geometries(fetcher);
+        }
+    }
+
     private static class BytesRefs extends BlockSourceReader {
         BytesRef scratch = new BytesRef();
 
@@ -149,6 +175,41 @@ public String toString() {
         }
     }
 
+    private static class Geometries extends BlockSourceReader {
+
+        Geometries(ValueFetcher fetcher) {
+            super(fetcher);
+        }
+
+        @Override
+        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 {
+                throw new IllegalArgumentException("Unsupported source type for point: " + v.getClass().getSimpleName());
+            }
+        }
+
+        @Override
+        public String toString() {
+            return "BlockSourceReader.Geometries";
+        }
+    }
+
     public static class DoublesBlockLoader extends SourceBlockLoader {
         private final ValueFetcher fetcher;