Skip to content

Commit ae2964c

Browse files
author
Olivier Favre
committed
Fix geo normalization for ranges and points
Geo normalization of latitude was not properly performed. 90°lat is the north pole, and +1°lat beyond is still the north pole, not the south pole. Latitude should not be wrapped like longitude but rather reflected, accompanied with a +180°lon shift. Normalizing ranges cannot be performed by normalizing its top-left and bottom-right points, but must be split into multiple ranges. And two "qeo"->"geo" type fixes as bonuses. Note: There is still a problem with GeoPolygon, like for ranges, but I'll fix this in a separate commit. See elasticsearch/elasticsearch issue elastic#1997.
1 parent a0d1b9b commit ae2964c

File tree

11 files changed

+538
-81
lines changed

11 files changed

+538
-81
lines changed

‎src/main/java/org/elasticsearch/index/mapper/geo/GeoPointFieldMapper.java

+10-10
Original file line numberDiff line numberDiff line change
@@ -372,11 +372,11 @@ private void parseObjectLatLon(ParseContext context) throws IOException {
372372
}
373373

374374
private void parseLatLon(ParseContext context, double lat, double lon) throws IOException {
375-
if (normalizeLon) {
376-
lon = GeoUtils.normalizeLon(lon);
377-
}
378-
if (normalizeLat) {
379-
lat = GeoUtils.normalizeLat(lat);
375+
if (normalizeLat || normalizeLon) {
376+
Point pt = new Point(lat, lon);
377+
GeoUtils.normalizePoint(pt, normalizeLat, normalizeLon);
378+
lat = pt.lat;
379+
lon = pt.lon;
380380
}
381381

382382
if (validateLat) {
@@ -409,11 +409,11 @@ private void parseGeohash(ParseContext context, String geohash) throws IOExcepti
409409
double lat = values[0];
410410
double lon = values[1];
411411

412-
if (normalizeLon) {
413-
lon = GeoUtils.normalizeLon(lon);
414-
}
415-
if (normalizeLat) {
416-
lat = GeoUtils.normalizeLat(lat);
412+
if (normalizeLat || normalizeLon) {
413+
Point pt = new Point(lat, lon);
414+
GeoUtils.normalizePoint(pt, normalizeLat, normalizeLon);
415+
lat = pt.lat;
416+
lon = pt.lon;
417417
}
418418

419419
if (validateLat) {

‎src/main/java/org/elasticsearch/index/query/GeoBoundingBoxFilterParser.java

+24-19
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121

2222
import org.apache.lucene.search.Filter;
2323
import org.elasticsearch.common.inject.Inject;
24+
import org.elasticsearch.common.lucene.search.XBooleanFilter;
2425
import org.elasticsearch.common.xcontent.XContentParser;
2526
import org.elasticsearch.index.cache.filter.support.CacheKeyFilter;
2627
import org.elasticsearch.index.mapper.FieldMapper;
@@ -29,6 +30,8 @@
2930
import org.elasticsearch.index.search.geo.*;
3031

3132
import java.io.IOException;
33+
import java.util.ArrayList;
34+
import java.util.List;
3235

3336
import static org.elasticsearch.index.query.support.QueryParsers.wrapSmartNameFilter;
3437

@@ -61,8 +64,7 @@ public Filter parse(QueryParseContext parseContext) throws IOException, QueryPar
6164
String filterName = null;
6265
String currentFieldName = null;
6366
XContentParser.Token token;
64-
boolean normalizeLon = true;
65-
boolean normalizeLat = true;
67+
boolean normalize = true;
6668

6769
String type = "memory";
6870

@@ -151,23 +153,21 @@ public Filter parse(QueryParseContext parseContext) throws IOException, QueryPar
151153
} else if ("_cache_key".equals(currentFieldName) || "_cacheKey".equals(currentFieldName)) {
152154
cacheKey = new CacheKeyFilter.Key(parser.text());
153155
} else if ("normalize".equals(currentFieldName)) {
154-
normalizeLat = parser.booleanValue();
155-
normalizeLon = parser.booleanValue();
156+
normalize = parser.booleanValue();
156157
} else if ("type".equals(currentFieldName)) {
157158
type = parser.text();
158159
} else {
159-
throw new QueryParsingException(parseContext.index(), "[qeo_bbox] filter does not support [" + currentFieldName + "]");
160+
throw new QueryParsingException(parseContext.index(), "[geo_bbox] filter does not support [" + currentFieldName + "]");
160161
}
161162
}
162163
}
163164

164-
if (normalizeLat) {
165-
topLeft.lat = GeoUtils.normalizeLat(topLeft.lat);
166-
bottomRight.lat = GeoUtils.normalizeLat(bottomRight.lat);
167-
}
168-
if (normalizeLon) {
169-
topLeft.lon = GeoUtils.normalizeLon(topLeft.lon);
170-
bottomRight.lon = GeoUtils.normalizeLon(bottomRight.lon);
165+
List<Range> ranges = null;
166+
if (normalize) {
167+
ranges = GeoUtils.normalizeRange(topLeft, bottomRight);
168+
} else {
169+
ranges = new ArrayList<Range>(1);
170+
ranges.add(new Range(topLeft, bottomRight));
171171
}
172172

173173
MapperService.SmartNameFieldMappers smartMappers = parseContext.smartFieldMappers(fieldName);
@@ -182,15 +182,20 @@ public Filter parse(QueryParseContext parseContext) throws IOException, QueryPar
182182

183183
fieldName = mapper.names().indexName();
184184

185-
Filter filter;
186-
if ("indexed".equals(type)) {
187-
filter = IndexedGeoBoundingBoxFilter.create(topLeft, bottomRight, geoMapper);
188-
} else if ("memory".equals(type)) {
189-
filter = new InMemoryGeoBoundingBoxFilter(topLeft, bottomRight, fieldName, parseContext.indexCache().fieldData());
190-
} else {
191-
throw new QueryParsingException(parseContext.index(), "geo bounding box type [" + type + "] not supported, either 'indexed' or 'memory' are allowed");
185+
XBooleanFilter boolFilter = new XBooleanFilter();
186+
for (Range r : ranges) {
187+
Filter f;
188+
if ("indexed".equals(type)) {
189+
f = IndexedGeoBoundingBoxFilter.create(topLeft, bottomRight, geoMapper);
190+
} else if ("memory".equals(type)) {
191+
f = new InMemoryGeoBoundingBoxFilter(topLeft, bottomRight, fieldName, parseContext.indexCache().fieldData());
192+
} else {
193+
throw new QueryParsingException(parseContext.index(), "geo bounding box type [" + type + "] not supported, either 'indexed' or 'memory' are allowed");
194+
}
195+
boolFilter.addShould(f);
192196
}
193197

198+
Filter filter = boolFilter;
194199
if (cache) {
195200
filter = parseContext.cacheFilter(filter, cacheKey);
196201
}

‎src/main/java/org/elasticsearch/index/query/GeoDistanceFilterParser.java

+5-5
Original file line numberDiff line numberDiff line change
@@ -168,11 +168,11 @@ public Filter parse(QueryParseContext parseContext) throws IOException, QueryPar
168168
}
169169
distance = geoDistance.normalize(distance, DistanceUnit.MILES);
170170

171-
if (normalizeLat) {
172-
lat = GeoUtils.normalizeLat(lat);
173-
}
174-
if (normalizeLon) {
175-
lon = GeoUtils.normalizeLon(lon);
171+
if (normalizeLat || normalizeLon) {
172+
Point pt = new Point(lat, lon);
173+
GeoUtils.normalizePoint(pt, normalizeLat, normalizeLon);
174+
lat = pt.lat;
175+
lon = pt.lon;
176176
}
177177

178178
MapperService.SmartNameFieldMappers smartMappers = parseContext.smartFieldMappers(fieldName);

‎src/main/java/org/elasticsearch/index/query/GeoDistanceRangeFilterParser.java

+5-5
Original file line numberDiff line numberDiff line change
@@ -224,11 +224,11 @@ public Filter parse(QueryParseContext parseContext) throws IOException, QueryPar
224224
to = geoDistance.normalize(to, DistanceUnit.MILES);
225225
}
226226

227-
if (normalizeLat) {
228-
lat = GeoUtils.normalizeLat(lat);
229-
}
230-
if (normalizeLon) {
231-
lon = GeoUtils.normalizeLon(lon);
227+
if (normalizeLat || normalizeLon) {
228+
Point pt = new Point(lat, lon);
229+
GeoUtils.normalizePoint(pt, normalizeLat, normalizeLon);
230+
lat = pt.lat;
231+
lon = pt.lon;
232232
}
233233

234234
MapperService.SmartNameFieldMappers smartMappers = parseContext.smartFieldMappers(fieldName);

‎src/main/java/org/elasticsearch/index/query/GeoPolygonFilterParser.java

+3-6
Original file line numberDiff line numberDiff line change
@@ -161,12 +161,9 @@ public Filter parse(QueryParseContext parseContext) throws IOException, QueryPar
161161
throw new QueryParsingException(parseContext.index(), "no points defined for geo_polygon filter");
162162
}
163163

164-
for (Point point : points) {
165-
if (normalizeLat) {
166-
point.lat = GeoUtils.normalizeLat(point.lat);
167-
}
168-
if (normalizeLon) {
169-
point.lon = GeoUtils.normalizeLon(point.lon);
164+
if (normalizeLat || normalizeLon) {
165+
for (Point point : points) {
166+
GeoUtils.normalizePoint(point, normalizeLat, normalizeLon);
170167
}
171168
}
172169

0 commit comments

Comments
 (0)