-
Notifications
You must be signed in to change notification settings - Fork 24.5k
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
GeoPolygonFilter not properly handling dateline and pole crossing #9171
Conversation
By default GeoPolygonFilter normalizes GeoPoints to a [-180:180] lon, [-90:90] lat coordinate boundary. This requires the GeoPolygonFilter be split into east/west, north/south regions to properly return points collection that wrap the dateline and poles. To keep queries fast, this simple fix converts the GeoPoints for both the target polygon and candidate points to a [0:360] lon, [0:180] lat coordinate range. This way the user (or the filter logic) doesn't have to split the filter in two parts. closes elastic#5968
lon += 360; | ||
} | ||
|
||
if (lat <0) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: space between <
and 0
LGTM. I left a couple minor thoughts. |
Can we remove the |
Sounds good @clintongormley. Not only do we not need the normalize filter option, this filter also suffers from the ambiguous poly problem. There's a bit that needs to be cleaned up here, including reusing logic written in #8762. Do you want me to just go ahead and close this PR and submit a new one when its ready? Or just keep this open? |
Closing in favor of #9339 |
By default GeoPolygonFilter normalizes GeoPoints to a [-180:180] lon, [-90:90] lat coordinate boundary. This requires the GeoPolygonFilter be split into east/west, north/south regions to properly return points collection that wrap the dateline and poles. To keep queries fast, this simple fix converts the GeoPoints for both the target polygon and candidate points to a [0:360] lon, [0:180] lat coordinate range. This way the user (or the filter logic) doesn't have to split the filter in two parts.
closes #5968