Skip to content

Commit

Permalink
fix error
Browse files Browse the repository at this point in the history
  • Loading branch information
@nevermore3
nevermore3 committed Sep 26, 2021
1 parent ddb47a3 commit 570d5f4
Show file tree
Hide file tree
Showing 11 changed files with 96 additions and 102 deletions.
2 changes: 1 addition & 1 deletion src/graph/context/ast/QueryAstContext.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ struct FetchEdgesContext final : public AstContext {

ExpressionProps exprProps;
YieldColumns* yieldExpr{nullptr};

std::string edgeName;
bool distinct{false};
// store the result of the previous sentence
std::string inputVarName;
Expand Down
28 changes: 20 additions & 8 deletions src/graph/planner/ngql/FetchEdgesPlanner.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,18 +5,12 @@
*/

#include "graph/planner/ngql/FetchEdgesPlanner.h"

#include "graph/util/ExpressionUtils.h"
#include "graph/util/QueryUtil.h"
#include "graph/util/SchemaUtil.h"
#include "graph/validator/Validator.h"

namespace nebula {
namespace graph {

std::unique_ptr<FetchEdgesPlanner::EdgeProps> FetchEdgesPlanner::buildEdgeProps() {
auto eProps = std::make_unique<EdgeProps>();
auto edgePropsMap = fetchCtx_->exprProps.edgeProps();
const auto &edgePropsMap = fetchCtx_->exprProps.edgeProps();
for (const auto &edgeProp : edgePropsMap) {
EdgeProp ep;
ep.set_type(edgeProp.first);
Expand All @@ -26,6 +20,22 @@ std::unique_ptr<FetchEdgesPlanner::EdgeProps> FetchEdgesPlanner::buildEdgeProps(
return eProps;
}

Expression *FetchEdgesPlanner::emptyEdgeFilter() {
auto *pool = fetchCtx_->qctx->objPool();
const auto &edgeName = fetchCtx_->edgeName;
auto notEmpty = [&pool](Expression *expr) {
return RelationalExpression::makeNE(pool, ConstantExpression::make(pool, Value::kEmpty), expr);
};
auto exprAnd = [&pool](Expression *left, Expression *right) {
return LogicalExpression::makeAnd(pool, left, right);
};

auto *srcNotEmpty = notEmpty(EdgeSrcIdExpression::make(pool, edgeName));
auto *dstNotEmpty = notEmpty(EdgeDstIdExpression::make(pool, edgeName));
auto *rankNotEmpty = notEmpty(EdgeRankExpression::make(pool, edgeName));
return exprAnd(srcNotEmpty, exprAnd(dstNotEmpty, rankNotEmpty));
}

StatusOr<SubPlan> FetchEdgesPlanner::transform(AstContext *astCtx) {
fetchCtx_ = static_cast<FetchEdgesContext *>(astCtx);
auto qctx = fetchCtx_->qctx;
Expand All @@ -44,7 +54,9 @@ StatusOr<SubPlan> FetchEdgesPlanner::transform(AstContext *astCtx) {
fetchCtx_->distinct);
getEdges->setInputVar(fetchCtx_->inputVarName);

subPlan.root = Project::make(qctx, getEdges, fetchCtx_->yieldExpr);
subPlan.root = Filter::make(qctx, getEdges, emptyEdgeFilter());

subPlan.root = Project::make(qctx, subPlan.root, fetchCtx_->yieldExpr);
if (fetchCtx_->distinct) {
subPlan.root = Dedup::make(qctx, subPlan.root);
}
Expand Down
2 changes: 2 additions & 0 deletions src/graph/planner/ngql/FetchEdgesPlanner.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ class FetchEdgesPlanner final : public Planner {

std::unique_ptr<EdgeProps> buildEdgeProps();

Expression* emptyEdgeFilter();

private:
FetchEdgesPlanner() = default;

Expand Down
62 changes: 37 additions & 25 deletions src/graph/validator/FetchEdgesValidator.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,16 @@ namespace nebula {
namespace graph {

Status FetchEdgesValidator::validateImpl() {
auto *fsentence = static_cast<FetchEdgesSentence *>(sentence_);
edgeName_ = fsentence->edgeName();
auto *sentence = static_cast<FetchEdgesSentence *>(sentence_);
if (sentence->edgeSize() != 1) {
return Status::SemanticError("only allow fetch on one edge");
}
edgeName_ = sentence->edgeName();
fetchCtx_ = getContext<FetchEdgesContext>();
NG_RETURN_IF_ERROR(validateEdgeName());
NG_RETURN_IF_ERROR(validateEdgeKey());
NG_RETURN_IF_ERROR(validateYield(fsentence->yieldClause()));
NG_RETURN_IF_ERROR(validateYield(sentence->yieldClause()));
fetchCtx_->edgeName = std::move(edgeName_);
return Status::OK();
}

Expand All @@ -40,7 +44,7 @@ StatusOr<std::string> FetchEdgesValidator::validateEdgeRef(const Expression *exp
Value::Type type) {
const auto &kind = expr->kind();
if (kind != Expression::Kind::kInputProperty && kind != EdgeExpression::Kind::kVarProperty) {
return Status::SemanticError("`%s', Only input and variable expression is acceptable",
return Status::SemanticError("`%s', only input and variable expression is acceptable",
expr->toString().c_str());
}
auto exprType = deduceExprType(expr);
Expand Down Expand Up @@ -74,15 +78,19 @@ Status FetchEdgesValidator::validateEdgeKey() {
result = validateEdgeRef(rankExpr, Value::Type::INT);
NG_RETURN_IF_ERROR(result);
if (inputVarName != result.value()) {
return Status::SemanticError("Can't refer to different variable as key at same time.");
return Status::SemanticError(
"`%s' the src dst and rank of the edge must use the same reference.",
rankExpr->toString().c_str());
}
}

auto *dstExpr = sentence->ref()->dstid();
result = validateEdgeRef(dstExpr, vidType_);
NG_RETURN_IF_ERROR(result);
if (inputVarName != result.value()) {
return Status::SemanticError("Can't refer to different variable as key at same time.");
return Status::SemanticError(
"`%s' the src dst and rank of the edge must use the same reference.",
dstExpr->toString().c_str());
}
fetchCtx_->src = srcExpr;
fetchCtx_->dst = dstExpr;
Expand All @@ -107,6 +115,7 @@ Status FetchEdgesValidator::validateEdgeKey() {
return Status::SemanticError(ss.str());
}
auto ranking = key->rank();

if (!evaluableExpr(key->dstid())) {
return Status::SemanticError("`%s' is not evaluable.", key->dstid()->toString().c_str());
}
Expand Down Expand Up @@ -142,29 +151,20 @@ void FetchEdgesValidator::extractEdgeProp(ExpressionProps &exprProps) {

Status FetchEdgesValidator::validateYield(const YieldClause *yield) {
auto pool = qctx_->objPool();
fetchCtx_->distinct = yield->isDistinct();
bool existEdge = false;
bool noYield = false;
if (yield == nullptr) {
// TODO: compatible with previous version, this will be deprecated in version 3.0.
auto *yieldColumns = new YieldColumns();
auto *edge = new YieldColumn(EdgeExpression::make(pool));
auto *edge = new YieldColumn(EdgeExpression::make(pool), "edges_");
yieldColumns->addColumn(edge);
yield = pool->add(new YieldClause(yieldColumns));
existEdge = true;
}
for (const auto &col : yield->columns()) {
if (col->expr()->kind() == Expression::Kind::kEdge) {
existEdge = true;
break;
}
// TODO return error when yield vertex
noYield = true;
}
auto size = yield->columns().size();
outputs_.reserve(size + 3);
fetchCtx_->distinct = yield->isDistinct();

ExpressionProps exprProps;
auto &exprProps = fetchCtx_->exprProps;
auto *newCols = pool->add(new YieldColumns());
if (!existEdge) {
if (!noYield) {
auto *src = new YieldColumn(EdgeSrcIdExpression::make(pool, edgeName_));
auto *dst = new YieldColumn(EdgeDstIdExpression::make(pool, edgeName_));
auto *rank = new YieldColumn(EdgeRankExpression::make(pool, edgeName_));
Expand All @@ -177,18 +177,30 @@ Status FetchEdgesValidator::validateYield(const YieldClause *yield) {
exprProps.insertEdgeProp(edgeType_, kSrc);
exprProps.insertEdgeProp(edgeType_, kDst);
exprProps.insertEdgeProp(edgeType_, kRank);
} else {
extractEdgeProp(exprProps);
}

for (const auto &col : yield->columns()) {
if (ExpressionUtils::hasAny(col->expr(), {Expression::Kind::kEdge})) {
extractEdgeProp(exprProps);
break;
}
}
auto size = yield->columns().size();
outputs_.reserve(size + 3);

for (auto col : yield->columns()) {
if (ExpressionUtils::hasAny(col->expr(),
{Expression::Kind::kVertex, Expression::Kind::kPathBuild})) {
return Status::SemanticError("illegal yield clauses `%s'", col->toString().c_str());
}
col->setExpr(ExpressionUtils::rewriteLabelAttr2EdgeProp(col->expr()));
NG_RETURN_IF_ERROR(ValidateUtil::invalidLabelIdentifiers(col->expr()));

auto colExpr = col->expr();
auto typeStatus = deduceExprType(colExpr);
NG_RETURN_IF_ERROR(typeStatus);
outputs_.emplace_back(col->name(), typeStatus.value());
newCols->addColumn(col->clone().release());

NG_RETURN_IF_ERROR(deduceProps(colExpr, exprProps));
}
Expand All @@ -203,10 +215,10 @@ Status FetchEdgesValidator::validateYield(const YieldClause *yield) {

for (const auto &edge : exprProps.edgeProps()) {
if (edge.first != edgeType_) {
return Status::SemanticError("MisMatch edge name");
return Status::SemanticError("should use edge name `%s'", edgeName_.c_str());
}
}

fetchCtx_->yieldExpr = newCols;
return Status::OK();
}

Expand Down
2 changes: 2 additions & 0 deletions src/graph/validator/FetchEdgesValidator.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ class FetchEdgesValidator final : public Validator {

Status validateYield(const YieldClause* yieldClause);

AstContext* getAstContext() override { return fetchCtx_.get(); }

private:
std::string edgeName_;

Expand Down
75 changes: 20 additions & 55 deletions src/graph/validator/test/FetchEdgesTest.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,10 @@ class FetchEdgesValidatorTest : public ValidatorTestBase {
};

TEST_F(FetchEdgesValidatorTest, FetchEdgesProp) {
auto src = VariablePropertyExpression::make(pool_.get(), "_VAR1_", kSrc);
auto src = ColumnExpression::make(pool_.get(), 0);
auto type = VariablePropertyExpression::make(pool_.get(), "_VAR2_", kType);
auto rank = VariablePropertyExpression::make(pool_.get(), "_VAR3_", kRank);
auto dst = VariablePropertyExpression::make(pool_.get(), "_VAR4_", kDst);
auto rank = ColumnExpression::make(pool_.get(), 1);
auto dst = ColumnExpression::make(pool_.get(), 2);
{
auto qctx = getQCtx("FETCH PROP ON like \"1\"->\"2\"");
auto *pool = qctx->objPool();
Expand All @@ -35,22 +35,12 @@ TEST_F(FetchEdgesValidatorTest, FetchEdgesProp) {
auto edgeType = edgeTypeResult.value();
storage::cpp2::EdgeProp prop;
prop.set_type(edgeType);
prop.set_props({kSrc, kDst, kRank, kType, "start", "end", "likeness"});
std::set<std::string> propNames{kSrc, kDst, kRank, kType, "start", "end", "likeness"};
prop.set_props(std::vector<std::string>(propNames.begin(), propNames.end()));
auto props = std::make_unique<std::vector<storage::cpp2::EdgeProp>>();
props->emplace_back(std::move(prop));
auto *ge = GetEdges::make(qctx, start, 1, src, type, rank, dst, std::move(props));
std::vector<std::string> colNames{std::string("like.") + kSrc,
std::string("like.") + kDst,
std::string("like.") + kRank,
std::string("like.") + kType,
"like.start",
"like.end",
"like.likeness"};
ge->setColNames(colNames);

// filter
auto *filter = Filter::make(qctx, ge, nullptr /*TODO*/);
filter->setColNames(colNames);

// project
auto yieldColumns = std::make_unique<YieldColumns>();
Expand All @@ -71,7 +61,8 @@ TEST_F(FetchEdgesValidatorTest, FetchEdgesProp) {
auto edgeType = edgeTypeResult.value();
storage::cpp2::EdgeProp prop;
prop.set_type(edgeType);
prop.set_props({kSrc, kDst, kRank, "start", "end"});
std::set<std::string> propNames{kSrc, kDst, kRank, "start", "end"};
prop.set_props(std::vector<std::string>(propNames.begin(), propNames.end()));
auto props = std::make_unique<std::vector<storage::cpp2::EdgeProp>>();
props->emplace_back(std::move(prop));
auto exprs = std::make_unique<std::vector<storage::cpp2::Expr>>();
Expand All @@ -83,16 +74,7 @@ TEST_F(FetchEdgesValidatorTest, FetchEdgesProp) {
exprs->emplace_back(std::move(expr2));
auto *ge =
GetEdges::make(qctx, start, 1, src, type, rank, dst, std::move(props), std::move(exprs));
std::vector<std::string> colNames{std::string("like.") + kSrc,
std::string("like.") + kDst,
std::string("like.") + kRank,
"like.start",
"like.end"};
ge->setColNames(colNames);

// filter
auto *filter = Filter::make(qctx, ge, nullptr /*TODO*/);
filter->setColNames(colNames);

// Project
auto yieldColumns = std::make_unique<YieldColumns>();
Expand Down Expand Up @@ -122,7 +104,8 @@ TEST_F(FetchEdgesValidatorTest, FetchEdgesProp) {
auto edgeType = edgeTypeResult.value();
storage::cpp2::EdgeProp prop;
prop.set_type(edgeType);
prop.set_props({kSrc, kDst, kRank, "start", "end"});
std::set<std::string> propNames{kSrc, kDst, kRank, "start", "end"};
prop.set_props(std::vector<std::string>(propNames.begin(), propNames.end()));
auto props = std::make_unique<std::vector<storage::cpp2::EdgeProp>>();
props->emplace_back(std::move(prop));
auto exprs = std::make_unique<std::vector<storage::cpp2::Expr>>();
Expand All @@ -134,16 +117,7 @@ TEST_F(FetchEdgesValidatorTest, FetchEdgesProp) {
exprs->emplace_back(std::move(expr2));
auto *ge =
GetEdges::make(qctx, start, 1, src, type, rank, dst, std::move(props), std::move(exprs));
std::vector<std::string> colNames{std::string("like.") + kSrc,
std::string("like.") + kDst,
std::string("like.") + kRank,
"like.start",
"like.end"};
ge->setColNames(colNames);

// filter
auto *filter = Filter::make(qctx, ge, nullptr /*TODO*/);
filter->setColNames(colNames);

// Project
auto yieldColumns = std::make_unique<YieldColumns>();
Expand Down Expand Up @@ -175,8 +149,8 @@ TEST_F(FetchEdgesValidatorTest, FetchEdgesProp) {
auto edgeType = edgeTypeResult.value();
storage::cpp2::EdgeProp prop;
prop.set_type(edgeType);
std::vector<std::string> propsName;
prop.set_props({kSrc, kDst, kRank, "start", "end"});
std::set<std::string> propNames{kSrc, kDst, kRank, "start", "end"};
prop.set_props(std::vector<std::string>(propNames.begin(), propNames.end()));
auto props = std::make_unique<std::vector<storage::cpp2::EdgeProp>>();
props->emplace_back(std::move(prop));
auto exprs = std::make_unique<std::vector<storage::cpp2::Expr>>();
Expand All @@ -188,16 +162,7 @@ TEST_F(FetchEdgesValidatorTest, FetchEdgesProp) {
exprs->emplace_back(std::move(expr1));
auto *ge =
GetEdges::make(qctx, start, 1, src, type, rank, dst, std::move(props), std::move(exprs));
std::vector<std::string> colNames{std::string("like.") + kSrc,
std::string("like.") + kDst,
std::string("like.") + kRank,
"like.start",
"like.end"};
ge->setColNames(colNames);

// filter
auto *filter = Filter::make(qctx, ge, nullptr /*TODO*/);
filter->setColNames(colNames);

// project, TODO(shylock) it's could push-down to storage if it supported
auto yieldColumns = std::make_unique<YieldColumns>();
Expand Down Expand Up @@ -228,7 +193,8 @@ TEST_F(FetchEdgesValidatorTest, FetchEdgesProp) {
auto edgeType = edgeTypeResult.value();
storage::cpp2::EdgeProp prop;
prop.set_type(edgeType);
prop.set_props({kSrc, kDst, kRank, "start", "end"});
std::set<std::string> propNames{kSrc, kDst, kRank, "start", "end"};
prop.set_props(std::vector<std::string>(propNames.begin(), propNames.end()));
auto props = std::make_unique<std::vector<storage::cpp2::EdgeProp>>();
props->emplace_back(std::move(prop));
auto exprs = std::make_unique<std::vector<storage::cpp2::Expr>>();
Expand All @@ -246,11 +212,9 @@ TEST_F(FetchEdgesValidatorTest, FetchEdgesProp) {
std::string("like.") + kRank,
"like.start",
"like.end"};
ge->setColNames(colNames);

// filter
auto *filter = Filter::make(qctx, ge, nullptr /*TODO*/);
filter->setColNames(colNames);

// project
auto yieldColumns = std::make_unique<YieldColumns>();
Expand All @@ -260,14 +224,9 @@ TEST_F(FetchEdgesValidatorTest, FetchEdgesProp) {
yieldColumns->addColumn(new YieldColumn(EdgePropertyExpression::make(pool, "like", "start")));
yieldColumns->addColumn(new YieldColumn(EdgePropertyExpression::make(pool, "like", "end")));
auto *project = Project::make(qctx, filter, yieldColumns.get());
project->setColNames({std::string("like.") + kSrc,
std::string("like.") + kDst,
std::string("like.") + kRank,
"like.start",
"like.end"});
project->setColNames(colNames);
// dedup
auto *dedup = Dedup::make(qctx, project);
dedup->setColNames(colNames);

// data collect
auto *dataCollect = DataCollect::make(qctx, DataCollect::DCKind::kRowBasedMove);
Expand Down Expand Up @@ -359,6 +318,12 @@ TEST_F(FetchEdgesValidatorTest, FetchEdgesPropFailed) {
// mismatched tag
ASSERT_FALSE(validate("FETCH PROP ON edge1 \"1\"->\"2\" YIELD edge2.prop2"));

ASSERT_FALSE(validate("FETCH PROP ON edge1 \"1\"->\"2\" YIELD edge"));
ASSERT_FALSE(validate("FETCH PROP ON edge1 \"1\"->\"2\" YIELD vertex"));
ASSERT_FALSE(validate("FETCH PROP ON edge1 \"1\"->\"2\" YIELD vertex as a, edge1.prop1"));
ASSERT_FALSE(validate("FETCH PROP ON edge1 \"1\"->\"2\" YIELD vertex as b"));
ASSERT_FALSE(validate("FETCH PROP ON edge1 \"1\"->\"2\" YIELD path as a"));

// notexist edge
ASSERT_FALSE(validate("FETCH PROP ON not_exist_edge \"1\"->\"2\" YIELD not_exist_edge.prop1"));

Expand Down
Loading

0 comments on commit 570d5f4

Please sign in to comment.