Lookup subgraph format yield (#3139)

* format lookup * fix lookup ci test * fix geo testcase * fix lookup ci test * disable missing yield clause in subgraph sentence * fix subgraph ci test * fix test error
vesoft-inc · Nov 23, 2021 · 6541438 · 6541438
1 parent e8b45d3
commit 6541438
Show file tree

Hide file tree

Showing 32 changed files with 1,882 additions and 3,255 deletions.
diff --git a/src/graph/validator/GetSubgraphValidator.cpp b/src/graph/validator/GetSubgraphValidator.cpp
@@ -94,30 +94,21 @@ Status GetSubgraphValidator::validateBothInOutBound(BothInOutClause* out) {
 }
 
 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));
+    return Status::SemanticError("Missing yield clause.");
   }
   auto size = yield->columns().size();
   outputs_.reserve(size);
+  auto pool = qctx_->objPool();
   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") {
+    const std::string& colStr = col->expr()->toString();
+    if (colStr == "VERTICES") {
       subgraphCtx_->getVertexProp = true;
       auto* newCol = new YieldColumn(InputPropertyExpression::make(pool, "VERTICES"), col->name());
       newCols->addColumn(newCol);
-    } else if (lowerStr == "edges" || lowerStr == "_edges") {
+    } else if (colStr == "EDGES") {
       if (subgraphCtx_->steps.steps() == 0) {
         return Status::SemanticError("Get Subgraph 0 STEPS only support YIELD vertices");
       }

diff --git a/src/graph/validator/LookupValidator.cpp b/src/graph/validator/LookupValidator.cpp
@@ -160,36 +160,21 @@ Status LookupValidator::validateYieldTag() {
 }
 
 Status LookupValidator::validateYield() {
-  auto pool = qctx_->objPool();
-  auto* newCols = pool->add(new YieldColumns());
-  lookupCtx_->yieldExpr = newCols;
-  if (lookupCtx_->isEdge) {
-    idxReturnCols_.emplace_back(kSrc);
-    idxReturnCols_.emplace_back(kDst);
-    idxReturnCols_.emplace_back(kRank);
-    idxReturnCols_.emplace_back(kType);
-    outputs_.emplace_back(kSrcVID, vidType_);
-    outputs_.emplace_back(kDstVID, vidType_);
-    outputs_.emplace_back(kRanking, Value::Type::INT);
-    newCols->addColumn(new YieldColumn(ColumnExpression::make(pool, 0), kSrcVID));
-    newCols->addColumn(new YieldColumn(ColumnExpression::make(pool, 1), kDstVID));
-    newCols->addColumn(new YieldColumn(ColumnExpression::make(pool, 2), kRanking));
-  } else {
-    idxReturnCols_.emplace_back(kVid);
-    outputs_.emplace_back(kVertexID, vidType_);
-    newCols->addColumn(new YieldColumn(ColumnExpression::make(pool, 0), kVertexID));
-  }
-
   auto yieldClause = sentence()->yieldClause();
   if (yieldClause == nullptr) {
-    extractExprProps();
-    return Status::OK();
+    return Status::SemanticError("Missing yield clause.");
   }
   lookupCtx_->dedup = yieldClause->isDistinct();
+  lookupCtx_->yieldExpr = qctx_->objPool()->add(new YieldColumns());
 
   if (lookupCtx_->isEdge) {
+    idxReturnCols_.emplace_back(nebula::kSrc);
+    idxReturnCols_.emplace_back(nebula::kDst);
+    idxReturnCols_.emplace_back(nebula::kRank);
+    idxReturnCols_.emplace_back(nebula::kType);
     NG_RETURN_IF_ERROR(validateYieldEdge());
   } else {
+    idxReturnCols_.emplace_back(nebula::kVid);
     NG_RETURN_IF_ERROR(validateYieldTag());
   }
   if (exprProps_.hasInputVarProperty()) {

diff --git a/src/graph/validator/test/GetSubgraphValidatorTest.cpp b/src/graph/validator/test/GetSubgraphValidatorTest.cpp
@@ -19,7 +19,7 @@ using PK = nebula::graph::PlanNode::Kind;
 
 TEST_F(GetSubgraphValidatorTest, Base) {
   {
-    std::string query = "GET SUBGRAPH FROM \"1\"";
+    std::string query = "GET SUBGRAPH FROM \"1\" YIELD vertices as nodes";
     std::vector<PlanNode::Kind> expected = {
         PK::kProject,
         PK::kDataCollect,
@@ -32,7 +32,7 @@ TEST_F(GetSubgraphValidatorTest, Base) {
     EXPECT_TRUE(checkResult(query, expected));
   }
   {
-    std::string query = "GET SUBGRAPH WITH PROP 3 STEPS FROM \"1\"";
+    std::string query = "GET SUBGRAPH WITH PROP 3 STEPS FROM \"1\" YIELD edges as relationships";
     std::vector<PlanNode::Kind> expected = {
         PK::kProject,
         PK::kDataCollect,
@@ -58,7 +58,8 @@ TEST_F(GetSubgraphValidatorTest, Base) {
     EXPECT_TRUE(checkResult(query, expected));
   }
   {
-    std::string query = "GET SUBGRAPH WITH PROP FROM \"1\", \"2\" IN like";
+    std::string query =
+        "GET SUBGRAPH WITH PROP FROM \"1\", \"2\" IN like YIELD vertices as a, edges as b";
     std::vector<PlanNode::Kind> expected = {
         PK::kProject,
         PK::kDataCollect,
@@ -76,7 +77,7 @@ TEST_F(GetSubgraphValidatorTest, Input) {
   {
     std::string query =
         "GO FROM \"1\" OVER like YIELD like._src AS src | GET SUBGRAPH WITH "
-        "PROP FROM $-.src";
+        "PROP FROM $-.src YIELD vertices as a, edges as b";
     std::vector<PlanNode::Kind> expected = {
         PK::kProject,
         PK::kDataCollect,
@@ -95,7 +96,7 @@ TEST_F(GetSubgraphValidatorTest, Input) {
   {
     std::string query =
         "$a = GO FROM \"1\" OVER like YIELD like._src AS src; GET SUBGRAPH "
-        "FROM $a.src";
+        "FROM $a.src YIELD vertices as a, edges as b";
     std::vector<PlanNode::Kind> expected = {
         PK::kProject,
         PK::kDataCollect,
@@ -112,7 +113,7 @@ TEST_F(GetSubgraphValidatorTest, Input) {
     EXPECT_TRUE(checkResult(query, expected));
   }
   {
-    std::string query = "GET SUBGRAPH 0 STEPS FROM \"1\"";
+    std::string query = "GET SUBGRAPH 0 STEPS FROM \"1\" YIELD vertices as nodes";
     std::vector<PlanNode::Kind> expected = {
         PK::kAggregate,
         PK::kGetVertices,
@@ -121,7 +122,8 @@ TEST_F(GetSubgraphValidatorTest, Input) {
     EXPECT_TRUE(checkResult(query, expected));
   }
   {
-    std::string query = "GET SUBGRAPH WITH PROP 0 STEPS FROM \"1\", \"2\", \"3\"";
+    std::string query =
+        "GET SUBGRAPH WITH PROP 0 STEPS FROM \"1\", \"2\", \"3\" YIELD vertices as nodes";
     std::vector<PlanNode::Kind> expected = {
         PK::kAggregate,
         PK::kGetVertices,
@@ -132,7 +134,7 @@ TEST_F(GetSubgraphValidatorTest, Input) {
   {
     std::string query =
         "GO FROM \"1\" OVER like YIELD like._src AS src | GET SUBGRAPH WITH "
-        "PROP 0 STEPS FROM $-.src";
+        "PROP 0 STEPS FROM $-.src YIELD vertices as nodes";
     std::vector<PlanNode::Kind> expected = {
         PK::kAggregate,
         PK::kGetVertices,
@@ -147,7 +149,7 @@ TEST_F(GetSubgraphValidatorTest, Input) {
   {
     std::string query =
         "$a = GO FROM \"1\" OVER like YIELD like._src AS src; GET SUBGRAPH "
-        "WITH PROP 0 STEPS FROM $a.src";
+        "WITH PROP 0 STEPS FROM $a.src YIELD vertices as nodes";
     std::vector<PlanNode::Kind> expected = {
         PK::kAggregate,
         PK::kGetVertices,
@@ -162,6 +164,11 @@ TEST_F(GetSubgraphValidatorTest, Input) {
 }
 
 TEST_F(GetSubgraphValidatorTest, invalidYield) {
+  {
+    std::string query = "GET SUBGRAPH WITH PROP FROM \"Tim Duncan\"";
+    auto result = checkResult(query);
+    EXPECT_EQ(std::string(result.message()), "SemanticError: Missing yield clause.");
+  }
   {
     std::string query = "GET SUBGRAPH WITH PROP FROM \"Tim Duncan\" YIELD vertice";
     auto result = checkResult(query);
@@ -202,19 +209,19 @@ TEST_F(GetSubgraphValidatorTest, invalidYield) {
 
 TEST_F(GetSubgraphValidatorTest, RefNotExist) {
   {
-    std::string query = "GET SUBGRAPH WITH PROP FROM $-.id";
+    std::string query = "GET SUBGRAPH WITH PROP FROM $-.id YIELD edges as b";
     auto result = checkResult(query);
     EXPECT_EQ(std::string(result.message()), "SemanticError: `$-.id', not exist prop `id'");
   }
   {
-    std::string query = "GET SUBGRAPH WITH PROP FROM $a.id";
+    std::string query = "GET SUBGRAPH WITH PROP FROM $a.id YIELD edges as b";
     auto result = checkResult(query);
     EXPECT_EQ(std::string(result.message()), "SemanticError: `$a.id', not exist variable `a'");
   }
   {
     std::string query =
         "GO FROM \"1\" OVER like YIELD $$.person.age AS id | GET SUBGRAPH WITH "
-        "PROP FROM $-.id";
+        "PROP FROM $-.id YIELD vertices as nodes";
     auto result = checkResult(query);
     EXPECT_EQ(std::string(result.message()),
               "SemanticError: `$-.id', the srcs should be type of "
@@ -223,7 +230,7 @@ TEST_F(GetSubgraphValidatorTest, RefNotExist) {
   {
     std::string query =
         "$a = GO FROM \"1\" OVER like YIELD $$.person.age AS ID; GET SUBGRAPH "
-        "FROM $a.ID";
+        "FROM $a.ID YIELD edges as relationships";
     auto result = checkResult(query);
     EXPECT_EQ(std::string(result.message()),
               "SemanticError: `$a.ID', the srcs should be type of "
@@ -232,21 +239,21 @@ TEST_F(GetSubgraphValidatorTest, RefNotExist) {
   {
     std::string query =
         "$a = GO FROM \"1\" OVER like YIELD like._src AS src; GET SUBGRAPH "
-        "WITH PROP FROM $b.src";
+        "WITH PROP FROM $b.src YIELD vertices as nodes";
     auto result = checkResult(query);
     EXPECT_EQ(std::string(result.message()), "SemanticError: `$b.src', not exist variable `b'");
   }
   {
     std::string query =
         "GO FROM \"1\" OVER like YIELD like._dst AS id, like._src AS id | GET "
-        "SUBGRAPH FROM $-.id";
+        "SUBGRAPH FROM $-.id YIELD edges as relationships";
     auto result = checkResult(query);
     EXPECT_EQ(std::string(result.message()), "SemanticError: Duplicate Column Name : `id'");
   }
   {
     std::string query =
         "$a = GO FROM \"1\" OVER like YIELD like._dst AS id, like._src AS id; "
-        "GET SUBGRAPH WITH PROP FROM $a.id";
+        "GET SUBGRAPH WITH PROP FROM $a.id YIELD vertices as nodes";
     auto result = checkResult(query);
     EXPECT_EQ(std::string(result.message()), "SemanticError: Duplicate Column Name : `id'");
   }

diff --git a/src/graph/validator/test/LookupValidatorTest.cpp b/src/graph/validator/test/LookupValidatorTest.cpp
@@ -18,8 +18,8 @@ TEST_F(LookupValidatorTest, InputOutput) {
   // pipe
   {
     const std::string query =
-        "LOOKUP ON person where person.age == 35 | "
-        "FETCH PROP ON person $-.VertexID YIELD vertex as node";
+        "LOOKUP ON person where person.age == 35 YIELD id(vertex) as id | "
+        "FETCH PROP ON person $-.id YIELD vertex as node";
     EXPECT_TRUE(checkResult(query,
                             {
                                 PlanNode::Kind::kProject,
@@ -48,8 +48,8 @@ TEST_F(LookupValidatorTest, InputOutput) {
   // variable
   {
     const std::string query =
-        "$a = LOOKUP ON person where person.age == 35; "
-        "FETCH PROP ON person $a.VertexID YIELD vertex as node";
+        "$a = LOOKUP ON person where person.age == 35 YIELD id(vertex) as id; "
+        "FETCH PROP ON person $a.id YIELD vertex as node";
     EXPECT_TRUE(checkResult(query,
                             {
                                 PlanNode::Kind::kProject,
@@ -63,8 +63,7 @@ TEST_F(LookupValidatorTest, InputOutput) {
   // var with yield
   {
     const std::string query =
-        "$a = LOOKUP ON person where person.age == 35 YIELD person.name AS "
-        "name;"
+        "$a = LOOKUP ON person where person.age == 35 YIELD person.name AS name;"
         "FETCH PROP ON person $a.name YIELD vertex as node";
     EXPECT_TRUE(checkResult(query,
                             {
@@ -79,7 +78,10 @@ TEST_F(LookupValidatorTest, InputOutput) {
 }
 
 TEST_F(LookupValidatorTest, InvalidYieldExpression) {
-  // TODO(shylock)
+  {
+    const std::string query = "LOOKUP ON person where person.age > 20;";
+    EXPECT_FALSE(checkResult(query, {}));
+  }
   {
     const std::string query =
         "LOOKUP ON person where person.age == 35 YIELD person.age + 1 AS age;";
@@ -95,40 +97,49 @@ TEST_F(LookupValidatorTest, InvalidYieldExpression) {
 
 TEST_F(LookupValidatorTest, InvalidFilterExpression) {
   {
-    const std::string query = "LOOKUP ON person where person.age == person.name;";
+    const std::string query =
+        "LOOKUP ON person where person.age == person.name YIELD vertex as node;";
     EXPECT_FALSE(checkResult(query, {}));
   }
   {
-    const std::string query = "LOOKUP ON person where person.age > person.name;";
+    const std::string query =
+        "LOOKUP ON person where person.age > person.name YIELD vertex as node;";
     EXPECT_FALSE(checkResult(query, {}));
   }
   {
-    const std::string query = "LOOKUP ON person where person.age != person.name;";
+    const std::string query =
+        "LOOKUP ON person where person.age != person.name YIELD vertex as node;";
     EXPECT_FALSE(checkResult(query, {}));
   }
   {
-    const std::string query = "LOOKUP ON person where person.age + 1 > 5;";
+    const std::string query = "LOOKUP ON person where person.age + 1 > 5 YIELD person.age;";
     EXPECT_FALSE(checkResult(query, {}));
   }
   {
-    const std::string query = "LOOKUP ON person where person.age > person.name + 5;";
+    const std::string query =
+        "LOOKUP ON person where person.age > person.name + 5 YIELD id(vertex);";
     EXPECT_FALSE(checkResult(query, {}));
   }
   {
-    const std::string query = "LOOKUP ON person where  1 + 5 < person.age;";
+    const std::string query = "LOOKUP ON person where  1 + 5 < person.age YIELD vertex as node;";
     EXPECT_TRUE(checkResult(query, {}));
   }
   {
-    const std::string query = "LOOKUP ON person where person.age > 1 + 5;";
+    const std::string query = "LOOKUP ON person where person.age > 1 + 5 YIELD vertex as node;";
     EXPECT_TRUE(checkResult(query, {}));
   }
   {
-    const std::string query = "LOOKUP ON person where person.age > abs(-5);";
+    const std::string query = "LOOKUP ON person where person.age > abs(-5) YIELD id(vertex);";
     EXPECT_TRUE(checkResult(query, {}));
   }
 }
 
 TEST_F(LookupValidatorTest, wrongYield) {
+  {
+    std::string query = "LOOKUP ON person";
+    auto result = checkResult(query);
+    EXPECT_EQ(std::string(result.message()), "SemanticError: Missing yield clause.");
+  }
   {
     std::string query = "LOOKUP ON person YIELD vertex";
     auto result = checkResult(query);

diff --git a/tests/tck/features/bugfix/TruncatedStringIndex.feature b/tests/tck/features/bugfix/TruncatedStringIndex.feature
@@ -25,16 +25,16 @@ Feature: Truncated string index
     Then the execution should be successful
     When executing query:
       """
-      LOOKUP ON person WHERE person.name=="abc"
+      LOOKUP ON person WHERE person.name=="abc" YIELD id(vertex) as id
       """
     Then the result should be, in any order:
-      | VertexID |
+      | id |
     When executing query:
       """
       LOOKUP ON person WHERE person.name=="abc" YIELD person.name
       """
     Then the result should be, in any order:
-      | VertexID | person.name |
+      | person.name |
     When executing query:
       """
       match (v:person) where v.name == "abc" return v;