Skip to content

Commit

Permalink
allow RQL geo args to be strings (#6934)
Browse files Browse the repository at this point in the history 
* allow RQL geo args to be strings

* clean up and address feedback
  • Loading branch information
@ironage
ironage authored Aug 31, 2023
1 parent 8b4b08d commit c258e26
Show file tree
Hide file tree
Showing 3 changed files with 79 additions and 22 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
### Enhancements
* <New feature description> (PR [#????](https://github.com/realm/realm-core/pull/????))
* Add a distinct error code for timeouts (SyncConnectTimeout) rather than using the same one as for less transient failures ([PR #6932](https://github.com/realm/realm-core/pull/6932)).
* Allow arguments to RQL to be a string representation of a geospatial object for GEOWITHIN queries. This enables SDKs using the CAPI to marshal geo objects to strings. ([PR 6934](https://github.com/realm/realm-core/issues/6934))

### Fixed
* <How do the end-user experience this issue? what was the impact?> ([#????](https://github.com/realm/realm-core/issues/????), since v?.?.?)
Expand Down
72 changes: 54 additions & 18 deletions src/realm/parser/driver.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -794,29 +794,65 @@ Query GeoWithinNode::visit(ParserDriver* drv)
auto geo_value = dynamic_cast<const ConstantGeospatialValue*>(right.get());
return link_column->geo_within(geo_value->get_mixed().get<Geospatial>());
}
else {
size_t arg_no = size_t(strtol(argument.substr(1).c_str(), nullptr, 10));
auto right_type = drv->m_args.is_argument_null(arg_no) ? DataType(-1) : drv->m_args.type_for_argument(arg_no);
if (right_type != type_Geospatial) {
throw InvalidQueryError(util::format("The right hand side of 'geoWithin' must be a geospatial constant "
"value. But the provided type is '%1'",
get_data_type_name(right_type)));
}
Geospatial geo = drv->m_args.geospatial_for_argument(arg_no);

if (geo.get_type() == Geospatial::Type::Invalid) {
throw InvalidQueryError(
util::format("The right hand side of 'geoWithin' must be a valid Geospatial value, got '%1'", geo));
REALM_ASSERT_3(argument.size(), >, 1);
REALM_ASSERT_3(argument[0], ==, '$');
size_t arg_no = size_t(strtol(argument.substr(1).c_str(), nullptr, 10));
auto right_type = drv->m_args.is_argument_null(arg_no) ? DataType(-1) : drv->m_args.type_for_argument(arg_no);

Geospatial geo_from_argument;
if (right_type == type_Geospatial) {
geo_from_argument = drv->m_args.geospatial_for_argument(arg_no);
}
else if (right_type == type_String) {
// This is a "hack" to allow users to pass in geospatial objects
// serialized as a string instead of as a native type. This is because
// the CAPI doesn't have support for marshalling polygons (of variable length)
// yet and that project was deprioritized to geospatial phase 2. This should be
// removed once SDKs are all using the binding generator.
std::string str_val = drv->m_args.string_for_argument(arg_no);
const std::string simulated_prefix = "simulated GEOWITHIN ";
str_val = simulated_prefix + str_val;
ParserDriver sub_driver;
try {
sub_driver.parse(str_val);
}
Status geo_status = geo.is_valid();
if (!geo_status.is_ok()) {
throw InvalidQueryError(
util::format("The Geospatial query argument region is invalid: '%1'", geo_status.reason()));
catch (const std::exception& ex) {
std::string doctored_err = ex.what();
size_t prefix_location = doctored_err.find(simulated_prefix);
if (prefix_location != std::string::npos) {
doctored_err.erase(prefix_location, simulated_prefix.size());
}
throw InvalidQueryError(util::format(
"Invalid syntax in serialized geospatial object at argument %1: '%2'", arg_no, doctored_err));
}
GeoWithinNode* node = dynamic_cast<GeoWithinNode*>(sub_driver.result);
REALM_ASSERT(node);
if (node->geo) {
if (node->geo->m_geo.get_type() != Geospatial::Type::Invalid) {
geo_from_argument = node->geo->m_geo;
}
else {
geo_from_argument = GeoPolygon{node->geo->m_points};
}
}
return link_column->geo_within(geo);
}
else {
throw InvalidQueryError(util::format("The right hand side of 'geoWithin' must be a geospatial constant "
"value. But the provided type is '%1'",
get_data_type_name(right_type)));
}

REALM_UNREACHABLE();
if (geo_from_argument.get_type() == Geospatial::Type::Invalid) {
throw InvalidQueryError(util::format(
"The right hand side of 'geoWithin' must be a valid Geospatial value, got '%1'", geo_from_argument));
}
Status geo_status = geo_from_argument.is_valid();
if (!geo_status.is_ok()) {
throw InvalidQueryError(
util::format("The Geospatial query argument region is invalid: '%1'", geo_status.reason()));
}
return link_column->geo_within(geo_from_argument);
}
#endif

Expand Down
28 changes: 24 additions & 4 deletions test/test_parser.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5775,11 +5775,21 @@ TEST(Parser_Geospatial)
GeoPolygon{{{GeoPoint{0, 0}, GeoPoint{1, 0}, GeoPoint{1, 1}, GeoPoint{0, 1}, GeoPoint{0, 0}}}}};
Geospatial invalid;
Geospatial point{GeoPoint{0, 0}};
std::vector<Mixed> args = {Mixed{&box}, Mixed{&circle}, Mixed{&polygon}, Mixed{&invalid},
Mixed{realm::null()}, Mixed{1.2}, Mixed{1000}, Mixed{"string value"}};
std::string str_of_box = box.to_string();
std::string str_of_circle = circle.to_string();
std::string str_of_polygon = polygon.to_string();
std::string str_of_point = point.to_string();
std::vector<Mixed> args = {Mixed{&box}, Mixed{&circle}, Mixed{&polygon},
Mixed{&invalid}, Mixed{realm::null()}, Mixed{1.2},
Mixed{1000}, Mixed{"string value"}, Mixed{str_of_box},
Mixed{str_of_circle}, Mixed{str_of_polygon}, Mixed{str_of_point}};

verify_query_sub(test_context, table, "location GEOWITHIN $0", args, 1);
verify_query_sub(test_context, table, "location GEOWITHIN $1", args, 4);
verify_query_sub(test_context, table, "location GEOWITHIN $2", args, 1);
verify_query_sub(test_context, table, "location GEOWITHIN $8", args, 1);
verify_query_sub(test_context, table, "location GEOWITHIN $9", args, 4);
verify_query_sub(test_context, table, "location GEOWITHIN $10", args, 1);

GeoCircle c = circle.get<GeoCircle>();
std::vector<Mixed> coord_args = {Mixed{c.center.longitude}, Mixed{c.center.latitude}, Mixed{c.radius_radians}};
Expand Down Expand Up @@ -5837,8 +5847,18 @@ TEST(Parser_Geospatial)
"But the provided type is 'int'") != std::string::npos));
CHECK_THROW_EX(
verify_query_sub(test_context, table, "location GEOWITHIN $7", args, 1), query_parser::InvalidQueryError,
CHECK(std::string(e.what()).find("The right hand side of 'geoWithin' must be a geospatial constant value. "
"But the provided type is 'string'") != std::string::npos));
CHECK(std::string(e.what()).find(
"Invalid syntax in serialized geospatial object at argument 7: 'Invalid predicate: 'string value': "
"syntax error, unexpected identifier, expecting geobox or geopolygon or geocircle or argument'") !=
std::string::npos));

CHECK_THROW_EX(verify_query_sub(test_context, table, "location GEOWITHIN $11", args, 1),
query_parser::InvalidQueryError,
CHECK(std::string(e.what()).find(
"Invalid syntax in serialized geospatial object at argument 11: 'Invalid predicate: "
"'GeoPoint([0, 0])': syntax error, unexpected identifier, expecting geobox or "
"geopolygon or geocircle or argument'") != std::string::npos));

CHECK_THROW_EX(verify_query_sub(test_context, table, "location GEOWITHIN $3", args, 0),
query_parser::InvalidQueryError,
CHECK(std::string(e.what()).find("The right hand side of 'geoWithin' must be a valid "
Expand Down

0 comments on commit c258e26

Please sign in to comment.