change VertexExpresion interface

vesoft-inc · Sep 30, 2021 · 5b5fa01 · 5b5fa01
1 parent dccaea0
commit 5b5fa01
Show file tree

Hide file tree

Showing 15 changed files with 258 additions and 138 deletions.
diff --git a/src/common/expression/VertexExpression.cpp b/src/common/expression/VertexExpression.cpp
@@ -10,11 +10,32 @@
 
 namespace nebula {
 
-const Value &VertexExpression::eval(ExpressionContext &ctx) {
+const Value& VertexExpression::eval(ExpressionContext& ctx) {
   result_ = ctx.getVertex();
   return result_;
 }
 
-void VertexExpression::accept(ExprVisitor *visitor) { visitor->visit(this); }
+bool VertexExpression::operator==(const Expression& rhs) const {
+  if (kind_ != rhs.kind()) {
+    return false;
+  }
+  const auto& expr = static_cast<const VertexExpression&>(rhs);
+  return name_ == expr.name();
+}
+
+void VertexExpression::writeTo(Encoder& encoder) const {
+  // kind_
+  encoder << kind_;
+
+  // name_
+  encoder << name_;
+}
+
+void VertexExpression::resetFrom(Decoder& decoder) {
+  // Read name_
+  name_ = decoder.readStr();
+}
+
+void VertexExpression::accept(ExprVisitor* visitor) { visitor->visit(this); }
 
 }  // namespace nebula
diff --git a/src/common/expression/VertexExpression.h b/src/common/expression/VertexExpression.h
@@ -19,26 +19,34 @@ namespace nebula {
  */
 class VertexExpression final : public Expression {
  public:
-  static VertexExpression *make(ObjectPool *pool) { return pool->add(new VertexExpression(pool)); }
+  // default name : VERTEX, $^ : startNode of EDGE, $$ : endNode of EDGE
+  // $$ & $^ only used in go sentence
+  static VertexExpression *make(ObjectPool *pool, const std::string &name = "VERTEX") {
+    return pool->add(new VertexExpression(pool, name));
+  }
 
   const Value &eval(ExpressionContext &ctx) override;
 
   void accept(ExprVisitor *visitor) override;
 
-  Expression *clone() const override { return VertexExpression::make(pool_); }
+  Expression *clone() const override { return VertexExpression::make(pool_, name()); }
 
-  std::string toString() const override { return "VERTEX"; }
+  std::string toString() const override { return name_; }
 
-  bool operator==(const Expression &expr) const override { return kind() == expr.kind(); }
+  const std::string &name() const { return name_; }
+
+  bool operator==(const Expression &expr) const override;
 
  private:
-  explicit VertexExpression(ObjectPool *pool) : Expression(pool, Kind::kVertex) {}
+  explicit VertexExpression(ObjectPool *pool, const std::string &name)
+      : Expression(pool, Kind::kVertex), name_(name) {}
 
-  void writeTo(Encoder &encoder) const override { encoder << kind(); }
+  void writeTo(Encoder &encoder) const override;
 
-  void resetFrom(Decoder &) override {}
+  void resetFrom(Decoder &) override;
 
  private:
+  std::string name_;
   Value result_;
 };
 

diff --git a/src/graph/validator/GoValidator.cpp b/src/graph/validator/GoValidator.cpp
@@ -17,6 +17,7 @@
 namespace nebula {
 namespace graph {
 
+static const char* VERTEX = "VERTEX";
 static const char* COLNAME_SRC_VERTEX = "$^";
 static const char* COLNAME_DST_VERTEX = "$$";
 static const char* COLNAME_EDGE = "EDGE";
@@ -140,22 +141,11 @@ Status GoValidator::validateYield(YieldClause* yield) {
 
   for (auto col : cols) {
     if (ExpressionUtils::hasAny(col->expr(),
-                                {Expression::Kind::kAggregate,
-                                 Expression::Kind::kVertex,
-                                 Expression::Kind::kPathBuild})) {
+                                {Expression::Kind::kAggregate, Expression::Kind::kPathBuild})) {
       return Status::SemanticError("`%s' is not support in go sentence.", col->toString().c_str());
     }
-
-    std::string exprStr = col->expr()->toString();
-    if (exprStr == SRC_VERTEX) {
-      colTypeMap_[col->name()] = true;
-      col->setExpr(rewriteLabel2Vertex(col->expr()));
-      NG_RETURN_IF_ERROR(extractVertexProp(exprProps, true));
-    }
-    if (exprStr == DST_VERTEX) {
-      colTypeMap_[col->name()] = false;
-      col->setExpr(rewriteLabel2Vertex(col->expr()));
-      NG_RETURN_IF_ERROR(extractVertexProp(exprProps, false));
+    if (hasAnyStr(col->expr(), VERTEX)) {
+      return Status::SemanticError("`%s' is not support in go sentence.", col->toString().c_str());
     }
 
     col->setExpr(ExpressionUtils::rewriteLabelAttr2EdgeProp(col->expr()));
@@ -165,6 +155,14 @@ Status GoValidator::validateYield(YieldClause* yield) {
     if (ExpressionUtils::hasAny(colExpr, {Expression::Kind::kEdge})) {
       extractEdgeProp(exprProps);
     }
+    if (hasAnyStr(colExpr, SRC_VERTEX)) {
+      colTypeMap_[col->name()] = true;
+      NG_RETURN_IF_ERROR(extractVertexProp(exprProps, true));
+    }
+    if (hasAnyStr(colExpr, DST_VERTEX)) {
+      colTypeMap_[col->name()] = false;
+      NG_RETURN_IF_ERROR(extractVertexProp(exprProps, false));
+    }
 
     auto typeStatus = deduceExprType(colExpr);
     NG_RETURN_IF_ERROR(typeStatus);
@@ -237,16 +235,18 @@ Expression* GoValidator::rewrite2VarProp(const Expression* expr) {
   return RewriteVisitor::transform(expr, matcher, rewriter);
 }
 
-Expression* GoValidator::rewriteLabel2Vertex(const Expression* expr) {
-  auto matcher = [this](const Expression* e) -> bool {
-    return e->toString() == SRC_VERTEX || e->toString() == DST_VERTEX;
-  };
-  auto rewriter = [this](const Expression* e) -> Expression* {
-    UNUSED(e);
-    return VertexExpression::make(qctx_->objPool());
+bool GoValidator::hasAnyStr(const Expression* self, const std::string& name) {
+  auto finder = [&name](const Expression* expr) -> bool {
+    if (expr->toString() == name) {
+      return true;
+    }
+    return false;
   };
 
-  return RewriteVisitor::transform(expr, matcher, rewriter);
+  FindVisitor visitor(finder);
+  const_cast<Expression*>(self)->accept(&visitor);
+  auto res = visitor.results();
+  return res.size() != 0;
 }
 
 Expression* GoValidator::rewriteVertex2VarProp(const Expression* expr) {

diff --git a/src/graph/validator/GoValidator.h b/src/graph/validator/GoValidator.h
@@ -41,19 +41,19 @@ class GoValidator final : public Validator {
 
   Status extractEdgeProp(ExpressionProps& exprProps);
 
-  Expression* rewriteLabel2Vertex(const Expression* expr);
-
   Expression* rewriteVertex2VarProp(const Expression* expr);
 
   Expression* rewriteEdge2VarProp(const Expression* expr);
 
+  bool hasAnyStr(const Expression* expr, const std::string& name);
+
  private:
   std::unique_ptr<GoContext> goCtx_;
 
   YieldColumns* inputPropCols_{nullptr};
   std::unordered_map<std::string, YieldColumn*> propExprColMap_;
 
-  // key : colName, value: bool (true: srcv, false: dstv)
+  // key : colName, value: bool (true: $^, false: $$)
   std::unordered_map<std::string, bool> colTypeMap_;
 };
 }  // namespace graph

diff --git a/src/graph/validator/test/GetSubgraphValidatorTest.cpp b/src/graph/validator/test/GetSubgraphValidatorTest.cpp
@@ -173,19 +173,19 @@ TEST_F(GetSubgraphValidatorTest, invalidYield) {
     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'");
+              "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'");
+              "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'");
+              "SyntaxError: please add alias when using `edges'. near `edges'");
   }
   {
     std::string query = "GET SUBGRAPH WITH PROP FROM \"Tim Duncan\" YIELD path";

diff --git a/src/graph/validator/test/LookupValidatorTest.cpp b/src/graph/validator/test/LookupValidatorTest.cpp
@@ -134,13 +134,13 @@ TEST_F(LookupValidatorTest, wrongYield) {
     std::string query = "LOOKUP ON person YIELD vertex";
     auto result = checkResult(query);
     EXPECT_EQ(std::string(result.message()),
-              "SyntaxError: please add alias when using vertex. near `vertex'");
+              "SyntaxError: please add alias when using `vertex'. near `vertex'");
   }
   {
     std::string query = "LOOKUP ON person YIELD vertex as node, edge";
     auto result = checkResult(query);
     EXPECT_EQ(std::string(result.message()),
-              "SyntaxError: please add alias when using edge. near `edge'");
+              "SyntaxError: please add alias when using `edge'. near `edge'");
   }
   {
     std::string query = "LOOKUP ON person YIELD edge as e";

diff --git a/src/graph/validator/test/QueryValidatorTest.cpp b/src/graph/validator/test/QueryValidatorTest.cpp
@@ -967,10 +967,16 @@ TEST_F(QueryValidatorTest, GoInvalid) {
     EXPECT_EQ(std::string(result.message()), "SemanticError: Invalid label identifiers: path");
   }
   {
-    std::string query = "GO FROM \"Tim\" OVER * YIELD id($$) as id, id($^) as src";
+    std::string query = "GO FROM \"Tim\" OVER * YIELD $$";
     auto result = checkResult(query);
     EXPECT_EQ(std::string(result.message()),
-              "SemanticError: `id(VERTEX) AS id' is not support in go sentence.");
+              "SyntaxError: please add alias when using `$$'. near `$$'");
+  }
+  {
+    std::string query = "GO FROM \"Tim\" OVER * YIELD $^";
+    auto result = checkResult(query);
+    EXPECT_EQ(std::string(result.message()),
+              "SyntaxError: please add alias when using `$^'. near `$^'");
   }
   {
     std::string query = "GO 1 TO 4 STEPS FROM \"Tim\" OVER * YIELD id(vertex) as id";

diff --git a/src/parser/parser.yy b/src/parser/parser.yy
@@ -1065,12 +1065,12 @@ argument_list
     }
     | SRC_REF {
         $$ = ArgumentList::make(qctx->objPool());
-        Expression* arg = VertexExpression::make(qctx->objPool());
+        Expression* arg = VertexExpression::make(qctx->objPool(), "$^");
         $$->addArgument(arg);
     }
     | DST_REF {
         $$ = ArgumentList::make(qctx->objPool());
-        Expression* arg = VertexExpression::make(qctx->objPool());
+        Expression* arg = VertexExpression::make(qctx->objPool(), "$$");
         $$->addArgument(arg);
     }
     | KW_EDGE {
@@ -1403,47 +1403,47 @@ yield_columns
 yield_column
     : KW_VERTEX {
         $$ = nullptr;
-        throw nebula::GraphParser::syntax_error(@1, "please add alias when using vertex.");
+        throw nebula::GraphParser::syntax_error(@1, "please add alias when using `vertex'.");
     }
     | KW_VERTEX KW_AS name_label {
         $$ = new YieldColumn(VertexExpression::make(qctx->objPool()), *$3);
         delete $3;
     }
     | SRC_REF {
         $$ = nullptr;
-        throw nebula::GraphParser::syntax_error(@1, "please add alias when using $^.");
+        throw nebula::GraphParser::syntax_error(@1, "please add alias when using `$^'.");
     }
     | SRC_REF KW_AS name_label {
-        $$ = new YieldColumn(LabelExpression::make(qctx->objPool(), "$^"), *$3);
+        $$ = new YieldColumn(VertexExpression::make(qctx->objPool(), "$^"), *$3);
         delete $3;
     }
     | DST_REF {
         $$ = nullptr;
-        throw nebula::GraphParser::syntax_error(@1, "please add alias when using $$.");
+        throw nebula::GraphParser::syntax_error(@1, "please add alias when using `$$'.");
     }
     | DST_REF KW_AS name_label {
-        $$ = new YieldColumn(LabelExpression::make(qctx->objPool(), "$$"), *$3);
+        $$ = new YieldColumn(VertexExpression::make(qctx->objPool(), "$$"), *$3);
         delete $3;
     }
     | KW_VERTICES {
         $$ = nullptr;
-        throw nebula::GraphParser::syntax_error(@1, "please add alias when using vertices.");
+        throw nebula::GraphParser::syntax_error(@1, "please add alias when using `vertices'.");
     }
     | KW_VERTICES KW_AS name_label {
         $$ = new YieldColumn(LabelExpression::make(qctx->objPool(), "VERTICES"), *$3);
         delete $3;
     }
     | KW_EDGE {
         $$ = nullptr;
-        throw nebula::GraphParser::syntax_error(@1, "please add alias when using edge.");
+        throw nebula::GraphParser::syntax_error(@1, "please add alias when using `edge'.");
     }
     | KW_EDGE KW_AS name_label {
         $$ = new YieldColumn(EdgeExpression::make(qctx->objPool()), *$3);
         delete $3;
     }
     | KW_EDGES {
         $$ = nullptr;
-        throw nebula::GraphParser::syntax_error(@1, "please add alias when using edges.");
+        throw nebula::GraphParser::syntax_error(@1, "please add alias when using `edges'.");
     }
     | KW_EDGES KW_AS name_label {
         $$ = new YieldColumn(LabelExpression::make(qctx->objPool(), "EDGES"), *$3);

diff --git a/tests/tck/features/aggregate/Agg.feature b/tests/tck/features/aggregate/Agg.feature
@@ -691,7 +691,7 @@ Feature: Basic Aggregate and GroupBy
       """
       GO FROM "Tim Duncan" OVER like YIELD count(*)
       """
-    Then a SemanticError should be raised at runtime: `count(*)', not support aggregate function in go sentence.
+    Then a SemanticError should be raised at runtime: `count(*)' is not support in go sentence.
     When executing query:
       """
       GO FROM "Tim Duncan" OVER like where COUNT(*) > 2
@@ -767,7 +767,7 @@ Feature: Basic Aggregate and GroupBy
          YIELD $$.team.name AS name,
                COUNT(serve._dst) AS id
       """
-    Then a SemanticError should be raised at runtime: `COUNT(serve._dst) AS id', not support aggregate function in go sentence.
+    Then a SemanticError should be raised at runtime: `COUNT(serve._dst) AS id' is not support in go sentence.
     When executing query:
       """
       MATCH (v:player)

diff --git a/tests/tck/features/fetch/FetchVertices.intVid.feature b/tests/tck/features/fetch/FetchVertices.intVid.feature
@@ -283,7 +283,7 @@ Feature: Fetch Int Vid Vertices
       """
       FETCH PROP ON player hash('Boris Diaw') YIELD vertex
       """
-    Then a SyntaxError should be raised at runtime: please add alias when using vertex. near `vertex'
+    Then a SyntaxError should be raised at runtime: please add alias when using `vertex'. near `vertex'
     When executing query:
       """
       FETCH PROP ON player hash('Boris Diaw') YIELD edge as a

diff --git a/tests/tck/features/fetch/FetchVertices.strVid.feature b/tests/tck/features/fetch/FetchVertices.strVid.feature
@@ -380,7 +380,7 @@ Feature: Fetch String Vertices
       """
       FETCH PROP ON player 'Boris Diaw' YIELD vertex
       """
-    Then a SyntaxError should be raised at runtime: please add alias when using vertex. near `vertex'
+    Then a SyntaxError should be raised at runtime: please add alias when using `vertex'. near `vertex'
     When executing query:
       """
       FETCH PROP ON player 'Boris Diaw' YIELD edge as a

diff --git a/tests/tck/features/go/GO.feature b/tests/tck/features/go/GO.feature
@@ -1136,7 +1136,7 @@ Feature: Go Sentence
       """
       GO FROM 'Tim Duncan' OVER like where like.likeness
       """
-    Then a SemanticError should be raised at runtime: `like.likeness', expected Boolean, but was `INT'
+    Then a SemanticError should be raised at runtime: `like.likeness', expected boolean, but was `INT'
 
   Scenario: contain
     When executing query: