honeymoose
/
OpenSearch
mirror of https://github.com/honeymoose/OpenSearch.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Remove .geohash suffix from GeoDistanceQuery and GeoDistanceRangeQuery 

Occasionally the .geohash suffix in Geo{Distance|DistanceRange}Query would conflict with a mapping that defines a sub-field by the same name. This occurs often with nested and multi-fields a mapping defines a geo_point sub-field using the field name "geohash". Since the QueryParser already handles parsing geohash encoded geopoints without requiring the ".geohash" suffix, the suffix parsing can be removed altogether.

This commit removes the .geohash suffix parsing, adds explicit test coverage for the nested query use-case, and adds random distance queries to the nested query test suite.
This commit is contained in:
Nicholas Knize 2016-01-08 22:32:41 -06:00
parent e411cbb060
commit d09ee3f174
7 changed files with 36 additions and 15 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
core/src/main/java/org/elasticsearch/index/mapper/geo/BaseGeoPointFieldMapper.java
View File

@ -65,7 +65,6 @@ public abstract class BaseGeoPointFieldMapper extends FieldMapper implements Arr
public static final String LON = "lon";
public static final String LON_SUFFIX = "." + LON;
public static final String GEOHASH = "geohash";
public static final String GEOHASH_SUFFIX = "." + GEOHASH;
public static final String IGNORE_MALFORMED = "ignore_malformed";
}

3
core/src/main/java/org/elasticsearch/index/query/GeoDistanceQueryParser.java
View File

@ -120,9 +120,6 @@ public class GeoDistanceQueryParser implements QueryParser<GeoDistanceQueryBuild
} else if (currentFieldName.endsWith(GeoPointFieldMapper.Names.LON_SUFFIX)) {
point.resetLon(parser.doubleValue());
fieldName = currentFieldName.substring(0, currentFieldName.length() - GeoPointFieldMapper.Names.LON_SUFFIX.length());
} else if (currentFieldName.endsWith(GeoPointFieldMapper.Names.GEOHASH_SUFFIX)) {
point.resetFromGeoHash(parser.text());
fieldName = currentFieldName.substring(0, currentFieldName.length() - GeoPointFieldMapper.Names.GEOHASH_SUFFIX.length());
} else if (parseContext.parseFieldMatcher().match(currentFieldName, AbstractQueryBuilder.NAME_FIELD)) {
queryName = parser.text();
} else if (parseContext.parseFieldMatcher().match(currentFieldName, AbstractQueryBuilder.BOOST_FIELD)) {

9
core/src/main/java/org/elasticsearch/index/query/GeoDistanceRangeQueryParser.java
View File

@ -196,15 +196,6 @@ public class GeoDistanceRangeQueryParser implements QueryParser<GeoDistanceRange
point = new GeoPoint();
}
point.resetLon(parser.doubleValue());
} else if (currentFieldName.endsWith(GeoPointFieldMapper.Names.GEOHASH_SUFFIX)) {
String maybeFieldName = currentFieldName.substring(0, currentFieldName.length() - GeoPointFieldMapper.Names.GEOHASH_SUFFIX.length());
if (fieldName == null || fieldName.equals(maybeFieldName)) {
fieldName = maybeFieldName;
} else {
throw new ParsingException(parser.getTokenLocation(), "[" + GeoDistanceRangeQueryBuilder.NAME +
"] field name already set to [" + fieldName + "] but found [" + currentFieldName + "]");
}
point = GeoPoint.fromGeohash(parser.text());
} else if (parseContext.parseFieldMatcher().match(currentFieldName, NAME_FIELD)) {
queryName = parser.text();
} else if (parseContext.parseFieldMatcher().match(currentFieldName, BOOST_FIELD)) {

3
core/src/test/java/org/elasticsearch/index/query/AbstractQueryTestCase.java
View File

@ -140,6 +140,7 @@ public abstract class AbstractQueryTestCase<QB extends AbstractQueryBuilder<QB>>
protected static final String DATE_FIELD_NAME = "mapped_date";
protected static final String OBJECT_FIELD_NAME = "mapped_object";
protected static final String GEO_POINT_FIELD_NAME = "mapped_geo_point";
protected static final String GEO_POINT_FIELD_MAPPING = "type=geo_point,lat_lon=true,geohash=true,geohash_prefix=true";
protected static final String GEO_SHAPE_FIELD_NAME = "mapped_geo_shape";
protected static final String[] MAPPED_FIELD_NAMES = new String[] { STRING_FIELD_NAME, INT_FIELD_NAME, DOUBLE_FIELD_NAME,
BOOLEAN_FIELD_NAME, DATE_FIELD_NAME, OBJECT_FIELD_NAME, GEO_POINT_FIELD_NAME, GEO_SHAPE_FIELD_NAME };
@ -300,7 +301,7 @@ public abstract class AbstractQueryTestCase<QB extends AbstractQueryBuilder<QB>>
BOOLEAN_FIELD_NAME, "type=boolean",
DATE_FIELD_NAME, "type=date",
OBJECT_FIELD_NAME, "type=object",
GEO_POINT_FIELD_NAME, "type=geo_point,lat_lon=true,geohash=true,geohash_prefix=true",
GEO_POINT_FIELD_NAME, GEO_POINT_FIELD_MAPPING,
GEO_SHAPE_FIELD_NAME, "type=geo_shape"
).string()), MapperService.MergeReason.MAPPING_UPDATE, false);
// also add mappings for two inner field in the object field

32
core/src/test/java/org/elasticsearch/index/query/GeoDistanceRangeQueryTests.java
View File

@ -24,10 +24,12 @@ import org.apache.lucene.spatial.geopoint.search.GeoPointDistanceRangeQuery;
import org.apache.lucene.spatial.util.GeoDistanceUtils;
import org.apache.lucene.util.NumericUtils;
import org.elasticsearch.Version;
import org.elasticsearch.common.compress.CompressedXContent;
import org.elasticsearch.common.geo.GeoDistance;
import org.elasticsearch.common.geo.GeoPoint;
import org.elasticsearch.common.geo.GeoUtils;
import org.elasticsearch.common.unit.DistanceUnit;
import org.elasticsearch.index.mapper.MapperService;
import org.elasticsearch.index.search.geo.GeoDistanceRangeQuery;
import org.elasticsearch.test.geo.RandomGeoGenerator;
@ -296,6 +298,36 @@ public class GeoDistanceRangeQueryTests extends AbstractQueryTestCase<GeoDistanc
}
}
public void testNestedRangeQuery() throws IOException {
// create a nested geo_point type with a subfield named "geohash" (explicit testing for ISSUE #15179)
MapperService mapperService = queryShardContext().getMapperService();
String nestedMapping =
"{\"nested_doc\" : {\"properties\" : {" +
"\"locations\": {\"properties\": {" +
"\"geohash\": {\"type\": \"geo_point\"}}," +
"\"type\": \"nested\"}" +
"}}}";
mapperService.merge("nested_doc", new CompressedXContent(nestedMapping), MapperService.MergeReason.MAPPING_UPDATE, false);
// create a range query on the nested locations.geohash sub-field
String queryJson =
"{\n" +
" \"nested\": {\n" +
" \"path\": \"locations\",\n" +
" \"query\": {\n" +
" \"geo_distance_range\": {\n" +
" \"from\": \"0.0km\",\n" +
" \"to\" : \"200.0km\",\n" +
" \"locations.geohash\": \"s7ws01wyd7ws\"\n" +
" }\n" +
" }\n" +
" }\n" +
"}\n";
NestedQueryBuilder builder = (NestedQueryBuilder) parseQuery(queryJson);
QueryShardContext context = createShardContext();
builder.toQuery(context);
}
public void testFromJson() throws IOException {
String json =
"{\n" +

2
core/src/test/java/org/elasticsearch/index/query/GeohashCellQueryBuilderTests.java
View File

@ -63,7 +63,7 @@ public class GeohashCellQueryBuilderTests extends AbstractQueryTestCase<Builder>
assertThat(query, instanceOf(TermQuery.class));
TermQuery termQuery = (TermQuery) query;
Term term = termQuery.getTerm();
assertThat(term.field(), equalTo(queryBuilder.fieldName() + GeoPointFieldMapper.Names.GEOHASH_SUFFIX));
assertThat(term.field(), equalTo(queryBuilder.fieldName() + "." + GeoPointFieldMapper.Names.GEOHASH));
String geohash = queryBuilder.geohash();
if (queryBuilder.precision() != null) {
int len = Math.min(queryBuilder.precision(), geohash.length());

1
core/src/test/java/org/elasticsearch/index/query/NestedQueryBuilderTests.java
View File

@ -52,6 +52,7 @@ public class NestedQueryBuilderTests extends AbstractQueryTestCase<NestedQueryBu
BOOLEAN_FIELD_NAME, "type=boolean",
DATE_FIELD_NAME, "type=date",
OBJECT_FIELD_NAME, "type=object",
GEO_POINT_FIELD_NAME, GEO_POINT_FIELD_MAPPING,
"nested1", "type=nested"
).string()), MapperService.MergeReason.MAPPING_UPDATE, false);
}