Skip to content

Commit 75508e2

Browse files
author
elasticsearchmachine
committed
Merge remote-tracking branch 'origin/main' into lucene_snapshot
2 parents 09342ab + 844ee68 commit 75508e2

File tree

8 files changed

+115
-15
lines changed

8 files changed

+115
-15
lines changed

‎docs/changelog/128259.yaml‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
pr: 128259
2+
summary: Added geometry validation for GEO types to exit early on invalid latitudes
3+
area: Geo
4+
type: bug
5+
issues:
6+
- 128234

‎muted-tests.yml‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -489,6 +489,12 @@ tests:
489489
- class: org.elasticsearch.xpack.search.CrossClusterAsyncSearchIT
490490
method: testCCSClusterDetailsWhereAllShardsSkippedInCanMatch
491491
issue: https://github.com/elastic/elasticsearch/issues/128418
492+
- class: org.elasticsearch.xpack.esql.action.ForkIT
493+
method: testProfile
494+
issue: https://github.com/elastic/elasticsearch/issues/128377
495+
- class: org.elasticsearch.xpack.esql.action.CrossClusterQueryWithFiltersIT
496+
method: testTimestampFilterFromQuery
497+
issue: https://github.com/elastic/elasticsearch/issues/127332
492498

493499
# Examples:
494500
#

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

Lines changed: 13 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,11 @@ 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+
// We validate the lat/lon values, and ignore any z values
39+
return GeographyValidator.instance(true);
40+
}
3541
},
3642
CARTESIAN {
3743

@@ -67,6 +73,10 @@ public long pointAsLong(double x, double y) {
6773
}
6874
};
6975

76+
protected GeometryValidator validator() {
77+
return GeometryValidator.NOOP;
78+
}
79+
7080
public abstract Point longAsPoint(long encoded);
7181

7282
public abstract long pointAsLong(double x, double y);
@@ -77,7 +87,7 @@ public long wkbAsLong(BytesRef wkb) {
7787
}
7888

7989
public Point wkbAsPoint(BytesRef wkb) {
80-
Geometry geometry = WellKnownBinary.fromWKB(GeometryValidator.NOOP, false, wkb.bytes, wkb.offset, wkb.length);
90+
Geometry geometry = WellKnownBinary.fromWKB(validator(), false, wkb.bytes, wkb.offset, wkb.length);
8191
if (geometry instanceof Point point) {
8292
return point;
8393
} else {
@@ -101,26 +111,18 @@ public BytesRef wktToWkb(String wkt) {
101111
// TODO: we should be able to transform WKT to WKB without building the geometry
102112
// we should as well use different validator for cartesian and geo?
103113
try {
104-
Geometry geometry = WellKnownText.fromWKT(GeometryValidator.NOOP, false, wkt);
114+
Geometry geometry = WellKnownText.fromWKT(validator(), false, wkt);
105115
return new BytesRef(WellKnownBinary.toWKB(geometry, ByteOrder.LITTLE_ENDIAN));
106116
} catch (Exception e) {
107117
throw new IllegalArgumentException("Failed to parse WKT: " + e.getMessage(), e);
108118
}
109119
}
110120

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-
119121
public String wkbToWkt(BytesRef wkb) {
120122
return WellKnownText.fromWKB(wkb.bytes, wkb.offset, wkb.length);
121123
}
122124

123125
public Geometry wkbToGeometry(BytesRef wkb) {
124-
return WellKnownBinary.fromWKB(GeometryValidator.NOOP, false, wkb.bytes, wkb.offset, wkb.length);
126+
return WellKnownBinary.fromWKB(validator(), false, wkb.bytes, wkb.offset, wkb.length);
125127
}
126128
}

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

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

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

‎x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlCapabilities.java‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,11 @@ public enum Cap {
5555
*/
5656
SPATIAL_SHAPES,
5757

58+
/**
59+
* Do validation check on geo_point and geo_shape fields. Done in #128259.
60+
*/
61+
GEO_VALIDATION,
62+
5863
/**
5964
* Support for spatial aggregation {@code ST_CENTROID}. Done in #104269.
6065
*/

‎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)