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

Geo Clean Up #4580

Merged
merged 1 commit into from Jan 11, 2014
Merged

Geo Clean Up #4580

merged 1 commit into from Jan 11, 2014

Conversation

chilling
Copy link
Contributor

@chilling chilling commented Jan 2, 2014

The default unit for measuring distances is MILES in most cases. This commit moves ES
over to the International System of Units and make it work on a default which relates
to METERS . Also the current structures of the GeoBoundingBox Filter changed in
order to define the Bounding by setting abitrary corners.

Distances

Since the default unit for measuring distances has changed to a default unit
DistanceUnit.DEFAULT relating to meters, the REST API has changed at the
following places:

  • ScriptDocValues.factorDistance() returns meters instead of miles
  • ScriptDocValues.factorDistanceWithDefault() returns meters instead of miles
  • ScriptDocValues.arcDistance() returns meters instead of miles
    one might use ScriptDocValues.arcDistanceInMiles()
  • ScriptDocValues.arcDistanceWithDefault() returns meters instead of miles
  • ScriptDocValues.distance() returns meters instead of miles
    one might use ScriptDocValues.distanceInMiles()
  • ScriptDocValues.distanceWithDefault() returns meters instead of miles
    one might use ScriptDocValues.distanceInMilesWithDefault()
  • GeoDistanceFilter default unit changes from kilometers to meters
  • GeoDistanceRangeFilter default unit changes from miles to meters
  • GeoDistanceFacet default unit changes from miles to meters

Geo Bounding Box Filter

The naming of the GeoBoundingBoxFilter properties allows to set arbitrary corners
(see #4084) namely top_right, top_left, bottom_right and bottom_left. This
change also includes the fields topRight and bottomLeft Also it is be possible to
set the single values by using just top, bottom, left and right parameters.

Closes #4515, #4084

@@ -293,10 +291,9 @@ public static GeoPoint decode(String geohash) {
* @param geohash Geohash to deocde
* @return Array with the latitude at index 0, and longitude at index 1
Copy link
Contributor

Choose a reason for hiding this comment

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

that @return statement seems wrong now

@chilling
Copy link
Contributor Author

chilling commented Jan 8, 2014

@s1monw I worked in your suggestions. Maybe you can have another look.

@chilling
Copy link
Contributor Author

chilling commented Jan 9, 2014

@s1monw I setup the changes we discussed. So please have another quick look.

@s1monw
Copy link
Contributor

s1monw commented Jan 9, 2014

@chilling I think it looks good though the only thing that I think we should change to use constants to parse the top_left etc fields instead of building all these dynamically? can you fix that?

@@ -149,4 +182,25 @@ public Filter parse(QueryParseContext parseContext) throws IOException, QueryPar
}
return filter;
}

private static final boolean matches(String fieldname, String prefix, String suffix) {
Copy link
Contributor

Choose a reason for hiding this comment

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

can we have constants for these instead of building them dynamically. I am not sure if it makes a difference perf wise but I think it's more consistent with all the other parsers.

@s1monw
Copy link
Contributor

s1monw commented Jan 10, 2014

I left more comments there are still inconsistencies with north vs. top

@chilling
Copy link
Contributor Author

@s1monw I hope I caught them now

@s1monw
Copy link
Contributor

s1monw commented Jan 10, 2014

LGTM +1 to push to master! Make sure you squash them before you push :)

============
The default unit for measuring distances is *MILES* in most cases. This commit moves ES
over to the *International System of Units* and make it work on a default which relates
to *METERS* . Also the current structures of the `GeoBoundingBox Filter` changed in
order to define the *Bounding* by setting abitrary corners.

Distances
---------
Since the default unit for measuring distances has changed to a default unit
`DistanceUnit.DEFAULT` relating to *meters*, the **REST API** has changed at the
following places:

  * `ScriptDocValues.factorDistance()` returns *meters* instead of *miles*
  * `ScriptDocValues.factorDistanceWithDefault()` returns *meters* instead of *miles*
  * `ScriptDocValues.arcDistance()` returns *meters* instead of *miles*
        one might use `ScriptDocValues.arcDistanceInMiles()`
  * `ScriptDocValues.arcDistanceWithDefault()` returns *meters* instead of *miles*
  * `ScriptDocValues.distance()` returns *meters* instead of *miles*
        one might use `ScriptDocValues.distanceInMiles()`
  * `ScriptDocValues.distanceWithDefault()` returns *meters* instead of *miles*
        one might use `ScriptDocValues.distanceInMilesWithDefault()`
  * `GeoDistanceFilter` default unit changes from *kilometers* to *meters*
  * `GeoDistanceRangeFilter` default unit changes from *miles* to *meters*
  * `GeoDistanceFacet` default unit changes from *miles* to *meters*

Geo Bounding Box Filter
-----------------------
The naming of the GeoBoundingBoxFilter properties allows to set arbitrary corners
(see elastic#4084) namely `top_right`, `top_left`, `bottom_right` and `bottom_left`. This
change also includes the fields `topRight` and `bottomLeft` Also it is be possible to
set the single values by using just `top`, `bottom`, `left` and `right` parameters.

Closes elastic#4515, elastic#4084
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Geo clean up
2 participants