Skip to content

Commit

Permalink
Format subgraph (vesoft-inc#2555)
Browse files Browse the repository at this point in the history 
* format subgraph

* add test case

* format code

* fix unit test error

* fix test error

* change vertex, edge to vertices, edges

* compatible with  column names

* fix yield colname

* add more test

* fix test colname

* address comment
  • Loading branch information
@nevermore3
nevermore3 authored Sep 15, 2021
1 parent dbea9c7 commit b8f56fc
Show file tree
Hide file tree
Showing 16 changed files with 1,969 additions and 34 deletions.
3 changes: 3 additions & 0 deletions .gitignore
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,3 +62,6 @@ venv/

#lock
*.lock

#ctags
.tags
1 change: 1 addition & 0 deletions .linters/cpp/checkKeyword.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
'KW_DESCRIBE',
'KW_DESC',
'KW_VERTEX',
'KW_VERTICES',
'KW_EDGE',
'KW_EDGES',
'KW_UPDATE',
Expand Down
7 changes: 5 additions & 2 deletions src/graph/context/Iterator.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -338,7 +338,7 @@ Value GetNeighborsIter::getVertex() const {
}

auto vidVal = getColumn(nebula::kVid);
if (!SchemaUtil::isValidVid(vidVal)) {
if (UNLIKELY(!SchemaUtil::isValidVid(vidVal))) {
return Value::kNullBadType;
}
Vertex vertex;
Expand All @@ -348,7 +348,7 @@ Value GetNeighborsIter::getVertex() const {
auto& row = *currentRow_;
auto& tagPropNameList = tagProp.second.propList;
auto tagColId = tagProp.second.colIdx;
if (!row[tagColId].isList()) {
if (UNLIKELY(!row[tagColId].isList())) {
// Ignore the bad value.
continue;
}
Expand All @@ -358,6 +358,9 @@ Value GetNeighborsIter::getVertex() const {
Tag tag;
tag.name = tagProp.first;
for (size_t i = 0; i < propList.size(); ++i) {
if (tagPropNameList[i] == nebula::kTag) {
continue;
}
tag.props.emplace(tagPropNameList[i], propList[i]);
}
vertex.tags.emplace_back(std::move(tag));
Expand Down
5 changes: 4 additions & 1 deletion src/graph/context/ast/QueryAstContext.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -120,9 +120,12 @@ struct SubgraphContext final : public AstContext {
Starts from;
StepClause steps;
std::string loopSteps;

YieldColumns* yieldExpr;
std::vector<std::string> colNames;
std::unordered_set<EdgeType> edgeTypes;
bool withProp{false};
bool getVertexProp{false};
bool getEdgeProp{false};
};

} // namespace graph
Expand Down
10 changes: 3 additions & 7 deletions src/graph/executor/query/DataCollectExecutor.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,8 +64,6 @@ folly::Future<Status> DataCollectExecutor::doCollect() {
Status DataCollectExecutor::collectSubgraph(const std::vector<std::string>& vars) {
DataSet ds;
ds.colNames = std::move(colNames_);
// the subgraph not need duplicate vertices or edges, so dedup here directly
std::unordered_set<Value> uniqueVids;
std::unordered_set<std::tuple<Value, EdgeType, EdgeRanking, Value>> uniqueEdges;
for (auto i = vars.begin(); i != vars.end(); ++i) {
const auto& hist = ectx_->getHistory(*i);
Expand All @@ -84,16 +82,14 @@ Status DataCollectExecutor::collectSubgraph(const std::vector<std::string>& vars
auto* gnIter = static_cast<GetNeighborsIter*>(iter.get());
auto originVertices = gnIter->getVertices();
for (auto& v : originVertices.values) {
if (!v.isVertex()) {
if (UNLIKELY(!v.isVertex())) {
continue;
}
if (uniqueVids.emplace(v.getVertex().vid).second) {
vertices.emplace_back(std::move(v));
}
vertices.emplace_back(std::move(v));
}
auto originEdges = gnIter->getEdges();
for (auto& edge : originEdges.values) {
if (!edge.isEdge()) {
if (UNLIKELY(!edge.isEdge())) {
continue;
}
const auto& e = edge.getEdge();
Expand Down
15 changes: 9 additions & 6 deletions src/graph/planner/ngql/SubgraphPlanner.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ namespace graph {

StatusOr<std::unique_ptr<std::vector<EdgeProp>>> SubgraphPlanner::buildEdgeProps() {
auto* qctx = subgraphCtx_->qctx;
auto withProp = subgraphCtx_->withProp;
bool getEdgeProp = subgraphCtx_->withProp && subgraphCtx_->getEdgeProp;
const auto& space = subgraphCtx_->space;
auto& edgeTypes = subgraphCtx_->edgeTypes;

Expand All @@ -31,7 +31,7 @@ StatusOr<std::unique_ptr<std::vector<EdgeProp>>> SubgraphPlanner::buildEdgeProps
}
}
std::vector<EdgeType> vEdgeTypes(edgeTypes.begin(), edgeTypes.end());
auto edgeProps = SchemaUtil::getEdgeProps(qctx, space, std::move(vEdgeTypes), withProp);
auto edgeProps = SchemaUtil::getEdgeProps(qctx, space, std::move(vEdgeTypes), getEdgeProp);
NG_RETURN_IF_ERROR(edgeProps);
return edgeProps;
}
Expand All @@ -55,7 +55,8 @@ StatusOr<SubPlan> SubgraphPlanner::nSteps(SubPlan& startVidPlan, const std::stri
const auto& steps = subgraphCtx_->steps;

auto* startNode = StartNode::make(qctx);
auto vertexProps = SchemaUtil::getAllVertexProp(qctx, space, subgraphCtx_->withProp);
bool getVertexProp = subgraphCtx_->withProp && subgraphCtx_->getVertexProp;
auto vertexProps = SchemaUtil::getAllVertexProp(qctx, space, getVertexProp);
NG_RETURN_IF_ERROR(vertexProps);
auto edgeProps = buildEdgeProps();
NG_RETURN_IF_ERROR(edgeProps);
Expand All @@ -78,10 +79,12 @@ StatusOr<SubPlan> SubgraphPlanner::nSteps(SubPlan& startVidPlan, const std::stri
auto* dc = DataCollect::make(qctx, DataCollect::DCKind::kSubgraph);
dc->addDep(loop);
dc->setInputVars({gn->outputVar(), oneMoreStepOutput});
dc->setColNames({kVertices, kEdges});
dc->setColNames({"VERTICES", "EDGES"});

auto* project = Project::make(qctx, dc, subgraphCtx_->yieldExpr);

SubPlan subPlan;
subPlan.root = dc;
subPlan.root = project;
subPlan.tail = startVidPlan.tail != nullptr ? startVidPlan.tail : loop;
return subPlan;
}
Expand All @@ -106,7 +109,7 @@ StatusOr<SubPlan> SubgraphPlanner::zeroStep(SubPlan& startVidPlan, const std::st
auto* func = AggregateExpression::make(pool, "COLLECT", vertexExpr, false);

auto* collect = Aggregate::make(qctx, getVertex, {}, {func});
collect->setColNames({kVertices});
collect->setColNames(std::move(subgraphCtx_->colNames));

SubPlan subPlan;
subPlan.root = collect;
Expand Down
48 changes: 41 additions & 7 deletions src/graph/validator/GetSubgraphValidator.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,13 +29,7 @@ Status GetSubgraphValidator::validateImpl() {
NG_RETURN_IF_ERROR(validateInBound(gsSentence->in()));
NG_RETURN_IF_ERROR(validateOutBound(gsSentence->out()));
NG_RETURN_IF_ERROR(validateBothInOutBound(gsSentence->both()));
// set output col & type
if (subgraphCtx_->steps.steps() == 0) {
outputs_.emplace_back(kVertices, Value::Type::VERTEX);
} else {
outputs_.emplace_back(kVertices, Value::Type::VERTEX);
outputs_.emplace_back(kEdges, Value::Type::EDGE);
}
NG_RETURN_IF_ERROR(validateYield(gsSentence->yield()));
return Status::OK();
}

Expand Down Expand Up @@ -102,7 +96,47 @@ Status GetSubgraphValidator::validateBothInOutBound(BothInOutClause* out) {
edgeTypes.emplace(v);
}
}
return Status::OK();
}

Status GetSubgraphValidator::validateYield(YieldClause* yield) {
auto pool = qctx_->objPool();
if (yield == nullptr) {
// version 3.0: return Status::SemanticError("No Yield Clause");
auto* yieldColumns = new YieldColumns();
auto* vertex = new YieldColumn(LabelExpression::make(pool, "_vertices"));
yieldColumns->addColumn(vertex);
if (subgraphCtx_->steps.steps() != 0) {
auto* edge = new YieldColumn(LabelExpression::make(pool, "_edges"));
yieldColumns->addColumn(edge);
}
yield = pool->add(new YieldClause(yieldColumns));
}
auto size = yield->columns().size();
outputs_.reserve(size);
YieldColumns* newCols = pool->add(new YieldColumns());

for (const auto& col : yield->columns()) {
std::string lowerStr = col->expr()->toString();
folly::toLowerAscii(lowerStr);
if (lowerStr == "vertices" || lowerStr == "_vertices") {
subgraphCtx_->getVertexProp = true;
auto* newCol = new YieldColumn(InputPropertyExpression::make(pool, "VERTICES"), col->name());
newCols->addColumn(newCol);
} else if (lowerStr == "edges" || lowerStr == "_edges") {
if (subgraphCtx_->steps.steps() == 0) {
return Status::SemanticError("Get Subgraph 0 STEPS only support YIELD vertices");
}
subgraphCtx_->getEdgeProp = true;
auto* newCol = new YieldColumn(InputPropertyExpression::make(pool, "EDGES"), col->name());
newCols->addColumn(newCol);
} else {
return Status::SemanticError("Get Subgraph only support YIELD vertices OR edges");
}
outputs_.emplace_back(col->name(), Value::Type::LIST);
}
subgraphCtx_->yieldExpr = newCols;
subgraphCtx_->colNames = getOutColNames();
return Status::OK();
}

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

Status validateBothInOutBound(BothInOutClause* out);

Status validateYield(YieldClause* yield);

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

private:
Expand Down
47 changes: 46 additions & 1 deletion src/graph/validator/test/GetSubgraphValidatorTest.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ TEST_F(GetSubgraphValidatorTest, Base) {
{
std::string query = "GET SUBGRAPH FROM \"1\"";
std::vector<PlanNode::Kind> expected = {
PK::kProject,
PK::kDataCollect,
PK::kLoop,
PK::kStart,
Expand All @@ -34,6 +35,7 @@ TEST_F(GetSubgraphValidatorTest, Base) {
{
std::string query = "GET SUBGRAPH WITH PROP 3 STEPS FROM \"1\"";
std::vector<PlanNode::Kind> expected = {
PK::kProject,
PK::kDataCollect,
PK::kLoop,
PK::kStart,
Expand All @@ -44,8 +46,9 @@ TEST_F(GetSubgraphValidatorTest, Base) {
EXPECT_TRUE(checkResult(query, expected));
}
{
std::string query = "GET SUBGRAPH WITH PROP FROM \"1\" BOTH like";
std::string query = "GET SUBGRAPH WITH PROP FROM \"1\" BOTH like YIELD vertices AS a";
std::vector<PlanNode::Kind> expected = {
PK::kProject,
PK::kDataCollect,
PK::kLoop,
PK::kStart,
Expand All @@ -58,6 +61,7 @@ TEST_F(GetSubgraphValidatorTest, Base) {
{
std::string query = "GET SUBGRAPH WITH PROP FROM \"1\", \"2\" IN like";
std::vector<PlanNode::Kind> expected = {
PK::kProject,
PK::kDataCollect,
PK::kLoop,
PK::kStart,
Expand All @@ -75,6 +79,7 @@ TEST_F(GetSubgraphValidatorTest, Input) {
"GO FROM \"1\" OVER like YIELD like._src AS src | GET SUBGRAPH WITH "
"PROP FROM $-.src";
std::vector<PlanNode::Kind> expected = {
PK::kProject,
PK::kDataCollect,
PK::kLoop,
PK::kDedup,
Expand All @@ -93,6 +98,7 @@ TEST_F(GetSubgraphValidatorTest, Input) {
"$a = GO FROM \"1\" OVER like YIELD like._src AS src; GET SUBGRAPH "
"FROM $a.src";
std::vector<PlanNode::Kind> expected = {
PK::kProject,
PK::kDataCollect,
PK::kLoop,
PK::kDedup,
Expand Down Expand Up @@ -156,6 +162,45 @@ TEST_F(GetSubgraphValidatorTest, Input) {
}
}

TEST_F(GetSubgraphValidatorTest, invalidYield) {
{
std::string query = "GET SUBGRAPH WITH PROP FROM \"Tim Duncan\" YIELD vertice";
auto result = checkResult(query);
EXPECT_EQ(std::string(result.message()),
"SemanticError: Get Subgraph only support YIELD vertices OR edges");
}
{
std::string query = "GET SUBGRAPH WITH PROP FROM \"Tim Duncan\" YIELD vertices";
auto result = checkResult(query);
EXPECT_EQ(std::string(result.message()),
"SyntaxError: please add alias when using vertices. near `vertices'");
}
{
std::string query = "GET SUBGRAPH WITH PROP FROM \"Tim Duncan\" YIELD vertices as a, edge";
auto result = checkResult(query);
EXPECT_EQ(std::string(result.message()),
"SyntaxError: please add alias when using edge. near `edge'");
}
{
std::string query = "GET SUBGRAPH WITH PROP FROM \"Tim Duncan\" YIELD vertices as a, edges";
auto result = checkResult(query);
EXPECT_EQ(std::string(result.message()),
"SyntaxError: please add alias when using edges. near `edges'");
}
{
std::string query = "GET SUBGRAPH WITH PROP FROM \"Tim Duncan\" YIELD path";
auto result = checkResult(query);
EXPECT_EQ(std::string(result.message()),
"SemanticError: Get Subgraph only support YIELD vertices OR edges");
}
{
std::string query = "GET SUBGRAPH WITH PROP FROM \"Tim Duncan\" YIELD 123";
auto result = checkResult(query);
EXPECT_EQ(std::string(result.message()),
"SemanticError: Get Subgraph only support YIELD vertices OR edges");
}
}

TEST_F(GetSubgraphValidatorTest, RefNotExist) {
{
std::string query = "GET SUBGRAPH WITH PROP FROM $-.id";
Expand Down
4 changes: 4 additions & 0 deletions src/parser/TraverseSentences.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -257,6 +257,10 @@ std::string GetSubgraphSentence::toString() const {
buf += " ";
buf += both_->toString();
}
if (yield_ != nullptr) {
buf += " ";
buf += yield_->toString();
}
return buf;
}
} // namespace nebula
7 changes: 6 additions & 1 deletion src/parser/TraverseSentences.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -432,14 +432,16 @@ class GetSubgraphSentence final : public Sentence {
FromClause* from,
InBoundClause* in,
OutBoundClause* out,
BothInOutClause* both) {
BothInOutClause* both,
YieldClause* yield) {
kind_ = Kind::kGetSubgraph;
withProp_ = withProp;
step_.reset(step);
from_.reset(from);
in_.reset(in);
out_.reset(out);
both_.reset(both);
yield_.reset(yield);
}

StepClause* step() const { return step_.get(); }
Expand All @@ -454,6 +456,8 @@ class GetSubgraphSentence final : public Sentence {

BothInOutClause* both() const { return both_.get(); }

YieldClause* yield() const { return yield_.get(); }

std::string toString() const override;

private:
Expand All @@ -463,6 +467,7 @@ class GetSubgraphSentence final : public Sentence {
std::unique_ptr<InBoundClause> in_;
std::unique_ptr<OutBoundClause> out_;
std::unique_ptr<BothInOutClause> both_;
std::unique_ptr<YieldClause> yield_;
};
} // namespace nebula
#endif // PARSER_TRAVERSESENTENCES_H_
Loading

0 comments on commit b8f56fc

Please sign in to comment.