Merge branch 'master' into master

vesoft-inc · Mar 9, 2023 · 4fec10d · 4fec10d
2 parents 5025796 + 8f800e4
commit 4fec10d
Show file tree

Hide file tree

Showing 18 changed files with 133 additions and 32 deletions.
diff --git a/src/clients/meta/MetaClient.cpp b/src/clients/meta/MetaClient.cpp
@@ -1264,11 +1264,13 @@ folly::Future<StatusOr<GraphSpaceID>> MetaClient::createSpace(meta::cpp2::SpaceD
 }
 
 folly::Future<StatusOr<GraphSpaceID>> MetaClient::createSpaceAs(const std::string& oldSpaceName,
-                                                                const std::string& newSpaceName) {
+                                                                const std::string& newSpaceName,
+                                                                bool ifNotExists) {
   memory::MemoryCheckOffGuard g;
   cpp2::CreateSpaceAsReq req;
   req.old_space_name_ref() = oldSpaceName;
   req.new_space_name_ref() = newSpaceName;
+  req.if_not_exists_ref() = ifNotExists;
   folly::Promise<StatusOr<GraphSpaceID>> promise;
   auto future = promise.getFuture();
   getResponse(

diff --git a/src/clients/meta/MetaClient.h b/src/clients/meta/MetaClient.h
@@ -275,7 +275,8 @@ class MetaClient : public BaseMetaClient {
                                                     bool ifNotExists = false);
 
   folly::Future<StatusOr<GraphSpaceID>> createSpaceAs(const std::string& oldSpaceName,
-                                                      const std::string& newSpaceName);
+                                                      const std::string& newSpaceName,
+                                                      bool ifNotExists);
 
   folly::Future<StatusOr<std::vector<SpaceIdName>>> listSpaces();
 

diff --git a/src/common/function/FunctionManager.cpp b/src/common/function/FunctionManager.cpp
@@ -2033,8 +2033,11 @@ FunctionManager::FunctionManager() {
           return v.vid;
         }
         case Value::Type::LIST: {
-          const auto &listVal = args[0].get().getList();
-          auto &lastVal = listVal.values.back();
+          const auto &listVal = args[0].get().getList().values;
+          if (listVal.empty()) {
+            return Value::kNullBadType;
+          }
+          auto &lastVal = listVal.back();
           if (lastVal.isEdge()) {
             return lastVal.getEdge().dst;
           } else if (lastVal.isVertex()) {
@@ -2167,7 +2170,8 @@ FunctionManager::FunctionManager() {
           return Value::kNullValue;
         }
         case Value::Type::LIST: {
-          return args[0].get().getList().values.front();
+          const auto &items = args[0].get().getList().values;
+          return items.empty() ? Value::kNullValue : items.front();
         }
         default: {
           return Value::kNullBadType;
@@ -2186,7 +2190,8 @@ FunctionManager::FunctionManager() {
           return Value::kNullValue;
         }
         case Value::Type::LIST: {
-          return args[0].get().getList().values.back();
+          const auto &items = args[0].get().getList().values;
+          return items.empty() ? Value::kNullValue : items.back();
         }
         default: {
           return Value::kNullBadType;

diff --git a/src/graph/executor/admin/SpaceExecutor.cpp b/src/graph/executor/admin/SpaceExecutor.cpp
@@ -34,11 +34,10 @@ folly::Future<Status> CreateSpaceAsExecutor::execute() {
   SCOPED_TIMER(&execTime_);
 
   auto *csaNode = asNode<CreateSpaceAsNode>(node());
-  auto oldSpace = csaNode->getOldSpaceName();
-  auto newSpace = csaNode->getNewSpaceName();
   return qctx()
       ->getMetaClient()
-      ->createSpaceAs(oldSpace, newSpace)
+      ->createSpaceAs(
+          csaNode->getOldSpaceName(), csaNode->getNewSpaceName(), csaNode->getIfNotExists())
       .via(runner())
       .thenValue([](StatusOr<bool> resp) {
         if (!resp.ok()) {

diff --git a/src/graph/executor/test/JoinTest.cpp b/src/graph/executor/test/JoinTest.cpp
@@ -149,7 +149,6 @@ void JoinTest::testInnerJoin(std::string left, std::string right, DataSet& expec
   EXPECT_EQ(result.state(), Result::State::kSuccess) << "LINE: " << line;
 }
 
-
 TEST_F(JoinTest, InnerJoin) {
   DataSet expected;
   expected.colNames = {"src", "dst", kVid, "tag_prop", "edge_prop", kDst};

diff --git a/src/graph/planner/ngql/GoPlanner.cpp b/src/graph/planner/ngql/GoPlanner.cpp
@@ -340,6 +340,7 @@ PlanNode* GoPlanner::lastStep(PlanNode* dep, PlanNode* join) {
     gn->setVertexProps(buildVertexProps(goCtx_->exprProps.srcTagProps()));
     gn->setEdgeProps(buildEdgeProps(false));
     gn->setInputVar(goCtx_->vidsVar);
+    gn->setEdgeDirection(goCtx_->over.direction);
 
     const auto& steps = goCtx_->steps;
     auto* sampleLimit = buildSampleLimit(gn, steps.isMToN() ? steps.nSteps() : steps.steps());
@@ -446,6 +447,7 @@ SubPlan GoPlanner::oneStepPlan(SubPlan& startVidPlan) {
     gn->setEdgeProps(buildEdgeProps(false));
     gn->setSrc(goCtx_->from.src);
     gn->setInputVar(goCtx_->vidsVar);
+    gn->setEdgeDirection(goCtx_->over.direction);
     scan = gn;
 
     auto* sampleLimit = buildSampleLimit(gn, 1 /* one step */);
@@ -495,6 +497,7 @@ SubPlan GoPlanner::nStepsPlan(SubPlan& startVidPlan) {
     gn->setSrc(goCtx_->from.src);
     gn->setEdgeProps(buildEdgeProps(true));
     gn->setInputVar(goCtx_->vidsVar);
+    gn->setEdgeDirection(goCtx_->over.direction);
     auto* sampleLimit = buildSampleLimit(gn);
 
     getDst = PlannerUtil::extractDstFromGN(qctx, sampleLimit, goCtx_->vidsVar);
@@ -569,6 +572,7 @@ SubPlan GoPlanner::mToNStepsPlan(SubPlan& startVidPlan) {
     gn->setVertexProps(buildVertexProps(goCtx_->exprProps.srcTagProps()));
     gn->setEdgeProps(buildEdgeProps(false));
     gn->setInputVar(goCtx_->vidsVar);
+    gn->setEdgeDirection(goCtx_->over.direction);
     auto* sampleLimit = buildSampleLimit(gn);
 
     getDst = PlannerUtil::extractDstFromGN(qctx, sampleLimit, goCtx_->vidsVar);

diff --git a/src/graph/planner/ngql/PathPlanner.cpp b/src/graph/planner/ngql/PathPlanner.cpp
@@ -98,6 +98,8 @@ PlanNode* PathPlanner::getNeighbors(PlanNode* dep, bool reverse) {
   gn->setEdgeProps(buildEdgeProps(reverse));
   gn->setInputVar(gnInputVar);
   gn->setDedup();
+  gn->setEdgeDirection(pathCtx_->over.direction);
+
   PlanNode* result = gn;
   if (pathCtx_->filter != nullptr) {
     auto* filterExpr = pathCtx_->filter->clone();

diff --git a/src/graph/planner/plan/Admin.cpp b/src/graph/planner/plan/Admin.cpp
@@ -24,6 +24,7 @@ std::unique_ptr<PlanNodeDescription> CreateSpaceAsNode::explain() const {
   auto desc = SingleDependencyNode::explain();
   addDescription("oldSpaceName", oldSpaceName_, desc.get());
   addDescription("newSpaceName", newSpaceName_, desc.get());
+  addDescription("ifNotExists", folly::toJson(util::toJson(ifNotExists_)), desc.get());
   return desc;
 }
 

diff --git a/src/graph/planner/plan/Admin.h b/src/graph/planner/plan/Admin.h
@@ -162,8 +162,10 @@ class CreateSpaceAsNode final : public SingleDependencyNode {
   static CreateSpaceAsNode* make(QueryContext* qctx,
                                  PlanNode* input,
                                  const std::string& oldSpaceName,
-                                 const std::string& newSpaceName) {
-    return qctx->objPool()->makeAndAdd<CreateSpaceAsNode>(qctx, input, oldSpaceName, newSpaceName);
+                                 const std::string& newSpaceName,
+                                 bool ifNotExists) {
+    return qctx->objPool()->makeAndAdd<CreateSpaceAsNode>(
+        qctx, input, oldSpaceName, newSpaceName, ifNotExists);
   }
 
   std::unique_ptr<PlanNodeDescription> explain() const override;
@@ -177,16 +179,26 @@ class CreateSpaceAsNode final : public SingleDependencyNode {
     return newSpaceName_;
   }
 
+  bool getIfNotExists() const {
+    return ifNotExists_;
+  }
+
  private:
   friend ObjectPool;
-  CreateSpaceAsNode(QueryContext* qctx, PlanNode* input, std::string oldName, std::string newName)
+  CreateSpaceAsNode(QueryContext* qctx,
+                    PlanNode* input,
+                    std::string oldName,
+                    std::string newName,
+                    bool ifNotExists)
       : SingleDependencyNode(qctx, Kind::kCreateSpaceAs, input),
         oldSpaceName_(std::move(oldName)),
-        newSpaceName_(std::move(newName)) {}
+        newSpaceName_(std::move(newName)),
+        ifNotExists_(ifNotExists) {}
 
  private:
   std::string oldSpaceName_;
   std::string newSpaceName_;
+  bool ifNotExists_{false};
 };
 
 class DropSpace final : public SingleDependencyNode {

diff --git a/src/graph/service/GraphService.cpp b/src/graph/service/GraphService.cpp
@@ -76,6 +76,13 @@ folly::Future<AuthResponse> GraphService::future_authenticate(const std::string&
   auto clientIp = peer->getAddressStr();
   LOG(INFO) << "Authenticating user " << username << " from " << peer->describe();
 
+  // We don't support ipv6 yet, if the client is using IPv4-mapped IPv6 address, we should convert
+  // it to IPv4 address.
+  if (peer->isIPv4Mapped()) {
+    folly::IPAddress v6map(clientIp);
+    clientIp = folly::IPAddress::createIPv4(v6map).str();
+  }
+
   auto ctx = std::make_unique<RequestContext<AuthResponse>>();
   auto future = ctx->future();
 

diff --git a/src/graph/validator/AdminValidator.cpp b/src/graph/validator/AdminValidator.cpp
@@ -157,11 +157,12 @@ Status CreateSpaceAsValidator::validateImpl() {
   auto sentence = static_cast<CreateSpaceAsSentence *>(sentence_);
   oldSpaceName_ = sentence->getOldSpaceName();
   newSpaceName_ = sentence->getNewSpaceName();
+  ifNotExist_ = sentence->isIfNotExist();
   return Status::OK();
 }
 
 Status CreateSpaceAsValidator::toPlan() {
-  auto *doNode = CreateSpaceAsNode::make(qctx_, nullptr, oldSpaceName_, newSpaceName_);
+  auto *doNode = CreateSpaceAsNode::make(qctx_, nullptr, oldSpaceName_, newSpaceName_, ifNotExist_);
   root_ = doNode;
   tail_ = root_;
   return Status::OK();

diff --git a/src/graph/validator/AdminValidator.h b/src/graph/validator/AdminValidator.h
@@ -46,6 +46,7 @@ class CreateSpaceAsValidator final : public Validator {
  private:
   std::string oldSpaceName_;
   std::string newSpaceName_;
+  bool ifNotExist_;
 };
 
 class AlterSpaceValidator final : public Validator {

diff --git a/src/interface/meta.thrift b/src/interface/meta.thrift
@@ -339,6 +339,7 @@ struct CreateSpaceReq {
 struct CreateSpaceAsReq {
     1: binary        old_space_name,
     2: binary        new_space_name,
+    3: bool          if_not_exists,
 }
 
 struct DropSpaceReq {

diff --git a/src/meta/processors/parts/CreateSpaceAsProcessor.cpp b/src/meta/processors/parts/CreateSpaceAsProcessor.cpp
@@ -33,6 +33,14 @@ void CreateSpaceAsProcessor::process(const cpp2::CreateSpaceAsReq &req) {
   }
 
   if (nebula::ok(newSpaceId)) {
+    if (req.get_if_not_exists()) {
+      rc_ = nebula::cpp2::ErrorCode::SUCCEEDED;
+      cpp2::ID id;
+      id.space_id_ref() = static_cast<GraphSpaceID>(nebula::value(newSpaceId));
+      resp_.id_ref() = id;
+      return;
+    }
+
     rc_ = nebula::cpp2::ErrorCode::E_EXISTED;
     LOG(INFO) << "Create Space [" << newSpaceName << "] as [" << oldSpaceName
               << "] failed. New space already exists.";

diff --git a/tests/tck/features/admin/Sessions.feature b/tests/tck/features/admin/Sessions.feature
@@ -13,8 +13,8 @@ Feature: Test sessions
       SHOW SESSIONS;
       """
     Then the result should contain:
-      | SessionId | UserName | SpaceName | CreateTime | UpdateTime | GraphAddr | Timezone | ClientIp            |
-      | /\d+/     | "root"   | ""        | /.*/       | /.*/       | /.*/      | 0        | /.*(127\.0\.0\.1)$/ |
+      | SessionId | UserName | SpaceName | CreateTime | UpdateTime | GraphAddr | Timezone | ClientIp                            |
+      | /\d+/     | "root"   | ""        | /.*/       | /.*/       | /.*/      | 0        | /\b(?:[0-9]{1,3}\.){3}[0-9]{1,3}\b/ |
     When executing query:
       """
       CREATE USER user1 WITH PASSWORD 'nebula1';
@@ -29,9 +29,9 @@ Feature: Test sessions
       SHOW SESSIONS;
       """
     Then the result should contain, replace the holders with cluster info:
-      | SessionId | UserName | SpaceName | CreateTime | UpdateTime | GraphAddr                                           | Timezone | ClientIp            |
-      | /\d+/     | "root"   | "s1"      | /.*/       | /.*/       | "127.0.0.1:${cluster.graphd_processes[0].tcp_port}" | 0        | /.*(127\.0\.0\.1)$/ |
-      | /\d+/     | "user1"  | ""        | /.*/       | /.*/       | "127.0.0.1:${cluster.graphd_processes[1].tcp_port}" | 0        | /.*(127\.0\.0\.1)$/ |
+      | SessionId | UserName | SpaceName | CreateTime | UpdateTime | GraphAddr                                           | Timezone | ClientIp                            |
+      | /\d+/     | "root"   | "s1"      | /.*/       | /.*/       | "127.0.0.1:${cluster.graphd_processes[0].tcp_port}" | 0        | /\b(?:[0-9]{1,3}\.){3}[0-9]{1,3}\b/ |
+      | /\d+/     | "user1"  | ""        | /.*/       | /.*/       | "127.0.0.1:${cluster.graphd_processes[1].tcp_port}" | 0        | /\b(?:[0-9]{1,3}\.){3}[0-9]{1,3}\b/ |
 
   @distonly
   Scenario: Show local sessions
@@ -40,8 +40,8 @@ Feature: Test sessions
       SHOW SESSIONS;
       """
     Then the result should contain:
-      | SessionId | UserName | SpaceName | CreateTime | UpdateTime | GraphAddr | Timezone | ClientIp            |
-      | /\d+/     | "root"   | ""        | /.*/       | /.*/       | /.*/      | 0        | /.*(127\.0\.0\.1)$/ |
+      | SessionId | UserName | SpaceName | CreateTime | UpdateTime | GraphAddr | Timezone | ClientIp                            |
+      | /\d+/     | "root"   | ""        | /.*/       | /.*/       | /.*/      | 0        | /\b(?:[0-9]{1,3}\.){3}[0-9]{1,3}\b/ |
     When executing query:
       """
       CREATE USER IF NOT EXISTS user1 WITH PASSWORD 'nebula1';
@@ -61,14 +61,14 @@ Feature: Test sessions
       SHOW SESSIONS;
       """
     Then the result should contain, replace the holders with cluster info:
-      | SessionId | UserName | SpaceName    | CreateTime | UpdateTime | GraphAddr                                           | Timezone | ClientIp            |
-      | /\d+/     | "root"   | "root_space" | /.*/       | /.*/       | "127.0.0.1:${cluster.graphd_processes[0].tcp_port}" | 0        | /.*(127\.0\.0\.1)$/ |
-      | /\d+/     | "user1"  | ""           | /.*/       | /.*/       | "127.0.0.1:${cluster.graphd_processes[1].tcp_port}" | 0        | /.*(127\.0\.0\.1)$/ |
-      | /\d+/     | "user2"  | ""           | /.*/       | /.*/       | "127.0.0.1:${cluster.graphd_processes[2].tcp_port}" | 0        | /.*(127\.0\.0\.1)$/ |
+      | SessionId | UserName | SpaceName    | CreateTime | UpdateTime | GraphAddr                                           | Timezone | ClientIp                            |
+      | /\d+/     | "root"   | "root_space" | /.*/       | /.*/       | "127.0.0.1:${cluster.graphd_processes[0].tcp_port}" | 0        | /\b(?:[0-9]{1,3}\.){3}[0-9]{1,3}\b/ |
+      | /\d+/     | "user1"  | ""           | /.*/       | /.*/       | "127.0.0.1:${cluster.graphd_processes[1].tcp_port}" | 0        | /\b(?:[0-9]{1,3}\.){3}[0-9]{1,3}\b/ |
+      | /\d+/     | "user2"  | ""           | /.*/       | /.*/       | "127.0.0.1:${cluster.graphd_processes[2].tcp_port}" | 0        | /\b(?:[0-9]{1,3}\.){3}[0-9]{1,3}\b/ |
     When executing query:
       """
       SHOW LOCAL SESSIONS;
       """
     Then the result should contain, replace the holders with cluster info:
-      | SessionId | UserName | SpaceName    | CreateTime | UpdateTime | GraphAddr                                           | Timezone | ClientIp            |
-      | /\d+/     | "root"   | "root_space" | /.*/       | /.*/       | "127.0.0.1:${cluster.graphd_processes[0].tcp_port}" | 0        | /.*(127\.0\.0\.1)$/ |
+      | SessionId | UserName | SpaceName    | CreateTime | UpdateTime | GraphAddr                                           | Timezone | ClientIp                            |
+      | /\d+/     | "root"   | "root_space" | /.*/       | /.*/       | "127.0.0.1:${cluster.graphd_processes[0].tcp_port}" | 0        | /\b(?:[0-9]{1,3}\.){3}[0-9]{1,3}\b/ |
diff --git a/tests/tck/features/go/GO.feature b/tests/tck/features/go/GO.feature
@@ -339,7 +339,7 @@ Feature: Go Sentence
       | serve._dst |
 
   Scenario: multi edges over all
-    When executing query:
+    When profiling query:
       """
       GO FROM "Russell Westbrook" OVER * REVERSELY YIELD serve._dst, like._dst
       """
@@ -348,15 +348,37 @@ Feature: Go Sentence
       | EMPTY      | "James Harden"    |
       | EMPTY      | "Dejounte Murray" |
       | EMPTY      | "Paul George"     |
+    And the execution plan should be:
+      | id | name         | dependencies | operator info                |
+      | 2  | Project      | 1            |                              |
+      | 1  | GetNeighbors | 0            | {"edgeDirection": "IN_EDGE"} |
+      | 0  | Start        |              |                              |
+    When profiling query:
+      """
+      GO FROM "Russell Westbrook" OVER * BIDIRECT YIELD serve._dst, like._dst
+      """
+    Then the result should be, in any order, with relax comparison:
+      | serve._dst | like._dst         |
+      |            | "Dejounte Murray" |
+      |            | "James Harden"    |
+      |            | "Paul George"     |
+      |            | "James Harden"    |
+      |            | "Paul George"     |
+      | "Thunders" |                   |
+    And the execution plan should be:
+      | id | name         | dependencies | operator info             |
+      | 2  | Project      | 1            |                           |
+      | 1  | GetNeighbors | 0            | {"edgeDirection": "BOTH"} |
+      | 0  | Start        |              |                           |
     When executing query:
       """
       GO FROM "Russell Westbrook" OVER * REVERSELY YIELD serve._src, like._src
       """
     Then the result should be, in any order, with relax comparison:
       | serve._src | like._src           |
-      | EMPTY      | "Russell Westbrook" |
-      | EMPTY      | "Russell Westbrook" |
-      | EMPTY      | "Russell Westbrook" |
+      |            | "Russell Westbrook" |
+      |            | "Russell Westbrook" |
+      |            | "Russell Westbrook" |
     When executing query:
       """
       GO FROM "Russell Westbrook" OVER * REVERSELY YIELD like._dst, serve._dst, teammate._dst

diff --git a/tests/tck/features/schema/CreateSpaceAs.feature b/tests/tck/features/schema/CreateSpaceAs.feature
@@ -165,3 +165,27 @@ Feature: Create space as another space
       | src | dst | rank |
       | "1" | "2" | 0    |
     Then drop the used space
+
+  Scenario: clone space if not exist
+    # Space
+    When executing query:
+      """
+      CREATE SPACE IF NOT EXISTS space_origin(vid_type=int);
+      """
+    Then the execution should be successful
+    When executing query:
+      """
+      CREATE SPACE space_clone AS space_origin;
+      """
+    Then the execution should be successful
+    When executing query:
+      """
+      CREATE SPACE IF NOT EXISTS space_clone AS space_origin;
+      """
+    Then the execution should be successful
+    When executing query:
+      """
+      CREATE SPACE space_clone AS space_origin;
+      """
+    Then a ExecutionError should be raised at runtime: Existed!
+    Then drop the used space
diff --git a/tests/tck/features/yield/return.feature b/tests/tck/features/yield/return.feature
@@ -14,6 +14,13 @@ Feature: Return. A standalone return sentence is actually a yield sentence
     Then the result should be, in any order:
       | sum |
       | 2   |
+    When executing query:
+      """
+      RETURN last(LIST[]) AS a, head(LIST[]) AS b
+      """
+    Then the result should be, in any order:
+      | a    | b    |
+      | NULL | NULL |
     When executing query:
       """
       RETURN 1- -1 AS sub
@@ -26,6 +33,11 @@ Feature: Return. A standalone return sentence is actually a yield sentence
       RETURN 1--1 AS sub
       """
     Then a SyntaxError should be raised at runtime: syntax error near `--'
+    When executing query:
+      """
+      MATCH (v:player) RETURN none_direct_dst(LIST[]) AS a
+      """
+    Then a SemanticError should be raised at runtime: Type error `none_direct_dst([])'
     When executing query:
       """
       RETURN  [2,3 ] - [3] AS sub