diff --git a/src/common/filter/Expressions.cpp b/src/common/filter/Expressions.cpp index 50c1e18d5fc..fb2733fc4a5 100644 --- a/src/common/filter/Expressions.cpp +++ b/src/common/filter/Expressions.cpp @@ -118,7 +118,7 @@ std::string AliasPropertyExpression::toString() const { return buf; } -VariantType AliasPropertyExpression::eval() const { +OptVariantType AliasPropertyExpression::eval() const { return context_->getters().getAliasProp(*alias_, *prop_); } @@ -165,7 +165,7 @@ Status InputPropertyExpression::prepare() { } -VariantType InputPropertyExpression::eval() const { +OptVariantType InputPropertyExpression::eval() const { return context_->getters().getInputProp(*prop_); } @@ -190,7 +190,7 @@ const char* InputPropertyExpression::decode(const char *pos, const char *end) { } -VariantType DestPropertyExpression::eval() const { +OptVariantType DestPropertyExpression::eval() const { return context_->getters().getDstTagProp(*alias_, *prop_); } @@ -234,11 +234,10 @@ const char* DestPropertyExpression::decode(const char *pos, const char *end) { } -VariantType VariablePropertyExpression::eval() const { +OptVariantType VariablePropertyExpression::eval() const { return context_->getters().getVariableProp(*prop_); } - Status VariablePropertyExpression::prepare() { context_->addVariableProp(*alias_, *prop_); return Status::OK(); @@ -278,7 +277,7 @@ const char* VariablePropertyExpression::decode(const char *pos, const char *end) } -VariantType EdgeTypeExpression::eval() const { +OptVariantType EdgeTypeExpression::eval() const { return *alias_; } @@ -309,7 +308,7 @@ const char* EdgeTypeExpression::decode(const char *pos, const char *end) { } -VariantType EdgeSrcIdExpression::eval() const { +OptVariantType EdgeSrcIdExpression::eval() const { return context_->getters().getAliasProp(*alias_, *prop_); } @@ -340,7 +339,7 @@ const char* EdgeSrcIdExpression::decode(const char *pos, const char *end) { } -VariantType EdgeDstIdExpression::eval() const { +OptVariantType EdgeDstIdExpression::eval() const { return context_->getters().getAliasProp(*alias_, *prop_); } @@ -371,7 +370,7 @@ const char* EdgeDstIdExpression::decode(const char *pos, const char *end) { } -VariantType EdgeRankExpression::eval() const { +OptVariantType EdgeRankExpression::eval() const { return context_->getters().getAliasProp(*alias_, *prop_); } @@ -402,7 +401,7 @@ const char* EdgeRankExpression::decode(const char *pos, const char *end) { } -VariantType SourcePropertyExpression::eval() const { +OptVariantType SourcePropertyExpression::eval() const { return context_->getters().getSrcTagProp(*alias_, *prop_); } @@ -463,8 +462,7 @@ std::string PrimaryExpression::toString() const { return buf; } - -VariantType PrimaryExpression::eval() const { +OptVariantType PrimaryExpression::eval() const { switch (operand_.which()) { case VAR_INT64: return boost::get(operand_); @@ -478,9 +476,9 @@ VariantType PrimaryExpression::eval() const { case VAR_STR: return boost::get(operand_); } - return std::string("Unknown"); -} + return OptVariantType(Status::Error("Unknown type")); +} Status PrimaryExpression::prepare() { return Status::OK(); @@ -563,17 +561,21 @@ std::string FunctionCallExpression::toString() const { return buf; } - -VariantType FunctionCallExpression::eval() const { +OptVariantType FunctionCallExpression::eval() const { std::vector args; - args.resize(args_.size()); - auto eval = [] (auto &expr) { - return expr->eval(); - }; - std::transform(args_.begin(), args_.end(), args.begin(), eval); - return function_(args); -} + for (auto it = args_.cbegin(); it != args_.cend(); ++it) { + auto result = (*it)->eval(); + if (!result.ok()) { + return result; + } + args.push_back(std::move(result.value())); + } + + // TODO(simon.liu) + auto r = function_(args); + return OptVariantType(r); +} Status FunctionCallExpression::prepare() { auto result = FunctionManager::get(*name_, args_.size()); @@ -650,23 +652,25 @@ std::string UnaryExpression::toString() const { return buf; } - -VariantType UnaryExpression::eval() const { +OptVariantType UnaryExpression::eval() const { auto value = operand_->eval(); - if (op_ == PLUS) { - return value; - } else if (op_ == NEGATE) { - if (isInt(value)) { - return -asInt(value); - } else if (isDouble(value)) { - return -asDouble(value); + if (value.ok()) { + if (op_ == PLUS) { + return value; + } else if (op_ == NEGATE) { + if (isInt(value.value())) { + return OptVariantType(-asInt(value.value())); + } else if (isDouble(value.value())) { + return OptVariantType(-asDouble(value.value())); + } + } else { + return OptVariantType(!asBool(value.value())); } - } else { - return !asBool(value); } - return value; -} + return OptVariantType(Status::Error(folly::sformat( + "attempt to perform unary arithmetic on a {}", value.value().type().name()))); +} Status UnaryExpression::prepare() { return operand_->prepare(); @@ -714,12 +718,10 @@ std::string TypeCastingExpression::toString() const { return ""; } - -VariantType TypeCastingExpression::eval() const { - return toString(); +OptVariantType TypeCastingExpression::eval() const { + return OptVariantType(toString()); } - Status TypeCastingExpression::prepare() { return operand_->prepare(); } @@ -757,48 +759,69 @@ std::string ArithmeticExpression::toString() const { return buf; } - -VariantType ArithmeticExpression::eval() const { +OptVariantType ArithmeticExpression::eval() const { auto left = left_->eval(); auto right = right_->eval(); + if (!left.ok()) { + return left; + } + + if (!right.ok()) { + return right; + } + + auto l = left.value(); + auto r = right.value(); + switch (op_) { - case ADD: - assert((isArithmetic(left) && isArithmetic(right)) - || (isString(left) && isString(right))); - if (isArithmetic(left) && isArithmetic(right)) { - if (isDouble(left) || isDouble(right)) { - return asDouble(left) + asDouble(right); + case ADD: + if (isArithmetic(l) && isArithmetic(r)) { + if (isDouble(l) || isDouble(r)) { + return OptVariantType(asDouble(l) + asDouble(r)); + } + return OptVariantType(asInt(l) + asInt(r)); } - return asInt(left) + asInt(right); - } - return asString(left) + asString(right); - case SUB: - assert(isArithmetic(left) && isArithmetic(right)); - if (isDouble(left) || isDouble(right)) { - return asDouble(left) - asDouble(right); - } - return asInt(left) - asInt(right); - case MUL: - assert(isArithmetic(left) && isArithmetic(right)); - if (isDouble(left) || isDouble(right)) { - return asDouble(left) * asDouble(right); - } - return asInt(left) * asInt(right); - case DIV: - assert(isArithmetic(left) && isArithmetic(right)); - if (isDouble(left) || isDouble(right)) { - return asDouble(left) / asDouble(right); - } - return asInt(left) / asInt(right); - case MOD: - assert(isInt(left) && isInt(right)); - return asInt(left) % asInt(right); - default: - DCHECK(false); + + if (isString(l) && isString(r)) { + return OptVariantType(asString(l) + asString(r)); + } + break; + case SUB: + if (isArithmetic(l) && isArithmetic(r)) { + if (isDouble(l) || isDouble(r)) { + return OptVariantType(asDouble(l) - asDouble(r)); + } + return OptVariantType(asInt(l) - asInt(r)); + } + break; + case MUL: + if (isArithmetic(l) && isArithmetic(r)) { + if (isDouble(l) || isDouble(r)) { + return OptVariantType(asDouble(l) * asDouble(r)); + } + return OptVariantType(asInt(l) * asInt(r)); + } + break; + case DIV: + if (isArithmetic(l) && isArithmetic(r)) { + if (isDouble(l) || isDouble(r)) { + return OptVariantType(asDouble(l) / asDouble(r)); + } + return OptVariantType(asInt(l) / asInt(r)); + } + break; + case MOD: + if (isInt(l) && isInt(r)) { + return OptVariantType(asInt(l) % asInt(r)); + } + break; + default: + DCHECK(false); } - return false; -} + return OptVariantType(Status::Error(folly::sformat( + "attempt to perform arithmetic on {} with {}", l.type().name(), r.type().name()))); +} Status ArithmeticExpression::prepare() { auto status = left_->prepare(); @@ -865,33 +888,49 @@ std::string RelationalExpression::toString() const { return buf; } - -VariantType RelationalExpression::eval() const { +OptVariantType RelationalExpression::eval() const { auto left = left_->eval(); auto right = right_->eval(); + + if (!left.ok()) { + return left; + } + + if (!right.ok()) { + return right; + } + + auto l = left.value(); + auto r = right.value(); switch (op_) { case LT: - return left < right; + return OptVariantType(l < r); case LE: - return left <= right; + return OptVariantType(l <= r); case GT: - return left > right; + return OptVariantType(l > r); case GE: - return left >= right; + return OptVariantType(l >= r); case EQ: - if (isDouble(left) || isDouble(right)) { - return almostEqual(asDouble(left), asDouble(right)); + if (isArithmetic(l) && isArithmetic(r)) { + if (isDouble(l) || isDouble(r)) { + return OptVariantType( + almostEqual(asDouble(l), asDouble(r))); + } } - return left == right; + return OptVariantType(l == r); case NE: - if (isDouble(left) || isDouble(right)) { - return !almostEqual(asDouble(left), asDouble(right)); + if (isArithmetic(l) && isArithmetic(r)) { + if (isDouble(l) || isDouble(r)) { + return OptVariantType( + !almostEqual(asDouble(l), asDouble(r))); + } } - return left != right; + return OptVariantType(l != r); } - return false; -} + return OptVariantType(Status::Error("Wrong operator")); +} Status RelationalExpression::prepare() { auto status = left_->prepare(); @@ -946,22 +985,31 @@ std::string LogicalExpression::toString() const { return buf; } +OptVariantType LogicalExpression::eval() const { + auto left = left_->eval(); + auto right = right_->eval(); + + if (!left.ok()) { + return left; + } + + if (!right.ok()) { + return right; + } -VariantType LogicalExpression::eval() const { if (op_ == AND) { - if (!asBool(left_->eval())) { - return false; + if (!asBool(left.value())) { + return OptVariantType(false); } - return asBool(right_->eval()); + return OptVariantType(asBool(right.value())); } else { - if (asBool(left_->eval())) { - return true; + if (asBool(left.value())) { + return OptVariantType(true); } - return asBool(right_->eval()); + return OptVariantType(asBool(right.value())); } } - Status LogicalExpression::prepare() { auto status = left_->prepare(); if (!status.ok()) { diff --git a/src/common/filter/Expressions.h b/src/common/filter/Expressions.h index a29174fdb33..6829e3727de 100644 --- a/src/common/filter/Expressions.h +++ b/src/common/filter/Expressions.h @@ -13,6 +13,7 @@ namespace nebula { class Cord; +using OptVariantType = StatusOr; enum ColumnType { INT, STRING, DOUBLE, BIGINT, BOOL, TIMESTAMP, @@ -88,12 +89,12 @@ class ExpressionContext final { } struct Getters { - std::function getEdgeRank; - std::function getInputProp; - std::function getVariableProp; - std::function getSrcTagProp; - std::function getDstTagProp; - std::function getAliasProp; + std::function getEdgeRank; + std::function getInputProp; + std::function getVariableProp; + std::function getSrcTagProp; + std::function getDstTagProp; + std::function getAliasProp; }; Getters& getters() { @@ -125,7 +126,7 @@ class Expression { virtual Status MUST_USE_RESULT prepare() = 0; - virtual VariantType eval() const = 0; + virtual OptVariantType eval() const = 0; virtual bool isInputExpression() const { return kind_ == kInputProp; @@ -297,7 +298,7 @@ class AliasPropertyExpression: public Expression { std::string toString() const override; - VariantType eval() const override; + OptVariantType eval() const override; Status MUST_USE_RESULT prepare() override; @@ -333,7 +334,7 @@ class InputPropertyExpression final : public AliasPropertyExpression { prop_.reset(prop); } - VariantType eval() const override; + OptVariantType eval() const override; Status MUST_USE_RESULT prepare() override; @@ -358,7 +359,7 @@ class DestPropertyExpression final : public AliasPropertyExpression { prop_.reset(prop); } - VariantType eval() const override; + OptVariantType eval() const override; Status MUST_USE_RESULT prepare() override; @@ -383,7 +384,7 @@ class VariablePropertyExpression final : public AliasPropertyExpression { prop_.reset(prop); } - VariantType eval() const override; + OptVariantType eval() const override; Status MUST_USE_RESULT prepare() override; @@ -408,7 +409,7 @@ class EdgeTypeExpression final : public AliasPropertyExpression { prop_.reset(new std::string("_type")); } - VariantType eval() const override; + OptVariantType eval() const override; Status MUST_USE_RESULT prepare() override; @@ -433,7 +434,7 @@ class EdgeSrcIdExpression final : public AliasPropertyExpression { prop_.reset(new std::string("_src")); } - VariantType eval() const override; + OptVariantType eval() const override; Status MUST_USE_RESULT prepare() override; @@ -458,7 +459,7 @@ class EdgeDstIdExpression final : public AliasPropertyExpression { prop_.reset(new std::string("_dst")); } - VariantType eval() const override; + OptVariantType eval() const override; Status MUST_USE_RESULT prepare() override; @@ -483,7 +484,7 @@ class EdgeRankExpression final : public AliasPropertyExpression { prop_.reset(new std::string("_rank")); } - VariantType eval() const override; + OptVariantType eval() const override; Status MUST_USE_RESULT prepare() override; @@ -508,7 +509,7 @@ class SourcePropertyExpression final : public AliasPropertyExpression { prop_.reset(prop); } - VariantType eval() const override; + OptVariantType eval() const override; Status MUST_USE_RESULT prepare() override; @@ -548,7 +549,7 @@ class PrimaryExpression final : public Expression { std::string toString() const override; - VariantType eval() const override; + OptVariantType eval() const override; Status MUST_USE_RESULT prepare() override; @@ -594,7 +595,7 @@ class FunctionCallExpression final : public Expression { std::string toString() const override; - VariantType eval() const override; + OptVariantType eval() const override; Status MUST_USE_RESULT prepare() override; @@ -637,7 +638,7 @@ class UnaryExpression final : public Expression { std::string toString() const override; - VariantType eval() const override; + OptVariantType eval() const override; Status MUST_USE_RESULT prepare() override; @@ -676,7 +677,7 @@ class TypeCastingExpression final : public Expression { std::string toString() const override; - VariantType eval() const override; + OptVariantType eval() const override; Status MUST_USE_RESULT prepare() override; @@ -725,7 +726,7 @@ class ArithmeticExpression final : public Expression { std::string toString() const override; - VariantType eval() const override; + OptVariantType eval() const override; Status MUST_USE_RESULT prepare() override; @@ -776,7 +777,7 @@ class RelationalExpression final : public Expression { std::string toString() const override; - VariantType eval() const override; + OptVariantType eval() const override; Status MUST_USE_RESULT prepare() override; @@ -828,7 +829,7 @@ class LogicalExpression final : public Expression { std::string toString() const override; - VariantType eval() const override; + OptVariantType eval() const override; Status MUST_USE_RESULT prepare() override; diff --git a/src/common/filter/test/ExpressionTest.cpp b/src/common/filter/test/ExpressionTest.cpp index f44ce0acded..f00cf72af78 100644 --- a/src/common/filter/test/ExpressionTest.cpp +++ b/src/common/filter/test/ExpressionTest.cpp @@ -46,20 +46,24 @@ TEST_F(ExpressionTest, LiteralConstants) { auto *expr = getFilterExpr(parsed.value().get()); \ ASSERT_NE(nullptr, expr); \ auto value = expr->eval(); \ - ASSERT_TRUE(Expression::is##type(value)); \ + ASSERT_TRUE(value.ok()); \ + auto v = value.value(); \ + ASSERT_TRUE(Expression::is##type(v)); \ if (#type == std::string("Double")) { \ - ASSERT_DOUBLE_EQ((expr_arg), Expression::as##type(value)); \ + ASSERT_DOUBLE_EQ((expr_arg), Expression::as##type(v)); \ } else { \ - ASSERT_EQ((expr_arg), Expression::as##type(value)); \ + ASSERT_EQ((expr_arg), Expression::as##type(v)); \ } \ auto decoded = Expression::decode(Expression::encode(expr)); \ ASSERT_TRUE(decoded.ok()) << decoded.status(); \ value = decoded.value()->eval(); \ - ASSERT_TRUE(Expression::is##type(value)); \ + ASSERT_TRUE(value.ok()); \ + v = value.value(); \ + ASSERT_TRUE(Expression::is##type(v)); \ if (#type == std::string("Double")) { \ - ASSERT_DOUBLE_EQ((expr_arg), Expression::as##type(value)); \ + ASSERT_DOUBLE_EQ((expr_arg), Expression::as##type(v)); \ } else { \ - ASSERT_EQ((expr_arg), Expression::as##type(value)); \ + ASSERT_EQ((expr_arg), Expression::as##type(v)); \ } \ } while (false) @@ -85,8 +89,10 @@ TEST_F(ExpressionTest, LiteralConstants) { auto *expr = getFilterExpr(parsed.value().get()); ASSERT_NE(nullptr, expr); auto value = expr->eval(); - ASSERT_TRUE(Expression::isString(value)); - ASSERT_EQ("string_literal", Expression::asString(value)); + ASSERT_TRUE(value.ok()); + auto v = value.value(); + ASSERT_TRUE(Expression::isString(v)); + ASSERT_EQ("string_literal", Expression::asString(v)); auto buffer = Expression::encode(expr); ASSERT_FALSE(buffer.empty()); @@ -94,8 +100,10 @@ TEST_F(ExpressionTest, LiteralConstants) { ASSERT_TRUE(decoded.ok()) << decoded.status(); ASSERT_NE(nullptr, decoded.value()); value = decoded.value()->eval(); - ASSERT_TRUE(Expression::isString(value)); - ASSERT_EQ("string_literal", Expression::asString(value)); + ASSERT_TRUE(value.ok()); + v = value.value(); + ASSERT_TRUE(Expression::isString(v)); + ASSERT_EQ("string_literal", Expression::asString(v)); } } @@ -110,24 +118,28 @@ TEST_F(ExpressionTest, LiteralContantsArithmetic) { auto *expr = getFilterExpr(parsed.value().get()); \ ASSERT_NE(nullptr, expr); \ auto value = expr->eval(); \ - ASSERT_TRUE(Expression::is##type(value)); \ + ASSERT_TRUE(value.ok()); \ + auto v = value.value(); \ + ASSERT_TRUE(Expression::is##type(v)); \ if (#type == std::string("Double")) { \ - ASSERT_DOUBLE_EQ((expr_arg), Expression::as##type(value)); \ - ASSERT_DOUBLE_EQ((expected), Expression::as##type(value)); \ + ASSERT_DOUBLE_EQ((expr_arg), Expression::as##type(v)); \ + ASSERT_DOUBLE_EQ((expected), Expression::as##type(v)); \ } else { \ - ASSERT_EQ((expr_arg), Expression::as##type(value)); \ - ASSERT_EQ((expected), Expression::as##type(value)); \ + ASSERT_EQ((expr_arg), Expression::as##type(v)); \ + ASSERT_EQ((expected), Expression::as##type(v)); \ } \ auto decoded = Expression::decode(Expression::encode(expr)); \ ASSERT_TRUE(decoded.ok()) << decoded.status(); \ value = decoded.value()->eval(); \ - ASSERT_TRUE(Expression::is##type(value)); \ + ASSERT_TRUE(value.ok()); \ + v = value.value(); \ + ASSERT_TRUE(Expression::is##type(v)); \ if (#type == std::string("Double")) { \ - ASSERT_DOUBLE_EQ((expr_arg), Expression::as##type(value)); \ - ASSERT_DOUBLE_EQ((expected), Expression::as##type(value)); \ + ASSERT_DOUBLE_EQ((expr_arg), Expression::as##type(v)); \ + ASSERT_DOUBLE_EQ((expected), Expression::as##type(v)); \ } else { \ - ASSERT_EQ((expr_arg), Expression::as##type(value)); \ - ASSERT_EQ((expected), Expression::as##type(value)); \ + ASSERT_EQ((expr_arg), Expression::as##type(v)); \ + ASSERT_EQ((expected), Expression::as##type(v)); \ } \ } while (false) @@ -195,8 +207,10 @@ TEST_F(ExpressionTest, LiteralContantsArithmetic) { auto *expr = getFilterExpr(parsed.value().get()); ASSERT_NE(nullptr, expr); auto value = expr->eval(); - ASSERT_TRUE(Expression::isInt(value)); - ASSERT_EQ(16, Expression::asInt(value)); + ASSERT_TRUE(value.ok()); + auto v = value.value(); + ASSERT_TRUE(Expression::isInt(v)); + ASSERT_EQ(16, Expression::asInt(v)); auto buffer = Expression::encode(expr); ASSERT_FALSE(buffer.empty()); @@ -204,8 +218,10 @@ TEST_F(ExpressionTest, LiteralContantsArithmetic) { ASSERT_TRUE(decoded.ok()) << decoded.status(); ASSERT_NE(nullptr, decoded.value()); value = decoded.value()->eval(); - ASSERT_TRUE(Expression::isInt(value)); - ASSERT_EQ(16, Expression::asInt(value)); + ASSERT_TRUE(value.ok()); + v = value.value(); + ASSERT_TRUE(Expression::isInt(v)); + ASSERT_EQ(16, Expression::asInt(v)); } } @@ -220,15 +236,19 @@ TEST_F(ExpressionTest, LiteralConstantsRelational) { auto *expr = getFilterExpr(parsed.value().get()); \ ASSERT_NE(nullptr, expr); \ auto value = expr->eval(); \ - ASSERT_TRUE(Expression::isBool(value)); \ - ASSERT_EQ((expr_arg), Expression::asBool(value)); \ - ASSERT_EQ((expected), Expression::asBool(value)); \ + ASSERT_TRUE(value.ok()); \ + auto v = value.value(); \ + ASSERT_TRUE(Expression::isBool(v)); \ + ASSERT_EQ((expr_arg), Expression::asBool(v)); \ + ASSERT_EQ((expected), Expression::asBool(v)); \ auto decoded = Expression::decode(Expression::encode(expr)); \ ASSERT_TRUE(decoded.ok()) << decoded.status(); \ value = decoded.value()->eval(); \ - ASSERT_TRUE(Expression::isBool(value)); \ - ASSERT_EQ((expr_arg), Expression::asBool(value)); \ - ASSERT_EQ((expected), Expression::asBool(value)); \ + ASSERT_TRUE(value.ok()); \ + v = value.value(); \ + ASSERT_TRUE(Expression::isBool(v)); \ + ASSERT_EQ((expr_arg), Expression::asBool(v)); \ + ASSERT_EQ((expected), Expression::asBool(v)); \ } while (false) TEST_EXPR(!-1, false); @@ -292,13 +312,17 @@ TEST_F(ExpressionTest, LiteralConstantsRelational) { auto *expr = getFilterExpr(parsed.value().get()); ASSERT_NE(nullptr, expr); auto value = expr->eval(); - ASSERT_TRUE(Expression::isBool(value)); - ASSERT_TRUE(Expression::asBool(value)); + ASSERT_TRUE(value.ok()); + auto v = value.value(); + ASSERT_TRUE(Expression::isBool(v)); + ASSERT_TRUE(Expression::asBool(v)); auto decoded = Expression::decode(Expression::encode(expr)); ASSERT_TRUE(decoded.ok()) << decoded.status(); value = decoded.value()->eval(); - ASSERT_TRUE(Expression::isBool(value)); - ASSERT_TRUE(Expression::asBool(value)); + ASSERT_TRUE(value.ok()); + v = value.value(); + ASSERT_TRUE(Expression::isBool(v)); + ASSERT_TRUE(Expression::asBool(v)); } { std::string query = "GO FROM 1 OVER follow WHERE 3.14 * 3 * 3 / 2 != 3.14 * 1.5 * 1.5 / 2"; @@ -307,13 +331,17 @@ TEST_F(ExpressionTest, LiteralConstantsRelational) { auto *expr = getFilterExpr(parsed.value().get()); ASSERT_NE(nullptr, expr); auto value = expr->eval(); - ASSERT_TRUE(Expression::isBool(value)); - ASSERT_TRUE(Expression::asBool(value)); + ASSERT_TRUE(value.ok()); + auto v = value.value(); + ASSERT_TRUE(Expression::isBool(v)); + ASSERT_TRUE(Expression::asBool(v)); auto decoded = Expression::decode(Expression::encode(expr)); ASSERT_TRUE(decoded.ok()) << decoded.status(); value = decoded.value()->eval(); - ASSERT_TRUE(Expression::isBool(value)); - ASSERT_TRUE(Expression::asBool(value)); + ASSERT_TRUE(value.ok()); + v = value.value(); + ASSERT_TRUE(Expression::isBool(v)); + ASSERT_TRUE(Expression::asBool(v)); } #undef TEST_EXPR @@ -330,15 +358,19 @@ TEST_F(ExpressionTest, LiteralConstantsLogical) { auto *expr = getFilterExpr(parsed.value().get()); \ ASSERT_NE(nullptr, expr); \ auto value = expr->eval(); \ - ASSERT_TRUE(Expression::isBool(value)); \ - ASSERT_EQ((expr_arg), Expression::asBool(value)); \ - ASSERT_EQ((expected), Expression::asBool(value)); \ + ASSERT_TRUE(value.ok()); \ + auto v = value.value(); \ + ASSERT_TRUE(Expression::isBool(v)); \ + ASSERT_EQ((expr_arg), Expression::asBool(v)); \ + ASSERT_EQ((expected), Expression::asBool(v)); \ auto decoded = Expression::decode(Expression::encode(expr)); \ ASSERT_TRUE(decoded.ok()) << decoded.status(); \ value = decoded.value()->eval(); \ - ASSERT_TRUE(Expression::isBool(value)); \ - ASSERT_EQ((expr_arg), Expression::asBool(value)); \ - ASSERT_EQ((expected), Expression::asBool(value)); \ + ASSERT_TRUE(value.ok()); \ + v = value.value(); \ + ASSERT_TRUE(Expression::isBool(v)); \ + ASSERT_EQ((expr_arg), Expression::asBool(v)); \ + ASSERT_EQ((expected), Expression::asBool(v)); \ } while (false) // AND @@ -420,8 +452,10 @@ TEST_F(ExpressionTest, InputReference) { }; expr->setContext(ctx.get()); auto value = expr->eval(); - ASSERT_TRUE(Expression::isString(value)); - ASSERT_EQ("Freddie", Expression::asString(value)); + ASSERT_TRUE(value.ok()); + auto v = value.value(); + ASSERT_TRUE(Expression::isString(v)); + ASSERT_EQ("Freddie", Expression::asString(v)); } { std::string query = "GO FROM 1 OVER follow WHERE $-.age >= 18"; @@ -439,8 +473,10 @@ TEST_F(ExpressionTest, InputReference) { }; expr->setContext(ctx.get()); auto value = expr->eval(); - ASSERT_TRUE(Expression::isBool(value)); - ASSERT_TRUE(Expression::asBool(value)); + ASSERT_TRUE(value.ok()); + auto v = value.value(); + ASSERT_TRUE(Expression::isBool(v)); + ASSERT_TRUE(Expression::asBool(v)); } } @@ -462,8 +498,10 @@ TEST_F(ExpressionTest, SourceTagReference) { }; expr->setContext(ctx.get()); auto value = expr->eval(); - ASSERT_TRUE(Expression::isBool(value)); - ASSERT_TRUE(Expression::asBool(value)); + ASSERT_TRUE(value.ok()); + auto v = value.value(); + ASSERT_TRUE(Expression::isBool(v)); + ASSERT_TRUE(Expression::asBool(v)); } } @@ -493,8 +531,10 @@ TEST_F(ExpressionTest, EdgeReference) { }; expr->setContext(ctx.get()); auto value = expr->eval(); - ASSERT_TRUE(Expression::isBool(value)); - ASSERT_FALSE(Expression::asBool(value)); + ASSERT_TRUE(value.ok()); + auto v = value.value(); + ASSERT_TRUE(Expression::isBool(v)); + ASSERT_FALSE(Expression::asBool(v)); } } @@ -515,15 +555,17 @@ TEST_F(ExpressionTest, FunctionCall) { auto status = decoded.value()->prepare(); \ ASSERT_TRUE(status.ok()) << status; \ auto value = decoded.value()->eval(); \ - ASSERT_TRUE(Expression::is##type(value)); \ + ASSERT_TRUE(value.ok()); \ + auto v = value.value(); \ + ASSERT_TRUE(Expression::is##type(v)); \ if (#type == std::string("Double")) { \ if (#op != std::string("EQ")) { \ - ASSERT_##op(expected, Expression::as##type(value)); \ + ASSERT_##op(expected, Expression::as##type(v)); \ } else { \ - ASSERT_DOUBLE_EQ(expected, Expression::as##type(value));\ + ASSERT_DOUBLE_EQ(expected, Expression::as##type(v)); \ } \ } else { \ - ASSERT_##op(expected, Expression::as##type(value)); \ + ASSERT_##op(expected, Expression::as##type(v)); \ } \ } while (false) @@ -576,4 +618,40 @@ TEST_F(ExpressionTest, FunctionCall) { #undef TEST_EXPR } +TEST_F(ExpressionTest, InvalidExpressionTest) { + GQLParser parser; + +#define TEST_EXPR(expr_arg) \ + do { \ + std::string query = "GO FROM 1 OVER follow WHERE " #expr_arg; \ + auto parsed = parser.parse(query); \ + ASSERT_TRUE(parsed.ok()) << parsed.status(); \ + auto *expr = getFilterExpr(parsed.value().get()); \ + ASSERT_NE(nullptr, expr); \ + auto decoded = Expression::decode(Expression::encode(expr)); \ + ASSERT_TRUE(decoded.ok()) << decoded.status(); \ + auto ctx = std::make_unique(); \ + decoded.value()->setContext(ctx.get()); \ + auto status = decoded.value()->prepare(); \ + ASSERT_TRUE(status.ok()) << status; \ + auto value = decoded.value()->eval(); \ + ASSERT_TRUE(!value.ok()); \ + } while (false) + + TEST_EXPR("a" + 1); + TEST_EXPR(3.14 + "a"); + TEST_EXPR("ab" - "c"); + TEST_EXPR(1 - "a"); + TEST_EXPR("a" * 1); + TEST_EXPR(1.0 * "a"); + TEST_EXPR(1 / "a"); + TEST_EXPR("a" / "b"); + TEST_EXPR(1.0 % 2.0); + TEST_EXPR(1.0 % 3); + TEST_EXPR(1.0 % "a"); + TEST_EXPR(-"A"); + TEST_EXPR(TRUE + FALSE); +#undef TEST_EXPR +} + } // namespace nebula diff --git a/src/graph/ConfigExecutor.cpp b/src/graph/ConfigExecutor.cpp index 2c62b07006d..4705b8ae977 100644 --- a/src/graph/ConfigExecutor.cpp +++ b/src/graph/ConfigExecutor.cpp @@ -101,7 +101,13 @@ void ConfigExecutor::setVariables() { name = *configItem_->getName(); } if (configItem_->getValue() != nullptr) { - value = configItem_->getValue()->eval(); + auto v = configItem_->getValue()->eval(); + if (!v.ok()) { + DCHECK(onError_); + onError_(v.status()); + return; + } + value = v.value(); } } diff --git a/src/graph/FetchEdgesExecutor.cpp b/src/graph/FetchEdgesExecutor.cpp index 1517aa70ab9..4876412ffdc 100644 --- a/src/graph/FetchEdgesExecutor.cpp +++ b/src/graph/FetchEdgesExecutor.cpp @@ -185,8 +185,16 @@ Status FetchEdgesExecutor::setupEdgeKeysFromExpr() { if (!status.ok()) { break; } - auto srcid = srcExpr->eval(); - auto dstid = dstExpr->eval(); + auto value = srcExpr->eval(); + if (!value.ok()) { + return value.status(); + } + auto srcid = value.value(); + value = dstExpr->eval(); + if (!value.ok()) { + return value.status(); + } + auto dstid = value.value(); if (!Expression::isInt(srcid) || !Expression::isInt(dstid)) { status = Status::Error("ID should be of type integer."); break; @@ -281,13 +289,18 @@ void FetchEdgesExecutor::processResult(RpcResponse &&result) { auto writer = std::make_unique(outputSchema); auto &getters = expCtx_->getters(); - getters.getAliasProp = [&] (const std::string&, const std::string &prop) { + getters.getAliasProp = [&](const std::string &, + const std::string &prop) -> OptVariantType { return collector->getProp(prop, &*iter); }; for (auto *column : yields_) { auto *expr = column->expr(); auto value = expr->eval(); - collector->collect(value, writer.get()); + if (!value.ok()) { + onError_(value.status()); + return; + } + collector->collect(value.value(), writer.get()); } // TODO Consider float/double, and need to reduce mem copy. diff --git a/src/graph/FetchExecutor.cpp b/src/graph/FetchExecutor.cpp index d61dfb86161..15202e3fef4 100644 --- a/src/graph/FetchExecutor.cpp +++ b/src/graph/FetchExecutor.cpp @@ -100,7 +100,11 @@ void FetchExecutor::getOutputSchema( for (auto *column : yields_) { auto *expr = column->expr(); auto value = expr->eval(); - record.emplace_back(std::move(value)); + if (!value.ok()) { + onError_(value.status()); + return; + } + record.emplace_back(std::move(value.value())); } using nebula::cpp2::SupportedType; diff --git a/src/graph/FetchVerticesExecutor.cpp b/src/graph/FetchVerticesExecutor.cpp index 4af6949169a..e699b0906cc 100644 --- a/src/graph/FetchVerticesExecutor.cpp +++ b/src/graph/FetchVerticesExecutor.cpp @@ -154,13 +154,18 @@ void FetchVerticesExecutor::processResult(RpcResponse &&result) { auto writer = std::make_unique(outputSchema); auto &getters = expCtx_->getters(); - getters.getAliasProp = [&] (const std::string&, const std::string &prop) { + getters.getAliasProp = [&](const std::string &, + const std::string &prop) -> OptVariantType { return collector->getProp(prop, vreader.get()); }; for (auto *column : yields_) { auto *expr = column->expr(); auto value = expr->eval(); - collector->collect(value, writer.get()); + if (!value.ok()) { + onError_(value.status()); + return; + } + collector->collect(value.value(), writer.get()); } // TODO Consider float/double, and need to reduce mem copy. std::string encode = writer->encode(); @@ -202,12 +207,16 @@ Status FetchVerticesExecutor::setupVidsFromExpr() { break; } auto value = expr->eval(); - if (!Expression::isInt(value)) { + if (!value.ok()) { + return value.status(); + } + auto v = value.value(); + if (!Expression::isInt(v)) { status = Status::Error("Vertex ID should be of type integer"); break; } - auto valInt = Expression::asInt(value); + auto valInt = Expression::asInt(v); if (distinct_) { auto result = uniqID->emplace(valInt); if (result.second) { diff --git a/src/graph/GoExecutor.cpp b/src/graph/GoExecutor.cpp index 21f1fc88722..6d744677578 100644 --- a/src/graph/GoExecutor.cpp +++ b/src/graph/GoExecutor.cpp @@ -10,6 +10,7 @@ #include "dataman/RowSetReader.h" #include "dataman/ResultSchemaProvider.h" + namespace nebula { namespace graph { @@ -162,11 +163,16 @@ Status GoExecutor::prepareFrom() { break; } auto value = expr->eval(); - if (!Expression::isInt(value)) { + if (!value.ok()) { + status = Status::Error(); + break; + } + auto v = value.value(); + if (!Expression::isInt(v)) { status = Status::Error("Vertex ID should be of type integer"); break; } - starts_.push_back(Expression::asInt(value)); + starts_.push_back(Expression::asInt(v)); } fromType_ = kInstantExpr; if (!status.ok()) { @@ -424,14 +430,18 @@ std::vector GoExecutor::getDstIdsFromResp(RpcResponse &rpcResp) const return std::vector(set.begin(), set.end()); } - void GoExecutor::finishExecution(RpcResponse &&rpcResp) { - auto outputs = setupInterimResult(std::move(rpcResp)); + std::unique_ptr outputs; + + if (!setupInterimResult(std::move(rpcResp), outputs)) { + return; + } if (onResult_) { onResult_(std::move(outputs)); } else { resp_ = std::make_unique(); resp_->set_column_names(getResultColumnNames()); + if (outputs != nullptr) { auto rows = outputs->getRows(); resp_->set_rows(std::move(rows)); @@ -441,7 +451,6 @@ void GoExecutor::finishExecution(RpcResponse &&rpcResp) { onFinish_(); } - StatusOr> GoExecutor::getStepOutProps() { std::vector props; { @@ -573,8 +582,7 @@ std::vector GoExecutor::getResultColumnNames() const { return result; } - -std::unique_ptr GoExecutor::setupInterimResult(RpcResponse &&rpcResp) { +bool GoExecutor::setupInterimResult(RpcResponse &&rpcResp, std::unique_ptr &result) { // Generic results std::shared_ptr schema; std::unique_ptr rsWriter; @@ -603,9 +611,9 @@ std::unique_ptr GoExecutor::setupInterimResult(RpcResponse &&rpcR LOG(FATAL) << "Unknown VariantType: " << record[i].which(); } schema->appendCol(colnames[i], type); - } // for + } // for rsWriter = std::make_unique(schema); - } // if + } // if RowWriter writer(schema); for (auto &column : record) { @@ -637,12 +645,14 @@ std::unique_ptr GoExecutor::setupInterimResult(RpcResponse &&rpcR rsWriter->addRow(std::move(encode)); } }; // cb - processFinalResult(rpcResp, cb); + if (!processFinalResult(rpcResp, cb)) { + return false; + } // No results populated - if (rsWriter == nullptr) { - return nullptr; + if (rsWriter != nullptr) { + result = std::make_unique(std::move(rsWriter)); } - return std::make_unique(std::move(rsWriter)); + return true; } @@ -656,7 +666,7 @@ void GoExecutor::onEmptyInputs() { } -void GoExecutor::processFinalResult(RpcResponse &rpcResp, Callback cb) const { +bool GoExecutor::processFinalResult(RpcResponse &rpcResp, Callback cb) const { auto all = rpcResp.responses(); for (auto &resp : all) { if (resp.get_vertices() == nullptr) { @@ -683,35 +693,48 @@ void GoExecutor::processFinalResult(RpcResponse &rpcResp, Callback cb) const { auto iter = rsReader.begin(); while (iter) { auto &getters = expCtx_->getters(); - getters.getAliasProp = - [&] (const std::string&, const std::string &prop) -> VariantType { + getters.getAliasProp = [&](const std::string &, + const std::string &prop) -> OptVariantType { auto res = RowReader::getPropByName(&*iter, prop); - CHECK(ok(res)); - return value(std::move(res)); + if (ok(res)) { + return value(res); + } + return Status::Error("get edge prop failed"); }; - getters.getSrcTagProp = [&] (const std::string &tagName, const std::string &prop) { + getters.getSrcTagProp = [&](const std::string &tagName, + const std::string &prop) -> OptVariantType { auto tagIter = this->srcTagProps_.find(std::make_pair(tagName, prop)); if (tagIter == this->srcTagProps_.end()) { - LOG(ERROR) << "Src tagName : " << tagName - << ", propName : " << prop << " is not exist"; + auto msg = folly::sformat( + "Src tagName : {} , propName : {} is not exist", tagName, prop); + LOG(ERROR) << msg; + return Status::Error(msg); } auto index = tagIter->second; - const nebula::cpp2::ValueType& type = vschema->getFieldType(index); + const nebula::cpp2::ValueType &type = vschema->getFieldType(index); if (type == CommonConstants::kInvalidValueType()) { - LOG(ERROR) << "Tag : " << tagName <<" no schema for the index " << index; + auto msg = + folly::sformat("Tag: {} no schema for the index {}", tagName, index); + LOG(ERROR) << msg; + return Status::Error(msg); } auto res = RowReader::getPropByIndex(vreader.get(), index); - CHECK(ok(res)); - return value(std::move(res)); + if (ok(res)) { + return value(std::move(res)); + } + return Status::Error(folly::sformat("{}.{} was not exist", tagName, prop)); }; - getters.getDstTagProp = [&] (const std::string &tagName, const std::string &prop) { + getters.getDstTagProp = [&](const std::string &tagName, + const std::string &prop) -> OptVariantType { auto res = RowReader::getPropByName(&*iter, "_dst"); CHECK(ok(res)); auto dst = value(std::move(res)); auto tagIter = this->dstTagProps_.find(std::make_pair(tagName, prop)); if (tagIter == this->dstTagProps_.end()) { - LOG(ERROR) << "Dst tagName : " << tagName - << ", propName : " << prop << " is not exist"; + auto msg = folly::sformat( + "Src tagName : {} , propName : {} is not exist", tagName, prop); + LOG(ERROR) << msg; + return Status::Error(msg); } auto index = tagIter->second; return vertexHolder_->get(boost::get(dst), index); @@ -725,7 +748,11 @@ void GoExecutor::processFinalResult(RpcResponse &rpcResp, Callback cb) const { // Evaluate filter if (filter_ != nullptr) { auto value = filter_->eval(); - if (!Expression::asBool(value)) { + if (!value.ok()) { + onError_(value.status()); + return false; + } + if (!Expression::asBool(value.value())) { ++iter; continue; } @@ -734,22 +761,27 @@ void GoExecutor::processFinalResult(RpcResponse &rpcResp, Callback cb) const { record.reserve(yields_.size()); for (auto *column : yields_) { auto *expr = column->expr(); - // TODO(dutor) `eval' may fail auto value = expr->eval(); - record.emplace_back(std::move(value)); + if (!value.ok()) { + onError_(value.status()); + return false; + } + record.emplace_back(std::move(value.value())); } cb(std::move(record)); ++iter; } // while `iter' } // for `vdata' } // for `resp' + return true; } -VariantType GoExecutor::VertexHolder::get(VertexID id, int64_t index) const { +OptVariantType GoExecutor::VertexHolder::get(VertexID id, int64_t index) const { + DCHECK(schema_ != nullptr); auto iter = data_.find(id); - // TODO(dutor) We need a type to represent NULL or non-existing prop + // TODO(dutor) We need a type to represent NULL CHECK(iter != data_.end()); auto reader = RowReader::getRowReader(iter->second, schema_); @@ -765,16 +797,24 @@ void GoExecutor::VertexHolder::add(const storage::cpp2::QueryResponse &resp) { if (vertices == nullptr) { return; } + + if (vertices->empty()) { + return; + } + if (resp.get_vertex_schema() == nullptr) { return; } + if (schema_ == nullptr) { schema_ = std::make_shared(resp.vertex_schema); } for (auto &vdata : *vertices) { DCHECK(vdata.__isset.vertex_data); - data_[vdata.vertex_id] = vdata.vertex_data; + if (!vdata.vertex_data.empty()) { + data_[vdata.vertex_id] = vdata.vertex_data; + } } } diff --git a/src/graph/GoExecutor.h b/src/graph/GoExecutor.h index 308d8001478..81ca88a9f82 100644 --- a/src/graph/GoExecutor.h +++ b/src/graph/GoExecutor.h @@ -131,7 +131,7 @@ class GoExecutor final : public TraverseExecutor { * To setup an intermediate representation of the execution result, * which is about to be piped to the next executor. */ - std::unique_ptr setupInterimResult(RpcResponse &&rpcResp); + bool setupInterimResult(RpcResponse &&rpcResp, std::unique_ptr &result); /** * To setup the header of the execution result, i.e. the column names. @@ -141,14 +141,14 @@ class GoExecutor final : public TraverseExecutor { /** * To setup the body of the execution result. */ - void setupResponseBody(RpcResponse &rpcResp, cpp2::ExecutionResponse &resp) const; + bool setupResponseBody(RpcResponse &rpcResp, cpp2::ExecutionResponse &resp) const; /** * To iterate on the final data collection, and evaluate the filter and yield columns. * For each row that matches the filter, `cb' would be invoked. */ using Callback = std::function)>; - void processFinalResult(RpcResponse &rpcResp, Callback cb) const; + bool processFinalResult(RpcResponse &rpcResp, Callback cb) const; /** * A container to hold the mapping from vertex id to its properties, used for lookups @@ -156,7 +156,7 @@ class GoExecutor final : public TraverseExecutor { */ class VertexHolder final { public: - VariantType get(VertexID id, int64_t index) const; + OptVariantType get(VertexID id, int64_t index) const; void add(const storage::cpp2::QueryResponse &resp); const auto* schema() const { return schema_.get(); diff --git a/src/graph/InsertEdgeExecutor.cpp b/src/graph/InsertEdgeExecutor.cpp index 16593eca964..9f2ade6c584 100644 --- a/src/graph/InsertEdgeExecutor.cpp +++ b/src/graph/InsertEdgeExecutor.cpp @@ -72,20 +72,33 @@ StatusOr> InsertEdgeExecutor::prepareEdges() { auto index = 0; for (auto i = 0u; i < rows_.size(); i++) { auto *row = rows_[i]; - auto status = row->srcid()->prepare(); + auto sid = row->srcid(); + auto status = sid->prepare(); if (!status.ok()) { return status; } - auto v = row->srcid()->eval(); + auto ovalue = sid->eval(); + if (!ovalue.ok()) { + return ovalue.status(); + } + + auto v = ovalue.value(); if (!Expression::isInt(v)) { return Status::Error("Vertex ID should be of type integer"); } auto src = Expression::asInt(v); - status = row->dstid()->prepare(); + + auto did = row->dstid(); + status = did->prepare(); if (!status.ok()) { return status; } - v = row->dstid()->eval(); + ovalue = did->eval(); + if (!ovalue.ok()) { + return ovalue.status(); + } + + v = ovalue.value(); if (!Expression::isInt(v)) { return Status::Error("Vertex ID should be of type integer"); } @@ -105,7 +118,11 @@ StatusOr> InsertEdgeExecutor::prepareEdges() { std::vector values; values.reserve(expressions.size()); for (auto *expr : expressions) { - values.emplace_back(expr->eval()); + ovalue = expr->eval(); + if (!ovalue.ok()) { + return ovalue.status(); + } + values.emplace_back(ovalue.value()); } RowWriter writer(schema_); diff --git a/src/graph/InsertVertexExecutor.cpp b/src/graph/InsertVertexExecutor.cpp index 26b1a788efe..a4da3ab8204 100644 --- a/src/graph/InsertVertexExecutor.cpp +++ b/src/graph/InsertVertexExecutor.cpp @@ -80,11 +80,17 @@ StatusOr> InsertVertexExecutor::prepareVertic std::vector vertices(rows_.size()); for (auto i = 0u; i < rows_.size(); i++) { auto *row = rows_[i]; - auto status = row->id()->prepare(); + auto rid = row->id(); + auto status = rid->prepare(); if (!status.ok()) { return status; } - auto v = row->id()->eval(); + auto ovalue = rid->eval(); + if (!ovalue.ok()) { + return ovalue.status(); + } + + auto v = ovalue.value(); if (!Expression::isInt(v)) { return Status::Error("Vertex ID should be of type integer"); } @@ -94,7 +100,11 @@ StatusOr> InsertVertexExecutor::prepareVertic std::vector values; values.reserve(expressions.size()); for (auto *expr : expressions) { - values.emplace_back(expr->eval()); + ovalue = expr->eval(); + if (!ovalue.ok()) { + return ovalue.status(); + } + values.emplace_back(ovalue.value()); } storage::cpp2::Vertex vertex; diff --git a/src/graph/YieldExecutor.cpp b/src/graph/YieldExecutor.cpp index f48d4323555..4679134b8ee 100644 --- a/src/graph/YieldExecutor.cpp +++ b/src/graph/YieldExecutor.cpp @@ -37,7 +37,13 @@ void YieldExecutor::execute() { values.reserve(size); for (auto *col : yields_) { - values.emplace_back(col->expr()->eval()); + auto expr = col->expr(); + auto v = expr->eval(); + if (!v.ok()) { + onError_(v.status()); + return; + } + values.emplace_back(v.value()); } std::vector rows; diff --git a/src/graph/test/TestEnv.cpp b/src/graph/test/TestEnv.cpp index 0dcf84d0bcb..140aef4a978 100644 --- a/src/graph/test/TestEnv.cpp +++ b/src/graph/test/TestEnv.cpp @@ -81,6 +81,7 @@ void TestEnv::TearDown() { sleep(FLAGS_load_data_interval_secs + 1); graphServer_.reset(); storageServer_.reset(); + mClient_.reset(); metaServer_.reset(); mClient_.reset(); } diff --git a/src/parser/AdminSentences.cpp b/src/parser/AdminSentences.cpp index 70f86d02725..157116a9713 100644 --- a/src/parser/AdminSentences.cpp +++ b/src/parser/AdminSentences.cpp @@ -113,7 +113,12 @@ std::string ConfigRowItem::toString() const { ss << *name_; } if (value_ != nullptr) { - ss << "=" << value_->eval(); + auto v = value_->eval(); + if (!v.ok()) { + ss << "= "; + } else { + ss << "=" << v.value(); + } } return ss.str(); } diff --git a/src/storage/QueryBaseProcessor.inl b/src/storage/QueryBaseProcessor.inl index cd167d0e2a7..0601b5ae47b 100644 --- a/src/storage/QueryBaseProcessor.inl +++ b/src/storage/QueryBaseProcessor.inl @@ -368,24 +368,28 @@ kvstore::ResultCode QueryBaseProcessor::collectEdgeProps( std::lock_guard lg(this->lock_); auto& getters = expCtx_->getters(); getters.getAliasProp = - [&] (const std::string&, const std::string &prop) -> VariantType { + [&] (const std::string&, const std::string &prop) -> OptVariantType { auto res = RowReader::getPropByName(reader.get(), prop); - CHECK(ok(res)); + if (!ok(res)) { + return Status::Error("Invalid Prop"); + } return value(std::move(res)); }; getters.getEdgeRank = [&] () -> VariantType { return rank; }; getters.getSrcTagProp = [&, this] (const std::string& tag, - const std::string& prop) -> VariantType { + const std::string& prop) -> OptVariantType { auto it = fcontext->tagFilters_.find(std::make_pair(tag, prop)); - CHECK(it != fcontext->tagFilters_.end()); + if (it == fcontext->tagFilters_.end()) { + return Status::Error("Invalid Tag Filter"); + } VLOG(1) << "Hit srcProp filter for tag " << tag << ", prop " << prop << ", value " << it->second; return it->second; }; auto value = exp_->eval(); - if (!Expression::asBool(value)) { + if (value.ok() && !Expression::asBool(value.value())) { VLOG(1) << "Filter the edge " << vId << "-> " << dstId << "@" << rank << ":" << edgeType; continue;