Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions docs/changelog/128259.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
pr: 128259
summary: Added geometry validation for GEO types to exit early on invalid latitudes
area: Geo
type: bug
issues:
- 128234
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import org.apache.lucene.util.BytesRef;
import org.elasticsearch.geometry.Geometry;
import org.elasticsearch.geometry.Point;
import org.elasticsearch.geometry.utils.GeographyValidator;
import org.elasticsearch.geometry.utils.GeometryValidator;
import org.elasticsearch.geometry.utils.WellKnownBinary;
import org.elasticsearch.geometry.utils.WellKnownText;
Expand All @@ -32,6 +33,10 @@ public long pointAsLong(double x, double y) {
int longitudeEncoded = encodeLongitude(x);
return (((long) latitudeEncoded) << 32) | (longitudeEncoded & 0xFFFFFFFFL);
}

public GeometryValidator validator() {
return GeographyValidator.instance(false);
}
},
CARTESIAN {

Expand Down Expand Up @@ -67,6 +72,10 @@ public long pointAsLong(double x, double y) {
}
};

protected GeometryValidator validator() {
return GeometryValidator.NOOP;
}

public abstract Point longAsPoint(long encoded);

public abstract long pointAsLong(double x, double y);
Expand All @@ -77,7 +86,7 @@ public long wkbAsLong(BytesRef wkb) {
}

public Point wkbAsPoint(BytesRef wkb) {
Geometry geometry = WellKnownBinary.fromWKB(GeometryValidator.NOOP, false, wkb.bytes, wkb.offset, wkb.length);
Geometry geometry = WellKnownBinary.fromWKB(validator(), false, wkb.bytes, wkb.offset, wkb.length);
if (geometry instanceof Point point) {
return point;
} else {
Expand All @@ -101,26 +110,18 @@ public BytesRef wktToWkb(String wkt) {
// TODO: we should be able to transform WKT to WKB without building the geometry
// we should as well use different validator for cartesian and geo?
try {
Geometry geometry = WellKnownText.fromWKT(GeometryValidator.NOOP, false, wkt);
Geometry geometry = WellKnownText.fromWKT(validator(), false, wkt);
return new BytesRef(WellKnownBinary.toWKB(geometry, ByteOrder.LITTLE_ENDIAN));
} catch (Exception e) {
throw new IllegalArgumentException("Failed to parse WKT: " + e.getMessage(), e);
}
}

public Geometry wktToGeometry(String wkt) {
try {
return WellKnownText.fromWKT(GeometryValidator.NOOP, false, wkt);
} catch (Exception e) {
throw new IllegalArgumentException("Failed to parse WKT: " + e.getMessage(), e);
}
}

public String wkbToWkt(BytesRef wkb) {
return WellKnownText.fromWKB(wkb.bytes, wkb.offset, wkb.length);
}

public Geometry wkbToGeometry(BytesRef wkb) {
return WellKnownBinary.fromWKB(GeometryValidator.NOOP, false, wkb.bytes, wkb.offset, wkb.length);
return WellKnownBinary.fromWKB(validator(), false, wkb.bytes, wkb.offset, wkb.length);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -1788,6 +1788,74 @@ count:long
12
;

airportsDistanceLessThanInvalidPoint
required_capability: st_distance

FROM airports
| EVAL distance = ST_DISTANCE(location, TO_GEOPOINT("POINT(-28.6359 153.5906)"))
| KEEP distance, location, name, scalerank
| WHERE distance < 500000 AND scalerank < 6
| STATS count = COUNT()
;

warning:Line 2:41: evaluation of [TO_GEOPOINT(\"POINT(-28.6359 153.5906)\")] failed, treating result as null. Only first 20 failures recorded.
warning:Line 2:41: java.lang.IllegalArgumentException: Failed to parse WKT: invalid latitude 153.5906; must be between -90.0 and 90.0

count:l
0
;

airportsDistanceLessThanInvalidPoint2
required_capability: st_distance

FROM airports
| EVAL distance = ST_DISTANCE(TO_GEOPOINT("POINT(-28.6359 153.5906)"), location)
| KEEP distance, location, name, scalerank
| WHERE distance < 500000 AND scalerank < 6
| STATS count = COUNT()
;

warning:Line 2:31: evaluation of [TO_GEOPOINT(\"POINT(-28.6359 153.5906)\")] failed, treating result as null. Only first 20 failures recorded.
warning:Line 2:31: java.lang.IllegalArgumentException: Failed to parse WKT: invalid latitude 153.5906; must be between -90.0 and 90.0

count:l
0
;

airportsDistanceGreaterThanInvalidPoint
required_capability: st_distance

FROM airports
| EVAL distance = ST_DISTANCE(location, TO_GEOPOINT("POINT(-28.6359 153.5906)"))
| KEEP distance, location, name, scalerank
| WHERE distance > 500 AND scalerank < 6
| STATS count = COUNT()
;

warning:Line 2:41: evaluation of [TO_GEOPOINT(\"POINT(-28.6359 153.5906)\")] failed, treating result as null. Only first 20 failures recorded.
warning:Line 2:41: java.lang.IllegalArgumentException: Failed to parse WKT: invalid latitude 153.5906; must be between -90.0 and 90.0

count:l
0
;

airportsDistanceGreaterThanInvalidPoint2
required_capability: st_distance

FROM airports
| EVAL distance = ST_DISTANCE(TO_GEOPOINT("POINT(-28.6359 153.5906)"), location)
| KEEP distance, location, name, scalerank
| WHERE distance > 500 AND scalerank < 6
| STATS count = COUNT()
;

warning:Line 2:31: evaluation of [TO_GEOPOINT(\"POINT(-28.6359 153.5906)\")] failed, treating result as null. Only first 20 failures recorded.
warning:Line 2:31: java.lang.IllegalArgumentException: Failed to parse WKT: invalid latitude 153.5906; must be between -90.0 and 90.0

count:l
0
;

###############################################
# Tests for Equality and casting with GEO_POINT

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
import static org.elasticsearch.xpack.esql.core.type.DataType.GEO_POINT;
import static org.elasticsearch.xpack.esql.core.type.DataType.KEYWORD;
import static org.elasticsearch.xpack.esql.core.type.DataType.TEXT;
import static org.elasticsearch.xpack.esql.type.EsqlDataTypeConverter.stringToSpatial;
import static org.elasticsearch.xpack.esql.type.EsqlDataTypeConverter.stringToGeo;

public class ToGeoPoint extends AbstractConvertFunction {
public static final NamedWriteableRegistry.Entry ENTRY = new NamedWriteableRegistry.Entry(
Expand Down Expand Up @@ -91,6 +91,6 @@ protected NodeInfo<? extends Expression> info() {

@ConvertEvaluator(extraName = "FromString", warnExceptions = { IllegalArgumentException.class })
static BytesRef fromKeyword(BytesRef in) {
return stringToSpatial(in.utf8ToString());
return stringToGeo(in.utf8ToString());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
import static org.elasticsearch.xpack.esql.core.type.DataType.GEO_SHAPE;
import static org.elasticsearch.xpack.esql.core.type.DataType.KEYWORD;
import static org.elasticsearch.xpack.esql.core.type.DataType.TEXT;
import static org.elasticsearch.xpack.esql.type.EsqlDataTypeConverter.stringToSpatial;
import static org.elasticsearch.xpack.esql.type.EsqlDataTypeConverter.stringToGeo;

public class ToGeoShape extends AbstractConvertFunction {
public static final NamedWriteableRegistry.Entry ENTRY = new NamedWriteableRegistry.Entry(
Expand Down Expand Up @@ -93,6 +93,6 @@ protected NodeInfo<? extends Expression> info() {

@ConvertEvaluator(extraName = "FromString", warnExceptions = { IllegalArgumentException.class })
static BytesRef fromKeyword(BytesRef in) {
return stringToSpatial(in.utf8ToString());
return stringToGeo(in.utf8ToString());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@
import static org.elasticsearch.xpack.esql.core.util.NumericUtils.asLongUnsigned;
import static org.elasticsearch.xpack.esql.core.util.NumericUtils.asUnsignedLong;
import static org.elasticsearch.xpack.esql.core.util.NumericUtils.unsignedLongAsNumber;
import static org.elasticsearch.xpack.esql.core.util.SpatialCoordinateTypes.GEO;
import static org.elasticsearch.xpack.esql.core.util.SpatialCoordinateTypes.UNSPECIFIED;

public class EsqlDataTypeConverter {
Expand Down Expand Up @@ -234,6 +235,9 @@ public static Converter converterFor(DataType from, DataType to) {
if (to == DataType.BOOLEAN) {
return EsqlConverter.STRING_TO_BOOLEAN;
}
if (DataType.isSpatialGeo(to)) {
return EsqlConverter.STRING_TO_GEO;
}
if (DataType.isSpatial(to)) {
return EsqlConverter.STRING_TO_SPATIAL;
}
Expand Down Expand Up @@ -540,6 +544,10 @@ public static String spatialToString(BytesRef field) {
return UNSPECIFIED.wkbToWkt(field);
}

public static BytesRef stringToGeo(String field) {
return GEO.wktToWkb(field);
}

public static BytesRef stringToSpatial(String field) {
return UNSPECIFIED.wktToWkb(field);
}
Expand Down Expand Up @@ -772,6 +780,7 @@ public enum EsqlConverter implements Converter {
STRING_TO_LONG(x -> EsqlDataTypeConverter.stringToLong((String) x)),
STRING_TO_INT(x -> EsqlDataTypeConverter.stringToInt((String) x)),
STRING_TO_BOOLEAN(x -> EsqlDataTypeConverter.stringToBoolean((String) x)),
STRING_TO_GEO(x -> EsqlDataTypeConverter.stringToGeo((String) x)),
STRING_TO_SPATIAL(x -> EsqlDataTypeConverter.stringToSpatial((String) x));

private static final String NAME = "esql-converter";
Expand Down
Loading