honeymoose/OpenSearch
1
0
Fork 0
mirror of https://github.com/honeymoose/OpenSearch.git synced 2025-03-25 09:28:27 +00:00
Code Issues Packages Projects Releases Wiki Activity

Deprecate optimize_bbox on geodistance queries

Browse Source 
Deprecates the optimize_bbox parameter on geodistance queries. This has no longer been needed since version 2.2 because lucene geo distance queries (postings and LatLonPoint) already optimize by bounding box.
This commit is contained in:
Nicholas Knize 2016-08-23 09:14:17 -05:00
parent e22074c97f
commit 28ed0e7abf
7 changed files with 80 additions and 67 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

38
core/src/main/java/org/elasticsearch/index/query/GeoDistanceQueryBuilder.java
View File

@ -63,6 +63,7 @@ public class GeoDistanceQueryBuilder extends AbstractQueryBuilder<GeoDistanceQue
/** Default for geo distance computation. */
public static final GeoDistance DEFAULT_GEO_DISTANCE = GeoDistance.DEFAULT;
/** Default for optimising query through pre computed bounding box query. */
@Deprecated
public static final String DEFAULT_OPTIMIZE_BBOX = "memory";
/**
@ -75,7 +76,9 @@ public class GeoDistanceQueryBuilder extends AbstractQueryBuilder<GeoDistanceQue
.withAllDeprecated("use validation_method instead");
private static final ParseField COERCE_FIELD = new ParseField("coerce", "normalize")
.withAllDeprecated("use validation_method instead");
private static final ParseField OPTIMIZE_BBOX_FIELD = new ParseField("optimize_bbox");
@Deprecated
private static final ParseField OPTIMIZE_BBOX_FIELD = new ParseField("optimize_bbox")
.withAllDeprecated("no replacement: `optimize_bbox` is no longer supported due to recent improvements");
private static final ParseField DISTANCE_TYPE_FIELD = new ParseField("distance_type");
private static final ParseField UNIT_FIELD = new ParseField("unit");
private static final ParseField DISTANCE_FIELD = new ParseField("distance");
@ -89,7 +92,7 @@ public class GeoDistanceQueryBuilder extends AbstractQueryBuilder<GeoDistanceQue
/** Algorithm to use for distance computation. */
private GeoDistance geoDistance = DEFAULT_GEO_DISTANCE;
/** Whether or not to use a bbox for pre-filtering. TODO change to enum? */
private String optimizeBbox = DEFAULT_OPTIMIZE_BBOX;
private String optimizeBbox = null;
/** How strict should geo coordinate validation be? */
private GeoValidationMethod validationMethod = GeoValidationMethod.DEFAULT;
@ -115,7 +118,7 @@ public class GeoDistanceQueryBuilder extends AbstractQueryBuilder<GeoDistanceQue
distance = in.readDouble();
validationMethod = GeoValidationMethod.readFromStream(in);
center = in.readGeoPoint();
optimizeBbox = in.readString();
optimizeBbox = in.readOptionalString();
geoDistance = GeoDistance.readFromStream(in);
ignoreUnmapped = in.readBoolean();
}
@ -126,7 +129,7 @@ public class GeoDistanceQueryBuilder extends AbstractQueryBuilder<GeoDistanceQue
out.writeDouble(distance);
validationMethod.writeTo(out);
out.writeGeoPoint(center);
out.writeString(optimizeBbox);
out.writeOptionalString(optimizeBbox);
geoDistance.writeTo(out);
out.writeBoolean(ignoreUnmapped);
}
@ -220,26 +223,20 @@ public class GeoDistanceQueryBuilder extends AbstractQueryBuilder<GeoDistanceQue
* Set this to memory or indexed if before running the distance
* calculation you want to limit the candidates to hits in the
* enclosing bounding box.
* @deprecated
**/
@Deprecated
public GeoDistanceQueryBuilder optimizeBbox(String optimizeBbox) {
if (optimizeBbox == null) {
throw new IllegalArgumentException("optimizeBbox must not be null");
}
switch (optimizeBbox) {
case "none":
case "memory":
case "indexed":
break;
default:
throw new IllegalArgumentException("optimizeBbox must be one of [none, memory, indexed]");
}
this.optimizeBbox = optimizeBbox;
return this;
}
/**
* Returns whether or not to run a BoundingBox query prior to
* distance query for optimization purposes.*/
* distance query for optimization purposes.
* @deprecated
**/
@Deprecated
public String optimizeBbox() {
return this.optimizeBbox;
}
@ -303,8 +300,9 @@ public class GeoDistanceQueryBuilder extends AbstractQueryBuilder<GeoDistanceQue
if (indexVersionCreated.before(Version.V_2_2_0)) {
GeoPointFieldMapperLegacy.GeoPointFieldType geoFieldType = ((GeoPointFieldMapperLegacy.GeoPointFieldType) fieldType);
IndexGeoPointFieldData indexFieldData = shardContext.getForField(fieldType);
String bboxOptimization = Strings.isEmpty(optimizeBbox) ? DEFAULT_OPTIMIZE_BBOX : optimizeBbox;
return new GeoDistanceRangeQuery(center, null, normDistance, true, false, geoDistance,
geoFieldType, indexFieldData, optimizeBbox);
geoFieldType, indexFieldData, bboxOptimization);
}
// if index created V_2_2 use (soon to be legacy) numeric encoding postings format
@ -324,7 +322,9 @@ public class GeoDistanceQueryBuilder extends AbstractQueryBuilder<GeoDistanceQue
builder.startArray(fieldName).value(center.lon()).value(center.lat()).endArray();
builder.field(DISTANCE_FIELD.getPreferredName(), distance);
builder.field(DISTANCE_TYPE_FIELD.getPreferredName(), geoDistance.name().toLowerCase(Locale.ROOT));
builder.field(OPTIMIZE_BBOX_FIELD.getPreferredName(), optimizeBbox);
if (Strings.isEmpty(optimizeBbox) == false) {
builder.field(OPTIMIZE_BBOX_FIELD.getPreferredName(), optimizeBbox);
}
builder.field(VALIDATION_METHOD_FIELD.getPreferredName(), validationMethod);
builder.field(IGNORE_UNMAPPED_FIELD.getPreferredName(), ignoreUnmapped);
printBoostAndQueryName(builder);
@ -344,7 +344,7 @@ public class GeoDistanceQueryBuilder extends AbstractQueryBuilder<GeoDistanceQue
Object vDistance = null;
DistanceUnit unit = GeoDistanceQueryBuilder.DEFAULT_DISTANCE_UNIT;
GeoDistance geoDistance = GeoDistanceQueryBuilder.DEFAULT_GEO_DISTANCE;
String optimizeBbox = GeoDistanceQueryBuilder.DEFAULT_OPTIMIZE_BBOX;
String optimizeBbox = null;
boolean coerce = GeoValidationMethod.DEFAULT_LENIENT_PARSING;
boolean ignoreMalformed = GeoValidationMethod.DEFAULT_LENIENT_PARSING;
GeoValidationMethod validationMethod = null;

31
core/src/main/java/org/elasticsearch/index/query/GeoDistanceRangeQueryBuilder.java
View File

@ -54,6 +54,7 @@ public class GeoDistanceRangeQueryBuilder extends AbstractQueryBuilder<GeoDistan
public static final boolean DEFAULT_INCLUDE_UPPER = true;
public static final GeoDistance DEFAULT_GEO_DISTANCE = GeoDistance.DEFAULT;
public static final DistanceUnit DEFAULT_UNIT = DistanceUnit.DEFAULT;
@Deprecated
public static final String DEFAULT_OPTIMIZE_BBOX = "memory";
/**
@ -73,7 +74,9 @@ public class GeoDistanceRangeQueryBuilder extends AbstractQueryBuilder<GeoDistan
private static final ParseField DISTANCE_TYPE_FIELD = new ParseField("distance_type");
private static final ParseField NAME_FIELD = new ParseField("_name");
private static final ParseField BOOST_FIELD = new ParseField("boost");
private static final ParseField OPTIMIZE_BBOX_FIELD = new ParseField("optimize_bbox");
@Deprecated
private static final ParseField OPTIMIZE_BBOX_FIELD = new ParseField("optimize_bbox")
.withAllDeprecated("no replacement: `optimize_bbox` is no longer supported due to recent improvements");
private static final ParseField COERCE_FIELD = new ParseField("coerce", "normalize")
.withAllDeprecated("use validation_method instead");
private static final ParseField IGNORE_MALFORMED_FIELD = new ParseField("ignore_malformed")
@ -96,7 +99,7 @@ public class GeoDistanceRangeQueryBuilder extends AbstractQueryBuilder<GeoDistan
private DistanceUnit unit = DEFAULT_UNIT;
private String optimizeBbox = DEFAULT_OPTIMIZE_BBOX;
private String optimizeBbox = null;
private GeoValidationMethod validationMethod = GeoValidationMethod.DEFAULT;
@ -132,7 +135,7 @@ public class GeoDistanceRangeQueryBuilder extends AbstractQueryBuilder<GeoDistan
includeUpper = in.readBoolean();
unit = DistanceUnit.valueOf(in.readString());
geoDistance = GeoDistance.readFromStream(in);
optimizeBbox = in.readString();
optimizeBbox = in.readOptionalString();
validationMethod = GeoValidationMethod.readFromStream(in);
ignoreUnmapped = in.readBoolean();
}
@ -147,7 +150,7 @@ public class GeoDistanceRangeQueryBuilder extends AbstractQueryBuilder<GeoDistan
out.writeBoolean(includeUpper);
out.writeString(unit.name());
geoDistance.writeTo(out);;
out.writeString(optimizeBbox);
out.writeOptionalString(optimizeBbox);
validationMethod.writeTo(out);
out.writeBoolean(ignoreUnmapped);
}
@ -242,22 +245,13 @@ public class GeoDistanceRangeQueryBuilder extends AbstractQueryBuilder<GeoDistan
return unit;
}
@Deprecated
public GeoDistanceRangeQueryBuilder optimizeBbox(String optimizeBbox) {
if (optimizeBbox == null) {
throw new IllegalArgumentException("optimizeBbox must not be null");
}
switch (optimizeBbox) {
case "none":
case "memory":
case "indexed":
break;
default:
throw new IllegalArgumentException("optimizeBbox must be one of [none, memory, indexed]");
}
this.optimizeBbox = optimizeBbox;
return this;
}
@Deprecated
public String optimizeBbox() {
return optimizeBbox;
}
@ -356,8 +350,9 @@ public class GeoDistanceRangeQueryBuilder extends AbstractQueryBuilder<GeoDistan
if (indexVersionCreated.before(Version.V_2_2_0)) {
GeoPointFieldMapperLegacy.GeoPointFieldType geoFieldType = ((GeoPointFieldMapperLegacy.GeoPointFieldType) fieldType);
IndexGeoPointFieldData indexFieldData = context.getForField(fieldType);
String bboxOptimization = Strings.isEmpty(optimizeBbox) ? DEFAULT_OPTIMIZE_BBOX : optimizeBbox;
return new GeoDistanceRangeQuery(point, fromValue, toValue, includeLower, includeUpper, geoDistance, geoFieldType,
indexFieldData, optimizeBbox);
indexFieldData, bboxOptimization);
}
// if index created V_2_2 use (soon to be legacy) numeric encoding postings format
@ -380,7 +375,9 @@ public class GeoDistanceRangeQueryBuilder extends AbstractQueryBuilder<GeoDistan
builder.field(INCLUDE_UPPER_FIELD.getPreferredName(), includeUpper);
builder.field(UNIT_FIELD.getPreferredName(), unit);
builder.field(DISTANCE_TYPE_FIELD.getPreferredName(), geoDistance.name().toLowerCase(Locale.ROOT));
builder.field(OPTIMIZE_BBOX_FIELD.getPreferredName(), optimizeBbox);
if (Strings.isEmpty(optimizeBbox) == false) {
builder.field(OPTIMIZE_BBOX_FIELD.getPreferredName(), optimizeBbox);
}
builder.field(VALIDATION_METHOD.getPreferredName(), validationMethod);
builder.field(IGNORE_UNMAPPED_FIELD.getPreferredName(), ignoreUnmapped);
printBoostAndQueryName(builder);

10
core/src/main/java/org/elasticsearch/index/search/geo/GeoDistanceRangeQuery.java
View File

@ -40,8 +40,10 @@ import org.elasticsearch.index.mapper.GeoPointFieldMapperLegacy;
import java.io.IOException;
/**
*
* Query geo_point fields by distance ranges. Used for indexes created prior to 2.2
* @deprecated
*/
@Deprecated
public class GeoDistanceRangeQuery extends Query {
private final double lat;
@ -91,7 +93,8 @@ public class GeoDistanceRangeQuery extends Query {
if ("memory".equals(optimizeBbox)) {
boundingBoxFilter = null;
} else if ("indexed".equals(optimizeBbox)) {
boundingBoxFilter = IndexedGeoBoundingBoxQuery.create(distanceBoundingCheck.topLeft(), distanceBoundingCheck.bottomRight(), fieldType);
boundingBoxFilter = IndexedGeoBoundingBoxQuery.create(distanceBoundingCheck.topLeft(),
distanceBoundingCheck.bottomRight(), fieldType);
distanceBoundingCheck = GeoDistance.ALWAYS_INSTANCE; // fine, we do the bounding box check using the filter
} else {
throw new IllegalArgumentException("type [" + optimizeBbox + "] for bounding box optimization not supported");
@ -207,7 +210,8 @@ public class GeoDistanceRangeQuery extends Query {
@Override
public String toString(String field) {
return "GeoDistanceRangeQuery(" + indexFieldData.getFieldName() + ", " + geoDistance + ", [" + inclusiveLowerPoint + " - " + inclusiveUpperPoint + "], " + lat + ", " + lon + ")";
return "GeoDistanceRangeQuery(" + indexFieldData.getFieldName() + ", " + geoDistance + ", ["
+ inclusiveLowerPoint + " - " + inclusiveUpperPoint + "], " + lat + ", " + lon + ")";
}
@Override

27
core/src/test/java/org/elasticsearch/index/query/GeoDistanceQueryBuilderTests.java
View File

@ -71,10 +71,6 @@ public class GeoDistanceQueryBuilderTests extends AbstractQueryTestCase<GeoDista
qb.setValidationMethod(randomFrom(GeoValidationMethod.values()));
}
if (randomBoolean()) {
qb.optimizeBbox(randomFrom("none", "memory", "indexed"));
}
if (randomBoolean()) {
qb.geoDistance(randomFrom(GeoDistance.values()));
}
@ -118,9 +114,6 @@ public class GeoDistanceQueryBuilderTests extends AbstractQueryTestCase<GeoDista
e = expectThrows(IllegalArgumentException.class, () -> query.geoDistance(null));
assertEquals("geoDistance must not be null", e.getMessage());
e = expectThrows(IllegalArgumentException.class, () -> query.optimizeBbox(null));
assertEquals("optimizeBbox must not be null", e.getMessage());
}
/**
@ -368,7 +361,6 @@ public class GeoDistanceQueryBuilderTests extends AbstractQueryTestCase<GeoDista
" \"pin.location\" : [ -70.0, 40.0 ],\n" +
" \"distance\" : 12000.0,\n" +
" \"distance_type\" : \"sloppy_arc\",\n" +
" \"optimize_bbox\" : \"memory\",\n" +
" \"validation_method\" : \"STRICT\",\n" +
" \"ignore_unmapped\" : false,\n" +
" \"boost\" : 1.0\n" +
@ -381,6 +373,23 @@ public class GeoDistanceQueryBuilderTests extends AbstractQueryTestCase<GeoDista
assertEquals(json, 12000.0, parsed.distance(), 0.0001);
}
public void testOptimizeBboxFails() throws IOException {
String json =
"{\n" +
" \"geo_distance\" : {\n" +
" \"pin.location\" : [ -70.0, 40.0 ],\n" +
" \"distance\" : 12000.0,\n" +
" \"distance_type\" : \"sloppy_arc\",\n" +
" \"optimize_bbox\" : \"memory\",\n" +
" \"validation_method\" : \"STRICT\",\n" +
" \"ignore_unmapped\" : false,\n" +
" \"boost\" : 1.0\n" +
" }\n" +
"}";
IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> parseQuery(json));
assertTrue(e.getMessage().startsWith("Deprecated field "));
}
public void testFromCoerceFails() throws IOException {
String json =
"{\n" +
@ -388,7 +397,6 @@ public class GeoDistanceQueryBuilderTests extends AbstractQueryTestCase<GeoDista
" \"pin.location\" : [ -70.0, 40.0 ],\n" +
" \"distance\" : 12000.0,\n" +
" \"distance_type\" : \"sloppy_arc\",\n" +
" \"optimize_bbox\" : \"memory\",\n" +
" \"coerce\" : true,\n" +
" \"ignore_unmapped\" : false,\n" +
" \"boost\" : 1.0\n" +
@ -405,7 +413,6 @@ public class GeoDistanceQueryBuilderTests extends AbstractQueryTestCase<GeoDista
" \"pin.location\" : [ -70.0, 40.0 ],\n" +
" \"distance\" : 12000.0,\n" +
" \"distance_type\" : \"sloppy_arc\",\n" +
" \"optimize_bbox\" : \"memory\",\n" +
" \"ignore_malformed\" : true,\n" +
" \"ignore_unmapped\" : false,\n" +
" \"boost\" : 1.0\n" +

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

@ -46,7 +46,6 @@ public class GeoDistanceRangeQueryTests extends AbstractQueryTestCase<GeoDistanc
@Override
protected GeoDistanceRangeQueryBuilder doCreateTestQueryBuilder() {
Version version = createShardContext().indexVersionCreated();
GeoDistanceRangeQueryBuilder builder;
GeoPoint randomPoint = RandomGeoGenerator.randomPointIn(random(), -180.0, -89.9, 180.0, 89.9);
if (randomBoolean()) {
@ -106,9 +105,6 @@ public class GeoDistanceRangeQueryTests extends AbstractQueryTestCase<GeoDistanc
if (randomBoolean()) {
builder.geoDistance(randomFrom(GeoDistance.values()));
}
if (randomBoolean() && version.before(Version.V_2_2_0)) {
builder.optimizeBbox(randomFrom("none", "memory", "indexed"));
}
builder.unit(fromToUnits);
if (randomBoolean()) {
builder.setValidationMethod(randomFrom(GeoValidationMethod.values()));
@ -245,14 +241,6 @@ public class GeoDistanceRangeQueryTests extends AbstractQueryTestCase<GeoDistanc
assertEquals("[to] must not be null", e.getMessage());
}
public void testInvalidOptimizeBBox() {
GeoDistanceRangeQueryBuilder builder = new GeoDistanceRangeQueryBuilder(GEO_POINT_FIELD_NAME, new GeoPoint());
IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> builder.optimizeBbox(null));
assertEquals("optimizeBbox must not be null", e.getMessage());
e = expectThrows(IllegalArgumentException.class, () -> builder.optimizeBbox("foo"));
assertEquals("optimizeBbox must be one of [none, memory, indexed]", e.getMessage());
}
public void testInvalidGeoDistance() {
GeoDistanceRangeQueryBuilder builder = new GeoDistanceRangeQueryBuilder(GEO_POINT_FIELD_NAME, new GeoPoint());
IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> builder.geoDistance(null));
@ -306,7 +294,6 @@ public class GeoDistanceRangeQueryTests extends AbstractQueryTestCase<GeoDistanc
" \"include_upper\" : true,\n" +
" \"unit\" : \"m\",\n" +
" \"distance_type\" : \"sloppy_arc\",\n" +
" \"optimize_bbox\" : \"memory\",\n" +
" \"validation_method\" : \"STRICT\",\n" +
" \"ignore_unmapped\" : false,\n" +
" \"boost\" : 1.0\n" +
@ -317,6 +304,26 @@ public class GeoDistanceRangeQueryTests extends AbstractQueryTestCase<GeoDistanc
assertEquals(json, -70.0, parsed.point().lon(), 0.0001);
}
public void testFromJsonOptimizeBboxFails() throws IOException {
String json =
"{\n" +
" \"geo_distance_range\" : {\n" +
" \"pin.location\" : [ -70.0, 40.0 ],\n" +
" \"from\" : \"200km\",\n" +
" \"to\" : \"400km\",\n" +
" \"include_lower\" : true,\n" +
" \"include_upper\" : true,\n" +
" \"unit\" : \"m\",\n" +
" \"distance_type\" : \"sloppy_arc\",\n" +
" \"optimize_bbox\" : \"memory\",\n" +
" \"ignore_unmapped\" : false,\n" +
" \"boost\" : 1.0\n" +
" }\n" +
"}";
IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> parseQuery(json));
assertTrue(e.getMessage().startsWith("Deprecated field "));
}
public void testFromJsonCoerceFails() throws IOException {
String json =
"{\n" +
@ -328,7 +335,6 @@ public class GeoDistanceRangeQueryTests extends AbstractQueryTestCase<GeoDistanc
" \"include_upper\" : true,\n" +
" \"unit\" : \"m\",\n" +
" \"distance_type\" : \"sloppy_arc\",\n" +
" \"optimize_bbox\" : \"memory\",\n" +
" \"coerce\" : true,\n" +
" \"ignore_unmapped\" : false,\n" +
" \"boost\" : 1.0\n" +
@ -349,7 +355,6 @@ public class GeoDistanceRangeQueryTests extends AbstractQueryTestCase<GeoDistanc
" \"include_upper\" : true,\n" +
" \"unit\" : \"m\",\n" +
" \"distance_type\" : \"sloppy_arc\",\n" +
" \"optimize_bbox\" : \"memory\",\n" +
" \"ignore_malformed\" : true,\n" +
" \"ignore_unmapped\" : false,\n" +
" \"boost\" : 1.0\n" +

2
docs/reference/query-dsl/geo-distance-query.asciidoc
View File

@ -199,7 +199,7 @@ The following are options allowed on the filter:
before the distance check. Defaults to `memory` which will do in memory
checks. Can also have values of `indexed` to use indexed value check (make
sure the `geo_point` type index lat lon in this case), or `none` which
disables bounding box optimization.
disables bounding box optimization. deprecated[2.2]
`_name`::

4
test/framework/src/main/java/org/elasticsearch/test/AbstractQueryTestCase.java
View File

@ -947,8 +947,8 @@ public abstract class AbstractQueryTestCase<QB extends AbstractQueryBuilder<QB>>
* <li> Take a reference documentation example.
* <li> Stick it into the createParseableQueryJson method of the respective query test.
* <li> Manually check that what the QueryBuilder generates equals the input json ignoring default options.
* <li> Put the manual checks into the asserQueryParsedFromJson method.
* <li> Now copy the generated json including default options into createParseableQueryJso
* <li> Put the manual checks into the assertQueryParsedFromJson method.
* <li> Now copy the generated json including default options into createParseableQueryJson
* <li> By now the roundtrip check for the json should be happy.
* </ul>
**/