Skip to content

Commit 612d2c5

Browse files
craigtavernercraigtaverner
committed
Added geometry validation for GEO types to exit early on invalid latitudes
1 parent 960222e commit 612d2c5

File tree

5 files changed

+93
-15
lines changed

5 files changed

+93
-15
lines changed

‎x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/util/SpatialCoordinateTypes.java‎

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
import org.apache.lucene.util.BytesRef;
1313
import org.elasticsearch.geometry.Geometry;
1414
import org.elasticsearch.geometry.Point;
15+
import org.elasticsearch.geometry.utils.GeographyValidator;
1516
import org.elasticsearch.geometry.utils.GeometryValidator;
1617
import org.elasticsearch.geometry.utils.WellKnownBinary;
1718
import org.elasticsearch.geometry.utils.WellKnownText;
@@ -32,6 +33,10 @@ public long pointAsLong(double x, double y) {
3233
int longitudeEncoded = encodeLongitude(x);
3334
return (((long) latitudeEncoded) << 32) | (longitudeEncoded & 0xFFFFFFFFL);
3435
}
36+
37+
public GeometryValidator validator() {
38+
return GeographyValidator.instance(false);
39+
}
3540
},
3641
CARTESIAN {
3742

@@ -67,6 +72,10 @@ public long pointAsLong(double x, double y) {
6772
}
6873
};
6974

75+
protected GeometryValidator validator() {
76+
return GeometryValidator.NOOP;
77+
}
78+
7079
public abstract Point longAsPoint(long encoded);
7180

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

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

111-
public Geometry wktToGeometry(String wkt) {
112-
try {
113-
return WellKnownText.fromWKT(GeometryValidator.NOOP, false, wkt);
114-
} catch (Exception e) {
115-
throw new IllegalArgumentException("Failed to parse WKT: " + e.getMessage(), e);
116-
}
117-
}
118-
119120
public String wkbToWkt(BytesRef wkb) {
120121
return WellKnownText.fromWKB(wkb.bytes, wkb.offset, wkb.length);
121122
}
122123

123124
public Geometry wkbToGeometry(BytesRef wkb) {
124-
return WellKnownBinary.fromWKB(GeometryValidator.NOOP, false, wkb.bytes, wkb.offset, wkb.length);
125+
return WellKnownBinary.fromWKB(validator(), false, wkb.bytes, wkb.offset, wkb.length);
125126
}
126127
}

‎x-pack/plugin/esql/qa/testFixtures/src/main/resources/spatial.csv-spec‎

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1788,6 +1788,74 @@ count:long
17881788
12
17891789
;
17901790

1791+
airportsDistanceLessThanInvalidPoint
1792+
required_capability: st_distance
1793+
1794+
FROM airports
1795+
| EVAL distance = ST_DISTANCE(location, TO_GEOPOINT("POINT(-28.6359 153.5906)"))
1796+
| KEEP distance, location, name, scalerank
1797+
| WHERE distance < 500000 AND scalerank < 6
1798+
| STATS count = COUNT()
1799+
;
1800+
1801+
warning:Line 2:41: evaluation of [TO_GEOPOINT(\"POINT(-28.6359 153.5906)\")] failed, treating result as null. Only first 20 failures recorded.
1802+
warning:Line 2:41: java.lang.IllegalArgumentException: Failed to parse WKT: invalid latitude 153.5906; must be between -90.0 and 90.0
1803+
1804+
count:l
1805+
0
1806+
;
1807+
1808+
airportsDistanceLessThanInvalidPoint2
1809+
required_capability: st_distance
1810+
1811+
FROM airports
1812+
| EVAL distance = ST_DISTANCE(TO_GEOPOINT("POINT(-28.6359 153.5906)"), location)
1813+
| KEEP distance, location, name, scalerank
1814+
| WHERE distance < 500000 AND scalerank < 6
1815+
| STATS count = COUNT()
1816+
;
1817+
1818+
warning:Line 2:31: evaluation of [TO_GEOPOINT(\"POINT(-28.6359 153.5906)\")] failed, treating result as null. Only first 20 failures recorded.
1819+
warning:Line 2:31: java.lang.IllegalArgumentException: Failed to parse WKT: invalid latitude 153.5906; must be between -90.0 and 90.0
1820+
1821+
count:l
1822+
0
1823+
;
1824+
1825+
airportsDistanceGreaterThanInvalidPoint
1826+
required_capability: st_distance
1827+
1828+
FROM airports
1829+
| EVAL distance = ST_DISTANCE(location, TO_GEOPOINT("POINT(-28.6359 153.5906)"))
1830+
| KEEP distance, location, name, scalerank
1831+
| WHERE distance > 500 AND scalerank < 6
1832+
| STATS count = COUNT()
1833+
;
1834+
1835+
warning:Line 2:41: evaluation of [TO_GEOPOINT(\"POINT(-28.6359 153.5906)\")] failed, treating result as null. Only first 20 failures recorded.
1836+
warning:Line 2:41: java.lang.IllegalArgumentException: Failed to parse WKT: invalid latitude 153.5906; must be between -90.0 and 90.0
1837+
1838+
count:l
1839+
0
1840+
;
1841+
1842+
airportsDistanceGreaterThanInvalidPoint2
1843+
required_capability: st_distance
1844+
1845+
FROM airports
1846+
| EVAL distance = ST_DISTANCE(TO_GEOPOINT("POINT(-28.6359 153.5906)"), location)
1847+
| KEEP distance, location, name, scalerank
1848+
| WHERE distance > 500 AND scalerank < 6
1849+
| STATS count = COUNT()
1850+
;
1851+
1852+
warning:Line 2:31: evaluation of [TO_GEOPOINT(\"POINT(-28.6359 153.5906)\")] failed, treating result as null. Only first 20 failures recorded.
1853+
warning:Line 2:31: java.lang.IllegalArgumentException: Failed to parse WKT: invalid latitude 153.5906; must be between -90.0 and 90.0
1854+
1855+
count:l
1856+
0
1857+
;
1858+
17911859
###############################################
17921860
# Tests for Equality and casting with GEO_POINT
17931861

‎x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/convert/ToGeoPoint.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@
2626
import static org.elasticsearch.xpack.esql.core.type.DataType.GEO_POINT;
2727
import static org.elasticsearch.xpack.esql.core.type.DataType.KEYWORD;
2828
import static org.elasticsearch.xpack.esql.core.type.DataType.TEXT;
29-
import static org.elasticsearch.xpack.esql.type.EsqlDataTypeConverter.stringToSpatial;
29+
import static org.elasticsearch.xpack.esql.type.EsqlDataTypeConverter.stringToGeo;
3030

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

9292
@ConvertEvaluator(extraName = "FromString", warnExceptions = { IllegalArgumentException.class })
9393
static BytesRef fromKeyword(BytesRef in) {
94-
return stringToSpatial(in.utf8ToString());
94+
return stringToGeo(in.utf8ToString());
9595
}
9696
}

‎x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/convert/ToGeoShape.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@
2727
import static org.elasticsearch.xpack.esql.core.type.DataType.GEO_SHAPE;
2828
import static org.elasticsearch.xpack.esql.core.type.DataType.KEYWORD;
2929
import static org.elasticsearch.xpack.esql.core.type.DataType.TEXT;
30-
import static org.elasticsearch.xpack.esql.type.EsqlDataTypeConverter.stringToSpatial;
30+
import static org.elasticsearch.xpack.esql.type.EsqlDataTypeConverter.stringToGeo;
3131

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

9494
@ConvertEvaluator(extraName = "FromString", warnExceptions = { IllegalArgumentException.class })
9595
static BytesRef fromKeyword(BytesRef in) {
96-
return stringToSpatial(in.utf8ToString());
96+
return stringToGeo(in.utf8ToString());
9797
}
9898
}

‎x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/type/EsqlDataTypeConverter.java‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,7 @@
104104
import static org.elasticsearch.xpack.esql.core.util.NumericUtils.asLongUnsigned;
105105
import static org.elasticsearch.xpack.esql.core.util.NumericUtils.asUnsignedLong;
106106
import static org.elasticsearch.xpack.esql.core.util.NumericUtils.unsignedLongAsNumber;
107+
import static org.elasticsearch.xpack.esql.core.util.SpatialCoordinateTypes.GEO;
107108
import static org.elasticsearch.xpack.esql.core.util.SpatialCoordinateTypes.UNSPECIFIED;
108109

109110
public class EsqlDataTypeConverter {
@@ -234,6 +235,9 @@ public static Converter converterFor(DataType from, DataType to) {
234235
if (to == DataType.BOOLEAN) {
235236
return EsqlConverter.STRING_TO_BOOLEAN;
236237
}
238+
if (DataType.isSpatialGeo(to)) {
239+
return EsqlConverter.STRING_TO_GEO;
240+
}
237241
if (DataType.isSpatial(to)) {
238242
return EsqlConverter.STRING_TO_SPATIAL;
239243
}
@@ -540,6 +544,10 @@ public static String spatialToString(BytesRef field) {
540544
return UNSPECIFIED.wkbToWkt(field);
541545
}
542546

547+
public static BytesRef stringToGeo(String field) {
548+
return GEO.wktToWkb(field);
549+
}
550+
543551
public static BytesRef stringToSpatial(String field) {
544552
return UNSPECIFIED.wktToWkb(field);
545553
}
@@ -772,6 +780,7 @@ public enum EsqlConverter implements Converter {
772780
STRING_TO_LONG(x -> EsqlDataTypeConverter.stringToLong((String) x)),
773781
STRING_TO_INT(x -> EsqlDataTypeConverter.stringToInt((String) x)),
774782
STRING_TO_BOOLEAN(x -> EsqlDataTypeConverter.stringToBoolean((String) x)),
783+
STRING_TO_GEO(x -> EsqlDataTypeConverter.stringToGeo((String) x)),
775784
STRING_TO_SPATIAL(x -> EsqlDataTypeConverter.stringToSpatial((String) x));
776785

777786
private static final String NAME = "esql-converter";

0 commit comments

Comments
 (0)