Skip to content

Commit 6da87aa

Browse files
authored
fix encoding bug when clipping geo-tile latitude mask (elastic#52274)
Due to how geometries are encoded, it is important to compare the bounds of a shape to that of the encoded latitude bounds for geo-tiles.
1 parent c95396d commit 6da87aa

File tree

5 files changed

+32
-13
lines changed

5 files changed

+32
-13
lines changed

server/src/main/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoGridTiler.java

+5-3
Original file line numberDiff line numberDiff line change
@@ -160,7 +160,8 @@ public long encode(double x, double y, int precision) {
160160
/**
161161
* Sets the values of the long[] underlying {@link GeoShapeCellValues}.
162162
*
163-
* If the shape resides between <code>GeoTileUtils.LATITUDE_MASK</code> and 90 degree latitudes, then
163+
* If the shape resides between <code>GeoTileUtils.NORMALIZED_LATITUDE_MASK</code> and 90 or
164+
* between <code>GeoTileUtils.NORMALIZED_NEGATIVE_LATITUDE_MASK</code> and -90 degree latitudes, then
164165
* the shape is not accounted for since geo-tiles are only defined within those bounds.
165166
*
166167
* @param values the bucket values
@@ -182,8 +183,9 @@ public int setValues(GeoShapeCellValues values, MultiGeoValues.GeoValue geoValue
182183

183184
// geo tiles are not defined at the extreme latitudes due to them
184185
// tiling the world as a square.
185-
if ((bounds.top > GeoTileUtils.LATITUDE_MASK && bounds.bottom > GeoTileUtils.LATITUDE_MASK)
186-
|| (bounds.top < -GeoTileUtils.LATITUDE_MASK && bounds.bottom < -GeoTileUtils.LATITUDE_MASK)) {
186+
if ((bounds.top > GeoTileUtils.NORMALIZED_LATITUDE_MASK && bounds.bottom > GeoTileUtils.NORMALIZED_LATITUDE_MASK)
187+
|| (bounds.top < GeoTileUtils.NORMALIZED_NEGATIVE_LATITUDE_MASK
188+
&& bounds.bottom < GeoTileUtils.NORMALIZED_NEGATIVE_LATITUDE_MASK)) {
187189
return 0;
188190
}
189191

server/src/main/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoTileUtils.java

+9-1
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
*/
1919
package org.elasticsearch.search.aggregations.bucket.geogrid;
2020

21+
import org.apache.lucene.geo.GeoEncodingUtils;
2122
import org.apache.lucene.util.SloppyMath;
2223
import org.elasticsearch.ElasticsearchParseException;
2324
import org.elasticsearch.common.geo.GeoPoint;
@@ -46,7 +47,14 @@ public final class GeoTileUtils {
4647
/**
4748
* The geo-tile map is clipped at 85.05112878 to 90 and -85.05112878 to -90
4849
*/
49-
public static final double LATITUDE_MASK = 85.05112878;
50+
public static final double LATITUDE_MASK = 85.0511287798066;
51+
52+
/**
53+
* Since shapes are encoded, their boundaries are to be compared to against the encoded/decoded values of <code>LATITUDE_MASK</code>
54+
*/
55+
static final double NORMALIZED_LATITUDE_MASK = GeoEncodingUtils.decodeLatitude(GeoEncodingUtils.encodeLatitude(LATITUDE_MASK));
56+
static final double NORMALIZED_NEGATIVE_LATITUDE_MASK =
57+
GeoEncodingUtils.decodeLatitude(GeoEncodingUtils.encodeLatitude(-LATITUDE_MASK));
5058

5159
private static final double PI_DIV_2 = Math.PI / 2;
5260

server/src/test/java/org/elasticsearch/common/geo/GeoTestUtils.java

+9
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
package org.elasticsearch.common.geo;
2020

2121
import org.apache.lucene.document.ShapeField;
22+
import org.apache.lucene.geo.GeoEncodingUtils;
2223
import org.apache.lucene.index.IndexableField;
2324
import org.apache.lucene.store.ByteBuffersDataOutput;
2425
import org.apache.lucene.util.BytesRef;
@@ -75,6 +76,14 @@ public static TriangleTreeReader triangleTreeReader(Geometry geometry, Coordinat
7576
return reader;
7677
}
7778

79+
public static double encodeDecodeLat(double lat) {
80+
return GeoEncodingUtils.decodeLatitude(GeoEncodingUtils.encodeLatitude(lat));
81+
}
82+
83+
public static double encodeDecodeLon(double lon) {
84+
return GeoEncodingUtils.decodeLongitude(GeoEncodingUtils.encodeLongitude(lon));
85+
}
86+
7887
public static String toGeoJsonString(Geometry geometry) throws IOException {
7988
XContentBuilder builder = XContentFactory.jsonBuilder();
8089
GeoJson.toXContent(geometry, builder, ToXContent.EMPTY_PARAMS);

server/src/test/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoGridTilerTests.java

+7-6
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,11 @@
4040
import java.util.Arrays;
4141
import java.util.List;
4242

43+
import static org.elasticsearch.common.geo.GeoTestUtils.encodeDecodeLat;
44+
import static org.elasticsearch.common.geo.GeoTestUtils.encodeDecodeLon;
4345
import static org.elasticsearch.common.geo.GeoTestUtils.triangleTreeReader;
44-
import static org.elasticsearch.search.aggregations.bucket.geogrid.GeoTileUtils.LATITUDE_MASK;
46+
import static org.elasticsearch.search.aggregations.bucket.geogrid.GeoTileUtils.NORMALIZED_LATITUDE_MASK;
47+
import static org.elasticsearch.search.aggregations.bucket.geogrid.GeoTileUtils.NORMALIZED_NEGATIVE_LATITUDE_MASK;
4548
import static org.hamcrest.Matchers.equalTo;
4649

4750
public class GeoGridTilerTests extends ESTestCase {
@@ -178,14 +181,12 @@ public void testGeoTileSetValuesBoundingBoxes_UnboundedGeoShapeCellValues() thro
178181
}
179182
}
180183

181-
@AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/37206")
182184
public void testTilerMatchPoint() throws Exception {
183185
int precision = randomIntBetween(0, 4);
184186
Point originalPoint = GeometryTestUtils.randomPoint(false);
185187
int xTile = GeoTileUtils.getXTile(originalPoint.getX(), 1 << precision);
186188
int yTile = GeoTileUtils.getYTile(originalPoint.getY(), 1 << precision);
187189
Rectangle bbox = GeoTileUtils.toBoundingBox(xTile, yTile, precision);
188-
long originalTileHash = GeoTileUtils.longEncode(originalPoint.getX(), originalPoint.getY(), precision);
189190

190191
Point[] pointCorners = new Point[] {
191192
// tile corners
@@ -207,7 +208,7 @@ public void testTilerMatchPoint() throws Exception {
207208
int numTiles = GEOTILE.setValues(unboundedCellValues, value, precision);
208209
assertThat(numTiles, equalTo(1));
209210
long tilerHash = unboundedCellValues.getValues()[0];
210-
long pointHash = GeoTileUtils.longEncode(point.getX(), point.getY(), precision);
211+
long pointHash = GeoTileUtils.longEncode(encodeDecodeLon(point.getX()), encodeDecodeLat(point.getY()), precision);
211212
assertThat(tilerHash, equalTo(pointHash));
212213
}
213214
}
@@ -270,8 +271,8 @@ private int numTiles(MultiGeoValues.GeoValue geoValue, int precision) {
270271
return 1;
271272
}
272273

273-
if ((bounds.top > LATITUDE_MASK && bounds.bottom > LATITUDE_MASK)
274-
|| (bounds.top < -LATITUDE_MASK && bounds.bottom < -LATITUDE_MASK)) {
274+
if ((bounds.top > NORMALIZED_LATITUDE_MASK && bounds.bottom > NORMALIZED_LATITUDE_MASK)
275+
|| (bounds.top < NORMALIZED_NEGATIVE_LATITUDE_MASK && bounds.bottom < NORMALIZED_NEGATIVE_LATITUDE_MASK)) {
275276
return 0;
276277
}
277278

server/src/test/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoTileUtilsTests.java

+2-3
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@
2323
import org.elasticsearch.geometry.Rectangle;
2424
import org.elasticsearch.test.ESTestCase;
2525

26-
import static org.elasticsearch.search.aggregations.bucket.geogrid.GeoTileUtils.LATITUDE_MASK;
2726
import static org.elasticsearch.search.aggregations.bucket.geogrid.GeoTileUtils.MAX_ZOOM;
2827
import static org.elasticsearch.search.aggregations.bucket.geogrid.GeoTileUtils.checkPrecisionRange;
2928
import static org.elasticsearch.search.aggregations.bucket.geogrid.GeoTileUtils.hashToGeoPoint;
@@ -223,8 +222,8 @@ public void testGeoTileAsLongRoutines() {
223222
* so ensure they are clipped correctly.
224223
*/
225224
public void testSingularityAtPoles() {
226-
double minLat = -LATITUDE_MASK;
227-
double maxLat = LATITUDE_MASK;
225+
double minLat = -GeoTileUtils.LATITUDE_MASK;
226+
double maxLat = GeoTileUtils.LATITUDE_MASK;
228227
double lon = randomIntBetween(-180, 180);
229228
double lat = randomBoolean()
230229
? randomDoubleBetween(-90, minLat, true)

0 commit comments

Comments
 (0)