Skip to content

Commit b2cd9fc

Browse files
committed
Fix GEORADIUS edge case with huge radius.
This commit closes issue redis#3698, at least for now, since the root cause was not fixed: the bounding box function, for huge radiuses, does not return a correct bounding box, there are points still within the radius that are left outside. So when using GEORADIUS queries with radiuses in the order of 5000 km or more, it was possible to see, at the edge of the area, certain points not correctly reported. Because the bounding box for now was used just as an optimization, and such huge radiuses are not common, for now the optimization is just switched off when the radius is near such magnitude. Three test cases found by the Continuous Integration test were added, so that we can easily trigger the bug again, both for regression testing and in order to properly fix it as some point in the future.
1 parent 26e638a commit b2cd9fc

File tree

2 files changed

+39
-20
lines changed

2 files changed

+39
-20
lines changed

src/geohash_helper.c

Lines changed: 36 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,21 @@ uint8_t geohashEstimateStepsByRadius(double range_meters, double lat) {
8585
/* Return the bounding box of the search area centered at latitude,longitude
8686
* having a radius of radius_meter. bounds[0] - bounds[2] is the minimum
8787
* and maxium longitude, while bounds[1] - bounds[3] is the minimum and
88-
* maximum latitude. */
88+
* maximum latitude.
89+
*
90+
* This function does not behave correctly with very large radius values, for
91+
* instance for the coordinates 81.634948934258375 30.561509253718668 and a
92+
* radius of 7083 kilometers, it reports as bounding boxes:
93+
*
94+
* min_lon 7.680495, min_lat -33.119473, max_lon 155.589402, max_lat 94.242491
95+
*
96+
* However, for instance, a min_lon of 7.680495 is not correct, because the
97+
* point -1.27579540014266968 61.33421815228281559 is at less than 7000
98+
* kilometers away.
99+
*
100+
* Since this function is currently only used as an optimization, the
101+
* optimization is not used for very big radiuses, however the function
102+
* should be fixed. */
89103
int geohashBoundingBox(double longitude, double latitude, double radius_meters,
90104
double *bounds) {
91105
if (!bounds) return 0;
@@ -154,25 +168,27 @@ GeoHashRadius geohashGetAreasByRadius(double longitude, double latitude, double
154168
}
155169

156170
/* Exclude the search areas that are useless. */
157-
if (area.latitude.min < min_lat) {
158-
GZERO(neighbors.south);
159-
GZERO(neighbors.south_west);
160-
GZERO(neighbors.south_east);
161-
}
162-
if (area.latitude.max > max_lat) {
163-
GZERO(neighbors.north);
164-
GZERO(neighbors.north_east);
165-
GZERO(neighbors.north_west);
166-
}
167-
if (area.longitude.min < min_lon) {
168-
GZERO(neighbors.west);
169-
GZERO(neighbors.south_west);
170-
GZERO(neighbors.north_west);
171-
}
172-
if (area.longitude.max > max_lon) {
173-
GZERO(neighbors.east);
174-
GZERO(neighbors.south_east);
175-
GZERO(neighbors.north_east);
171+
if (steps >= 2) {
172+
if (area.latitude.min < min_lat) {
173+
GZERO(neighbors.south);
174+
GZERO(neighbors.south_west);
175+
GZERO(neighbors.south_east);
176+
}
177+
if (area.latitude.max > max_lat) {
178+
GZERO(neighbors.north);
179+
GZERO(neighbors.north_east);
180+
GZERO(neighbors.north_west);
181+
}
182+
if (area.longitude.min < min_lon) {
183+
GZERO(neighbors.west);
184+
GZERO(neighbors.south_west);
185+
GZERO(neighbors.north_west);
186+
}
187+
if (area.longitude.max > max_lon) {
188+
GZERO(neighbors.east);
189+
GZERO(neighbors.south_east);
190+
GZERO(neighbors.north_east);
191+
}
176192
}
177193
radius.hash = hash;
178194
radius.neighbors = neighbors;

tests/unit/geo.tcl

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,9 @@ proc compare_lists {List1 List2} {
4949
#
5050
# The format is: seed km lon lat
5151
set regression_vectors {
52+
{1482225976969 7083 81.634948934258375 30.561509253718668}
53+
{1482340074151 5416 -70.863281847379767 -46.347003465679947}
54+
{1499014685896 6064 -89.818768962202014 -40.463868561416803}
5255
{1412 156 149.29737817929004 15.95807862745508}
5356
{441574 143 59.235461856813856 66.269555127373678}
5457
{160645 187 -101.88575239939883 49.061997951502917}

0 commit comments

Comments
 (0)