Geospatial queries for points #6562

ironage · 2023-05-02T17:54:09Z

Initial implementation of geospatial queries for points. Most of the code additions come from adding the s2 library to our code base. Other changes have been reviewed in separate PRs previously.

☑️ ToDos

📝 Changelog update
🚦 Tests (or not relevant)

Geospatial prototype

Merge master, adapt exceptions

…ort_for_geowithin Geospatial Polygon and CenterSphere support for geowithin operator

update from latest

* integrate Realm logging and assertions * remove some string utilities * remove encoder/decoder * remove some base files * remove hash * fix header search path for s2 swift builds

* scoped_ptr -> std::unique_ptr * use std type_traits * Replace deprecated is_pod with is_trivial --------- Co-authored-by: Kirill Burtsev <kirill.burtsev@mongodb.com>

…queries_for_points

* Don't use optional for sphere radius to reduce the size of Geospatial * Use radians for CenterSphere radius, expose constant * Add comments about points in Box and Polygon * Don't construct optional needlessly on Geospatial comparison

* geospatial query parser support WIP * some error handling * RQL geoSphere * RQL geoPolygon * query geospatial arguments * C-API geospatial in mixed * add a Geospatial type checking API * format * fix warnings on Windows * Alternative syntax definition * API changes from feedback and testing * remove type_GeoPoint as Geospatial is an umbrella for this * additional tests * fix an unused warning * review feedback * Use NaN for GeoPoint (#6490) * Use NaN for altitude, expose to public * Check for NaN for lat/lon --------- Co-authored-by: James Stone <james.stone@mongodb.com> * formatting * remove GeospatialRef and use Geospatial* remove wrong C-API implementation (#6518) --------- Co-authored-by: Jørgen Edelbo <jorgen.edelbo@mongodb.com> Co-authored-by: Kirill Burtsev <kirill.burtsev@mongodb.com>

* Allow to disable compilation of whole geospatial support * Turn on the geo feature for swift build by default

* Geospatial uses a variant polygon has holes * support holes in polygons in RQL * touch ups * Add REALM_ENABLE_GEOSPATIAL to config.h * add missing include * Fix test build for geo feature, optimize copying (#6550) * Fix build without geospatial * Don't force to copy primitives on get<> * Allow less curly braces for GeoPolygon on init * format --------- Co-authored-by: Jørgen Edelbo <jorgen.edelbo@mongodb.com> Co-authored-by: Kirill Burtsev <kirill.burtsev@mongodb.com>

evergreen/config.yml

kiburtse · 2023-05-03T10:51:31Z

We should exclude somehow s2 from CodeFactor, right?

jedelbo

LGTM

nicola-cab

Really nice.

nicola-cab · 2023-05-03T14:49:01Z

src/realm/data_type.hpp

@@ -152,6 +153,9 @@ static constexpr DataType type_OldDateTime = DataType{7};
 static_assert(!type_OldTable.is_valid());
 static_assert(!type_OldDateTime.is_valid());
 static constexpr DataType type_TypeOfValue = DataType{18};
+#if REALM_ENABLE_GEOSPATIAL
+static constexpr DataType type_Geospatial = DataType{19};


This will collide with nested collections, whoever gets first into master will have to adjust this value (nested collections are using 19,20,21) ... maybe we can agree to leave 19 free or whatever, also if we intend to keep this behind the GEOSPATIAL macro maybe it is better if we reserve the slot 22.

Thanks for the heads up. The geospatial type is not part of the file format so its value doesn't really matter yet. I changed it to 22 so it doesn't conflict with nested collections.

src/realm/geospatial.hpp

nicola-cab · 2023-05-03T15:15:52Z

src/realm/geospatial.hpp

+    {
+        return !(*this == other);
+    }
+    bool operator>(const Geospatial&) const = delete;


Why do you need to delete the operators? Is this done for better compiler error messages?

Right, we don't want to allow these kinds of comparisons and by deleting them we make sure that they aren't being used anywhere in the query system or elsewhere.

src/realm/util/file_mapper.cpp

src/realm/util/serializer.hpp

src/realm/utilities.hpp

nicola-cab · 2023-05-03T15:41:59Z

src/realm/geospatial.cpp

+    return true;
+}
+
+Geospatial Geospatial::from_obj(const Obj& obj, ColKey type_col, ColKey coords_col)


So a Geospatial type is a Obj which has a list of points (doubles) and a string to identify if it is longitude or latitude? Am I right? When it will come to deal with polygons, will you flatten a matrix into a list?

The string is to identify what type of geometry is in this Geospatial type (polygon, line), but for now it's just 'point' which is supported. Second question is an open one. It remains to be seen how this is going to be implemented. Scope is yet to come for discussion.

tgoyne · 2023-05-03T15:45:37Z

Package.swift

@@ -16,6 +16,7 @@ var cxxSettings: [CXXSetting] = [
    .define("REALM_ENABLE_ASSERTIONS", to: "1"),
    .define("REALM_ENABLE_ENCRYPTION", to: "1"),
    .define("REALM_ENABLE_SYNC", to: "1"),
+    .define("REALM_ENABLE_GEOSPATIAL", to: "1"),


If the intention is to have this disabled until we expose it in the SDK then it needs to be set to 0 here (and in tools/build-cocoa.sh).

Yes thanks. I updated both places to disable it by default for cocoa builds.

I don't think there was such intention, we just added it this way to check that we can optionally disable whole geospatial if only through configure flag. Most of current implementation is useable without any work in sdks like rql queries on local data. Default was set to allow straight forward experimentation without official release of this feature.

Cocoa does not use the core query parser.

Right... @ironage i think that would need a bit more work to properly disable in in Package.swift. It's not just this define.

I don't think there's a reason to disable it by default. It doesn't upgrade the file format and the feature existing wouldn't impact Cocoa users besides a slight binary size increase. Whether we increase the binary size today or in a month when we add support for the feature is most likely inconsequential for the majority of our users.

ironage · 2023-05-03T18:21:18Z

We should exclude somehow s2 from CodeFactor, right?

I updated the project settings over at codefactor.io to ignore src/external/** and src/realm/parser/generated/**

kiburtse and others added 30 commits February 1, 2023 14:47

Add s2geometry sources for geo queries support from mongodb repo as is

5c08b06

s2: cleanup/remove unneeded files

41edb32

s2: add to build

ca39b9f

Add basic test for s2geometry link

f941de7

Object store Geospatial mappings

3bc4421

starting queries

de502cc

Merge pull request #6284 from realm/js/geospatial

3ef4a4b

Geospatial prototype

s2: cleanup tests related functions

726bb9c

Merge branch 'master' into kb/geospatial_merge_master

57d1ccc

Fix build due to removed error categories

8c1d759

Replace std exceptions with realm specific

3f13ca9

Merge pull request #6361 from realm/kb/geospatial_merge_master

d949d83

Merge master, adapt exceptions

Move geo query tests to separate test file for future expansion

5792653

Add init types for polygon and center sphere

388090d

Add basic geo_within support for Polygon and CenterSphere

95b55b8

Fix windows build

13d0ec6

Style: rename geo constants

70f2a8f

Fix MacOS compatibility build issue

0d102f8

More win/mac build fixes

60933bd

Add s2geometry library for swift package build

018c63e

Suppress s2 warnings for gcc

c2a7376

Merge pull request #6346 from realm/kb/geospatial_polygon_sphere_supp…

c985968

…ort_for_geowithin Geospatial Polygon and CenterSphere support for geowithin operator

Merge branch 'master' into js/update-from-base

fa7d977

fix clang-format complaint

bd5267a

Merge branch 'master' into js/update-from-base

13ad86a

Merge pull request #6450 from realm/js/update-from-base

e97c98d

update from latest

Trim down unnecessary S2 sources (#6456)

314121b

* integrate Realm logging and assertions * remove some string utilities * remove encoder/decoder * remove some base files * remove hash * fix header search path for s2 swift builds

Fix include of external when built as subproject of sdk

29b8d2e

Use modern language features (#6466)

8130093

* scoped_ptr -> std::unique_ptr * use std type_traits * Replace deprecated is_pod with is_trivial --------- Co-authored-by: Kirill Burtsev <kirill.burtsev@mongodb.com>

Merge remote-tracking branch 'origin/master' into feature/geospatial_…

ff7d333

…queries_for_points

kiburtse and others added 7 commits April 13, 2023 17:07

Add REALM_ENABLE_GEOSPATIAL cmake option to disable feature (#6525)

e079865

* Allow to disable compilation of whole geospatial support * Turn on the geo feature for swift build by default

Remove non-compiling hash_value function (#6533)

eb37390

Merge branch 'master' into feature/geospatial_queries_for_points

1d670f9

changelog and test without geospatial on CI

46c0a73

ironage requested review from jedelbo, nicola-cab and kiburtse May 2, 2023 17:54

cla-bot bot added the cla: yes label May 2, 2023

github-actions bot assigned ironage May 2, 2023

kiburtse reviewed May 3, 2023

View reviewed changes

evergreen/config.yml Show resolved Hide resolved

jedelbo reviewed May 3, 2023

View reviewed changes

nicola-cab reviewed May 3, 2023

View reviewed changes

tgoyne reviewed May 3, 2023

View reviewed changes

ironage added 2 commits May 3, 2023 10:50

disable geospatial by default for cocoa builds, review feedback

cdef8e0

Merge branch 'master' into feature/geospatial_queries_for_points

0e73ede

ironage added 3 commits May 4, 2023 14:32

Merge branch 'master' into feature/geospatial_queries_for_points

5b3b87b

turn off geospatial in swift builds for now

0c255ad

formatting

a3c8cd8

kiburtse approved these changes May 5, 2023

View reviewed changes

nicola-cab self-requested a review May 5, 2023 17:41

nicola-cab approved these changes May 5, 2023

View reviewed changes

Merge branch 'master' into feature/geospatial_queries_for_points

420fb0c

ironage merged commit a4ff1b0 into master May 8, 2023
2 checks passed

ironage deleted the feature/geospatial_queries_for_points branch May 8, 2023 16:48

github-actions bot locked as resolved and limited conversation to collaborators Mar 21, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Geospatial queries for points #6562

Geospatial queries for points #6562

ironage commented May 2, 2023

kiburtse commented May 3, 2023

jedelbo left a comment

nicola-cab left a comment

nicola-cab May 3, 2023

ironage May 3, 2023

nicola-cab May 3, 2023

ironage May 3, 2023

nicola-cab May 3, 2023

kiburtse May 3, 2023

tgoyne May 3, 2023

ironage May 3, 2023

kiburtse May 3, 2023

tgoyne May 3, 2023

kiburtse May 3, 2023

nirinchev May 3, 2023

ironage commented May 3, 2023

Geospatial queries for points #6562

Geospatial queries for points #6562

Conversation

ironage commented May 2, 2023

☑️ ToDos

kiburtse commented May 3, 2023

jedelbo left a comment

Choose a reason for hiding this comment

nicola-cab left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ironage commented May 3, 2023