tursom/memgraph
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Fix an error where null AND false returned null

Browse Source 
Summary:
Tests have been updated to catch this error and other behaviour. Other
than this change, `AND` should behave as before.

Reviewers: florijan, mislav.bradac

Reviewed By: florijan

Subscribers: pullbot

Differential Revision: https://phabricator.memgraph.io/D970
This commit is contained in:
Teon Banek 2017-11-08 13:13:58 +01:00
parent 4b8076f41a
commit 7fabdda938
2 changed files with 38 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
src/query/interpret/eval.hpp
View File

@ -115,11 +115,17 @@ class ExpressionEvaluator : public TreeVisitor<TypedValue> {
TypedValue Visit(AndOperator &op) override {
auto value1 = op.expression1_->Accept(*this);
if (value1.IsNull() || !value1.Value<bool>()) {
// If first expression is null or false, don't execute the second one.
if (value1.IsBool() && !value1.Value<bool>()) {
// If first expression is false, don't evaluate the second one.
return value1;
}
return op.expression2_->Accept(*this);
auto value2 = op.expression2_->Accept(*this);
try {
return value1 && value2;
} catch (const TypedValueException &) {
throw QueryRuntimeException("Invalid types: {} and {} for 'AND'",
value1.type(), value2.type());
}
}
TypedValue Visit(IfOperator &if_operator) override {

29
tests/unit/query_expression_evaluator.cpp
View File

@ -113,12 +113,41 @@ TEST(ExpressionEvaluator, AndOperatorShortCircuit) {
EXPECT_EQ(value.Value<bool>(), false);
}
{
auto *op =
storage.Create<AndOperator>(storage.Create<PrimitiveLiteral>(5),
storage.Create<PrimitiveLiteral>(false));
// We are evaluating left to right, so we don't short circuit here and raise
// due to `5`. This differs from neo4j, where they evaluate both sides and
// return `false` without checking for type of the first expression.
EXPECT_THROW(op->Accept(eval.eval), QueryRuntimeException);
}
}
TEST(ExpressionEvaluator, AndOperatorNull) {
AstTreeStorage storage;
NoContextExpressionEvaluator eval;
{
// Null doesn't short circuit
auto *op = storage.Create<AndOperator>(
storage.Create<PrimitiveLiteral>(TypedValue::Null),
storage.Create<PrimitiveLiteral>(5));
EXPECT_THROW(op->Accept(eval.eval), QueryRuntimeException);
}
{
auto *op = storage.Create<AndOperator>(
storage.Create<PrimitiveLiteral>(TypedValue::Null),
storage.Create<PrimitiveLiteral>(true));
auto value = op->Accept(eval.eval);
EXPECT_TRUE(value.IsNull());
}
{
auto *op = storage.Create<AndOperator>(
storage.Create<PrimitiveLiteral>(TypedValue::Null),
storage.Create<PrimitiveLiteral>(false));
auto value = op->Accept(eval.eval);
ASSERT_TRUE(value.IsBool());
EXPECT_EQ(value.Value<bool>(), false);
}
}
TEST(ExpressionEvaluator, AdditionOperator) {