-
Notifications
You must be signed in to change notification settings - Fork 167
Add support for geospatial queries #3300
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
Conversation
Pull Request Test Coverage Report for Build 5188514108
💛 - Coveralls |
{ | ||
foreach (var polygon in linearRings) | ||
{ | ||
Argument.Ensure(polygon.Count > 3, $"Each linear ring (both the outer one and any holes) must have at least 4 points, but {LinearRingToString(polygon)} only had {polygon.Count}.", nameof(linearRings)); |
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.
In the docs for the class it says that each ring needs to have at least 3 points, not 4
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.
The docs say 3 distinct points and the first and the last must match, implying at least 4 points.
foreach (var polygon in linearRings) | ||
{ | ||
Argument.Ensure(polygon.Count > 3, $"Each linear ring (both the outer one and any holes) must have at least 4 points, but {LinearRingToString(polygon)} only had {polygon.Count}.", nameof(linearRings)); | ||
Argument.Ensure(polygon[0] == polygon[polygon.Count - 1], $"The first and the last points of the polygon {LinearRingToString(polygon)} must be the same.", nameof(linearRings)); |
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.
Same here, I thought in this case the polygon would be considered close anyways
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.
That was the case at some point, but Core changed it, so we now need to explicitly close the polygon. Will update the docs.
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.
Actually, I can see that MongoDB is more relaxed than GeoJSON, so might be worth doing it for the SDKs as well.
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.
Apart from those few notes it looks fine
Co-authored-by: Ferdinando Papale <4850119+papafe@users.noreply.github.com>
* main: (29 commits) Add platform-specific helpers package (#3323) Allow geobox to be constructed from 4 coordinates (#3341) Add support for geospatial queries (#3300) Update UWP certificate, add instructions on how to do it (#3338) Don't throw an exception when setting an embedded property to the same value (#3337) Automatically handle object -> embedded object migrations (#3322) Add a check for the thread in Realm.IsInTransaction (#3336) Update to latest Core (#3334) Fix a few typos Move the SG and weaver binaries to the Realm package (#3319) Update ci-actions to latest commit (#3318) Fix a couple of warnings (#3317) Prepare for vNext (#3316) Prepare for 11.0.0 (#3315) Add support for arm on Linux (#3267) Update changelog Add tests for Set.PropertyChanged notifications (#3309) Change testing targets to net7.0 (#3305) Adjust codeql config (#3311) Allow publishing unity package on prerelease workflow (#3310) ... # Conflicts: # Realm/Realm.UnityWeaver/UnityWeaver.cs # wrappers/realm-core
* main: (169 commits) Prepare for vNext (#3361) Prepare for 11.1.2 (#3360) Fix maui namespaces (#3357) Add a task to verify namespaces (#3358) Prepare for vNext (#3356) Prepare for 11.1.1 (#3355) Fix namespaces to ensure we don't have Realm as a namespace (#3353) Better warning for skipped properties with attributes (#3354) Revert "Better warning for skipped properties with attributes" Better warning for skipped properties with attributes Add nuget readme (#3350) Prepare for vNext (#3349) Prepare for 11.1.0 (#3348) Remove SG package reference from XUnit tests Use a different MSVC version (#3347) Overhaul metrics [feature branch] (#3209) Bump NuGet.Protocol from 5.9.3 to 6.0.5 (#3346) Add platform-specific helpers package (#3323) Allow geobox to be constructed from 4 coordinates (#3341) Add support for geospatial queries (#3300) ... # Conflicts: # .github/templates/main.yml # .github/templates/pr.yml # .github/templates/test-net-core.yml # .github/templates/test-uwp-managed.yml # .github/workflows/pr.yml # .github/workflows/test-uwp-managed.yml
Description
Fixes #3299
Summary
Adds initial support for geo queries. In this first version, we're not introducing new datatypes, so users will need to make sure their own types conform to the shape expected for geo queries. We're providing a few helper shapes to write queries against, though those can only be used for querying and cannot be stored in the database.
Example point definition
Example query
TODO