diff --git a/src/query/typed_value.cpp b/src/query/typed_value.cpp index b72d0dde2..77a025075 100644 --- a/src/query/typed_value.cpp +++ b/src/query/typed_value.cpp @@ -698,52 +698,47 @@ TypedValue operator%(const TypedValue &a, const TypedValue &b) { } } -inline bool IsLogicallyOk(const TypedValue &a) { - return a.type() == TypedValue::Type::Bool || - a.type() == TypedValue::Type::Null; +inline void EnsureLogicallyOk(const TypedValue &a, const TypedValue &b, + const std::string &op_name) { + if ((a.type() != TypedValue::Type::Bool && + a.type() != TypedValue::Type::Null) || + (b.type() != TypedValue::Type::Bool && + b.type() != TypedValue::Type::Null)) + throw TypedValueException("Invalid {} operand types({} && {})", op_name, + a.type(), b.type()); } -// TODO: Fix bugs in && and ||. null or true -> true; false and null -> false TypedValue operator&&(const TypedValue &a, const TypedValue &b) { - if (IsLogicallyOk(a) && IsLogicallyOk(b)) { - if (a.type() == TypedValue::Type::Null || - b.type() == TypedValue::Type::Null) { - return TypedValue::Null; - } else { - return a.Value() && b.Value(); - } - } else { - throw TypedValueException("Invalid logical and operand types({} && {})", - a.type(), b.type()); - } + EnsureLogicallyOk(a, b, "logical AND"); + // at this point we only have null and bool + // if either operand is false, the result is false + if (a.type() == TypedValue::Type::Bool && !a.Value()) return false; + if (b.type() == TypedValue::Type::Bool && !b.Value()) return false; + if (a.type() == TypedValue::Type::Null || b.type() == TypedValue::Type::Null) + return TypedValue::Null; + // neither is false, neither is null, thus both are true + return true; } TypedValue operator||(const TypedValue &a, const TypedValue &b) { - if (IsLogicallyOk(a) && IsLogicallyOk(b)) { - if (a.type() == TypedValue::Type::Null || - b.type() == TypedValue::Type::Null) { - return TypedValue::Null; - } else { - return a.Value() || b.Value(); - } - } else { - throw TypedValueException("Invalid logical and operand types({} && {})", - a.type(), b.type()); - } + EnsureLogicallyOk(a, b, "logical OR"); + // at this point we only have null and bool + // if either operand is true, the result is true + if (a.type() == TypedValue::Type::Bool && a.Value()) return true; + if (b.type() == TypedValue::Type::Bool && b.Value()) return true; + if (a.type() == TypedValue::Type::Null || b.type() == TypedValue::Type::Null) + return TypedValue::Null; + // neither is true, neither is null, thus both are false + return false; } TypedValue operator^(const TypedValue &a, const TypedValue &b) { - if (IsLogicallyOk(a) && IsLogicallyOk(b)) { - if (a.type() == TypedValue::Type::Null || - b.type() == TypedValue::Type::Null) { - return TypedValue::Null; - } else { - return static_cast(a.Value() ^ b.Value()); - } - } else { - throw TypedValueException("Invalid logical and operand types({} && {})", - a.type(), b.type()); - } + EnsureLogicallyOk(a, b, "logical XOR"); + // at this point we only have null and bool + if (a.type() == TypedValue::Type::Null || b.type() == TypedValue::Type::Null) + return TypedValue::Null; + else + return static_cast(a.Value() ^ b.Value()); } bool TypedValue::BoolEqual::operator()(const TypedValue &lhs, diff --git a/tests/unit/typed_value.cpp b/tests/unit/typed_value.cpp index 38a2e0dc8..db836fc96 100644 --- a/tests/unit/typed_value.cpp +++ b/tests/unit/typed_value.cpp @@ -379,7 +379,9 @@ void TestLogicalThrows( TEST(TypedValue, LogicalAnd) { TestLogicalThrows( [](const TypedValue &p1, const TypedValue &p2) { return p1 && p2; }); + EXPECT_PROP_ISNULL(TypedValue::Null && TypedValue(true)); + EXPECT_PROP_EQ(TypedValue::Null && TypedValue(false), TypedValue(false)); EXPECT_PROP_EQ(TypedValue(true) && TypedValue(true), TypedValue(true)); EXPECT_PROP_EQ(TypedValue(false) && TypedValue(true), TypedValue(false)); } @@ -387,7 +389,9 @@ TEST(TypedValue, LogicalAnd) { TEST(TypedValue, LogicalOr) { TestLogicalThrows( [](const TypedValue &p1, const TypedValue &p2) { return p1 || p2; }); - EXPECT_PROP_ISNULL(TypedValue::Null && TypedValue(true)); + + EXPECT_PROP_ISNULL(TypedValue::Null || TypedValue(false)); + EXPECT_PROP_EQ(TypedValue::Null || TypedValue(true), TypedValue(true)); EXPECT_PROP_EQ(TypedValue(true) || TypedValue(true), TypedValue(true)); EXPECT_PROP_EQ(TypedValue(false) || TypedValue(true), TypedValue(true)); } @@ -395,6 +399,7 @@ TEST(TypedValue, LogicalOr) { TEST(TypedValue, LogicalXor) { TestLogicalThrows( [](const TypedValue &p1, const TypedValue &p2) { return p1 ^ p2; }); + EXPECT_PROP_ISNULL(TypedValue::Null && TypedValue(true)); EXPECT_PROP_EQ(TypedValue(true) ^ TypedValue(true), TypedValue(false)); EXPECT_PROP_EQ(TypedValue(false) ^ TypedValue(true), TypedValue(true));