Browse Source

Remove bounding box query type parameter (#74536)

The parameter has been deprecates in 7.14 as it is a no-op.
Ignacio Vera 4 years ago
parent
commit
d7ef5b6d21

+ 11 - 0
docs/reference/migration/migrate_8_0/search.asciidoc

@@ -173,5 +173,16 @@ If you query date fields without a specified `format`, check if the values in yo
 actually meant to be milliseconds-since-epoch and use a numeric value in this case. If not, use
 a string value which gets parsed by either the date format set on the field in the mappings or
 by `strict_date_optional_time` by default.
+====
+
+.The `geo_bounding_box` query's `type` parameter has been removed.
+[%collapsible]
+====
+*Details* +
+The `geo_bounding_box` query's `type` parameter was deprecated in 7.14.0 and has
+been removed in 8.0.0. This parameter is a no-op and has no effect on the query.
 
+*Impact* +
+Discontinue use of the `type` parameter. `geo_bounding_box` queries that include
+this parameter will return an error.
 ====

+ 0 - 44
docs/reference/query-dsl/geo-bounding-box-query.asciidoc

@@ -169,9 +169,6 @@ GET my_locations,my_geoshapes/_search
 accept geo points with invalid latitude or longitude, set to
 `COERCE` to also try to infer correct latitude or longitude. (default is `STRICT`).
 
-|`type` |Set to one of `indexed` or `memory` to defines whether this filter will
-be executed in memory or indexed. See <<geo-bbox-type,Type>> below for further details
-Default is `memory`.
 |=======================================================================
 
 [[query-dsl-geo-bounding-box-query-accepted-formats]]
@@ -388,47 +385,6 @@ The filter can work with multiple locations / points per document. Once
 a single location / point matches the filter, the document will be
 included in the filter
 
-[discrete]
-[[geo-bbox-type]]
-==== Type
-
-The type of the bounding box execution by default is set to `memory`,
-which means in memory checks if the doc falls within the bounding box
-range. In some cases, an `indexed` option will perform faster (but note
-that the `geo_point` type must have lat and lon indexed in this case).
-Note, when using the indexed option, multi locations per document field
-are not supported. Here is an example:
-
-[source,console]
---------------------------------------------------
-GET my_locations/_search
-{
-  "query": {
-    "bool": {
-      "must": {
-        "match_all": {}
-      },
-      "filter": {
-        "geo_bounding_box": {
-          "pin.location": {
-            "top_left": {
-              "lat": 40.73,
-              "lon": -74.1
-            },
-            "bottom_right": {
-              "lat": 40.10,
-              "lon": -71.12
-            }
-          },
-          "type": "indexed"
-        }
-      }
-    }
-  }
-}
---------------------------------------------------
-// TEST[warning:Deprecated field [type] used, this field is unused and will be removed entirely]
-
 [discrete]
 ==== Ignore Unmapped
 

+ 3 - 9
server/src/internalClusterTest/java/org/elasticsearch/search/geo/AbstractGeoBoundingBoxQueryIT.java

@@ -100,7 +100,7 @@ abstract class AbstractGeoBoundingBoxQueryIT extends ESIntegTestCase {
         }
 
         searchResponse = client().prepareSearch() // from NY
-                .setQuery(geoBoundingBoxQuery("location").setCorners(40.73, -74.1, 40.717, -73.99).type("indexed"))
+                .setQuery(geoBoundingBoxQuery("location").setCorners(40.73, -74.1, 40.717, -73.99))
                 .get();
         assertThat(searchResponse.getHits().getTotalHits().value, equalTo(2L));
         assertThat(searchResponse.getHits().getHits().length, equalTo(2));
@@ -150,8 +150,7 @@ abstract class AbstractGeoBoundingBoxQueryIT extends ESIntegTestCase {
         searchResponse = client().prepareSearch()
                 .setQuery(
                         boolQuery().must(termQuery("userid", 880)).filter(
-                                geoBoundingBoxQuery("location").setCorners(74.579421999999994, 143.5, -66.668903999999998, 113.96875)
-                                        .type("indexed"))
+                                geoBoundingBoxQuery("location").setCorners(74.579421999999994, 143.5, -66.668903999999998, 113.96875))
                 ).get();
         assertThat(searchResponse.getHits().getTotalHits().value, equalTo(1L));
 
@@ -164,8 +163,7 @@ abstract class AbstractGeoBoundingBoxQueryIT extends ESIntegTestCase {
         searchResponse = client().prepareSearch()
                 .setQuery(
                         boolQuery().must(termQuery("userid", 534)).filter(
-                                geoBoundingBoxQuery("location").setCorners(74.579421999999994, 143.5, -66.668903999999998, 113.96875)
-                                        .type("indexed"))
+                                geoBoundingBoxQuery("location").setCorners(74.579421999999994, 143.5, -66.668903999999998, 113.96875))
                 ).get();
         assertThat(searchResponse.getHits().getTotalHits().value, equalTo(1L));
 
@@ -216,7 +214,6 @@ abstract class AbstractGeoBoundingBoxQueryIT extends ESIntegTestCase {
         searchResponse = client().prepareSearch()
                 .setQuery(
                         geoBoundingBoxQuery("location").setValidationMethod(GeoValidationMethod.COERCE).setCorners(50, -180, -50, 180)
-                            .type("indexed")
                 ).get();
         assertThat(searchResponse.getHits().getTotalHits().value, equalTo(1L));
         searchResponse = client().prepareSearch()
@@ -227,7 +224,6 @@ abstract class AbstractGeoBoundingBoxQueryIT extends ESIntegTestCase {
         searchResponse = client().prepareSearch()
                 .setQuery(
                         geoBoundingBoxQuery("location").setValidationMethod(GeoValidationMethod.COERCE).setCorners(90, -180, -90, 180)
-                            .type("indexed")
                 ).get();
         assertThat(searchResponse.getHits().getTotalHits().value, equalTo(2L));
 
@@ -239,7 +235,6 @@ abstract class AbstractGeoBoundingBoxQueryIT extends ESIntegTestCase {
         searchResponse = client().prepareSearch()
                 .setQuery(
                         geoBoundingBoxQuery("location").setValidationMethod(GeoValidationMethod.COERCE).setCorners(50, 0, -50, 360)
-                                .type("indexed")
                 ).get();
         assertThat(searchResponse.getHits().getTotalHits().value, equalTo(1L));
         searchResponse = client().prepareSearch()
@@ -250,7 +245,6 @@ abstract class AbstractGeoBoundingBoxQueryIT extends ESIntegTestCase {
         searchResponse = client().prepareSearch()
                 .setQuery(
                         geoBoundingBoxQuery("location").setValidationMethod(GeoValidationMethod.COERCE).setCorners(90, 0, -90, 360)
-                                .type("indexed")
                 ).get();
         assertThat(searchResponse.getHits().getTotalHits().value, equalTo(2L));
 

+ 8 - 39
server/src/main/java/org/elasticsearch/index/query/GeoBoundingBoxQueryBuilder.java

@@ -11,6 +11,7 @@ package org.elasticsearch.index.query;
 import org.apache.lucene.search.MatchNoDocsQuery;
 import org.apache.lucene.search.Query;
 import org.elasticsearch.ElasticsearchParseException;
+import org.elasticsearch.Version;
 import org.elasticsearch.common.Numbers;
 import org.elasticsearch.common.xcontent.ParseField;
 import org.elasticsearch.common.ParsingException;
@@ -41,15 +42,11 @@ import java.util.Objects;
 public class GeoBoundingBoxQueryBuilder extends AbstractQueryBuilder<GeoBoundingBoxQueryBuilder> {
     public static final String NAME = "geo_bounding_box";
 
-    /** Default type for executing this query (memory as of this writing). */
-    public static final GeoExecType DEFAULT_TYPE = GeoExecType.MEMORY;
-
     /**
      * The default value for ignore_unmapped.
      */
     public static final boolean DEFAULT_IGNORE_UNMAPPED = false;
 
-    private static final ParseField TYPE_FIELD = new ParseField("type").withAllDeprecated();
     private static final ParseField VALIDATION_METHOD_FIELD = new ParseField("validation_method");
     private static final ParseField IGNORE_UNMAPPED_FIELD = new ParseField("ignore_unmapped");
 
@@ -59,8 +56,6 @@ public class GeoBoundingBoxQueryBuilder extends AbstractQueryBuilder<GeoBounding
     private GeoBoundingBox geoBoundingBox = new GeoBoundingBox(new GeoPoint(Double.NaN, Double.NaN), new GeoPoint(Double.NaN, Double.NaN));
     /** How to deal with incorrect coordinates.*/
     private GeoValidationMethod validationMethod = GeoValidationMethod.DEFAULT;
-    /** How the query should be run. */
-    private GeoExecType type = DEFAULT_TYPE;
 
     private boolean ignoreUnmapped = DEFAULT_IGNORE_UNMAPPED;
 
@@ -82,7 +77,9 @@ public class GeoBoundingBoxQueryBuilder extends AbstractQueryBuilder<GeoBounding
         super(in);
         fieldName = in.readString();
         geoBoundingBox = new GeoBoundingBox(in);
-        type = GeoExecType.readFromStream(in);
+        if (in.getVersion().before(Version.V_8_0_0)) {
+            in.readVInt(); // ignore value
+        }
         validationMethod = GeoValidationMethod.readFromStream(in);
         ignoreUnmapped = in.readBoolean();
     }
@@ -91,7 +88,9 @@ public class GeoBoundingBoxQueryBuilder extends AbstractQueryBuilder<GeoBounding
     protected void doWriteTo(StreamOutput out) throws IOException {
         out.writeString(fieldName);
         geoBoundingBox.writeTo(out);
-        type.writeTo(out);
+        if (out.getVersion().before(Version.V_8_0_0)) {
+            out.writeVInt(0);
+        }
         validationMethod.writeTo(out);
         out.writeBoolean(ignoreUnmapped);
     }
@@ -213,30 +212,6 @@ public class GeoBoundingBoxQueryBuilder extends AbstractQueryBuilder<GeoBounding
         return this.validationMethod;
     }
 
-    /**
-     * Sets the type of executing of the geo bounding box. Can be either `memory` or `indexed`. Defaults
-     * to `memory`.
-     */
-    public GeoBoundingBoxQueryBuilder type(GeoExecType type) {
-        if (type == null) {
-            throw new IllegalArgumentException("Type is not allowed to be null.");
-        }
-        this.type = type;
-        return this;
-    }
-
-    /**
-     * For BWC: Parse type from type name.
-     * */
-    public GeoBoundingBoxQueryBuilder type(String type) {
-        this.type = GeoExecType.fromString(type);
-        return this;
-    }
-    /** Returns the execution type of the geo bounding box.*/
-    public GeoExecType type() {
-        return type;
-    }
-
     /** Returns the name of the field to base the bounding box computation on. */
     public String fieldName() {
         return this.fieldName;
@@ -341,7 +316,6 @@ public class GeoBoundingBoxQueryBuilder extends AbstractQueryBuilder<GeoBounding
         geoBoundingBox.toXContentFragment(builder, false);
         builder.endObject();
         builder.field(VALIDATION_METHOD_FIELD.getPreferredName(), validationMethod);
-        builder.field(TYPE_FIELD.getPreferredName(), type);
         builder.field(IGNORE_UNMAPPED_FIELD.getPreferredName(), ignoreUnmapped);
 
         printBoostAndQueryName(builder);
@@ -361,7 +335,6 @@ public class GeoBoundingBoxQueryBuilder extends AbstractQueryBuilder<GeoBounding
 
         // bottom (minLat), top (maxLat), left (minLon), right (maxLon)
         GeoBoundingBox bbox = null;
-        String type = "memory";
 
         while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) {
             if (token == XContentParser.Token.FIELD_NAME) {
@@ -382,8 +355,6 @@ public class GeoBoundingBoxQueryBuilder extends AbstractQueryBuilder<GeoBounding
                     validationMethod = GeoValidationMethod.fromString(parser.text());
                 } else if (IGNORE_UNMAPPED_FIELD.match(currentFieldName, parser.getDeprecationHandler())) {
                     ignoreUnmapped = parser.booleanValue();
-                } else if (TYPE_FIELD.match(currentFieldName, parser.getDeprecationHandler())) {
-                    type = parser.text();
                 } else {
                     throw new ParsingException(parser.getTokenLocation(), "failed to parse [{}] query. unexpected field [{}]",
                             NAME, currentFieldName);
@@ -399,7 +370,6 @@ public class GeoBoundingBoxQueryBuilder extends AbstractQueryBuilder<GeoBounding
         builder.setCorners(bbox.topLeft(), bbox.bottomRight());
         builder.queryName(queryName);
         builder.boost(boost);
-        builder.type(GeoExecType.fromString(type));
         builder.ignoreUnmapped(ignoreUnmapped);
         if (validationMethod != null) {
             // ignore deprecated coerce/ignoreMalformed settings if validationMethod is set
@@ -411,7 +381,6 @@ public class GeoBoundingBoxQueryBuilder extends AbstractQueryBuilder<GeoBounding
     @Override
     protected boolean doEquals(GeoBoundingBoxQueryBuilder other) {
         return Objects.equals(geoBoundingBox, other.geoBoundingBox) &&
-                Objects.equals(type, other.type) &&
                 Objects.equals(validationMethod, other.validationMethod) &&
                 Objects.equals(fieldName, other.fieldName) &&
                 Objects.equals(ignoreUnmapped, other.ignoreUnmapped);
@@ -419,7 +388,7 @@ public class GeoBoundingBoxQueryBuilder extends AbstractQueryBuilder<GeoBounding
 
     @Override
     protected int doHashCode() {
-        return Objects.hash(geoBoundingBox, type, validationMethod, fieldName, ignoreUnmapped);
+        return Objects.hash(geoBoundingBox, validationMethod, fieldName, ignoreUnmapped);
     }
 
     @Override

+ 0 - 55
server/src/main/java/org/elasticsearch/index/query/GeoExecType.java

@@ -1,55 +0,0 @@
-/*
- * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
- * or more contributor license agreements. Licensed under the Elastic License
- * 2.0 and the Server Side Public License, v 1; you may not use this file except
- * in compliance with, at your election, the Elastic License 2.0 or the Server
- * Side Public License, v 1.
- */
-
-package org.elasticsearch.index.query;
-
-import org.elasticsearch.ElasticsearchException;
-import org.elasticsearch.common.io.stream.StreamInput;
-import org.elasticsearch.common.io.stream.StreamOutput;
-import org.elasticsearch.common.io.stream.Writeable;
-
-import java.io.IOException;
-
-/** Specifies how a geo query should be run. */
-public enum GeoExecType implements Writeable {
-
-    MEMORY(0), INDEXED(1);
-
-    private final int ordinal;
-
-    GeoExecType(int ordinal) {
-        this.ordinal = ordinal;
-    }
-
-    public static GeoExecType readFromStream(StreamInput in) throws IOException {
-        int ord = in.readVInt();
-        switch(ord) {
-            case(0): return MEMORY;
-            case(1): return INDEXED;
-        }
-        throw new ElasticsearchException("unknown serialized type [" + ord + "]");
-    }
-
-    @Override
-    public void writeTo(StreamOutput out) throws IOException {
-        out.writeVInt(this.ordinal);
-    }
-
-    public static GeoExecType fromString(String typeName) {
-        if (typeName == null) {
-            throw new IllegalArgumentException("cannot parse type from null string");
-        }
-
-        for (GeoExecType type : GeoExecType.values()) {
-            if (type.name().equalsIgnoreCase(typeName)) {
-                return type;
-            }
-        }
-        throw new IllegalArgumentException("no type can be parsed from ordinal " + typeName);
-    }
-}

+ 1 - 48
server/src/test/java/org/elasticsearch/index/query/GeoBoundingBoxQueryBuilderTests.java

@@ -78,7 +78,6 @@ public class GeoBoundingBoxQueryBuilderTests extends AbstractQueryTestCase<GeoBo
             builder.ignoreUnmapped(randomBoolean());
         }
 
-        builder.type(randomFrom(GeoExecType.values()));
         return builder;
     }
 
@@ -87,18 +86,6 @@ public class GeoBoundingBoxQueryBuilderTests extends AbstractQueryTestCase<GeoBo
         assertEquals("Field name must not be empty.", e.getMessage());
     }
 
-    public void testValidationNullType() {
-        GeoBoundingBoxQueryBuilder qb = new GeoBoundingBoxQueryBuilder("teststring");
-        IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> qb.type((GeoExecType) null));
-        assertEquals("Type is not allowed to be null.", e.getMessage());
-    }
-
-    public void testValidationNullTypeString() {
-        GeoBoundingBoxQueryBuilder qb = new GeoBoundingBoxQueryBuilder("teststring");
-        IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> qb.type((String) null));
-        assertEquals("cannot parse type from null string", e.getMessage());
-    }
-
     public void testExceptionOnMissingTypes() {
         SearchExecutionContext context = createShardContextWithNoType();
         GeoBoundingBoxQueryBuilder qb = createTestQueryBuilder();
@@ -376,8 +363,7 @@ public class GeoBoundingBoxQueryBuilderTests extends AbstractQueryTestCase<GeoBo
                 "      \"bottom_right\" : [ -71.12, 40.01 ]\n" +
                 "    },\n" +
                 "    \"validation_method\" : \"STRICT\",\n" +
-                "    \"type\" : \"MEMORY\",\n" +
-                        "    \"ignore_unmapped\" : false,\n" +
+                 "    \"ignore_unmapped\" : false,\n" +
                 "    \"boost\" : 1.0\n" +
                 "  }\n" +
                 "}";
@@ -389,8 +375,6 @@ public class GeoBoundingBoxQueryBuilderTests extends AbstractQueryTestCase<GeoBo
         assertEquals(json, -71.12, parsed.bottomRight().getLon(), 0.0001);
         assertEquals(json, 40.01, parsed.bottomRight().getLat(), 0.0001);
         assertEquals(json, 1.0, parsed.boost(), 0.0001);
-        assertEquals(json, GeoExecType.MEMORY, parsed.type());
-        assertDeprecationWarning();
     }
 
     public void testFromWKT() throws IOException {
@@ -401,7 +385,6 @@ public class GeoBoundingBoxQueryBuilderTests extends AbstractQueryTestCase<GeoBo
                 "      \"wkt\" : \"BBOX (-74.1, -71.12, 40.73, 40.01)\"\n" +
                 "    },\n" +
                 "    \"validation_method\" : \"STRICT\",\n" +
-                "    \"type\" : \"MEMORY\",\n" +
                 "    \"ignore_unmapped\" : false,\n" +
                 "    \"boost\" : 1.0\n" +
                 "  }\n" +
@@ -417,7 +400,6 @@ public class GeoBoundingBoxQueryBuilderTests extends AbstractQueryTestCase<GeoBo
                 "      \"bottom_right\" : [ -71.12, 40.01 ]\n" +
                 "    },\n" +
                 "    \"validation_method\" : \"STRICT\",\n" +
-                "    \"type\" : \"MEMORY\",\n" +
                 "    \"ignore_unmapped\" : false,\n" +
                 "    \"boost\" : 1.0\n" +
                 "  }\n" +
@@ -434,8 +416,6 @@ public class GeoBoundingBoxQueryBuilderTests extends AbstractQueryTestCase<GeoBo
         assertEquals(expectedJson, -71.12, parsed.bottomRight().getLon(), delta);
         assertEquals(expectedJson, 40.01, parsed.bottomRight().getLat(), delta);
         assertEquals(expectedJson, 1.0, parsed.boost(), delta);
-        assertEquals(expectedJson, GeoExecType.MEMORY, parsed.type());
-        assertDeprecationWarning();
     }
 
     public void testFromGeohash() throws IOException {
@@ -447,7 +427,6 @@ public class GeoBoundingBoxQueryBuilderTests extends AbstractQueryTestCase<GeoBo
                 "      \"bottom_right\" : \"dq\"\n" +
                 "    },\n" +
                 "    \"validation_method\" : \"STRICT\",\n" +
-                "    \"type\" : \"MEMORY\",\n" +
                 "    \"ignore_unmapped\" : false,\n" +
                 "    \"boost\" : 1.0\n" +
                 "  }\n" +
@@ -461,7 +440,6 @@ public class GeoBoundingBoxQueryBuilderTests extends AbstractQueryTestCase<GeoBo
                 "      \"bottom_right\" : [ -67.5, 33.75 ]\n" +
                 "    },\n" +
                 "    \"validation_method\" : \"STRICT\",\n" +
-                "    \"type\" : \"MEMORY\",\n" +
                 "    \"ignore_unmapped\" : false,\n" +
                 "    \"boost\" : 1.0\n" +
                 "  }\n" +
@@ -474,8 +452,6 @@ public class GeoBoundingBoxQueryBuilderTests extends AbstractQueryTestCase<GeoBo
         assertEquals(json, -67.5, parsed.bottomRight().getLon(), 0.0001);
         assertEquals(json, 33.75, parsed.bottomRight().getLat(), 0.0001);
         assertEquals(json, 1.0, parsed.boost(), 0.0001);
-        assertEquals(json, GeoExecType.MEMORY, parsed.type());
-        assertDeprecationWarning();
     }
 
     public void testMalformedGeohashes() {
@@ -487,7 +463,6 @@ public class GeoBoundingBoxQueryBuilderTests extends AbstractQueryTestCase<GeoBo
                 "      \"wkt\" : \"BBOX (-74.1, -71.12, 40.73, 40.01)\"\n" +
                 "    },\n" +
                 "    \"validation_method\" : \"STRICT\",\n" +
-                "    \"type\" : \"MEMORY\",\n" +
                 "    \"ignore_unmapped\" : false,\n" +
                 "    \"boost\" : 1.0\n" +
                 "  }\n" +
@@ -534,26 +509,4 @@ public class GeoBoundingBoxQueryBuilderTests extends AbstractQueryTestCase<GeoBo
         QueryShardException e = expectThrows(QueryShardException.class, () -> failingQueryBuilder.toQuery(searchExecutionContext));
         assertThat(e.getMessage(), containsString("failed to find geo field [unmapped]"));
     }
-
-    @Override
-    public void testValidOutput() throws IOException {
-        super.testValidOutput();
-        assertDeprecationWarning();
-    }
-
-    @Override
-    public void testUnknownField() throws IOException {
-        super.testUnknownField();
-        assertDeprecationWarning();
-    }
-
-    @Override
-    public void testFromXContent() throws IOException {
-        super.testFromXContent();
-        assertDeprecationWarning();
-    }
-
-    private void assertDeprecationWarning() {
-        assertWarnings("Deprecated field [type] used, this field is unused and will be removed entirely");
-    }
 }