Skip to content

GEOSEARCH bybox bug fixes and new fuzzy tester#8445

Merged
oranagra merged 1 commit into
redis:unstablefrom
yangbodong22011:feature-geosearch-new-algorithm
Feb 4, 2021
Merged

GEOSEARCH bybox bug fixes and new fuzzy tester#8445
oranagra merged 1 commit into
redis:unstablefrom
yangbodong22011:feature-geosearch-new-algorithm

Conversation

@yangbodong22011

@yangbodong22011 yangbodong22011 commented Feb 4, 2021

Copy link
Copy Markdown
Contributor

Fix errors of GEOSEARCH bybox search due to:

  1. projection of the box to a trapezoid (when the meter box is converted to long / lat it's no longer a box).
  2. width and height mismatch

Changes:

  • New GEOSEARCH point in rectangle algorithm
  • Fix GEOSEARCH bybox width and height mismatch bug
  • Add GEOSEARCH bybox testing to the existing "GEOADD + GEORANGE randomized test"
  • Add new fuzzy test to stress test the bybox corners and edges
  • Add some tests for edge cases of the bybox algorithm

@yangbodong22011

Copy link
Copy Markdown
Contributor Author

@oranagra Thank you for the improvement of the original PR #8375, especially the algorithm that has a flash of light, I have actually put it in this PR.

Note:

  • I deleted the bybox test in "GEOADD + GEORANGE randomized test", because trapezoids or polygons always have errors in theory, in order to prevent one day from failing to confuse others.
  • In the latest fuzzy test, it is only judged whether all points are included or not, because the boundary points obtained may not be judged due to the error itself.

Comment thread src/geohash_helper.c Outdated
Comment thread tests/unit/geo.tcl Outdated
Comment thread src/geohash_helper.c Outdated
Comment thread src/geohash_helper.c Outdated
Comment thread tests/unit/geo.tcl Outdated
@oranagra oranagra added the release-notes indication that this issue needs to be mentioned in the release notes label Feb 4, 2021
- New GEOSEARCH point in rectangle algorithm
- Fix GEOSEARCH bybox width and height mismatch
- Add GEOSEARCH bybox testing to the existing "GEOADD + GEORANGE randomized test"
- Fix errors of GEOSEARCH bybox search due to projection of the box to a
trapezoid (when the meter box is converted to long / lat it's no longer a box).
- Add new fuzzy test to stress test the bybox corners and edges
- Add some tests for edge cases of the bybox algorithm
@yangbodong22011

Copy link
Copy Markdown
Contributor Author

updated, please review again, thanks @oranagra

@oranagra oranagra merged commit ded1655 into redis:unstable Feb 4, 2021
@oranagra oranagra mentioned this pull request Feb 22, 2021
JackieXie168 pushed a commit to JackieXie168/redis that referenced this pull request Mar 2, 2021
Fix errors of GEOSEARCH bybox search due to:
1. projection of the box to a trapezoid (when the meter box is converted to long / lat it's no longer a box).
2. width and height mismatch

Changes:
- New GEOSEARCH point in rectangle algorithm
- Fix GEOSEARCH bybox width and height mismatch bug
- Add GEOSEARCH bybox testing to the existing "GEOADD + GEORANGE randomized test"
- Add new fuzzy test to stress test the bybox corners and edges
- Add some tests for edge cases of the bybox algorithm

Co-authored-by: Oran Agra <oran@redislabs.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release-notes indication that this issue needs to be mentioned in the release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants