-
-
Notifications
You must be signed in to change notification settings - Fork 21
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
Add GeoBoundingBoxCondition #420
Add GeoBoundingBoxCondition #420
Conversation
9bcdfad
to
c149865
Compare
I rebased the branch as I merged the Algolia v4 upgrade: #414 |
$this->escapeFilterValue($filter->minLatitude), | ||
$this->escapeFilterValue($filter->minLongitude), | ||
$this->escapeFilterValue($filter->maxLatitude), | ||
$this->escapeFilterValue($filter->maxLongitude), |
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.
Meilisearch seems to be Top Right to Bottom Left while others are the other way around. I even think about make that default because JS libraries like Google Maps and Mapbox return NorthEast (TopRight) -> SouthWest (BottomLeft) as the bounded context for a map.
Loupe supports in the 0.7 version now also GeoBoundingBox: https://github.com/loupe-php/loupe/releases/tag/0.7.0 |
…Top Right -> Bottom Left
e206d9e
to
59e3088
Compare
$filter instanceof Condition\GeoBoundingBoxCondition => ($filters[] = \sprintf( | ||
'@%s:[WITHIN $filter_%s]', | ||
$this->getFilterField($search->indexes, $filter->field), | ||
$key, | ||
)) && ($parameters['filter_' . $key] = \sprintf( | ||
'POLYGON((%s %s, %s %s, %s %s, %s %s, %s %s))', | ||
$filter->westLongitude, | ||
$filter->northLatitude, | ||
$filter->westLongitude, | ||
$filter->southLatitude, | ||
$filter->eastLongitude, | ||
$filter->southLatitude, | ||
$filter->eastLongitude, | ||
$filter->northLatitude, | ||
$filter->westLongitude, | ||
$filter->northLatitude, | ||
)), |
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.
GEO type does not support yet filtering by Polygon. Open issue is here: RediSearch/RediSearch#680. Maybe it for them easier to support the bounding box. Lets wait for feedback from them: RediSearch/RediSearch#5032
Loupe is implemented now. So only thing we need is to wait feedback from the @RediSearch team about there support for GeoBoundingBox or even Polygon based queries. |
I will go with merging the current state maybe we get a response from @RediSearch team soon how the state of the support for bounding box and polygon based queries is on there side. |
This PR introduce
GeoBoundingBoxCondition
:ToDos
fix #365