Merge branch 'master' into fix

.github/workflows/nightly.yml

@@ -82,8 +82,7 @@ jobs:
         with:
           context: .
           file: ./docker/Dockerfile.${{ matrix.service }}
-          # platforms: linux/amd64,linux/arm64
-          platforms: linux/amd64
+          platforms: linux/amd64,linux/arm64
           tags: |
             vesoft/nebula-${{ matrix.service }}:nightly
           push: true

.github/workflows/rc.yml

@@ -99,8 +99,7 @@ jobs:
         with:
           context: .
           file: ./docker/Dockerfile.${{ matrix.service }}
-          # platforms: linux/amd64,linux/arm64
-          platforms: linux/amd64
+          platforms: linux/amd64,linux/arm64
           tags: |
             ${{ secrets.HARBOR_REGISTRY }}/vesoft/nebula-${{ matrix.service }}:${{ steps.tagname.outputs.tag }}
             ${{ secrets.HARBOR_REGISTRY }}/vesoft/nebula-${{ matrix.service }}:${{ steps.tagname.outputs.majorver }}

conf/nebula-storaged.conf.production

@@ -62,6 +62,9 @@
 # The default block cache size used in BlockBasedTable. (MB)
 # recommend: 1/3 of all memory
 --rocksdb_block_cache=4096
+# Disable page cache to better control memory used by rocksdb.
+# Caution: Make sure to allocate enough block cache if disabling page cache!
+--disable_page_cache=false
 
 # Compression algorithm, options: no,snappy,lz4,lz4hc,zlib,bzip2,zstd
 # For the sake of binary compatibility, the default value is snappy.

src/clients/meta/MetaClient.h

@@ -220,8 +220,8 @@ class MetaClient {
   FRIEND_TEST(MetaClientTest, RetryUntilLimitTest);
   FRIEND_TEST(MetaClientTest, RocksdbOptionsTest);
   FRIEND_TEST(MetaClientTest, VerifyClientTest);
-  friend class KillQueryMetaWrapper;
   FRIEND_TEST(ChainAddEdgesTest, AddEdgesLocalTest);
+  friend class KillQueryMetaWrapper;
   friend class storage::MetaClientTestUpdater;
 
  public:

src/clients/storage/GeneralStorageClient.h

@@ -6,8 +6,6 @@
 #ifndef CLIENTS_STORAGE_GENERALSTORAGECLIENT_H_
 #define CLIENTS_STORAGE_GENERALSTORAGECLIENT_H_
 
-#include <gtest/gtest_prod.h>
-
 #include "clients/meta/MetaClient.h"
 #include "clients/storage/StorageClientBase.h"
 #include "common/base/Base.h"

src/clients/storage/InternalStorageClient.h

@@ -6,8 +6,6 @@
 #ifndef CLIENTS_STORAGE_INTERNALSTORAGEClient_H_
 #define CLIENTS_STORAGE_INTERNALSTORAGEClient_H_
 
-#include <gtest/gtest_prod.h>
-
 #include "clients/storage/StorageClientBase.h"
 #include "common/base/Base.h"
 #include "common/base/ErrorOr.h"

src/clients/storage/StorageClient.h

@@ -6,8 +6,6 @@
 #ifndef CLIENTS_STORAGE_STORAGECLIENT_H
 #define CLIENTS_STORAGE_STORAGECLIENT_H
 
-#include <gtest/gtest_prod.h>
-
 #include "clients/storage/StorageClientBase.h"
 #include "common/base/Base.h"
 #include "common/thrift/ThriftClientManager.h"

src/graph/optimizer/rule/CollapseProjectRule.cpp

@@ -64,9 +64,11 @@ StatusOr<OptRule::TransformResult> CollapseProjectRule::transform(
   // disable this case to avoid the expression in ProjBelow being eval multiple
   // times
   std::unordered_set<std::string> uniquePropRefNames;
+  std::unordered_set<std::string> multiRefColNames;
   for (auto p : allPropRefNames) {
     if (!uniquePropRefNames.insert(p).second) {
-      return TransformResult::noTransform();
+      // Records PropRefNames that are referenced multiple times
+      multiRefColNames.insert(p);
     }
   }
 
@@ -75,7 +77,13 @@ StatusOr<OptRule::TransformResult> CollapseProjectRule::transform(
   auto colNames = projBelow->colNames();
   for (size_t i = 0; i < colNames.size(); ++i) {
     if (uniquePropRefNames.count(colNames[i])) {
-      rewriteMap[colNames[i]] = colsBelow[i]->expr();
+      auto colExpr = colsBelow[i]->expr();
+      // disable this case to avoid the expression in ProjBelow being eval multiple
+      // times
+      if (!graph::ExpressionUtils::isPropertyExpr(colExpr) && multiRefColNames.count(colNames[i])) {
+        return TransformResult::noTransform();
+      }
+      rewriteMap[colNames[i]] = colExpr;
     }
   }
 

src/graph/planner/match/MatchClausePlanner.cpp

@@ -409,7 +409,7 @@ Status MatchClausePlanner::projectColumnsBySymbols(MatchClauseContext* matchClau
     auto iter = std::find_if(aliases.begin(), aliases.end(), [](const auto& alias) {
       return alias.second == AliasType::kPath;
     });
-    if (iter != aliases.end()) {
+    if (iter != aliases.end() && path.pathBuild != nullptr) {
       auto& alias = iter->first;
       columns->addColumn(buildPathColumn(path.pathBuild, alias));
       colNames.emplace_back(alias);

src/graph/planner/plan/PlanNodeVisitor.h

@@ -22,6 +22,7 @@ class PlanNodeVisitor {
   virtual void visit(Aggregate *node) = 0;
   virtual void visit(Traverse *node) = 0;
   virtual void visit(AppendVertices *node) = 0;
+  virtual void visit(Dedup *node) = 0;
   virtual void visit(BiJoin *node) = 0;
 };
 

src/graph/planner/plan/Query.cpp

@@ -544,6 +544,10 @@ Dedup::Dedup(QueryContext* qctx, PlanNode* input) : SingleInputNode(qctx, Kind::
   copyInputColNames(input);
 }
 
+void Dedup::accept(PlanNodeVisitor* visitor) {
+  visitor->visit(this);
+}
+
 PlanNode* Dedup::clone() const {
   auto* newDedup = Dedup::make(qctx_, nullptr);
   newDedup->cloneMembers(*this);

src/graph/planner/plan/Query.h

@@ -1168,6 +1168,8 @@ class Dedup final : public SingleInputNode {
     return qctx->objPool()->add(new Dedup(qctx, input));
   }
 
+  void accept(PlanNodeVisitor* visitor) override;
+
   PlanNode* clone() const override;
 
  private:
@@ -1565,7 +1567,7 @@ class AppendVertices final : public GetVertices {
                     nullptr,
                     false,
                     {},
-                    0,
+                    -1,  // means no limit
                     nullptr) {}
 
   void cloneMembers(const AppendVertices& a);

src/graph/validator/AdminValidator.cpp

@@ -15,6 +15,8 @@
 
 namespace nebula {
 namespace graph {
+
+// Validate options of create space sentence, and fill them into spaceDesc_
 Status CreateSpaceValidator::validateImpl() {
   auto sentence = static_cast<CreateSpaceSentence *>(sentence_);
   ifNotExist_ = sentence->isIfNotExist();
@@ -150,6 +152,7 @@ Status CreateSpaceValidator::toPlan() {
   return Status::OK();
 }
 
+// Validate sentence to create space by clone existed one. Just clone meta data, don't clone data.
 Status CreateSpaceAsValidator::validateImpl() {
   auto sentence = static_cast<CreateSpaceAsSentence *>(sentence_);
   oldSpaceName_ = sentence->getOldSpaceName();
@@ -164,6 +167,7 @@ Status CreateSpaceAsValidator::toPlan() {
   return Status::OK();
 }
 
+// Alter options of space: zone
 Status AlterSpaceValidator::validateImpl() {
   return Status::OK();
 }
@@ -212,10 +216,13 @@ Status DropSpaceValidator::toPlan() {
   return Status::OK();
 }
 
+// Show the sentence to create this space. It's created from options of space, so maybe is different
+// from the origin sentence to create this space.
 Status ShowCreateSpaceValidator::validateImpl() {
   return Status::OK();
 }
 
+// Show create space need read permission on target space.
 Status ShowCreateSpaceValidator::checkPermission() {
   auto sentence = static_cast<ShowCreateSpaceSentence *>(sentence_);
   auto spaceIdResult = qctx_->schemaMng()->toGraphSpaceID(*sentence->spaceName());
@@ -323,6 +330,7 @@ Status ShowListenerValidator::toPlan() {
   return Status::OK();
 }
 
+// Register hosts, unregistered host won't be allowed to join cluster.
 Status AddHostsValidator::validateImpl() {
   auto sentence = static_cast<AddHostsSentence *>(sentence_);
   auto hosts = sentence->hosts()->hosts();
@@ -527,6 +535,7 @@ Status ShowStatusValidator::validateImpl() {
   return Status::OK();
 }
 
+// Plan to show stats of vertices and edges
 Status ShowStatusValidator::toPlan() {
   auto *node = ShowStats::make(qctx_, nullptr);
   root_ = node;
@@ -538,6 +547,7 @@ Status ShowServiceClientsValidator::validateImpl() {
   return Status::OK();
 }
 
+// Plan to show external service clients, e.g. Text Search client
 Status ShowServiceClientsValidator::toPlan() {
   auto sentence = static_cast<ShowServiceClientsSentence *>(sentence_);
   auto type = sentence->getType();
@@ -551,6 +561,7 @@ Status SignInServiceValidator::validateImpl() {
   return Status::OK();
 }
 
+// Plan to sign in external services, e.g. Text Search
 Status SignInServiceValidator::toPlan() {
   auto sentence = static_cast<SignInServiceSentence *>(sentence_);
   std::vector<meta::cpp2::ServiceClient> clients;
@@ -568,6 +579,7 @@ Status SignOutServiceValidator::validateImpl() {
   return Status::OK();
 }
 
+// Plan to sign out external services, e.g. Text Search
 Status SignOutServiceValidator::toPlan() {
   auto sentence = static_cast<SignOutServiceSentence *>(sentence_);
   auto type = sentence->getType();
@@ -639,6 +651,7 @@ Status KillQueryValidator::validateImpl() {
   return Status::OK();
 }
 
+// Plan to kill query by execution plan id
 Status KillQueryValidator::toPlan() {
   auto sentence = static_cast<KillQuerySentence *>(sentence_);
   auto *node = KillQuery::make(qctx_, nullptr, sentence->sessionId(), sentence->epId());

src/graph/validator/AssignmentValidator.cpp

@@ -21,6 +21,7 @@ Status AssignmentValidator::validateImpl() {
   return Status::OK();
 }
 
+// Plan to process assignment in nGQL, e.g. $a = GO FROM <vid_list> OVER <edge_type> YIELD ...
 Status AssignmentValidator::toPlan() {
   root_ = validator_->root();
   auto *var = qctx_->symTable()->newVariable(var_);

src/graph/validator/DownloadValidator.cpp

@@ -11,6 +11,7 @@
 namespace nebula {
 namespace graph {
 
+// Plan to download SST file from HDFS
 Status DownloadValidator::toPlan() {
   auto sentence = static_cast<DownloadSentence *>(sentence_);
   if (sentence->host() == nullptr || sentence->port() == 0 || sentence->path() == nullptr) {

src/graph/validator/ExplainValidator.cpp

@@ -31,6 +31,7 @@ ExplainValidator::ExplainValidator(Sentence* sentence, QueryContext* context)
   }
 }
 
+// Check validity of format type string, and convert to lower case
 static StatusOr<std::string> toExplainFormatType(const std::string& formatType) {
   if (formatType.empty()) {
     return kAllowedFmtType.front();

src/graph/validator/FetchEdgesValidator.cpp

@@ -27,6 +27,7 @@ Status FetchEdgesValidator::validateImpl() {
   return Status::OK();
 }
 
+// Check validity of edge type specified in sentence
 Status FetchEdgesValidator::validateEdgeName() {
   auto &spaceID = space_.id;
   auto status = qctx_->schemaMng()->toEdgeType(spaceID, edgeName_);
@@ -39,6 +40,8 @@ Status FetchEdgesValidator::validateEdgeName() {
   return Status::OK();
 }
 
+// Check validity of edge key(src, type, rank, dst)
+// from Input/Variable expression specified in sentence
 StatusOr<std::string> FetchEdgesValidator::validateEdgeRef(const Expression *expr,
                                                            Value::Type type) {
   const auto &kind = expr->kind();
@@ -62,11 +65,12 @@ StatusOr<std::string> FetchEdgesValidator::validateEdgeRef(const Expression *exp
   return propExpr->sym();
 }
 
+// Check validity of edge key(src, type, rank, dst) from Variable/Constants specified in sentence
 Status FetchEdgesValidator::validateEdgeKey() {
   auto pool = qctx_->objPool();
   auto *sentence = static_cast<FetchEdgesSentence *>(sentence_);
   std::string inputVarName;
-  if (sentence->isRef()) {
+  if (sentence->isRef()) {  // edge keys from Input/Variable
     auto *srcExpr = sentence->ref()->srcid();
     auto result = validateEdgeRef(srcExpr, vidType_);
     NG_RETURN_IF_ERROR(result);
@@ -97,45 +101,48 @@ Status FetchEdgesValidator::validateEdgeKey() {
     fetchCtx_->type = ConstantExpression::make(pool, edgeType_);
     fetchCtx_->inputVarName = std::move(inputVarName);
     return Status::OK();
-  }
-
-  DataSet edgeKeys{{kSrc, kRank, kDst}};
-  QueryExpressionContext ctx;
-  auto keys = sentence->keys()->keys();
-  edgeKeys.rows.reserve(keys.size());
-  for (const auto &key : keys) {
-    if (!ExpressionUtils::isEvaluableExpr(key->srcid(), qctx_)) {
-      return Status::SemanticError("`%s' is not evaluable.", key->srcid()->toString().c_str());
-    }
-    auto src = key->srcid()->eval(ctx);
-    if (src.type() != vidType_) {
-      std::stringstream ss;
-      ss << "the src should be type of " << vidType_ << ", but was`" << src.type() << "'";
-      return Status::SemanticError(ss.str());
-    }
-    auto ranking = key->rank();
+  } else {  // Edge keys from constants
+    DataSet edgeKeys{{kSrc, kRank, kDst}};
+    QueryExpressionContext ctx;
+    auto keys = sentence->keys()->keys();
+    edgeKeys.rows.reserve(keys.size());
+    for (const auto &key : keys) {
+      if (!ExpressionUtils::isEvaluableExpr(key->srcid(), qctx_)) {
+        return Status::SemanticError("`%s' is not evaluable.", key->srcid()->toString().c_str());
+      }
+      auto src = key->srcid()->eval(ctx);
+      if (src.type() != vidType_) {
+        std::stringstream ss;
+        ss << "the src should be type of " << vidType_ << ", but was`" << src.type() << "'";
+        return Status::SemanticError(ss.str());
+      }
+      auto ranking = key->rank();
 
-    if (!ExpressionUtils::isEvaluableExpr(key->dstid(), qctx_)) {
-      return Status::SemanticError("`%s' is not evaluable.", key->dstid()->toString().c_str());
-    }
-    auto dst = key->dstid()->eval(ctx);
-    if (dst.type() != vidType_) {
-      std::stringstream ss;
-      ss << "the dst should be type of " << vidType_ << ", but was`" << dst.type() << "'";
-      return Status::SemanticError(ss.str());
+      if (!ExpressionUtils::isEvaluableExpr(key->dstid(), qctx_)) {
+        return Status::SemanticError("`%s' is not evaluable.", key->dstid()->toString().c_str());
+      }
+      auto dst = key->dstid()->eval(ctx);
+      if (dst.type() != vidType_) {
+        std::stringstream ss;
+        ss << "the dst should be type of " << vidType_ << ", but was`" << dst.type() << "'";
+        return Status::SemanticError(ss.str());
+      }
+      edgeKeys.emplace_back(nebula::Row({std::move(src), ranking, std::move(dst)}));
     }
-    edgeKeys.emplace_back(nebula::Row({std::move(src), ranking, std::move(dst)}));
+    inputVarName = vctx_->anonVarGen()->getVar();
+    qctx_->ectx()->setResult(inputVarName,
+                             ResultBuilder().value(Value(std::move(edgeKeys))).build());
+    fetchCtx_->src = ColumnExpression::make(pool, 0);
+    fetchCtx_->rank = ColumnExpression::make(pool, 1);
+    fetchCtx_->dst = ColumnExpression::make(pool, 2);
+    fetchCtx_->type = ConstantExpression::make(pool, edgeType_);
+    fetchCtx_->inputVarName = std::move(inputVarName);
+    return Status::OK();
   }
-  inputVarName = vctx_->anonVarGen()->getVar();
-  qctx_->ectx()->setResult(inputVarName, ResultBuilder().value(Value(std::move(edgeKeys))).build());
-  fetchCtx_->src = ColumnExpression::make(pool, 0);
-  fetchCtx_->rank = ColumnExpression::make(pool, 1);
-  fetchCtx_->dst = ColumnExpression::make(pool, 2);
-  fetchCtx_->type = ConstantExpression::make(pool, edgeType_);
-  fetchCtx_->inputVarName = std::move(inputVarName);
-  return Status::OK();
 }
 
+// Validate columns of yield clause, rewrites expression to fit its sementic
+// and disable some invalid expression types.
 Status FetchEdgesValidator::validateYield(const YieldClause *yield) {
   if (yield == nullptr) {
     return Status::SemanticError("Missing yield clause.");

src/graph/validator/FetchVerticesValidator.cpp

@@ -23,6 +23,7 @@ Status FetchVerticesValidator::validateImpl() {
   return Status::OK();
 }
 
+// Check validity of tags specified in sentence
 Status FetchVerticesValidator::validateTag(const NameLabelList *nameLabels) {
   if (nameLabels == nullptr) {
     // all tag
@@ -50,6 +51,8 @@ Status FetchVerticesValidator::validateTag(const NameLabelList *nameLabels) {
   return Status::OK();
 }
 
+// Validate columns of yield clause, rewrites expression to fit its sementic
+// and disable some invalid expression types.
 Status FetchVerticesValidator::validateYield(YieldClause *yield) {
   if (yield == nullptr) {
     return Status::SemanticError("Missing yield clause.");

src/graph/validator/FindPathValidator.cpp

@@ -30,6 +30,8 @@ Status FindPathValidator::validateImpl() {
   return Status::OK();
 }
 
+// Check validity of filter expression, rewrites expression to fit its sementic,
+// disable some invalid expression types, collect properties used in filter.
 Status FindPathValidator::validateWhere(WhereClause* where) {
   if (where == nullptr) {
     return Status::OK();
@@ -62,6 +64,7 @@ Status FindPathValidator::validateWhere(WhereClause* where) {
   return Status::OK();
 }
 
+// Check validity of yield columns
 Status FindPathValidator::validateYield(YieldClause* yield) {
   if (yield == nullptr) {
     return Status::SemanticError("Missing yield clause.");