Skip to content

Commit

Permalink
RCORE-2131: Don't throw immediately when convertion of string to numb…
Browse files Browse the repository at this point in the history
…er fails (#7715)

The parser will throw later anyway if the comparison cannot be done.
  • Loading branch information
jedelbo authored May 22, 2024
1 parent de18fc3 commit 3648014
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 42 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

### Fixed
* A non-streaming progress notifier would not immediately call its callback after registration. Instead you would have to wait for a download message to be received to get your first update - if you were already caught up when you registered the notifier you could end up waiting a long time for the server to deliver a download that would call/expire your notifier ([#7627](https://github.com/realm/realm-core/issues/7627), since v14.6.0).
* Comparing a numeric property with an argument list containing a string would throw. ([#7714](https://github.com/realm/realm-core/issues/7714), since v14.7.0)

### Breaking changes
* None.
Expand Down
63 changes: 23 additions & 40 deletions src/realm/parser/driver.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -127,53 +127,26 @@ inline bool try_parse_specials(std::string str, T& ret)
}

template <typename T>
inline const char* get_type_name()
{
return "unknown";
}
template <>
inline const char* get_type_name<int64_t>()
{
return "number";
}
template <>
inline const char* get_type_name<float>()
{
return "floating point number";
}
template <>
inline const char* get_type_name<double>()
{
return "floating point number";
}

template <>
inline const char* get_type_name<Decimal128>()
{
return "decimal number";
}

template <typename T>
inline T string_to(const std::string& s)
inline std::optional<T> string_to(const std::string& s)
{
std::istringstream iss(s);
iss.imbue(std::locale::classic());
T value;
iss >> value;
if (iss.fail()) {
if (!try_parse_specials(s, value)) {
throw InvalidQueryArgError(util::format("Cannot convert '%1' to a %2", s, get_type_name<T>()));
return {};
}
}
return value;
}

template <>
inline Decimal128 string_to<Decimal128>(const std::string& s)
inline std::optional<Decimal128> string_to<Decimal128>(const std::string& s)
{
Decimal128 value(s);
if (value.is_nan()) {
throw InvalidQueryArgError(util::format("Cannot convert '%1' to a %2", s, get_type_name<Decimal128>()));
return {};
}
return value;
}
Expand Down Expand Up @@ -1391,16 +1364,24 @@ std::unique_ptr<Subexpr> ConstantNode::visit(ParserDriver* drv, DataType hint)
StringData str = value.get_string();
switch (hint) {
case type_Int:
value = string_to<int64_t>(str);
if (auto val = string_to<int64_t>(str)) {
value = *val;
}
break;
case type_Float:
value = string_to<float>(str);
if (auto val = string_to<float>(str)) {
value = *val;
}
break;
case type_Double:
value = string_to<double>(str);
if (auto val = string_to<double>(str)) {
value = *val;
}
break;
case type_Decimal:
value = string_to<Decimal128>(str);
if (auto val = string_to<Decimal128>(str)) {
value = *val;
}
break;
default:
break;
Expand Down Expand Up @@ -1443,11 +1424,6 @@ std::unique_ptr<Subexpr> ConstantNode::visit(ParserDriver* drv, DataType hint)
}
else {
explain_value_message = util::format("argument %1 with value '%2'", explain_value_message, value);
if (!(m_target_table || Mixed::data_types_are_comparable(value.get_type(), hint) ||
Mixed::is_numeric(hint) || (value.is_type(type_String) && hint == type_TypeOfValue))) {
throw InvalidQueryArgError(
util::format("Cannot compare %1 to a %2", explain_value_message, get_data_type_name(hint)));
}
}
}
}
Expand Down Expand Up @@ -1485,6 +1461,13 @@ std::unique_ptr<Subexpr> ConstantNode::visit(ParserDriver* drv, DataType hint)

convert_if_needed(value);

if (type == Type::ARG && !(m_target_table || Mixed::data_types_are_comparable(value.get_type(), hint) ||
(value.is_type(type_TypedLink) && hint == type_Link) ||
(value.is_type(type_String) && hint == type_TypeOfValue))) {
throw InvalidQueryArgError(
util::format("Cannot compare %1 to a %2", explain_value_message, get_data_type_name(hint)));
}

switch (value.get_type()) {
case type_Int: {
ret = std::make_unique<Value<int64_t>>(value.get_int());
Expand Down
4 changes: 2 additions & 2 deletions test/test_parser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2266,7 +2266,7 @@ TEST(Parser_list_of_primitive_ints)
message,
"Unsupported comparison operator 'endswith' against type 'int', right side must be a string or binary type");
CHECK_THROW_ANY_GET_MESSAGE(verify_query(test_context, t, "integers == 'string'", 0), message);
CHECK_EQUAL(message, "Cannot convert 'string' to a number");
CHECK_EQUAL(message, "Unsupported comparison between type 'int' and type 'string'");
}

TEST_TYPES(Parser_list_of_primitive_strings, std::true_type, std::false_type)
Expand Down Expand Up @@ -3346,7 +3346,7 @@ TEST(Parser_BacklinkCount)
std::string message;
// backlink count requires comparison to a numeric type
CHECK_THROW_ANY_GET_MESSAGE(verify_query(test_context, items, "@links.@count == 'string'", -1), message);
CHECK_EQUAL(message, "Cannot convert 'string' to a number");
CHECK_EQUAL(message, "Unsupported comparison between type 'int' and type 'string'");
CHECK_THROW_ANY_GET_MESSAGE(verify_query(test_context, items, "@links.@count == 2018-04-09@14:21:0", -1),
message);
CHECK_EQUAL(message, "Unsupported comparison between type 'int' and type 'timestamp'");
Expand Down

0 comments on commit 3648014

Please sign in to comment.