Geo: add validator that only checks altitude (#43893)

By default, we don't check ranges while indexing geo_shapes. As a
result, it is possible to index geoshapes that contain contain
coordinates outside of -90 +90 and -180 +180 ranges. Such geoshapes
will currently break SQL and ML retrieval mechanism. This commit removes
these restriction from the validator is used in SQL and ML retrieval.
This commit is contained in:
Igor Motov 2019-07-10 10:20:39 -04:00
parent 8fda49a834
commit df2e1fb43e
18 changed files with 248 additions and 11 deletions

View File

@ -0,0 +1,128 @@
/*
* Licensed to Elasticsearch under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
package org.elasticsearch.geo.utils;
import org.elasticsearch.geo.geometry.Circle;
import org.elasticsearch.geo.geometry.Geometry;
import org.elasticsearch.geo.geometry.GeometryCollection;
import org.elasticsearch.geo.geometry.GeometryVisitor;
import org.elasticsearch.geo.geometry.Line;
import org.elasticsearch.geo.geometry.LinearRing;
import org.elasticsearch.geo.geometry.MultiLine;
import org.elasticsearch.geo.geometry.MultiPoint;
import org.elasticsearch.geo.geometry.MultiPolygon;
import org.elasticsearch.geo.geometry.Point;
import org.elasticsearch.geo.geometry.Polygon;
import org.elasticsearch.geo.geometry.Rectangle;
/**
* Validator that only checks that altitude only shows up if ignoreZValue is set to true.
*/
public class StandardValidator implements GeometryValidator {
private final boolean ignoreZValue;
public StandardValidator(boolean ignoreZValue) {
this.ignoreZValue = ignoreZValue;
}
protected void checkAltitude(double zValue) {
if (ignoreZValue == false && Double.isNaN(zValue) == false) {
throw new IllegalArgumentException("found Z value [" + zValue + "] but [ignore_z_value] "
+ "parameter is [" + ignoreZValue + "]");
}
}
@Override
public void validate(Geometry geometry) {
if (ignoreZValue == false) {
geometry.visit(new GeometryVisitor<Void, RuntimeException>() {
@Override
public Void visit(Circle circle) throws RuntimeException {
checkAltitude(circle.getAlt());
return null;
}
@Override
public Void visit(GeometryCollection<?> collection) throws RuntimeException {
for (Geometry g : collection) {
g.visit(this);
}
return null;
}
@Override
public Void visit(Line line) throws RuntimeException {
for (int i = 0; i < line.length(); i++) {
checkAltitude(line.getAlt(i));
}
return null;
}
@Override
public Void visit(LinearRing ring) throws RuntimeException {
for (int i = 0; i < ring.length(); i++) {
checkAltitude(ring.getAlt(i));
}
return null;
}
@Override
public Void visit(MultiLine multiLine) throws RuntimeException {
return visit((GeometryCollection<?>) multiLine);
}
@Override
public Void visit(MultiPoint multiPoint) throws RuntimeException {
return visit((GeometryCollection<?>) multiPoint);
}
@Override
public Void visit(MultiPolygon multiPolygon) throws RuntimeException {
return visit((GeometryCollection<?>) multiPolygon);
}
@Override
public Void visit(Point point) throws RuntimeException {
checkAltitude(point.getAlt());
return null;
}
@Override
public Void visit(Polygon polygon) throws RuntimeException {
polygon.getPolygon().visit(this);
for (int i = 0; i < polygon.getNumberOfHoles(); i++) {
polygon.getHole(i).visit(this);
}
return null;
}
@Override
public Void visit(Rectangle rectangle) throws RuntimeException {
checkAltitude(rectangle.getMinAlt());
checkAltitude(rectangle.getMaxAlt());
return null;
}
});
}
}
}

View File

@ -21,6 +21,7 @@ package org.elasticsearch.geo.geometry;
import org.elasticsearch.geo.utils.GeographyValidator;
import org.elasticsearch.geo.utils.GeometryValidator;
import org.elasticsearch.geo.utils.StandardValidator;
import org.elasticsearch.geo.utils.WellKnownText;
import java.io.IOException;
@ -59,5 +60,10 @@ public class CircleTests extends BaseGeometryTestCase<Circle> {
ex = expectThrows(IllegalArgumentException.class, () -> validator.validate(new Circle(10, 200, 1)));
assertEquals("invalid longitude 200.0; must be between -180.0 and 180.0", ex.getMessage());
ex = expectThrows(IllegalArgumentException.class, () -> new StandardValidator(false).validate(new Circle(10, 200, 1, 20)));
assertEquals("found Z value [1.0] but [ignore_z_value] parameter is [false]", ex.getMessage());
new StandardValidator(true).validate(new Circle(10, 200, 1, 20));
}
}

View File

@ -20,6 +20,7 @@
package org.elasticsearch.geo.geometry;
import org.elasticsearch.geo.utils.GeographyValidator;
import org.elasticsearch.geo.utils.StandardValidator;
import org.elasticsearch.geo.utils.WellKnownText;
import java.io.IOException;
@ -58,5 +59,11 @@ public class GeometryCollectionTests extends BaseGeometryTestCase<GeometryCollec
ex = expectThrows(IllegalArgumentException.class, () -> new GeometryCollection<>(
Arrays.asList(new Point(10, 20), new Point(10, 20, 30))));
assertEquals("all elements of the collection should have the same number of dimension", ex.getMessage());
ex = expectThrows(IllegalArgumentException.class, () -> new StandardValidator(false).validate(
new GeometryCollection<Geometry>(Collections.singletonList(new Point(10, 20, 30)))));
assertEquals("found Z value [30.0] but [ignore_z_value] parameter is [false]", ex.getMessage());
new StandardValidator(true).validate(new GeometryCollection<Geometry>(Collections.singletonList(new Point(10, 20, 30))));
}
}

View File

@ -21,6 +21,7 @@ package org.elasticsearch.geo.geometry;
import org.elasticsearch.geo.utils.GeographyValidator;
import org.elasticsearch.geo.utils.GeometryValidator;
import org.elasticsearch.geo.utils.StandardValidator;
import org.elasticsearch.geo.utils.WellKnownText;
import java.io.IOException;
@ -59,6 +60,12 @@ public class LineTests extends BaseGeometryTestCase<Line> {
ex = expectThrows(IllegalArgumentException.class,
() -> validator.validate(new Line(new double[]{1, 100, 3, 1}, new double[]{3, 4, 5, 3})));
assertEquals("invalid latitude 100.0; must be between -90.0 and 90.0", ex.getMessage());
ex = expectThrows(IllegalArgumentException.class, () -> new StandardValidator(false).validate(
new Line(new double[]{1, 2}, new double[]{3, 4}, new double[]{6, 5})));
assertEquals("found Z value [6.0] but [ignore_z_value] parameter is [false]", ex.getMessage());
new StandardValidator(true).validate(new Line(new double[]{1, 2}, new double[]{3, 4}, new double[]{6, 5}));
}
public void testWKTValidation() {

View File

@ -21,6 +21,7 @@ package org.elasticsearch.geo.geometry;
import org.elasticsearch.geo.utils.GeographyValidator;
import org.elasticsearch.geo.utils.GeometryValidator;
import org.elasticsearch.geo.utils.StandardValidator;
import org.elasticsearch.geo.utils.WellKnownText;
import org.elasticsearch.test.ESTestCase;
@ -58,6 +59,12 @@ public class LinearRingTests extends ESTestCase {
ex = expectThrows(IllegalArgumentException.class,
() -> validator.validate(new LinearRing(new double[]{1, 100, 3, 1}, new double[]{3, 4, 5, 3})));
assertEquals("invalid latitude 100.0; must be between -90.0 and 90.0", ex.getMessage());
ex = expectThrows(IllegalArgumentException.class, () -> new StandardValidator(false).validate(
new LinearRing(new double[]{1, 2, 3, 1}, new double[]{3, 4, 5, 3}, new double[]{1, 1, 1, 1})));
assertEquals("found Z value [1.0] but [ignore_z_value] parameter is [false]", ex.getMessage());
new StandardValidator(true).validate(new LinearRing(new double[]{1, 2, 3, 1}, new double[]{3, 4, 5, 3}, new double[]{1, 1, 1, 1}));
}
public void testVisitor() {

View File

@ -20,6 +20,7 @@
package org.elasticsearch.geo.geometry;
import org.elasticsearch.geo.utils.GeographyValidator;
import org.elasticsearch.geo.utils.StandardValidator;
import org.elasticsearch.geo.utils.WellKnownText;
import java.io.IOException;
@ -50,4 +51,13 @@ public class MultiLineTests extends BaseGeometryTestCase<MultiLine> {
assertEquals("multilinestring EMPTY", wkt.toWKT(MultiLine.EMPTY));
assertEquals(MultiLine.EMPTY, wkt.fromWKT("multilinestring EMPTY)"));
}
public void testValidation() {
IllegalArgumentException ex = expectThrows(IllegalArgumentException.class, () -> new StandardValidator(false).validate(
new MultiLine(Collections.singletonList(new Line(new double[]{1, 2}, new double[]{3, 4}, new double[]{6, 5})))));
assertEquals("found Z value [6.0] but [ignore_z_value] parameter is [false]", ex.getMessage());
new StandardValidator(true).validate(
new MultiLine(Collections.singletonList(new Line(new double[]{1, 2}, new double[]{3, 4}, new double[]{6, 5}))));
}
}

View File

@ -20,6 +20,7 @@
package org.elasticsearch.geo.geometry;
import org.elasticsearch.geo.utils.GeographyValidator;
import org.elasticsearch.geo.utils.StandardValidator;
import org.elasticsearch.geo.utils.WellKnownText;
import java.io.IOException;
@ -61,4 +62,12 @@ public class MultiPointTests extends BaseGeometryTestCase<MultiPoint> {
assertEquals("multipoint EMPTY", wkt.toWKT(MultiPoint.EMPTY));
assertEquals(MultiPoint.EMPTY, wkt.fromWKT("multipoint EMPTY)"));
}
public void testValidation() {
IllegalArgumentException ex = expectThrows(IllegalArgumentException.class, () -> new StandardValidator(false).validate(
new MultiPoint(Collections.singletonList(new Point(1, 2 ,3)))));
assertEquals("found Z value [3.0] but [ignore_z_value] parameter is [false]", ex.getMessage());
new StandardValidator(true).validate(new MultiPoint(Collections.singletonList(new Point(1, 2 ,3))));
}
}

View File

@ -20,6 +20,7 @@
package org.elasticsearch.geo.geometry;
import org.elasticsearch.geo.utils.GeographyValidator;
import org.elasticsearch.geo.utils.StandardValidator;
import org.elasticsearch.geo.utils.WellKnownText;
import java.io.IOException;
@ -52,4 +53,16 @@ public class MultiPolygonTests extends BaseGeometryTestCase<MultiPolygon> {
assertEquals("multipolygon EMPTY", wkt.toWKT(MultiPolygon.EMPTY));
assertEquals(MultiPolygon.EMPTY, wkt.fromWKT("multipolygon EMPTY)"));
}
public void testValidation() {
IllegalArgumentException ex = expectThrows(IllegalArgumentException.class, () -> new StandardValidator(false).validate(
new MultiPolygon(Collections.singletonList(
new Polygon(new LinearRing(new double[]{1, 2, 3, 1}, new double[]{3, 4, 5, 3}, new double[]{1, 2, 3, 1}))
))));
assertEquals("found Z value [1.0] but [ignore_z_value] parameter is [false]", ex.getMessage());
new StandardValidator(true).validate(
new MultiPolygon(Collections.singletonList(
new Polygon(new LinearRing(new double[]{1, 2, 3, 1}, new double[]{3, 4, 5, 3}, new double[]{1, 2, 3, 1})))));
}
}

View File

@ -21,6 +21,7 @@ package org.elasticsearch.geo.geometry;
import org.elasticsearch.geo.utils.GeographyValidator;
import org.elasticsearch.geo.utils.GeometryValidator;
import org.elasticsearch.geo.utils.StandardValidator;
import org.elasticsearch.geo.utils.WellKnownText;
import java.io.IOException;
@ -51,6 +52,11 @@ public class PointTests extends BaseGeometryTestCase<Point> {
ex = expectThrows(IllegalArgumentException.class, () -> validator.validate(new Point(10, 500)));
assertEquals("invalid longitude 500.0; must be between -180.0 and 180.0", ex.getMessage());
ex = expectThrows(IllegalArgumentException.class, () -> new StandardValidator(false).validate(new Point(1, 2, 3)));
assertEquals("found Z value [3.0] but [ignore_z_value] parameter is [false]", ex.getMessage());
new StandardValidator(true).validate(new Point(1, 2, 3));
}
public void testWKTValidation() {

View File

@ -20,6 +20,7 @@
package org.elasticsearch.geo.geometry;
import org.elasticsearch.geo.utils.GeographyValidator;
import org.elasticsearch.geo.utils.StandardValidator;
import org.elasticsearch.geo.utils.WellKnownText;
import java.io.IOException;
@ -70,6 +71,13 @@ public class PolygonTests extends BaseGeometryTestCase<Polygon> {
() -> new Polygon(new LinearRing(new double[]{1, 2, 3, 1}, new double[]{3, 4, 5, 3}, new double[]{5, 4, 3, 5}),
Collections.singletonList(new LinearRing(new double[]{1, 2, 3, 1}, new double[]{3, 4, 5, 3}))));
assertEquals("holes must have the same number of dimensions as the polygon", ex.getMessage());
ex = expectThrows(IllegalArgumentException.class, () -> new StandardValidator(false).validate(
new Polygon(new LinearRing(new double[]{1, 2, 3, 1}, new double[]{3, 4, 5, 3}, new double[]{1, 2, 3, 1}))));
assertEquals("found Z value [1.0] but [ignore_z_value] parameter is [false]", ex.getMessage());
new StandardValidator(true).validate(
new Polygon(new LinearRing(new double[]{1, 2, 3, 1}, new double[]{3, 4, 5, 3}, new double[]{1, 2, 3, 1})));
}
public void testWKTValidation() {

View File

@ -21,6 +21,7 @@ package org.elasticsearch.geo.geometry;
import org.elasticsearch.geo.utils.GeographyValidator;
import org.elasticsearch.geo.utils.GeometryValidator;
import org.elasticsearch.geo.utils.StandardValidator;
import org.elasticsearch.geo.utils.WellKnownText;
import java.io.IOException;
@ -59,5 +60,11 @@ public class RectangleTests extends BaseGeometryTestCase<Rectangle> {
ex = expectThrows(IllegalArgumentException.class,
() -> validator.validate(new Rectangle(1, 2, 2, 3, 5, Double.NaN)));
assertEquals("only one altitude value is specified", ex.getMessage());
ex = expectThrows(IllegalArgumentException.class, () -> new StandardValidator(false).validate(
new Rectangle(30, 40, 50, 10, 20, 60)));
assertEquals("found Z value [20.0] but [ignore_z_value] parameter is [false]", ex.getMessage());
new StandardValidator(true).validate(new Rectangle(30, 40, 50, 10, 20, 60));
}
}

View File

@ -24,7 +24,7 @@ import org.elasticsearch.common.xcontent.ToXContent;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentParser;
import org.elasticsearch.geo.geometry.Geometry;
import org.elasticsearch.geo.utils.GeographyValidator;
import org.elasticsearch.geo.utils.StandardValidator;
import org.elasticsearch.geo.utils.GeometryValidator;
import org.elasticsearch.geo.utils.WellKnownText;
@ -38,10 +38,9 @@ public final class GeometryParser {
private final GeoJson geoJsonParser;
private final WellKnownText wellKnownTextParser;
private final GeometryValidator validator;
public GeometryParser(boolean rightOrientation, boolean coerce, boolean ignoreZValue) {
validator = new GeographyValidator(ignoreZValue);
GeometryValidator validator = new StandardValidator(ignoreZValue);
geoJsonParser = new GeoJson(rightOrientation, coerce, validator);
wellKnownTextParser = new WellKnownText(coerce, validator);
}

View File

@ -26,6 +26,7 @@ import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentFactory;
import org.elasticsearch.common.xcontent.XContentParseException;
import org.elasticsearch.common.xcontent.XContentParser;
import org.elasticsearch.geo.geometry.Line;
import org.elasticsearch.geo.geometry.LinearRing;
import org.elasticsearch.geo.geometry.Point;
import org.elasticsearch.geo.geometry.Polygon;
@ -114,6 +115,20 @@ public class GeometryParserTests extends ESTestCase {
newGeoJson.endObject();
assertEquals("{\"val\":\"point (100.0 10.0)\"}", Strings.toString(newGeoJson));
}
// Make sure we can parse values outside the normal lat lon boundaries
XContentBuilder lineGeoJson = XContentFactory.jsonBuilder()
.startObject()
.field("foo", "LINESTRING (100 0, 200 10)")
.endObject();
try (XContentParser parser = createParser(lineGeoJson)) {
parser.nextToken(); // Start object
parser.nextToken(); // Field Name
parser.nextToken(); // Field Value
assertEquals(new Line(new double[]{0, 10}, new double[]{100, 200} ),
new GeometryParser(true, randomBoolean(), randomBoolean()).parse(parser));
}
}
public void testNullParsing() throws Exception {

View File

@ -9,7 +9,7 @@ import org.elasticsearch.common.document.DocumentField;
import org.elasticsearch.geo.geometry.Geometry;
import org.elasticsearch.geo.geometry.Point;
import org.elasticsearch.geo.geometry.ShapeType;
import org.elasticsearch.geo.utils.GeographyValidator;
import org.elasticsearch.geo.utils.StandardValidator;
import org.elasticsearch.geo.utils.WellKnownText;
import org.elasticsearch.search.SearchHit;
@ -127,7 +127,7 @@ public abstract class ExtractedField {
}
private static class GeoShapeField extends FromSource {
private static final WellKnownText wkt = new WellKnownText(true, new GeographyValidator(true));
private static final WellKnownText wkt = new WellKnownText(true, new StandardValidator(true));
GeoShapeField(String alias, String name) {
super(alias, name);

View File

@ -5,7 +5,7 @@
*/
package org.elasticsearch.xpack.sql.jdbc;
import org.elasticsearch.geo.utils.GeographyValidator;
import org.elasticsearch.geo.utils.StandardValidator;
import org.elasticsearch.geo.utils.WellKnownText;
import org.elasticsearch.xpack.sql.proto.StringUtils;
@ -55,7 +55,7 @@ import static org.elasticsearch.xpack.sql.jdbc.JdbcDateUtils.timeAsTime;
*/
final class TypeConverter {
private static WellKnownText WKT = new WellKnownText(true, new GeographyValidator(true));
private static WellKnownText WKT = new WellKnownText(true, new StandardValidator(true));
private TypeConverter() {}

View File

@ -10,7 +10,7 @@ import com.carrotsearch.hppc.IntObjectHashMap;
import org.apache.logging.log4j.Logger;
import org.elasticsearch.geo.geometry.Geometry;
import org.elasticsearch.geo.geometry.Point;
import org.elasticsearch.geo.utils.GeographyValidator;
import org.elasticsearch.geo.utils.StandardValidator;
import org.elasticsearch.geo.utils.WellKnownText;
import org.elasticsearch.xpack.sql.jdbc.EsType;
import org.elasticsearch.xpack.sql.proto.StringUtils;
@ -52,7 +52,7 @@ public class JdbcAssert {
private static final IntObjectHashMap<EsType> SQL_TO_TYPE = new IntObjectHashMap<>();
private static final WellKnownText WKT = new WellKnownText(true, new GeographyValidator(true));
private static final WellKnownText WKT = new WellKnownText(true, new StandardValidator(true));
static {
for (EsType type : EsType.values()) {

View File

@ -286,3 +286,18 @@ Phoenix |Americas |-111.97350500151515
Chicago |Americas |-87.63787407428026
New York |Americas |-73.9900270756334
;
selectLargeLat
SELECT ST_X(ST_WKTToSQL('LINESTRING (200 100, 300 400)')) x;
x:d
200.0
;
selectLargeLon
SELECT ST_Y(ST_WKTToSQL('LINESTRING (200 100, 300 400)')) y;
y:d
100.0
// end::y
;

View File

@ -29,7 +29,7 @@ import org.elasticsearch.geo.geometry.MultiPolygon;
import org.elasticsearch.geo.geometry.Point;
import org.elasticsearch.geo.geometry.Polygon;
import org.elasticsearch.geo.geometry.Rectangle;
import org.elasticsearch.geo.utils.GeographyValidator;
import org.elasticsearch.geo.utils.StandardValidator;
import org.elasticsearch.geo.utils.GeometryValidator;
import org.elasticsearch.geo.utils.WellKnownText;
import org.elasticsearch.xpack.sql.SqlIllegalArgumentException;
@ -51,7 +51,7 @@ public class GeoShape implements ToXContentFragment, NamedWriteable {
private final Geometry shape;
private static final GeometryValidator validator = new GeographyValidator(true);
private static final GeometryValidator validator = new StandardValidator(true);
private static final GeometryParser GEOMETRY_PARSER = new GeometryParser(true, true, true);