Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Format subgraph #2555

Merged
merged 12 commits into from
Sep 15, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .gitignore
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
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
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
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
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;
Shylock-Hg marked this conversation as resolved.
Show resolved Hide resolved
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
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
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") {
nevermore3 marked this conversation as resolved.
Show resolved Hide resolved
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
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
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
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
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