From c75a1849dc26e4d6be22127677fd1a392271f60c Mon Sep 17 00:00:00 2001
From: jimingquan <mingquan.ji@vesoft.com>
Date: Wed, 22 Sep 2021 10:58:24 +0800
Subject: [PATCH] address comment

---
 .../validator/FetchVerticesValidator.cpp      |  3 +--
 src/graph/validator/MatchValidator.cpp        |  4 ++++
 .../validator/test/MatchValidatorTest.cpp     | 20 +++++++++++++++++++
 .../validator/test/QueryValidatorTest.cpp     |  6 ++++++
 4 files changed, 31 insertions(+), 2 deletions(-)

diff --git a/src/graph/validator/FetchVerticesValidator.cpp b/src/graph/validator/FetchVerticesValidator.cpp
index 0cb012fa0e9..418f332f2a3 100644
--- a/src/graph/validator/FetchVerticesValidator.cpp
+++ b/src/graph/validator/FetchVerticesValidator.cpp
@@ -70,8 +70,7 @@ Status FetchVerticesValidator::validateYield(YieldClause *yield) {
   auto pool = qctx_->objPool();
   bool noYield = false;
   if (yield == nullptr) {
-    // version 3.0: return Status::SemanticError("No YIELD Clause");
-    // compatible with previous versions
+    // TODO: compatible with previous version, this will be deprecated in version 3.0.
     auto *yieldColumns = new YieldColumns();
     auto *vertex = new YieldColumn(VertexExpression::make(pool), "vertices_");
     yieldColumns->addColumn(vertex);
diff --git a/src/graph/validator/MatchValidator.cpp b/src/graph/validator/MatchValidator.cpp
index d6f969cbdb4..5d1cba60826 100644
--- a/src/graph/validator/MatchValidator.cpp
+++ b/src/graph/validator/MatchValidator.cpp
@@ -353,6 +353,10 @@ Status MatchValidator::validateReturn(MatchReturn *ret,
   }
   if (ret->returnItems()->columns()) {
     for (auto *column : ret->returnItems()->columns()->columns()) {
+      if (ExpressionUtils::hasAny(column->expr(),
+                                  {Expression::Kind::kVertex, Expression::Kind::kEdge})) {
+        return Status::SemanticError("illegal return clause `%s'", column->toString().c_str());
+      }
       columns->addColumn(column->clone().release());
     }
   }
diff --git a/src/graph/validator/test/MatchValidatorTest.cpp b/src/graph/validator/test/MatchValidatorTest.cpp
index 03947c59366..f0d51777b79 100644
--- a/src/graph/validator/test/MatchValidatorTest.cpp
+++ b/src/graph/validator/test/MatchValidatorTest.cpp
@@ -611,6 +611,26 @@ TEST_F(MatchValidatorTest, validateAlias) {
     EXPECT_EQ(std::string(result.message()),
               "SemanticError: Path `p' does not have the type attribute");
   }
+  {
+    std::string query = "MATCH  (v:person) return id(vertex)";
+    auto result = checkResult(query);
+    EXPECT_EQ(std::string(result.message()), "SemanticError: illegal return clause `id(VERTEX)'");
+  }
+  {
+    std::string query = "MATCH  (v:person) return vertex as a";
+    auto result = checkResult(query);
+    EXPECT_EQ(std::string(result.message()), "SemanticError: illegal return clause `VERTEX AS a'");
+  }
+  {
+    std::string query = "MATCH  (v:person)-[e]-(v2) return src(edge)";
+    auto result = checkResult(query);
+    EXPECT_EQ(std::string(result.message()), "SemanticError: illegal return clause `src(EDGE)'");
+  }
+  {
+    std::string query = "MATCH  (v:person)-[e]-(v2) return edge as b";
+    auto result = checkResult(query);
+    EXPECT_EQ(std::string(result.message()), "SemanticError: illegal return clause `EDGE AS b'");
+  }
 }
 
 }  // namespace graph
diff --git a/src/graph/validator/test/QueryValidatorTest.cpp b/src/graph/validator/test/QueryValidatorTest.cpp
index 025e3475f5a..ac931a86420 100644
--- a/src/graph/validator/test/QueryValidatorTest.cpp
+++ b/src/graph/validator/test/QueryValidatorTest.cpp
@@ -950,6 +950,12 @@ TEST_F(QueryValidatorTest, GoInvalid) {
     auto result = checkResult(query);
     EXPECT_EQ(std::string(result.message()), "SemanticError: Duplicate Column Name : `id'");
   }
+  {
+    std::string query = "GO FROM id(vertex) OVER * ";
+    auto result = checkResult(query);
+    EXPECT_EQ(std::string(result.message()),
+              "SemanticError: `id(VERTEX)' is not an evaluable expression.");
+  }
 }
 
 TEST_F(QueryValidatorTest, Limit) {