Skip to content

Commit

Permalink
Disable index range scan of string data type in graph layer (#2283)
Browse files Browse the repository at this point in the history 
* Disable string range scan in graph lyear

* rename mothod dataTypeCheckForRange to supportedDataTypeForRange

* fixed typo error

* Does not support left expr and right expr are both kAliasProp
  • Loading branch information
@bright-starry-sky
bright-starry-sky authored Sep 17, 2020
1 parent 0d312e6 commit 9154dde
Show file tree
Hide file tree
Showing 3 changed files with 79 additions and 20 deletions.
88 changes: 72 additions & 16 deletions src/graph/LookupExecutor.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<const LogicalExpression*>(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<const RelationalExpression*>(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<const AliasPropertyExpression*>(left);
auto st = checkAliasProperty(aExpr);
if (!st.ok()) {
Expand All @@ -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<const FunctionCallExpression*>(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());
Expand All @@ -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;
}
Expand Down Expand Up @@ -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
5 changes: 4 additions & 1 deletion src/graph/LookupExecutor.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down Expand Up @@ -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<std::string, RelationalExpression::Operator>;
Expand Down
6 changes: 3 additions & 3 deletions src/graph/test/LookupTest.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down

0 comments on commit 9154dde

Please sign in to comment.