diff --git a/src/graph/LookupExecutor.cpp b/src/graph/LookupExecutor.cpp index e460d569a0d..8ca84674ae6 100644 --- a/src/graph/LookupExecutor.cpp +++ b/src/graph/LookupExecutor.cpp @@ -171,29 +171,43 @@ Status LookupExecutor::optimize() { return status; } -Status LookupExecutor::traversalExpr(const Expression *expr) { +Status LookupExecutor::traversalExpr(const Expression *expr, const meta::SchemaProviderIf* schema) { switch (expr->kind()) { case nebula::Expression::kLogical : { + Status ret = Status::OK(); auto* lExpr = dynamic_cast(expr); - if (lExpr->op() == LogicalExpression::Operator::XOR) { - return Status::SyntaxError("Syntax error : %s", lExpr->toString().c_str()); + if (lExpr->op() == LogicalExpression::Operator::XOR || + lExpr->op() == LogicalExpression::Operator::OR) { + return Status::SyntaxError("OR and XOR are not supported " + "in lookup where clause :%s", + lExpr->toString().c_str()); } auto* left = lExpr->left(); - traversalExpr(left); + ret = traversalExpr(left, schema); + if (!ret.ok()) { + return ret; + } auto* right = lExpr->right(); - traversalExpr(right); + ret = traversalExpr(right, schema); + if (!ret.ok()) { + return ret; + } break; } case nebula::Expression::kRelational : { std::string prop; VariantType v; auto* rExpr = dynamic_cast(expr); + auto ret = relationalExprCheck(rExpr->op()); + if (!ret.ok()) { + return ret; + } auto* left = rExpr->left(); auto* right = rExpr->right(); - /** - * TODO (sky) : Does not support left expr and right expr are both kAliasProp. - */ - if (left->kind() == nebula::Expression::kAliasProp) { + if (left->kind() == nebula::Expression::kAliasProp && + right->kind() == nebula::Expression::kAliasProp) { + return Status::SyntaxError("Does not support left and right are both property"); + } else if (left->kind() == nebula::Expression::kAliasProp) { auto* aExpr = dynamic_cast(left); auto st = checkAliasProperty(aExpr); if (!st.ok()) { @@ -213,15 +227,19 @@ Status LookupExecutor::traversalExpr(const Expression *expr) { return Status::SyntaxError("Unsupported expression :%s", rExpr->toString().c_str()); } + + if (rExpr->op() != RelationalExpression::Operator::EQ && + rExpr->op() != RelationalExpression::Operator::NE) { + auto type = schema->getFieldType(prop).type; + if (!supportedDataTypeForRange(type)) { + return Status::SyntaxError("Data type of field %s not support range scan", + prop.c_str()); + } + } break; } case nebula::Expression::kFunctionCall : { - auto* fExpr = dynamic_cast(expr); - auto* name = fExpr->name(); - if (*name == "udf_is_in") { - return Status::SyntaxError("Unsupported function : %s", name->c_str()); - } - break; + return Status::SyntaxError("Function expressions are not supported yet"); } default : { return Status::SyntaxError("Syntax error : %s", expr->toString().c_str()); @@ -231,7 +249,15 @@ Status LookupExecutor::traversalExpr(const Expression *expr) { } Status LookupExecutor::checkFilter() { - auto status = traversalExpr(sentence_->whereClause()->filter()); + auto *sm = ectx()->schemaManager(); + auto schema = isEdge_ + ? sm->getEdgeSchema(spaceId_, tagOrEdge_) + : sm->getTagSchema(spaceId_, tagOrEdge_); + if (schema == nullptr) { + return Status::Error("No schema found %s", from_->c_str()); + } + + auto status = traversalExpr(sentence_->whereClause()->filter(), schema.get()); if (!status.ok()) { return status; } @@ -826,5 +852,35 @@ bool LookupExecutor::processFinalVertexResult(RpcResponse &rpcResp, return true; } +Status LookupExecutor::relationalExprCheck(RelationalExpression::Operator op) const { + // Compile will fail after added new relational operations. + // Need to consider the logic in method 'traversalExpr' after new relational operation added. + switch (op) { + case RelationalExpression::Operator::EQ : + case RelationalExpression::Operator::GE : + case RelationalExpression::Operator::GT : + case RelationalExpression::Operator::LE : + case RelationalExpression::Operator::LT : + case RelationalExpression::Operator::NE : { + return Status::OK(); + } + case RelationalExpression::Operator::CONTAINS : { + return Status::SyntaxError("Unsupported 'CONTAINS' in where clause"); + } + } + return Status::OK(); +} + +bool LookupExecutor::supportedDataTypeForRange(nebula::cpp2::SupportedType type) const { + switch (type) { + case nebula::cpp2::SupportedType::INT: + case nebula::cpp2::SupportedType::DOUBLE: + case nebula::cpp2::SupportedType::FLOAT: + case nebula::cpp2::SupportedType::TIMESTAMP: + return true; + default: + return false; + } +} } // namespace graph } // namespace nebula diff --git a/src/graph/LookupExecutor.h b/src/graph/LookupExecutor.h index 1cbf25e3021..6e10c635447 100644 --- a/src/graph/LookupExecutor.h +++ b/src/graph/LookupExecutor.h @@ -49,7 +49,7 @@ class LookupExecutor final : public TraverseExecutor { Status optimize(); - Status traversalExpr(const Expression *expr); + Status traversalExpr(const Expression *expr, const meta::SchemaProviderIf* schema); Status checkFilter(); @@ -85,6 +85,9 @@ class LookupExecutor final : public TraverseExecutor { bool processFinalVertexResult(RpcResponse &rpcResp, const Callback& cb) const; + Status relationalExprCheck(RelationalExpression::Operator op) const; + + bool supportedDataTypeForRange(nebula::cpp2::SupportedType type) const; private: using FilterItem = std::pair; diff --git a/src/graph/test/LookupTest.cpp b/src/graph/test/LookupTest.cpp index 3f8c65c0cf1..35b80f4e20c 100644 --- a/src/graph/test/LookupTest.cpp +++ b/src/graph/test/LookupTest.cpp @@ -236,7 +236,7 @@ TEST_F(LookupTest, VertexConditionScan) { auto query = "LOOKUP ON lookup_tag_2 WHERE lookup_tag_2.col2 == 100 " "OR lookup_tag_2.col2 == 200"; auto code = client_->execute(query, resp); - ASSERT_EQ(cpp2::ErrorCode::E_EXECUTION_ERROR, code); + ASSERT_EQ(cpp2::ErrorCode::E_SYNTAX_ERROR, code); } { cpp2::ExecutionResponse resp; @@ -398,7 +398,7 @@ TEST_F(LookupTest, EdgeConditionScan) { auto query = "LOOKUP ON lookup_edge_2 WHERE lookup_edge_2.col2 == 100 " "OR lookup_edge_2.col2 == 200"; auto code = client_->execute(query, resp); - ASSERT_EQ(cpp2::ErrorCode::E_EXECUTION_ERROR, code); + ASSERT_EQ(cpp2::ErrorCode::E_SYNTAX_ERROR, code); } { cpp2::ExecutionResponse resp; @@ -660,7 +660,7 @@ TEST_F(LookupTest, FunctionExprTest) { cpp2::ExecutionResponse resp; auto query = "LOOKUP ON lookup_tag_2 WHERE lookup_tag_2.col2 != lookup_tag_2.col3"; auto code = client_->execute(query, resp); - ASSERT_EQ(cpp2::ErrorCode::E_EXECUTION_ERROR, code); + ASSERT_EQ(cpp2::ErrorCode::E_SYNTAX_ERROR, code); } { cpp2::ExecutionResponse resp;