From 9bbf5c08353576da75e2c012502bf851ccef8009 Mon Sep 17 00:00:00 2001 From: Nicholas Knize Date: Fri, 16 Dec 2016 11:11:30 -0600 Subject: [PATCH 1/2] Add multipolygon support to GeoPolygonQueryBuilder LatLonPoint polygon query in Lucene 6 supports multipolygons and polygons with holes. This commit exposes this capability within elasticsearch. --- .../elasticsearch/common/geo/GeoUtils.java | 49 ++-- .../index/query/GeoPolygonQueryBuilder.java | 223 ++++++++++++------ .../query/GeoPolygonQueryBuilderTests.java | 31 ++- 3 files changed, 217 insertions(+), 86 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/common/geo/GeoUtils.java b/server/src/main/java/org/elasticsearch/common/geo/GeoUtils.java index 795cc235ce759..411c975799907 100644 --- a/server/src/main/java/org/elasticsearch/common/geo/GeoUtils.java +++ b/server/src/main/java/org/elasticsearch/common/geo/GeoUtils.java @@ -498,22 +498,7 @@ public static GeoPoint parseGeoPoint(XContentParser parser, GeoPoint point, fina } } else if(parser.currentToken() == Token.START_ARRAY) { - int element = 0; - while(parser.nextToken() != Token.END_ARRAY) { - if(parser.currentToken() == Token.VALUE_NUMBER) { - element++; - if(element == 1) { - lon = parser.doubleValue(); - } else if(element == 2) { - lat = parser.doubleValue(); - } else { - GeoPoint.assertZValue(ignoreZValue, parser.doubleValue()); - } - } else { - throw new ElasticsearchParseException("numeric value expected"); - } - } - return point.reset(lat, lon); + return parseNumericArray(parser, point); } else if(parser.currentToken() == Token.VALUE_STRING) { String val = parser.text(); if (val.contains(",")) { @@ -545,6 +530,38 @@ private static GeoPoint parseGeoHash(GeoPoint point, String geohash, EffectivePo } } + public static GeoPoint parseNumericArray(XContentParser parser, GeoPoint point) throws IOException { + double lat = Double.NaN; + double lon = Double.NaN; + int element = 0; + while(parser.nextToken() != Token.END_ARRAY) { + if(parser.currentToken() == Token.VALUE_NUMBER) { + element++; + if(element == 1) { + lon = parser.doubleValue(); + } else if(element == 2) { + lat = parser.doubleValue(); + } else { + throw new ElasticsearchParseException("only two values allowed"); + } + } else { + throw new ElasticsearchParseException("numeric value expected"); + } + } + return point.reset(lat, lon); + } + + /** parse a {@link GeoPoint} from a String */ + public static GeoPoint parseGeoPoint(String data, GeoPoint point) { + int comma = data.indexOf(','); + if (comma > 0) { + double lat = Double.parseDouble(data.substring(0, comma).trim()); + double lon = Double.parseDouble(data.substring(comma + 1).trim()); + return point.reset(lat, lon); + } + return point.resetFromGeoHash(data); + } + /** * Parse a precision that can be expressed as an integer or a distance measure like "1km", "10m". * diff --git a/server/src/main/java/org/elasticsearch/index/query/GeoPolygonQueryBuilder.java b/server/src/main/java/org/elasticsearch/index/query/GeoPolygonQueryBuilder.java index a07b4186ed594..a29de79e709ad 100644 --- a/server/src/main/java/org/elasticsearch/index/query/GeoPolygonQueryBuilder.java +++ b/server/src/main/java/org/elasticsearch/index/query/GeoPolygonQueryBuilder.java @@ -50,62 +50,84 @@ public class GeoPolygonQueryBuilder extends AbstractQueryBuilder shell; + private final List>> polygons; private GeoValidationMethod validationMethod = GeoValidationMethod.DEFAULT; private boolean ignoreUnmapped = DEFAULT_IGNORE_UNMAPPED; - public GeoPolygonQueryBuilder(String fieldName, List points) { + public GeoPolygonQueryBuilder(String fieldName, List multiPoly) { if (Strings.isEmpty(fieldName)) { throw new IllegalArgumentException("fieldName must not be null"); } - if (points == null || points.isEmpty()) { + if (multiPoly == null || multiPoly.size() == 0) { throw new IllegalArgumentException("polygon must not be null or empty"); - } else { - GeoPoint start = points.get(0); - if (start.equals(points.get(points.size() - 1))) { - if (points.size() < 4) { - throw new IllegalArgumentException("too few points defined for geo_polygon query"); - } - } else { - if (points.size() < 3) { - throw new IllegalArgumentException("too few points defined for geo_polygon query"); - } - } } this.fieldName = fieldName; - this.shell = new ArrayList<>(points); - if (!shell.get(shell.size() - 1).equals(shell.get(0))) { - shell.add(shell.get(0)); + if (multiPoly.get(0) instanceof List) { + // handle multipolygon queries + this.polygons = new ArrayList<>(multiPoly); + // seal the boundaries + for (int p=0; p)polygon.get(b)); + } + } + } else if (multiPoly.get(0) instanceof GeoPoint) { + // handles { points : [ ... for backward compatibility + List shell = new ArrayList<>(multiPoly); + closeBoundary(shell); + List> poly = new ArrayList<>(); + poly.add(shell); + this.polygons = new ArrayList<>(); + this.polygons.add(poly); + } else { + throw new IllegalArgumentException("invalid multipolygon found"); } } - /** - * Read from a stream. - */ + /** Read from a stream. */ public GeoPolygonQueryBuilder(StreamInput in) throws IOException { super(in); fieldName = in.readString(); - int size = in.readVInt(); - shell = new ArrayList<>(size); - for (int i = 0; i < size; i++) { - shell.add(in.readGeoPoint()); + int numPolys = in.readVInt(); + this.polygons = new ArrayList<>(numPolys); + for (int p=0; p> boundaries = new ArrayList<>(numBoundaries); + for (int b=0; b points = new ArrayList<>(numPoints); + for (int i=0; i> polygon : polygons) { + out.writeVInt(polygon.size()); + for (List boundary : polygon) { + out.writeVInt(boundary.size()); + for (GeoPoint point : boundary) { + out.writeGeoPoint(point); + } + } } validationMethod.writeTo(out); out.writeBoolean(ignoreUnmapped); @@ -115,8 +137,8 @@ public String fieldName() { return fieldName; } - public List points() { - return shell; + public List>> points() { + return polygons; } /** Sets the validation method to use for geo coordinates. */ @@ -163,43 +185,39 @@ protected Query doToQuery(QueryShardContext context) throws IOException { throw new QueryShardException(context, "field [" + fieldName + "] is not a geo_point field"); } - List shell = new ArrayList<>(this.shell.size()); - for (GeoPoint geoPoint : this.shell) { - shell.add(new GeoPoint(geoPoint)); - } - final int shellSize = shell.size(); - - // validation was not available prior to 2.x, so to support bwc - // percolation queries we only ignore_malformed on 2.x created indexes - if (!GeoValidationMethod.isIgnoreMalformed(validationMethod)) { - for (GeoPoint point : shell) { - if (!GeoUtils.isValidLatitude(point.lat())) { - throw new QueryShardException(context, "illegal latitude value [{}] for [{}]", point.lat(), - GeoPolygonQueryBuilder.NAME); + List>> polys = new ArrayList<>(this.polygons); + Polygon[] multiPoly = new Polygon[polys.size()]; + int p = 0; + for (List> poly : polys) { + assert poly.size() > 0; + Polygon[] holes = new Polygon[poly.size() - 1]; + double[] shellLat = null; + double[] shellLon = null; + for (int b=0; b boundary = poly.get(b); + int l = 0; + double[] lat = new double[boundary.size()]; + double[] lon = new double[boundary.size()]; + for (GeoPoint point : boundary) { + // if validation method is set to coerce or ignoreMalformed; normalize the point + if (GeoValidationMethod.isCoerce(validationMethod) + || GeoValidationMethod.isIgnoreMalformed(validationMethod)) { + GeoUtils.normalizePoint(point, true, true); + } + lat[l] = point.lat(); + lon[l++] = point.lon(); } - if (!GeoUtils.isValidLongitude(point.lon())) { - throw new QueryShardException(context, "illegal longitude value [{}] for [{}]", point.lon(), - GeoPolygonQueryBuilder.NAME); + if (b == 0) { + shellLat = lat; + shellLon = lon; + } else { + holes[b-1] = new Polygon(lat, lon); } } + multiPoly[p++] = new Polygon(shellLat, shellLon, holes); } - if (GeoValidationMethod.isCoerce(validationMethod)) { - for (GeoPoint point : shell) { - GeoUtils.normalizePoint(point, true, true); - } - } - - double[] lats = new double[shellSize]; - double[] lons = new double[shellSize]; - GeoPoint p; - for (int i=0; i> polygon = polygons.get(p); + builder.startArray(); + for (int b=0; b points = polygon.get(b); + builder.startArray(); + for (GeoPoint point : points) { + builder.startArray().value(point.lon()).value(point.lat()).endArray(); + } + builder.endArray(); + } + builder.endArray(); } builder.endArray(); builder.endObject(); @@ -221,10 +249,56 @@ protected void doXContent(XContentBuilder builder, Params params) throws IOExcep builder.endObject(); } - public static GeoPolygonQueryBuilder fromXContent(XContentParser parser) throws IOException { + protected static List parseBoundary(XContentParser parser) throws IOException { + XContentParser.Token token; + List boundary = new ArrayList<>(); + while ((token = parser.nextToken()) != Token.END_ARRAY) { + if (token == Token.START_ARRAY) { + boundary.add(GeoUtils.parseGeoPoint(parser, new GeoPoint())); + } else { + throw new ParsingException(parser.getTokenLocation(), + "[geo_polygon] query does not support token type [" + token.name() + "]"); + } + } + + return boundary; + } + + protected static List> parsePolygon(XContentParser parser) throws IOException { + XContentParser.Token token; + List> boundaries = new ArrayList<>(); + // loop through boundaries + while ((token = parser.nextToken()) != Token.END_ARRAY) { + if (token == Token.START_ARRAY) { + boundaries.add(parseBoundary(parser)); + } else { + throw new ParsingException(parser.getTokenLocation(), + "[geo_polygon] query does not support token type [" + token.name() + "]"); + } + } + return boundaries; + } + + protected static List>> parseMultiPolygon(XContentParser parser) throws IOException { + XContentParser.Token token; + List>> polygons = new ArrayList<>(); + // loop through all polygons + while ((token = parser.nextToken()) != Token.END_ARRAY) { + if (token == Token.START_ARRAY) { + polygons.add(parsePolygon(parser)); + } else { + throw new ParsingException(parser.getTokenLocation(), + "[geo_polygon] query does not support token type [" + token.name() + "]"); + } + } + return polygons; + } + + public static GeoPolygonQueryBuilder fromXContent(QueryParseContext parseContext) throws IOException { + XContentParser parser = parseContext.parser(); String fieldName = null; - List shell = null; + List shell = null; Float boost = null; GeoValidationMethod validationMethod = null; @@ -248,6 +322,10 @@ public static GeoPolygonQueryBuilder fromXContent(XContentParser parser) throws while ((token = parser.nextToken()) != Token.END_ARRAY) { shell.add(GeoUtils.parseGeoPoint(parser)); } + } else if (parseContext.getParseFieldMatcher().match(currentFieldName, MULTIPOLYGON_FIELD)) { + // parse a multipolygon as defined in the GeoJSON spec + // Note: shape validation is not performed, possible todo but perhaps overkill + shell = parseMultiPolygon(parser); } else { throw new ParsingException(parser.getTokenLocation(), "[geo_polygon] query does not support [" + currentFieldName + "]"); @@ -294,17 +372,26 @@ public static GeoPolygonQueryBuilder fromXContent(XContentParser parser) throws protected boolean doEquals(GeoPolygonQueryBuilder other) { return Objects.equals(validationMethod, other.validationMethod) && Objects.equals(fieldName, other.fieldName) - && Objects.equals(shell, other.shell) + && Objects.equals(polygons, other.polygons) && Objects.equals(ignoreUnmapped, other.ignoreUnmapped); } @Override protected int doHashCode() { - return Objects.hash(validationMethod, fieldName, shell, ignoreUnmapped); + return Objects.hash(validationMethod, fieldName, polygons, ignoreUnmapped); } @Override public String getWriteableName() { return NAME; } + + private void closeBoundary(List boundary) { + if (!boundary.get(boundary.size() - 1).equals(boundary.get(0))) { + boundary.add(boundary.get(0)); + } + if (boundary.size() < 4) { + throw new IllegalArgumentException("too few points defined for geo_polygon query"); + } + } } diff --git a/server/src/test/java/org/elasticsearch/index/query/GeoPolygonQueryBuilderTests.java b/server/src/test/java/org/elasticsearch/index/query/GeoPolygonQueryBuilderTests.java index 92f017a7bdb7d..73f9b77d82bdf 100644 --- a/server/src/test/java/org/elasticsearch/index/query/GeoPolygonQueryBuilderTests.java +++ b/server/src/test/java/org/elasticsearch/index/query/GeoPolygonQueryBuilderTests.java @@ -195,6 +195,22 @@ public void testParsingAndToQuery4() throws IOException { assertGeoPolygonQuery(query); } + public void testParsingAndToQuery5() throws IOException { + assumeTrue("test runs only when at least a type is registered", getCurrentTypes().length > 0); + String query = "{\n" + + " \"geo_polygon\":{\n" + + " \"" + GEO_POINT_FIELD_NAME + "\":{\n" + + " \"multipolygon\":[ [ [\n" + + " [-70, 40],\n" + + " [-80, 30],\n" + + " [-90, 20]\n" + + " ] ] ]\n" + + " }\n" + + " }\n" + + "}\n"; + assertGeoPolygonQuery(query); + } + private void assertGeoPolygonQuery(String query) throws IOException { QueryShardContext context = createShardContext(); parseQuery(query).toQuery(context); @@ -214,9 +230,20 @@ public void testFromJson() throws IOException { " \"boost\" : 1.0\n" + " }\n" + "}"; + String expected = + "{\n" + + " \"geo_polygon\" : {\n" + + " \"person.location\" : {\n" + + " \"multipolygon\" : [ [ [ [ -70.0, 40.0 ], [ -80.0, 30.0 ], [ -90.0, 20.0 ], [ -70.0, 40.0 ] ] ] ]\n" + + " },\n" + + " \"validation_method\" : \"STRICT\",\n" + + " \"ignore_unmapped\" : false,\n" + + " \"boost\" : 1.0\n" + + " }\n" + + "}"; GeoPolygonQueryBuilder parsed = (GeoPolygonQueryBuilder) parseQuery(json); - checkGeneratedJson(json, parsed); - assertEquals(json, 4, parsed.points().size()); + checkGeneratedJson(expected, parsed); + assertEquals(json, 4, parsed.points().get(0).get(0).size()); } public void testIgnoreUnmapped() throws IOException { From 0cd36c5d6dc206e85f316f2874098217401843a1 Mon Sep 17 00:00:00 2001 From: Nicholas Knize Date: Wed, 6 Feb 2019 10:24:12 -0600 Subject: [PATCH 2/2] rebase old PR to master. initial round of updates --- .../index/query/GeoPolygonQueryBuilder.java | 5 ++-- .../query/GeoPolygonQueryBuilderTests.java | 28 +++++++++++++++---- 2 files changed, 24 insertions(+), 9 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/index/query/GeoPolygonQueryBuilder.java b/server/src/main/java/org/elasticsearch/index/query/GeoPolygonQueryBuilder.java index a29de79e709ad..3d76d963b5845 100644 --- a/server/src/main/java/org/elasticsearch/index/query/GeoPolygonQueryBuilder.java +++ b/server/src/main/java/org/elasticsearch/index/query/GeoPolygonQueryBuilder.java @@ -294,8 +294,7 @@ protected static List>> parseMultiPolygon(XContentParser par return polygons; } - public static GeoPolygonQueryBuilder fromXContent(QueryParseContext parseContext) throws IOException { - XContentParser parser = parseContext.parser(); + public static GeoPolygonQueryBuilder fromXContent(XContentParser parser) throws IOException { String fieldName = null; List shell = null; @@ -322,7 +321,7 @@ public static GeoPolygonQueryBuilder fromXContent(QueryParseContext parseContext while ((token = parser.nextToken()) != Token.END_ARRAY) { shell.add(GeoUtils.parseGeoPoint(parser)); } - } else if (parseContext.getParseFieldMatcher().match(currentFieldName, MULTIPOLYGON_FIELD)) { + } else if (MULTIPOLYGON_FIELD.match(currentFieldName, parser.getDeprecationHandler())) { // parse a multipolygon as defined in the GeoJSON spec // Note: shape validation is not performed, possible todo but perhaps overkill shell = parseMultiPolygon(parser); diff --git a/server/src/test/java/org/elasticsearch/index/query/GeoPolygonQueryBuilderTests.java b/server/src/test/java/org/elasticsearch/index/query/GeoPolygonQueryBuilderTests.java index 73f9b77d82bdf..eeece602ebf09 100644 --- a/server/src/test/java/org/elasticsearch/index/query/GeoPolygonQueryBuilderTests.java +++ b/server/src/test/java/org/elasticsearch/index/query/GeoPolygonQueryBuilderTests.java @@ -196,15 +196,31 @@ public void testParsingAndToQuery4() throws IOException { } public void testParsingAndToQuery5() throws IOException { - assumeTrue("test runs only when at least a type is registered", getCurrentTypes().length > 0); String query = "{\n" + " \"geo_polygon\":{\n" + " \"" + GEO_POINT_FIELD_NAME + "\":{\n" + - " \"multipolygon\":[ [ [\n" + - " [-70, 40],\n" + - " [-80, 30],\n" + - " [-90, 20]\n" + - " ] ] ]\n" + + " \"multipolygon\":[\n" + + " [\n" + + " [[-70, 40],\n" + + " [-80, 30],\n" + + " [-90, 20]]\n" + + " ],\n" + + " [\n" + + " [[70, 40],\n" + + " [80, 30],\n" + + " [90, 20]]\n" + + " ],\n" + + " [\n" + + " [[-10, -10],\n" + + " [10, -10],\n" + + " [10, 10],\n" + + " [-10, 10]],\n" + + " [[-5, -5],\n" + + " [5, -5],\n" + + " [5, 5],\n" + + " [-5, 5]]\n" + + " ]\n" + + " ]\n" + " }\n" + " }\n" + "}\n";