From 52153f66a502b507215786f7fabcb7d1b97a53a0 Mon Sep 17 00:00:00 2001 From: Yee <2520865+yixinglu@users.noreply.github.com> Date: Fri, 23 Oct 2020 17:46:29 +0800 Subject: [PATCH 1/2] Return semantic error in validators (#349) * Return error code string if the status message content is empty * Return error directly * Return semantic error in validators * Fix comments * Fix ci failure --- src/validator/ACLValidator.cpp | 32 +++--- src/validator/AdminValidator.cpp | 20 ++-- src/validator/FetchEdgesValidator.cpp | 19 ++-- src/validator/FetchVerticesValidator.cpp | 11 +- src/validator/GetSubgraphValidator.cpp | 48 ++------- src/validator/GoValidator.cpp | 38 +++---- src/validator/MaintainValidator.cpp | 119 +++++++-------------- src/validator/MutateValidator.cpp | 126 ++++++++--------------- src/validator/ReportError.h | 4 +- src/validator/SequentialValidator.cpp | 4 +- src/validator/SetValidator.cpp | 3 +- src/validator/TraversalValidator.cpp | 25 ++--- src/validator/Validator.cpp | 34 ++---- src/validator/Validator.h | 4 +- tests/query/v1/test_yield.py | 4 +- tests/query/v2/test_explain.py | 2 +- 16 files changed, 186 insertions(+), 307 deletions(-) diff --git a/src/validator/ACLValidator.cpp b/src/validator/ACLValidator.cpp index 9c5ed3ce4..49232af06 100644 --- a/src/validator/ACLValidator.cpp +++ b/src/validator/ACLValidator.cpp @@ -22,10 +22,12 @@ static std::size_t kPasswordMaxLength = 24; Status CreateUserValidator::validateImpl() { const auto *sentence = static_cast(sentence_); if (sentence->getAccount()->size() > kUsernameMaxLength) { - return Status::Error("Username exceed maximum length %ld characters.", kUsernameMaxLength); + return Status::SemanticError("Username exceed maximum length %ld characters.", + kUsernameMaxLength); } if (sentence->getPassword()->size() > kPasswordMaxLength) { - return Status::Error("Password exceed maximum length %ld characters.", kPasswordMaxLength); + return Status::SemanticError("Password exceed maximum length %ld characters.", + kPasswordMaxLength); } return Status::OK(); } @@ -40,7 +42,8 @@ Status CreateUserValidator::toPlan() { Status DropUserValidator::validateImpl() { const auto *sentence = static_cast(sentence_); if (sentence->getAccount()->size() > kUsernameMaxLength) { - return Status::Error("Username exceed maximum length %ld characters.", kUsernameMaxLength); + return Status::SemanticError("Username exceed maximum length %ld characters.", + kUsernameMaxLength); } return Status::OK(); } @@ -54,10 +57,12 @@ Status DropUserValidator::toPlan() { Status UpdateUserValidator::validateImpl() { const auto *sentence = static_cast(sentence_); if (sentence->getAccount()->size() > kUsernameMaxLength) { - return Status::Error("Username exceed maximum length %ld characters.", kUsernameMaxLength); + return Status::SemanticError("Username exceed maximum length %ld characters.", + kUsernameMaxLength); } if (sentence->getPassword()->size() > kPasswordMaxLength) { - return Status::Error("Password exceed maximum length %ld characters.", kPasswordMaxLength); + return Status::SemanticError("Password exceed maximum length %ld characters.", + kPasswordMaxLength); } return Status::OK(); } @@ -80,15 +85,16 @@ Status ShowUsersValidator::toPlan() { Status ChangePasswordValidator::validateImpl() { const auto *sentence = static_cast(sentence_); if (sentence->getAccount()->size() > kUsernameMaxLength) { - return Status::Error("Username exceed maximum length %ld characters.", kUsernameMaxLength); + return Status::SemanticError("Username exceed maximum length %ld characters.", + kUsernameMaxLength); } if (sentence->getOldPwd()->size() > kPasswordMaxLength) { - return Status::Error("Old password exceed maximum length %ld characters.", - kPasswordMaxLength); + return Status::SemanticError("Old password exceed maximum length %ld characters.", + kPasswordMaxLength); } if (sentence->getNewPwd()->size() > kPasswordMaxLength) { - return Status::Error("New password exceed maximum length %ld characters.", - kPasswordMaxLength); + return Status::SemanticError("New password exceed maximum length %ld characters.", + kPasswordMaxLength); } return Status::OK(); } @@ -103,7 +109,8 @@ Status ChangePasswordValidator::toPlan() { Status GrantRoleValidator::validateImpl() { const auto *sentence = static_cast(sentence_); if (sentence->getAccount()->size() > kUsernameMaxLength) { - return Status::Error("Username exceed maximum length %ld characters.", kUsernameMaxLength); + return Status::SemanticError("Username exceed maximum length %ld characters.", + kUsernameMaxLength); } return Status::OK(); } @@ -119,7 +126,8 @@ Status GrantRoleValidator::toPlan() { Status RevokeRoleValidator::validateImpl() { const auto *sentence = static_cast(sentence_); if (sentence->getAccount()->size() > kUsernameMaxLength) { - return Status::Error("Username exceed maximum length %ld characters.", kUsernameMaxLength); + return Status::SemanticError("Username exceed maximum length %ld characters.", + kUsernameMaxLength); } return Status::OK(); } diff --git a/src/validator/AdminValidator.cpp b/src/validator/AdminValidator.cpp index c33d863e4..49f28b041 100644 --- a/src/validator/AdminValidator.cpp +++ b/src/validator/AdminValidator.cpp @@ -30,14 +30,15 @@ Status CreateSpaceValidator::validateImpl() { case SpaceOptItem::PARTITION_NUM: { spaceDesc_.partition_num = item->getPartitionNum(); if (spaceDesc_.partition_num <= 0) { - return Status::Error("Partition_num value should be greater than zero"); + return Status::SemanticError("Partition_num value should be greater than zero"); } break; } case SpaceOptItem::REPLICA_FACTOR: { spaceDesc_.replica_factor = item->getReplicaFactor(); if (spaceDesc_.replica_factor <= 0) { - return Status::Error("Replica_factor value should be greater than zero"); + return Status::SemanticError( + "Replica_factor value should be greater than zero"); } break; } @@ -48,7 +49,7 @@ Status CreateSpaceValidator::validateImpl() { std::stringstream ss; ss << "Only support FIXED_STRING or INT64 vid type, but was given " << meta::cpp2::_PropertyType_VALUES_TO_NAMES.at(typeDef.type); - return Status::Error(ss.str()); + return Status::SemanticError(ss.str()); } spaceDesc_.vid_type.set_type(typeDef.type); @@ -56,10 +57,11 @@ Status CreateSpaceValidator::validateImpl() { spaceDesc_.vid_type.set_type_length(8); } else { if (!typeDef.__isset.type_length) { - return Status::Error("type length is not set for fixed string type"); + return Status::SemanticError( + "type length is not set for fixed string type"); } if (*typeDef.get_type_length() <= 0) { - return Status::Error("Vid size should be a positive number: %d", + return Status::SemanticError("Vid size should be a positive number: %d", *typeDef.get_type_length()); } spaceDesc_.vid_type.set_type_length(*typeDef.get_type_length()); @@ -294,7 +296,7 @@ Status SetConfigValidator::validateImpl() { auto sentence = static_cast(sentence_); auto item = sentence->configItem(); if (item == nullptr) { - return Status::Error("Empty config item"); + return Status::SemanticError("Empty config item"); } if (item->getName() != nullptr) { name_ = *item->getName(); @@ -318,14 +320,14 @@ Status SetConfigValidator::validateImpl() { std::string name; Value value; if (updateItem->getFieldName() == nullptr || updateItem->value() == nullptr) { - return Status::Error("Empty item"); + return Status::SemanticError("Empty item"); } name = *updateItem->getFieldName(); value = Expression::eval(const_cast(updateItem->value()), ctx(nullptr)); if (value.isNull() || (!value.isNumeric() && !value.isStr() && !value.isBool())) { - return Status::Error("Wrong value: %s", name.c_str()); + return Status::SemanticError("Wrong value: `%s'", name.c_str()); } configs.kvs.emplace(std::move(name), std::move(value)); } @@ -350,7 +352,7 @@ Status GetConfigValidator::validateImpl() { auto sentence = static_cast(sentence_); auto item = sentence->configItem(); if (item == nullptr) { - return Status::Error("Empty config item"); + return Status::SemanticError("Empty config item"); } module_ = item->getModule(); diff --git a/src/validator/FetchEdgesValidator.cpp b/src/validator/FetchEdgesValidator.cpp index 131d1f00b..5ae5f5798 100644 --- a/src/validator/FetchEdgesValidator.cpp +++ b/src/validator/FetchEdgesValidator.cpp @@ -84,7 +84,7 @@ Status FetchEdgesValidator::check() { schema_ = qctx_->schemaMng()->getEdgeSchema(spaceId_, edgeType_); if (schema_ == nullptr) { LOG(ERROR) << "No schema found for " << sentence->edge(); - return Status::Error("No schema found for `%s'", sentence->edge()->c_str()); + return Status::SemanticError("No schema found for `%s'", sentence->edge()->c_str()); } return Status::OK(); @@ -103,14 +103,15 @@ Status FetchEdgesValidator::prepareEdges() { result = checkRef(rankRef_, Value::Type::INT); NG_RETURN_IF_ERROR(result); if (inputVar_ != result.value()) { - return Status::Error("Can't refer to different variable as key at same time."); + return Status::SemanticError( + "Can't refer to different variable as key at same time."); } } dstRef_ = sentence->ref()->dstid(); result = checkRef(dstRef_, Value::Type::STRING); NG_RETURN_IF_ERROR(result); if (inputVar_ != result.value()) { - return Status::Error("Can't refer to different variable as key at same time."); + return Status::SemanticError("Can't refer to different variable as key at same time."); } return Status::OK(); } @@ -185,8 +186,8 @@ Status FetchEdgesValidator::preparePropertiesWithYield(const YieldClause *yield) } const auto *invalidExpr = findInvalidYieldExpression(col->expr()); if (invalidExpr != nullptr) { - return Status::Error("Invalid newYield_ expression `%s'.", - col->expr()->toString().c_str()); + return Status::SemanticError("Invalid newYield_ expression `%s'.", + col->expr()->toString().c_str()); } // The properties from storage directly push down only // The other will be computed in Project Executor @@ -194,16 +195,16 @@ Status FetchEdgesValidator::preparePropertiesWithYield(const YieldClause *yield) for (const auto &storageExpr : storageExprs) { const auto *expr = static_cast(storageExpr); if (*expr->sym() != edgeTypeName_) { - return Status::Error("Mismatched edge type name"); + return Status::SemanticError("Mismatched edge type name"); } // Check is prop name in schema if (schema_->getFieldIndex(*expr->prop()) < 0 && reservedProperties.find(*expr->prop()) == reservedProperties.end()) { LOG(ERROR) << "Unknown column `" << *expr->prop() << "' in edge `" << edgeTypeName_ << "'."; - return Status::Error("Unknown column `%s' in edge `%s'", - expr->prop()->c_str(), - edgeTypeName_.c_str()); + return Status::SemanticError("Unknown column `%s' in edge `%s'", + expr->prop()->c_str(), + edgeTypeName_.c_str()); } propsName.emplace_back(*expr->prop()); geColNames_.emplace_back(*expr->sym() + "." + *expr->prop()); diff --git a/src/validator/FetchVerticesValidator.cpp b/src/validator/FetchVerticesValidator.cpp index f00bdd5dc..597bff302 100644 --- a/src/validator/FetchVerticesValidator.cpp +++ b/src/validator/FetchVerticesValidator.cpp @@ -74,7 +74,7 @@ Status FetchVerticesValidator::check() { auto schema = qctx_->schemaMng()->getTagSchema(space_.id, tagId); if (schema == nullptr) { LOG(ERROR) << "No schema found for " << tagName; - return Status::Error("No schema found for `%s'", tagName.c_str()); + return Status::SemanticError("No schema found for `%s'", tagName.c_str()); } tagsSchema_.emplace(tagId, schema); } else { @@ -157,13 +157,14 @@ Status FetchVerticesValidator::preparePropertiesWithYield(const YieldClause *yie return std::move(deducePropsVisitor).status(); } if (exprProps.hasInputVarProperty()) { - return Status::Error("Unsupported input/variable property expression in yield."); + return Status::SemanticError( + "Unsupported input/variable property expression in yield."); } if (!exprProps.edgeProps().empty()) { - return Status::Error("Unsupported edge property expression in yield."); + return Status::SemanticError("Unsupported edge property expression in yield."); } if (exprProps.hasSrcDstTagProperty()) { - return Status::Error("Unsupported src/dst property expression in yield."); + return Status::SemanticError("Unsupported src/dst property expression in yield."); } colNames_.emplace_back(deduceColName(col)); @@ -173,7 +174,7 @@ Status FetchVerticesValidator::preparePropertiesWithYield(const YieldClause *yie // TODO(shylock) think about the push-down expr } if (exprProps.tagProps().empty()) { - return Status::Error("Unsupported empty tag property expression in yield."); + return Status::SemanticError("Unsupported empty tag property expression in yield."); } if (onStar_) { diff --git a/src/validator/GetSubgraphValidator.cpp b/src/validator/GetSubgraphValidator.cpp index 3d189e999..6b459d689 100644 --- a/src/validator/GetSubgraphValidator.cpp +++ b/src/validator/GetSubgraphValidator.cpp @@ -18,33 +18,13 @@ namespace nebula { namespace graph { Status GetSubgraphValidator::validateImpl() { - Status status; auto* gsSentence = static_cast(sentence_); - status = validateStep(gsSentence->step(), steps_); - if (!status.ok()) { - return status; - } - - status = validateStarts(gsSentence->from(), from_); - if (!status.ok()) { - return status; - } - - status = validateInBound(gsSentence->in()); - if (!status.ok()) { - return status; - } - - status = validateOutBound(gsSentence->out()); - if (!status.ok()) { - return status; - } - - status = validateBothInOutBound(gsSentence->both()); - if (!status.ok()) { - return status; - } + NG_RETURN_IF_ERROR(validateStep(gsSentence->step(), steps_)); + NG_RETURN_IF_ERROR(validateStarts(gsSentence->from(), from_)); + NG_RETURN_IF_ERROR(validateInBound(gsSentence->in())); + NG_RETURN_IF_ERROR(validateOutBound(gsSentence->out())); + NG_RETURN_IF_ERROR(validateBothInOutBound(gsSentence->both())); return Status::OK(); } @@ -56,13 +36,11 @@ Status GetSubgraphValidator::validateInBound(InBoundClause* in) { edgeTypes_.reserve(edgeTypes_.size() + edges.size()); for (auto* e : edges) { if (e->alias() != nullptr) { - return Status::Error("Get Subgraph not support rename edge name."); + return Status::SemanticError("Get Subgraph not support rename edge name."); } auto et = qctx_->schemaMng()->toEdgeType(space.id, *e->edge()); - if (!et.ok()) { - return et.status(); - } + NG_RETURN_IF_ERROR(et); auto v = -et.value(); edgeTypes_.emplace(v); @@ -79,13 +57,11 @@ Status GetSubgraphValidator::validateOutBound(OutBoundClause* out) { edgeTypes_.reserve(edgeTypes_.size() + edges.size()); for (auto* e : out->edges()) { if (e->alias() != nullptr) { - return Status::Error("Get Subgraph not support rename edge name."); + return Status::SemanticError("Get Subgraph not support rename edge name."); } auto et = qctx_->schemaMng()->toEdgeType(space.id, *e->edge()); - if (!et.ok()) { - return et.status(); - } + NG_RETURN_IF_ERROR(et); edgeTypes_.emplace(et.value()); } @@ -101,13 +77,11 @@ Status GetSubgraphValidator::validateBothInOutBound(BothInOutClause* out) { edgeTypes_.reserve(edgeTypes_.size() + edges.size()); for (auto* e : out->edges()) { if (e->alias() != nullptr) { - return Status::Error("Get Subgraph not support rename edge name."); + return Status::SemanticError("Get Subgraph not support rename edge name."); } auto et = qctx_->schemaMng()->toEdgeType(space.id, *e->edge()); - if (!et.ok()) { - return et.status(); - } + NG_RETURN_IF_ERROR(et); auto v = et.value(); edgeTypes_.emplace(v); diff --git a/src/validator/GoValidator.cpp b/src/validator/GoValidator.cpp index 21eccf7a6..e15006203 100644 --- a/src/validator/GoValidator.cpp +++ b/src/validator/GoValidator.cpp @@ -27,16 +27,17 @@ Status GoValidator::validateImpl() { NG_RETURN_IF_ERROR(validateYield(goSentence->yieldClause())); if (!exprProps_.inputProps().empty() && from_.fromType != kPipe) { - return Status::Error("$- must be referred in FROM before used in WHERE or YIELD"); + return Status::SemanticError("$- must be referred in FROM before used in WHERE or YIELD"); } if (!exprProps_.varProps().empty() && from_.fromType != kVariable) { - return Status::Error("A variable must be referred in FROM before used in WHERE or YIELD"); + return Status::SemanticError( + "A variable must be referred in FROM before used in WHERE or YIELD"); } if ((!exprProps_.inputProps().empty() && !exprProps_.varProps().empty()) || exprProps_.varProps().size() > 1) { - return Status::Error("Only support single input in a go sentence."); + return Status::SemanticError("Only support single input in a go sentence."); } NG_RETURN_IF_ERROR(buildColumns()); @@ -53,35 +54,28 @@ Status GoValidator::validateWhere(WhereClause* where) { if (filter_->kind() == Expression::Kind::kLabelAttribute) { auto laExpr = static_cast(filter_); - where->setFilter(ExpressionUtils::rewriteLabelAttribute( - laExpr)); + where->setFilter(ExpressionUtils::rewriteLabelAttribute(laExpr)); } else { ExpressionUtils::rewriteLabelAttribute(filter_); } auto typeStatus = deduceExprType(filter_); - if (!typeStatus.ok()) { - return typeStatus.status(); - } - + NG_RETURN_IF_ERROR(typeStatus); auto type = typeStatus.value(); if (type != Value::Type::BOOL && type != Value::Type::NULLVALUE) { std::stringstream ss; ss << "`" << filter_->toString() << "', Filter only accpet bool/null value, " << "but was `" << type << "'"; - return Status::Error(ss.str()); + return Status::SemanticError(ss.str()); } - auto status = deduceProps(filter_, exprProps_); - if (!status.ok()) { - return status; - } + NG_RETURN_IF_ERROR(deduceProps(filter_, exprProps_)); return Status::OK(); } Status GoValidator::validateYield(YieldClause* yield) { if (yield == nullptr) { - return Status::Error("Yield clause nullptr."); + return Status::SemanticError("Yield clause nullptr."); } distinct_ = yield->isDistinct(); @@ -112,7 +106,7 @@ Status GoValidator::validateYield(YieldClause* yield) { } if (!col->getAggFunName().empty()) { - return Status::Error( + return Status::SemanticError( "`%s', not support aggregate function in go sentence.", col->toString().c_str()); } @@ -129,7 +123,7 @@ Status GoValidator::validateYield(YieldClause* yield) { for (auto& e : exprProps_.edgeProps()) { auto found = std::find(over_.edgeTypes.begin(), over_.edgeTypes.end(), e.first); if (found == over_.edgeTypes.end()) { - return Status::Error("Edges should be declared first in over clause."); + return Status::SemanticError("Edges should be declared first in over clause."); } } yields_ = yield->yields(); @@ -262,10 +256,7 @@ Status GoValidator::buildNStepsPlan() { loopBody, // body buildNStepLoopCondition(steps_.steps - 1)); - auto status = oneStep(loop, dedupDstVids->outputVar(), projectFromJoin); - if (!status.ok()) { - return status; - } + NG_RETURN_IF_ERROR(oneStep(loop, dedupDstVids->outputVar(), projectFromJoin)); // reset tail_ if (projectStartVid_ != nullptr) { tail_ = projectStartVid_; @@ -616,10 +607,7 @@ Status GoValidator::buildOneStepPlan() { startVidsVar = dedupStartVid->outputVar(); } - auto status = oneStep(dedupStartVid, startVidsVar, nullptr); - if (!status.ok()) { - return status; - } + NG_RETURN_IF_ERROR(oneStep(dedupStartVid, startVidsVar, nullptr)); if (projectStartVid_ != nullptr) { tail_ = projectStartVid_; diff --git a/src/validator/MaintainValidator.cpp b/src/validator/MaintainValidator.cpp index 16e7c5af9..2d623b7e0 100644 --- a/src/validator/MaintainValidator.cpp +++ b/src/validator/MaintainValidator.cpp @@ -50,9 +50,7 @@ Status SchemaValidator::validateColumns(const std::vector auto defaultValueExpr = spec->getDefaultValue(); auto& value = defaultValueExpr->eval(ctx(nullptr)); auto valStatus = SchemaUtil::toSchemaValue(type, value); - if (!valStatus.ok()) { - return valStatus.status(); - } + NG_RETURN_IF_ERROR(valStatus); // When the timestamp value is string, need to save the int value, // TODO: if support timestamp value is an expression, need to remove the code if (type == meta::cpp2::PropertyType::TIMESTAMP && value.isStr()) { @@ -70,35 +68,21 @@ Status SchemaValidator::validateColumns(const std::vector Status CreateTagValidator::validateImpl() { auto sentence = static_cast(sentence_); - auto status = Status::OK(); name_ = *sentence->name(); ifNotExist_ = sentence->isIfNotExist(); - do { - // Check the validateContext has the same name schema - auto pro = vctx_->getSchema(name_); - if (pro != nullptr) { - status = Status::Error("Has the same name `%s' in the SequentialSentences", - name_.c_str()); - break; - } - status = validateColumns(sentence->columnSpecs(), schema_); - if (!status.ok()) { - VLOG(1) << status; - break; - } - status = SchemaUtil::validateProps(sentence->getSchemaProps(), schema_); - if (!status.ok()) { - VLOG(1) << status; - break; - } - } while (false); - // Save the schema in validateContext - if (status.ok()) { - auto schemaPro = SchemaUtil::generateSchemaProvider(0, schema_); - vctx_->addSchema(name_, schemaPro); + // Check the validateContext has the same name schema + auto pro = vctx_->getSchema(name_); + if (pro != nullptr) { + return Status::SemanticError("Has the same name `%s' in the SequentialSentences", + name_.c_str()); } - return status; + NG_RETURN_IF_ERROR(validateColumns(sentence->columnSpecs(), schema_)); + NG_RETURN_IF_ERROR(SchemaUtil::validateProps(sentence->getSchemaProps(), schema_)); + // Save the schema in validateContext + auto schemaPro = SchemaUtil::generateSchemaProvider(0, schema_); + vctx_->addSchema(name_, schemaPro); + return Status::OK(); } Status CreateTagValidator::toPlan() { @@ -115,33 +99,18 @@ Status CreateEdgeValidator::validateImpl() { auto status = Status::OK(); name_ = *sentence->name(); ifNotExist_ = sentence->isIfNotExist(); - do { - // Check the validateContext has the same name schema - auto pro = vctx_->getSchema(name_); - if (pro != nullptr) { - status = Status::Error("Has the same name `%s' in the SequentialSentences", - name_.c_str()); - break; - } - - status = validateColumns(sentence->columnSpecs(), schema_); - if (!status.ok()) { - VLOG(1) << status; - break; - } - status = SchemaUtil::validateProps(sentence->getSchemaProps(), schema_); - if (!status.ok()) { - VLOG(1) << status; - break; - } - } while (false); - - // Save the schema in validateContext - if (status.ok()) { - auto schemaPro = SchemaUtil::generateSchemaProvider(0, schema_); - vctx_->addSchema(name_, schemaPro); + // Check the validateContext has the same name schema + auto pro = vctx_->getSchema(name_); + if (pro != nullptr) { + return Status::SemanticError("Has the same name `%s' in the SequentialSentences", + name_.c_str()); } - return status; + NG_RETURN_IF_ERROR(validateColumns(sentence->columnSpecs(), schema_)); + NG_RETURN_IF_ERROR(SchemaUtil::validateProps(sentence->getSchemaProps(), schema_)); + // Save the schema in validateContext + auto schemaPro = SchemaUtil::generateSchemaProvider(0, schema_); + vctx_->addSchema(name_, schemaPro); + return Status::OK(); } Status CreateEdgeValidator::toPlan() { @@ -210,22 +179,18 @@ Status AlterValidator::alterSchema(const std::vector& schem switch (propType) { case SchemaPropItem::TTL_DURATION: retInt = schemaProp->getTtlDuration(); - if (!retInt.ok()) { - return retInt.status(); - } + NG_RETURN_IF_ERROR(retInt); ttlDuration = retInt.value(); schemaProp_.set_ttl_duration(ttlDuration); break; case SchemaPropItem::TTL_COL: // Check the legality of the column in meta retStr = schemaProp->getTtlCol(); - if (!retStr.ok()) { - return retStr.status(); - } + NG_RETURN_IF_ERROR(retStr); schemaProp_.set_ttl_col(retStr.value()); break; default: - return Status::Error("Property type not support"); + return Status::SemanticError("Property type not support"); } } return Status::OK(); @@ -357,13 +322,11 @@ Status CreateTagIndexValidator::validateImpl() { auto status = Status::OK(); do { auto tagStatus = qctx_->schemaMng()->toTagID(space_.id, name_); - if (!tagStatus.ok()) { - return tagStatus.status(); - } + NG_RETURN_IF_ERROR(tagStatus); auto schema_ = qctx_->schemaMng()->getTagSchema(space_.id, tagStatus.value()); if (schema_ == nullptr) { - return Status::Error("No schema found for '%s'", name_.c_str()); + return Status::SemanticError("No schema found for '%s'", name_.c_str()); } status = IndexUtil::validateColumns(fields_); @@ -396,26 +359,16 @@ Status CreateEdgeIndexValidator::validateImpl() { fields_ = sentence->columns(); ifNotExist_ = sentence->isIfNotExist(); - auto status = Status::OK(); - do { - auto edgeStatus = qctx_->schemaMng()->toEdgeType(space_.id, name_); - if (!edgeStatus.ok()) { - return edgeStatus.status(); - } - - auto schema_ = qctx_->schemaMng()->getEdgeSchema(space_.id, edgeStatus.value()); - if (schema_ == nullptr) { - return Status::Error("No schema found for '%s'", name_.c_str()); - } + auto edgeStatus = qctx_->schemaMng()->toEdgeType(space_.id, name_); + NG_RETURN_IF_ERROR(edgeStatus); + auto schema_ = qctx_->schemaMng()->getEdgeSchema(space_.id, edgeStatus.value()); + if (schema_ == nullptr) { + return Status::SemanticError("No schema found for '%s'", name_.c_str()); + } - status = IndexUtil::validateColumns(fields_); - if (!status.ok()) { - VLOG(1) << status; - break; - } - } while (false); + NG_RETURN_IF_ERROR(IndexUtil::validateColumns(fields_)); // TODO(darion) Save the index - return status; + return Status::OK(); } Status CreateEdgeIndexValidator::toPlan() { diff --git a/src/validator/MutateValidator.cpp b/src/validator/MutateValidator.cpp index bc9291860..cae9fa129 100644 --- a/src/validator/MutateValidator.cpp +++ b/src/validator/MutateValidator.cpp @@ -46,7 +46,7 @@ Status InsertVerticesValidator::check() { auto sentence = static_cast(sentence_); rows_ = sentence->rows(); if (rows_.empty()) { - return Status::Error("VALUES cannot be empty"); + return Status::SemanticError("VALUES cannot be empty"); } auto tagItems = sentence->tagItems(); @@ -59,14 +59,14 @@ Status InsertVerticesValidator::check() { auto tagStatus = qctx_->schemaMng()->toTagID(spaceId_, *tagName); if (!tagStatus.ok()) { LOG(ERROR) << "No schema found for " << *tagName; - return Status::Error("No schema found for `%s'", tagName->c_str()); + return Status::SemanticError("No schema found for `%s'", tagName->c_str()); } auto tagId = tagStatus.value(); auto schema = qctx_->schemaMng()->getTagSchema(spaceId_, tagId); if (schema == nullptr) { LOG(ERROR) << "No schema found for " << *tagName; - return Status::Error("No schema found for `%s'", tagName->c_str()); + return Status::SemanticError("No schema found for `%s'", tagName->c_str()); } std::vector names; @@ -75,7 +75,7 @@ Status InsertVerticesValidator::check() { for (auto *it : props) { if (schema->getFieldIndex(*it) < 0) { LOG(ERROR) << "Unknown column `" << *it << "' in schema"; - return Status::Error("Unknown column `%s' in schema", it->c_str()); + return Status::SemanticError("Unknown column `%s' in schema", it->c_str()); } propSize_++; names.emplace_back(*it); @@ -91,29 +91,27 @@ Status InsertVerticesValidator::prepareVertices() { for (auto i = 0u; i < rows_.size(); i++) { auto *row = rows_[i]; if (propSize_ != row->values().size()) { - return Status::Error("Column count doesn't match value count."); + return Status::SemanticError("Column count doesn't match value count."); } if (!evaluableExpr(row->id())) { LOG(ERROR) << "Wrong vid expression `" << row->id()->toString() << "\""; - return Status::Error("Wrong vid expression `%s'", row->id()->toString().c_str()); + return Status::SemanticError("Wrong vid expression `%s'", + row->id()->toString().c_str()); } auto idStatus = SchemaUtil::toVertexID(row->id()); - if (!idStatus.ok()) { - return idStatus.status(); - } + NG_RETURN_IF_ERROR(idStatus); auto vertexId = std::move(idStatus).value(); // check value expr for (auto &value : row->values()) { if (!evaluableExpr(value)) { LOG(ERROR) << "Insert wrong value: `" << value->toString() << "'."; - return Status::Error("Insert wrong value: `%s'.", value->toString().c_str()); + return Status::SemanticError("Insert wrong value: `%s'.", + value->toString().c_str()); } } auto valsRet = SchemaUtil::toValueVec(row->values()); - if (!valsRet.ok()) { - return valsRet.status(); - } + NG_RETURN_IF_ERROR(valsRet); auto values = std::move(valsRet).value(); std::vector tags(schemas_.size()); @@ -127,9 +125,7 @@ Status InsertVerticesValidator::prepareVertices() { for (auto index = 0u; index < propNames.size(); index++) { auto schemaType = schema->getFieldType(propNames[index]); auto valueStatus = SchemaUtil::toSchemaValue(schemaType, values[handleValueNum]); - if (!valueStatus.ok()) { - return valueStatus.status(); - } + NG_RETURN_IF_ERROR(valueStatus); props.emplace_back(std::move(valueStatus).value()); handleValueNum++; } @@ -148,19 +144,9 @@ Status InsertVerticesValidator::prepareVertices() { Status InsertEdgesValidator::validateImpl() { spaceId_ = vctx_->whichSpace().id; - auto status = Status::OK(); - do { - status = check(); - if (!status.ok()) { - break; - } - - status = prepareEdges(); - if (!status.ok()) { - break; - } - } while (false); - return status; + NG_RETURN_IF_ERROR(check()); + NG_RETURN_IF_ERROR(prepareEdges()); + return Status::OK(); } Status InsertEdgesValidator::toPlan() { @@ -179,9 +165,7 @@ Status InsertEdgesValidator::check() { auto sentence = static_cast(sentence_); overwritable_ = sentence->overwritable(); auto edgeStatus = qctx_->schemaMng()->toEdgeType(spaceId_, *sentence->edge()); - if (!edgeStatus.ok()) { - return edgeStatus.status(); - } + NG_RETURN_IF_ERROR(edgeStatus); edgeType_ = edgeStatus.value(); auto props = sentence->properties(); rows_ = sentence->rows(); @@ -189,14 +173,14 @@ Status InsertEdgesValidator::check() { schema_ = qctx_->schemaMng()->getEdgeSchema(spaceId_, edgeType_); if (schema_ == nullptr) { LOG(ERROR) << "No schema found for " << sentence->edge(); - return Status::Error("No schema found for `%s'", sentence->edge()->c_str()); + return Status::SemanticError("No schema found for `%s'", sentence->edge()->c_str()); } // Check prop name is in schema for (auto *it : props) { if (schema_->getFieldIndex(*it) < 0) { LOG(ERROR) << "Unknown column `" << *it << "' in schema"; - return Status::Error("Unknown column `%s' in schema", it->c_str()); + return Status::SemanticError("Unknown column `%s' in schema", it->c_str()); } propNames_.emplace_back(*it); } @@ -209,27 +193,25 @@ Status InsertEdgesValidator::prepareEdges() {; for (auto i = 0u; i < rows_.size(); i++) { auto *row = rows_[i]; if (propNames_.size() != row->values().size()) { - return Status::Error("Column count doesn't match value count."); + return Status::SemanticError("Column count doesn't match value count."); } if (!evaluableExpr(row->srcid())) { LOG(ERROR) << "Wrong src vid expression `" << row->srcid()->toString() << "\""; - return Status::Error("Wrong src vid expression `%s'", row->srcid()->toString().c_str()); + return Status::SemanticError("Wrong src vid expression `%s'", + row->srcid()->toString().c_str()); } if (!evaluableExpr(row->dstid())) { LOG(ERROR) << "Wrong dst vid expression `" << row->dstid()->toString() << "\""; - return Status::Error("Wrong dst vid expression `%s'", row->dstid()->toString().c_str()); + return Status::SemanticError("Wrong dst vid expression `%s'", + row->dstid()->toString().c_str()); } auto idStatus = SchemaUtil::toVertexID(row->srcid()); - if (!idStatus.ok()) { - return idStatus.status(); - } + NG_RETURN_IF_ERROR(idStatus); auto srcId = std::move(idStatus).value(); idStatus = SchemaUtil::toVertexID(row->dstid()); - if (!idStatus.ok()) { - return idStatus.status(); - } + NG_RETURN_IF_ERROR(idStatus); auto dstId = std::move(idStatus).value(); int64_t rank = row->rank(); @@ -238,22 +220,19 @@ Status InsertEdgesValidator::prepareEdges() {; for (auto &value : row->values()) { if (!evaluableExpr(value)) { LOG(ERROR) << "Insert wrong value: `" << value->toString() << "'."; - return Status::Error("Insert wrong value: `%s'.", value->toString().c_str()); + return Status::SemanticError("Insert wrong value: `%s'.", + value->toString().c_str()); } } auto valsRet = SchemaUtil::toValueVec(row->values()); - if (!valsRet.ok()) { - return valsRet.status(); - } + NG_RETURN_IF_ERROR(valsRet); auto values = std::move(valsRet).value(); std::vector props; for (auto index = 0u; index < propNames_.size(); index++) { auto schemaType = schema_->getFieldType(propNames_[index]); auto valueStatus = SchemaUtil::toSchemaValue(schemaType, values[index]); - if (!valueStatus.ok()) { - return valueStatus.status(); - } + NG_RETURN_IF_ERROR(valueStatus); props.emplace_back(std::move(valueStatus).value()); } @@ -284,36 +263,28 @@ Status DeleteVerticesValidator::validateImpl() { if (sentence->isRef()) { vidRef_ = sentence->vidRef(); auto type = deduceExprType(vidRef_); - if (!type.ok()) { - return type.status(); - } + NG_RETURN_IF_ERROR(type); if (type.value() != Value::Type::STRING) { std::stringstream ss; ss << "The vid should be string type, " << "but input is `" << type.value() << "'"; - return Status::Error(ss.str()); + return Status::SemanticError(ss.str()); } } else { auto vIds = sentence->vidList()->vidList(); for (auto vId : vIds) { auto idStatus = SchemaUtil::toVertexID(vId); - if (!idStatus.ok()) { - return idStatus.status(); - } + NG_RETURN_IF_ERROR(idStatus); vertices_.emplace_back(std::move(idStatus).value()); } } auto ret = qctx_->schemaMng()->getAllEdge(spaceId_); - if (!ret.ok()) { - return ret.status(); - } + NG_RETURN_IF_ERROR(ret); edgeNames_ = std::move(ret).value(); for (auto &name : edgeNames_) { auto edgeStatus = qctx_->schemaMng()->toEdgeType(spaceId_, name); - if (!edgeStatus.ok()) { - return edgeStatus.status(); - } + NG_RETURN_IF_ERROR(edgeStatus); auto edgeType = edgeStatus.value(); edgeTypes_.emplace_back(edgeType); } @@ -414,17 +385,12 @@ Status DeleteEdgesValidator::validateImpl() { auto sentence = static_cast(sentence_); auto spaceId = vctx_->whichSpace().id; auto edgeStatus = qctx_->schemaMng()->toEdgeType(spaceId, *sentence->edge()); - if (!edgeStatus.ok()) { - return edgeStatus.status(); - } + NG_RETURN_IF_ERROR(edgeStatus); auto edgeType = edgeStatus.value(); if (sentence->isRef()) { edgeKeyRefs_.emplace_back(sentence->edgeKeyRef()); (*edgeKeyRefs_.begin())->setType(new ConstantExpression(edgeType)); - auto status = checkInput(); - if (!status.ok()) { - return status; - } + NG_RETURN_IF_ERROR(checkInput()); } else { return buildEdgeKeyRef(sentence->edgeKeys()->keys(), edgeType); } @@ -440,13 +406,9 @@ Status DeleteEdgesValidator::buildEdgeKeyRef(const std::vector &edgeKe Row row; storage::cpp2::EdgeKey key; auto srcIdStatus = SchemaUtil::toVertexID(edgeKey->srcid()); - if (!srcIdStatus.ok()) { - return srcIdStatus.status(); - } + NG_RETURN_IF_ERROR(srcIdStatus); auto dstIdStatus = SchemaUtil::toVertexID(edgeKey->dstid()); - if (!dstIdStatus.ok()) { - return dstIdStatus.status(); - } + NG_RETURN_IF_ERROR(dstIdStatus); auto srcId = std::move(srcIdStatus).value(); auto dstId = std::move(dstIdStatus).value(); @@ -484,11 +446,11 @@ Status DeleteEdgesValidator::checkInput() { } if (!exprProps_.inputProps().empty() && !exprProps_.varProps().empty()) { - return Status::Error("Not support both input and variable."); + return Status::SemanticError("Not support both input and variable."); } if (!exprProps_.varProps().empty() && exprProps_.varProps().size() > 1) { - return Status::Error("Only one variable allowed to use."); + return Status::SemanticError("Only one variable allowed to use."); } auto status = deduceExprType(edgeKeyRef->srcid()); @@ -601,7 +563,7 @@ Status UpdateValidator::getUpdateProps() { if (symNames.size() != 1) { auto errorMsg = "Multi schema name: " + folly::join(",", symNames); LOG(ERROR) << errorMsg; - return Status::Error(std::move(errorMsg)); + return Status::SemanticError(std::move(errorMsg)); } if (symName != nullptr) { name_ = *symName; @@ -616,7 +578,7 @@ Status UpdateValidator::checkAndResetSymExpr(Expression* inExpr, bool hasWrongType = false; auto symExpr = rewriteSymExpr(inExpr, symName, hasWrongType, isEdge_); if (hasWrongType) { - return Status::Error("Has wrong expr in `%s'", + return Status::SemanticError("Has wrong expr in `%s'", inExpr->toString().c_str()); } if (symExpr != nullptr) { @@ -650,7 +612,7 @@ Status UpdateVertexValidator::validateImpl() { auto ret = qctx_->schemaMng()->toTagID(spaceId_, name_); if (!ret.ok()) { LOG(ERROR) << "No schema found for " << name_; - return Status::Error("No schema found for `%s'", name_.c_str()); + return Status::SemanticError("No schema found for `%s'", name_.c_str()); } tagId_ = ret.value(); return Status::OK(); @@ -692,7 +654,7 @@ Status UpdateEdgeValidator::validateImpl() { auto ret = qctx_->schemaMng()->toEdgeType(spaceId_, name_); if (!ret.ok()) { LOG(ERROR) << "No schema found for " << name_; - return Status::Error("No schema found for `%s'", name_.c_str()); + return Status::SemanticError("No schema found for `%s'", name_.c_str()); } edgeType_ = ret.value(); return Status::OK(); diff --git a/src/validator/ReportError.h b/src/validator/ReportError.h index 6d8d91e32..f8b10f2c1 100644 --- a/src/validator/ReportError.h +++ b/src/validator/ReportError.h @@ -20,12 +20,12 @@ class ReportError final : public Validator { private: Status validateImpl() override { - return Status::Error("Not support sentence type: %ld, query: %s", + return Status::SemanticError("Not support sentence type: %ld, query: %s", static_cast(sentence_->kind()), sentence_->toString().c_str()); } Status toPlan() override { - return Status::Error("Not support sentence type: %ld, query: %s", + return Status::SemanticError("Not support sentence type: %ld, query: %s", static_cast(sentence_->kind()), sentence_->toString().c_str()); } }; diff --git a/src/validator/SequentialValidator.cpp b/src/validator/SequentialValidator.cpp index 6d10ff818..bd4e59318 100644 --- a/src/validator/SequentialValidator.cpp +++ b/src/validator/SequentialValidator.cpp @@ -18,7 +18,7 @@ namespace graph { Status SequentialValidator::validateImpl() { Status status; if (sentence_->kind() != Sentence::Kind::kSequential) { - return Status::Error( + return Status::SemanticError( "Sequential validator validates a SequentialSentences, but %ld is given.", static_cast(sentence_->kind())); } @@ -26,7 +26,7 @@ Status SequentialValidator::validateImpl() { auto sentences = seqSentence->sentences(); if (sentences.size() > static_cast(FLAGS_max_allowed_statements)) { - return Status::Error("The maximum number of statements allowed has been exceeded"); + return Status::SemanticError("The maximum number of statements allowed has been exceeded"); } DCHECK(!sentences.empty()); diff --git a/src/validator/SetValidator.cpp b/src/validator/SetValidator.cpp index b3f99cd97..52fc8688c 100644 --- a/src/validator/SetValidator.cpp +++ b/src/validator/SetValidator.cpp @@ -70,7 +70,8 @@ Status SetValidator::toPlan() { break; } default: - return Status::Error("Unknown operator: %ld", static_cast(setSentence->op())); + return Status::SemanticError("Unknown operator: %ld", + static_cast(setSentence->op())); } bNode->setLeftVar(lRoot->outputVar()); diff --git a/src/validator/TraversalValidator.cpp b/src/validator/TraversalValidator.cpp index 74a7ffeef..25ba65b2f 100644 --- a/src/validator/TraversalValidator.cpp +++ b/src/validator/TraversalValidator.cpp @@ -13,13 +13,13 @@ namespace graph { Status TraversalValidator::validateStarts(const VerticesClause* clause, Starts& starts) { if (clause == nullptr) { - return Status::Error("From clause nullptr."); + return Status::SemanticError("From clause nullptr."); } if (clause->isRef()) { auto* src = clause->ref(); if (src->kind() != Expression::Kind::kInputProperty && src->kind() != Expression::Kind::kVarProperty) { - return Status::Error( + return Status::SemanticError( "`%s', Only input and variable expression is acceptable" " when starts are evaluated at runtime.", src->toString().c_str()); } else { @@ -34,7 +34,7 @@ Status TraversalValidator::validateStarts(const VerticesClause* clause, Starts& ss << "`" << src->toString() << "', the srcs should be type of " << meta::cpp2::_PropertyType_VALUES_TO_NAMES.at(vidType) << ", but was`" << type.value() << "'"; - return Status::Error(ss.str()); + return Status::SemanticError(ss.str()); } starts.srcRef = src; auto* propExpr = static_cast(src); @@ -48,7 +48,7 @@ Status TraversalValidator::validateStarts(const VerticesClause* clause, Starts& QueryExpressionContext ctx; for (auto* expr : vidList) { if (!evaluableExpr(expr)) { - return Status::Error("`%s' is not an evaluable expression.", + return Status::SemanticError("`%s' is not an evaluable expression.", expr->toString().c_str()); } auto vid = expr->eval(ctx(nullptr)); @@ -56,7 +56,7 @@ Status TraversalValidator::validateStarts(const VerticesClause* clause, Starts& if (!SchemaUtil::isValidVid(vid, vidType)) { std::stringstream ss; ss << "Vid should be a " << meta::cpp2::_PropertyType_VALUES_TO_NAMES.at(vidType); - return Status::Error(ss.str()); + return Status::SemanticError(ss.str()); } starts.vids.emplace_back(std::move(vid)); startVidList_->add(expr->clone().release()); @@ -67,7 +67,7 @@ Status TraversalValidator::validateStarts(const VerticesClause* clause, Starts& Status TraversalValidator::validateOver(const OverClause* clause, Over& over) { if (clause == nullptr) { - return Status::Error("Over clause nullptr."); + return Status::SemanticError("Over clause nullptr."); } over.direction = clause->direction(); @@ -77,13 +77,13 @@ Status TraversalValidator::validateOver(const OverClause* clause, Over& over) { NG_RETURN_IF_ERROR(allEdgeStatus); auto edges = std::move(allEdgeStatus).value(); if (edges.empty()) { - return Status::Error("No edge type found in space %s", + return Status::SemanticError("No edge type found in space `%s'", space_.name.c_str()); } for (auto edge : edges) { auto edgeType = schemaMng->toEdgeType(space_.id, edge); if (!edgeType.ok()) { - return Status::Error("%s not found in space [%s].", + return Status::SemanticError("`%s' not found in space [`%s'].", edge.c_str(), space_.name.c_str()); } over.edgeTypes.emplace_back(edgeType.value()); @@ -96,7 +96,7 @@ Status TraversalValidator::validateOver(const OverClause* clause, Over& over) { auto edgeName = *edge->edge(); auto edgeType = schemaMng->toEdgeType(space_.id, edgeName); if (!edgeType.ok()) { - return Status::Error("%s not found in space [%s].", + return Status::SemanticError("%s not found in space [%s].", edgeName.c_str(), space_.name.c_str()); } over.edgeTypes.emplace_back(edgeType.value()); @@ -107,7 +107,7 @@ Status TraversalValidator::validateOver(const OverClause* clause, Over& over) { Status TraversalValidator::validateStep(const StepClause* clause, Steps& step) { if (clause == nullptr) { - return Status::Error("Step clause nullptr."); + return Status::SemanticError("Step clause nullptr."); } if (clause->isMToN()) { auto* mToN = qctx_->objPool()->makeAndAdd(); @@ -122,8 +122,9 @@ Status TraversalValidator::validateStep(const StepClause* clause, Steps& step) { mToN->mSteps = 1; } if (mToN->nSteps < mToN->mSteps) { - return Status::Error("`%s', upper bound steps should be greater than lower bound.", - clause->toString().c_str()); + return Status::SemanticError( + "`%s', upper bound steps should be greater than lower bound.", + clause->toString().c_str()); } if (mToN->mSteps == mToN->nSteps) { steps_.steps = mToN->mSteps; diff --git a/src/validator/Validator.cpp b/src/validator/Validator.cpp index 867e4c2ff..3ad7fabcb 100644 --- a/src/validator/Validator.cpp +++ b/src/validator/Validator.cpp @@ -221,7 +221,7 @@ Status Validator::validate(Sentence* sentence, QueryContext* qctx) { auto root = validator->root(); if (!root) { - return Status::Error("Get null plan from sequential validator"); + return Status::SemanticError("Get null plan from sequential validator"); } qctx->plan()->setRoot(root); return Status::OK(); @@ -260,28 +260,17 @@ Status Validator::validate() { if (!noSpaceRequired_) { space_ = vctx_->whichSpace(); - VLOG(1) << "Space chosen, name: " << space_.spaceDesc.space_name - << " id: " << space_.id; + VLOG(1) << "Space chosen, name: " << space_.spaceDesc.space_name << " id: " << space_.id; } - auto status = validateImpl(); - if (!status.ok()) { - if (status.isSemanticError() || status.isPermissionError()) { - return status; - } - return Status::SemanticError(status.message()); - } + NG_RETURN_IF_ERROR(validateImpl()); // Execute after validateImpl because need field from it if (FLAGS_enable_authorize) { NG_RETURN_IF_ERROR(checkPermission()); } - status = toPlan(); - if (!status.ok()) { - if (status.isSemanticError()) return status; - return Status::SemanticError(status.message()); - } + NG_RETURN_IF_ERROR(toPlan()); return Status::OK(); } @@ -354,28 +343,27 @@ StatusOr Validator::checkRef(const Expression* ref, Value::Type typ ColDef col(*propExpr->prop(), type); const auto find = std::find(inputs_.begin(), inputs_.end(), col); if (find == inputs_.end()) { - return Status::Error("No input property %s", propExpr->prop()->c_str()); + return Status::SemanticError("No input property `%s'", propExpr->prop()->c_str()); } return inputVarName_; } else if (ref->kind() == Expression::Kind::kVarProperty) { const auto* propExpr = static_cast(ref); ColDef col(*propExpr->prop(), type); - const auto &outputVar = *propExpr->sym(); - const auto &var = vctx_->getVar(outputVar); + const auto& outputVar = *propExpr->sym(); + const auto& var = vctx_->getVar(outputVar); if (var.empty()) { - return Status::Error("No variable %s", outputVar.c_str()); + return Status::SemanticError("No variable `%s'", outputVar.c_str()); } const auto find = std::find(var.begin(), var.end(), col); if (find == var.end()) { - return Status::Error("No property %s in variable %s", - propExpr->prop()->c_str(), - outputVar.c_str()); + return Status::SemanticError( + "No property `%s' in variable `%s'", propExpr->prop()->c_str(), outputVar.c_str()); } return outputVar; } else { // it's guranteed by parser DLOG(FATAL) << "Unexpected expression " << ref->kind(); - return Status::Error("Unexpected expression."); + return Status::SemanticError("Unexpected expression."); } } diff --git a/src/validator/Validator.h b/src/validator/Validator.h index 15704f83f..db6874b0e 100644 --- a/src/validator/Validator.h +++ b/src/validator/Validator.h @@ -100,8 +100,8 @@ class Validator { bool evaluableExpr(const Expression* expr) const; static StatusOr checkPropNonexistOrDuplicate(const ColsDef& cols, - folly::StringPiece prop, - const std::string& validator); + folly::StringPiece prop, + const std::string& validator); static Status appendPlan(PlanNode* plan, PlanNode* appended); diff --git a/tests/query/v1/test_yield.py b/tests/query/v1/test_yield.py index 0082cc462..41b7491cc 100644 --- a/tests/query/v1/test_yield.py +++ b/tests/query/v1/test_yield.py @@ -321,12 +321,12 @@ def test_error(self): query = '''$var = GO FROM "Boris Diaw" OVER serve YIELD $^.player.name AS name, serve.start_year AS start, $$.team.name AS team; \ YIELD $$.a.team''' resp = self.execute_query(query) - self.check_resp_failed(resp, ttypes.ErrorCode.E_SEMANTIC_ERROR) + self.check_resp_failed(resp, ttypes.ErrorCode.E_EXECUTION_ERROR) query = '''$var = GO FROM "Boris Diaw" OVER serve YIELD $^.player.name AS name, serve.start_year AS start, $$.team.name AS team; \ YIELD $^.a.team''' resp = self.execute_query(query) - self.check_resp_failed(resp, ttypes.ErrorCode.E_SEMANTIC_ERROR) + self.check_resp_failed(resp, ttypes.ErrorCode.E_EXECUTION_ERROR) query = '''$var = GO FROM "Boris Diaw" OVER serve YIELD $^.player.name AS name, serve.start_year AS start, $$.team.name AS team; \ YIELD a.team''' diff --git a/tests/query/v2/test_explain.py b/tests/query/v2/test_explain.py index 28c762811..97944c433 100644 --- a/tests/query/v2/test_explain.py +++ b/tests/query/v2/test_explain.py @@ -16,7 +16,7 @@ ('FORMAT="row"', None), ('FORMAT="dot"', None), ('FORMAT="dot:struct"', None), - ('FORMAT="unknown"', ttypes.ErrorCode.E_SEMANTIC_ERROR), + ('FORMAT="unknown"', ttypes.ErrorCode.E_SYNTAX_ERROR), ] From f87d5900409fde199b7b42dc55a9b1e4e3c6f04c Mon Sep 17 00:00:00 2001 From: Shylock Hg <33566796+Shylock-Hg@users.noreply.github.com> Date: Sat, 24 Oct 2020 10:39:32 +0800 Subject: [PATCH 2/2] Add object time_utils. (#359) Co-authored-by: dutor <440396+dutor@users.noreply.github.com> --- src/context/test/CMakeLists.txt | 1 + src/daemons/CMakeLists.txt | 1 + src/executor/test/CMakeLists.txt | 1 + src/mock/test/CMakeLists.txt | 1 + src/optimizer/test/CMakeLists.txt | 1 + src/parser/test/CMakeLists.txt | 1 + src/util/test/CMakeLists.txt | 1 + src/validator/test/CMakeLists.txt | 1 + src/visitor/test/CMakeLists.txt | 1 + 9 files changed, 9 insertions(+) diff --git a/src/context/test/CMakeLists.txt b/src/context/test/CMakeLists.txt index 77621617c..0cc01868a 100644 --- a/src/context/test/CMakeLists.txt +++ b/src/context/test/CMakeLists.txt @@ -26,6 +26,7 @@ SET(CONTEXT_TEST_LIBS $ $ $ + $ $ $ $ diff --git a/src/daemons/CMakeLists.txt b/src/daemons/CMakeLists.txt index 7f76d5bf3..87371040a 100644 --- a/src/daemons/CMakeLists.txt +++ b/src/daemons/CMakeLists.txt @@ -56,6 +56,7 @@ nebula_add_executable( $ $ $ + $ LIBRARIES proxygenhttpserver proxygenlib diff --git a/src/executor/test/CMakeLists.txt b/src/executor/test/CMakeLists.txt index a92319281..4d3c80af4 100644 --- a/src/executor/test/CMakeLists.txt +++ b/src/executor/test/CMakeLists.txt @@ -31,6 +31,7 @@ SET(EXEC_QUERY_TEST_OBJS $ $ $ + $ $ $ $ diff --git a/src/mock/test/CMakeLists.txt b/src/mock/test/CMakeLists.txt index 88f56a44d..2a748be3f 100644 --- a/src/mock/test/CMakeLists.txt +++ b/src/mock/test/CMakeLists.txt @@ -50,6 +50,7 @@ set(GRAPH_TEST_LIB $ $ $ + $ ) nebula_add_test( diff --git a/src/optimizer/test/CMakeLists.txt b/src/optimizer/test/CMakeLists.txt index fcc235b5b..65b65760f 100644 --- a/src/optimizer/test/CMakeLists.txt +++ b/src/optimizer/test/CMakeLists.txt @@ -28,6 +28,7 @@ set(OPTIMIZER_TEST_LIB $ $ $ + $ $ $ $ diff --git a/src/parser/test/CMakeLists.txt b/src/parser/test/CMakeLists.txt index dc3f5053f..fce4289b3 100644 --- a/src/parser/test/CMakeLists.txt +++ b/src/parser/test/CMakeLists.txt @@ -29,6 +29,7 @@ set(PARSER_TEST_LIBS $ $ $ + $ $ $ $ diff --git a/src/util/test/CMakeLists.txt b/src/util/test/CMakeLists.txt index c2cb340d2..a1abf32ec 100644 --- a/src/util/test/CMakeLists.txt +++ b/src/util/test/CMakeLists.txt @@ -29,6 +29,7 @@ nebula_add_test( $ $ $ + $ $ $ $ diff --git a/src/validator/test/CMakeLists.txt b/src/validator/test/CMakeLists.txt index 8c4efbdbc..b93e73ea9 100644 --- a/src/validator/test/CMakeLists.txt +++ b/src/validator/test/CMakeLists.txt @@ -46,6 +46,7 @@ set(VALIDATOR_TEST_LIBS $ $ $ + $ ) nebula_add_test( diff --git a/src/visitor/test/CMakeLists.txt b/src/visitor/test/CMakeLists.txt index d3d5e5036..957452e78 100644 --- a/src/visitor/test/CMakeLists.txt +++ b/src/visitor/test/CMakeLists.txt @@ -46,6 +46,7 @@ nebula_add_test( $ $ $ + $ LIBRARIES gtest gtest_main