Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deprecate optimize_bbox on geodistance queries #20032

Closed
wants to merge 1 commit into from

Conversation

nknize
Copy link
Contributor

@nknize nknize commented Aug 17, 2016

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.

closes #20014

@nknize nknize added review :Analytics/Geo Indexing, search aggregations of geo points and shapes v2.4.0 labels Aug 17, 2016
@nknize
Copy link
Contributor Author

nknize commented Aug 17, 2016

@clintongormley I updated the docs with deprecation in 2.2 since the optimize_bbox parameter has been ignored since then.

@@ -48,6 +52,9 @@

public static final String NAME = "geo_distance";

public static final ParseField OPTIMIZE_BBOX_FIELD = new ParseField("optimize_bbox", "optimizeBbox");
protected static final DeprecationLogger deprecationLogger = new DeprecationLogger(Loggers.getLogger(BaseGeoPointFieldMapper.class));
Copy link
Contributor

@jpountz jpountz Aug 18, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would just do

public static final ParseField OPTIMIZE_BBOX_FIELD = new ParseField("optimize_bbox", "optimizeBbox").withAllDeprecated("no replacement: `optimize_bbox` became useless thanks to recent improvements")

and let ParseField emit deprecation warnings by itself. I don't think we need to only emit the deprecation warning for 2.2+ indices.

Copy link
Contributor Author

@nknize nknize Aug 18, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

++ I had this before but was unsure if this approach would issue only a warning or fail the query (I see it uses deprecation logger). I'll revert to this and commit. Thx Adrien!

@jpountz
Copy link
Contributor

jpountz commented Aug 18, 2016

I left a minor comment about a potential simplification, but feel free to push as-is if you disagree.

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.
@nknize
Copy link
Contributor Author

nknize commented Aug 19, 2016

closing as merged in: afb16b8

@jpountz
Copy link
Contributor

jpountz commented Aug 23, 2016

@nknize I think you should push this to master as well, otherwise the optimize_bbox will still exist in 5.0 and will not be deprecated, which would be weird since it was deprecated in 2.4.

@nknize
Copy link
Contributor Author

nknize commented Aug 23, 2016

I'm wrapping it up now.

@clintongormley clintongormley added :Search/Search Search-related issues that do not fall into other categories and removed :Query DSL labels Feb 14, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Analytics/Geo Indexing, search aggregations of geo points and shapes >deprecation :Search/Search Search-related issues that do not fall into other categories v2.4.0 v5.0.0-beta1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants