Deprecate optimize_bbox on geodistance queries

Add deprecation to the optimize_bbox parameter on geodistance queries. This is no longer needed since lucene geo distance queries already optimize by bounding box.
elastic · Aug 18, 2016 · fead1ee · fead1ee
1 parent d9b807f
commit fead1ee
Show file tree

Hide file tree

Showing 5 changed files with 30 additions and 7 deletions.
diff --git a/core/src/main/java/org/elasticsearch/index/query/GeoDistanceQueryParser.java b/core/src/main/java/org/elasticsearch/index/query/GeoDistanceQueryParser.java
@@ -23,14 +23,18 @@
 import org.apache.lucene.spatial.geopoint.document.GeoPointField;
 import org.apache.lucene.spatial.geopoint.search.GeoPointDistanceQuery;
 import org.elasticsearch.Version;
+import org.elasticsearch.common.ParseField;
 import org.elasticsearch.common.geo.GeoDistance;
 import org.elasticsearch.common.geo.GeoPoint;
 import org.elasticsearch.common.geo.GeoUtils;
 import org.elasticsearch.common.inject.Inject;
+import org.elasticsearch.common.logging.DeprecationLogger;
+import org.elasticsearch.common.logging.Loggers;
 import org.elasticsearch.common.unit.DistanceUnit;
 import org.elasticsearch.common.xcontent.XContentParser;
 import org.elasticsearch.index.fielddata.IndexGeoPointFieldData;
 import org.elasticsearch.index.mapper.MappedFieldType;
+import org.elasticsearch.index.mapper.geo.BaseGeoPointFieldMapper;
 import org.elasticsearch.index.mapper.geo.GeoPointFieldMapper;
 import org.elasticsearch.index.search.geo.GeoDistanceRangeQuery;
 
@@ -48,6 +52,9 @@ public class GeoDistanceQueryParser implements QueryParser {
 
     public static final String NAME = "geo_distance";
 
+    public static final ParseField OPTIMIZE_BBOX_FIELD = new ParseField("optimize_bbox", "optimizeBbox")
+        .withAllDeprecated("no replacement: `optimize_bbox` is no longer supported due to recent improvements");
+
     @Inject
     public GeoDistanceQueryParser() {
     }
@@ -73,6 +80,7 @@ public Query parse(QueryParseContext parseContext) throws IOException, QueryPars
         GeoDistance geoDistance = GeoDistance.DEFAULT;
         String optimizeBbox = "memory";
         final boolean indexCreatedBeforeV2_0 = parseContext.indexVersionCreated().before(Version.V_2_0_0);
+        final boolean indexCreatedBeforeV2_2 = parseContext.indexVersionCreated().before(Version.V_2_2_0);
         boolean coerce = false;
         boolean ignoreMalformed = false;
         while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) {
@@ -121,8 +129,10 @@ public Query parse(QueryParseContext parseContext) throws IOException, QueryPars
                     fieldName = currentFieldName.substring(0, currentFieldName.length() - GeoPointFieldMapper.Names.LON_SUFFIX.length());
                 } else if ("_name".equals(currentFieldName)) {
                     queryName = parser.text();
-                } else if ("optimize_bbox".equals(currentFieldName) || "optimizeBbox".equals(currentFieldName)) {
-                    optimizeBbox = parser.textOrNull();
+                } else if (parseContext.parseFieldMatcher().match(currentFieldName, OPTIMIZE_BBOX_FIELD)) {
+                    if (indexCreatedBeforeV2_2) {
+                        optimizeBbox = parser.textOrNull();
+                    }
                 } else if ("coerce".equals(currentFieldName) || (indexCreatedBeforeV2_0 && "normalize".equals(currentFieldName))) {
                     coerce = parser.booleanValue();
                     if (coerce == true) {

diff --git a/core/src/main/java/org/elasticsearch/index/query/GeoDistanceRangeQueryParser.java b/core/src/main/java/org/elasticsearch/index/query/GeoDistanceRangeQueryParser.java
@@ -29,10 +29,13 @@
 import org.elasticsearch.common.geo.GeoPoint;
 import org.elasticsearch.common.geo.GeoUtils;
 import org.elasticsearch.common.inject.Inject;
+import org.elasticsearch.common.logging.DeprecationLogger;
+import org.elasticsearch.common.logging.Loggers;
 import org.elasticsearch.common.unit.DistanceUnit;
 import org.elasticsearch.common.xcontent.XContentParser;
 import org.elasticsearch.index.fielddata.IndexGeoPointFieldData;
 import org.elasticsearch.index.mapper.MappedFieldType;
+import org.elasticsearch.index.mapper.geo.BaseGeoPointFieldMapper;
 import org.elasticsearch.index.mapper.geo.GeoPointFieldMapper;
 import org.elasticsearch.index.search.geo.GeoDistanceRangeQuery;
 
@@ -51,6 +54,7 @@
 public class GeoDistanceRangeQueryParser implements QueryParser {
 
     public static final String NAME = "geo_distance_range";
+    protected static final DeprecationLogger deprecationLogger = new DeprecationLogger(Loggers.getLogger(BaseGeoPointFieldMapper.class));
 
     @Inject
     public GeoDistanceRangeQueryParser() {
@@ -156,8 +160,12 @@ public Query parse(QueryParseContext parseContext) throws IOException, QueryPars
                     point.resetLon(parser.doubleValue());
                 } else if ("_name".equals(currentFieldName)) {
                     queryName = parser.text();
-                } else if ("optimize_bbox".equals(currentFieldName) || "optimizeBbox".equals(currentFieldName)) {
-                    optimizeBbox = parser.textOrNull();
+                } else if (parseContext.parseFieldMatcher().match(currentFieldName, GeoDistanceQueryParser.OPTIMIZE_BBOX_FIELD)) {
+                    if (indexCreatedBeforeV2_2) {
+                        optimizeBbox = parser.textOrNull();
+                    } else {
+                        deprecationLogger.deprecated(NAME + " parameter [{}] is deprecated", GeoDistanceQueryParser.OPTIMIZE_BBOX_FIELD);
+                    }
                 } else if ("coerce".equals(currentFieldName) || (indexCreatedBeforeV2_0 && "normalize".equals(currentFieldName))) {
                     coerce = parser.booleanValue();
                     if (coerce == true) {

diff --git a/core/src/test/java/org/elasticsearch/ExceptionSerializationTests.java b/core/src/test/java/org/elasticsearch/ExceptionSerializationTests.java
@@ -787,7 +787,7 @@ public void testIds() {
                 reverse.put(entry.getValue(), entry.getKey());
             }
         }
-        
+
         for (Map.Entry<Integer, Constructor<? extends ElasticsearchException>> entry : ElasticsearchException.ID_TO_SUPPLIER.entrySet()) {
             assertNotNull(Integer.toString(entry.getKey()), reverse.get(entry.getValue().getDeclaringClass()));
             assertEquals(reverse.get(entry.getValue().getDeclaringClass()), entry.getKey());

diff --git a/docs/reference/migration/migrate_2_4.asciidoc b/docs/reference/migration/migrate_2_4.asciidoc
@@ -8,4 +8,9 @@
 The setting `bootstrap.mlockall` has been renamed to
 <<bootstrap.memory_lock,`bootstrap.memory_lock`>>. Future versions of
 Elasticsearch will refuse to start if `bootstrap.mlockall` is set. You
-can switch to the new setting now.
+can switch to the new setting now.
+
+=== Query DSL
+
+The `optimize_bbox` parameter for `geo_distance` and `geo_distance_range` queries
+has been deprecated when querying indexes created after 2.1.
diff --git a/docs/reference/query-dsl/geo-distance-query.asciidoc b/docs/reference/query-dsl/geo-distance-query.asciidoc
@@ -156,7 +156,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`::