Format subgraph (#2555)

* 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
vesoft-inc · Sep 15, 2021 · b8f56fc · b8f56fc
1 parent dbea9c7
commit b8f56fc
Show file tree

Hide file tree

Showing 16 changed files with 1,969 additions and 34 deletions.
diff --git a/.gitignore b/.gitignore
@@ -62,3 +62,6 @@ venv/
 
 #lock
 *.lock
+
+#ctags
+.tags
diff --git a/.linters/cpp/checkKeyword.py b/.linters/cpp/checkKeyword.py
@@ -30,6 +30,7 @@
     'KW_DESCRIBE',
     'KW_DESC',
     'KW_VERTEX',
+    'KW_VERTICES',
     'KW_EDGE',
     'KW_EDGES',
     'KW_UPDATE',

diff --git a/src/graph/context/Iterator.cpp b/src/graph/context/Iterator.cpp
@@ -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;
@@ -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;
     }
@@ -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));

diff --git a/src/graph/context/ast/QueryAstContext.h b/src/graph/context/ast/QueryAstContext.h
@@ -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

diff --git a/src/graph/executor/query/DataCollectExecutor.cpp b/src/graph/executor/query/DataCollectExecutor.cpp
@@ -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);
@@ -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();

diff --git a/src/graph/planner/ngql/SubgraphPlanner.cpp b/src/graph/planner/ngql/SubgraphPlanner.cpp
@@ -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;
 
@@ -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;
 }
@@ -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);
@@ -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;
 }
@@ -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;

diff --git a/src/graph/validator/GetSubgraphValidator.cpp b/src/graph/validator/GetSubgraphValidator.cpp
@@ -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();
 }
 
@@ -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();
 }
 

diff --git a/src/graph/validator/GetSubgraphValidator.h b/src/graph/validator/GetSubgraphValidator.h
@@ -27,6 +27,8 @@ class GetSubgraphValidator final : public TraversalValidator {
 
   Status validateBothInOutBound(BothInOutClause* out);
 
+  Status validateYield(YieldClause* yield);
+
   AstContext* getAstContext() override { return subgraphCtx_.get(); }
 
  private:

diff --git a/src/graph/validator/test/GetSubgraphValidatorTest.cpp b/src/graph/validator/test/GetSubgraphValidatorTest.cpp
@@ -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,
@@ -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,
@@ -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,
@@ -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,
@@ -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,
@@ -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,
@@ -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";

diff --git a/src/parser/TraverseSentences.cpp b/src/parser/TraverseSentences.cpp
@@ -257,6 +257,10 @@ std::string GetSubgraphSentence::toString() const {
     buf += " ";
     buf += both_->toString();
   }
+  if (yield_ != nullptr) {
+    buf += " ";
+    buf += yield_->toString();
+  }
   return buf;
 }
 }  // namespace nebula
diff --git a/src/parser/TraverseSentences.h b/src/parser/TraverseSentences.h
@@ -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(); }
@@ -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:
@@ -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_