Merge branch 'master' into fix_restart

.github/workflows/nightly.yml

@@ -189,7 +189,7 @@ jobs:
  timeout-minutes: 2
  - name: coverage
  run: |
- ~/.local/bin/fastcov -d build -l -o fastcov.info -p --exclude /usr/include --exclude=/opt/vesoft --exclude scanner.lex
+ ~/.local/bin/fastcov -d build -l -o fastcov.info -p --exclude /usr/include /usr/lib /opt/vesoft build/ tests/ /test .lex .yy
  - uses: codecov/codecov-action@v2
  with:
  files: fastcov.info

.github/workflows/pull_request.yml

@@ -185,7 +185,7 @@ jobs:
  - name: coverage
  if: ${{ matrix.compiler == 'gcc-9.3' && matrix.os == 'ubuntu2004' }}
  run: |
- ~/.local/bin/fastcov -d build -l -o fastcov.info -p --exclude /usr/include --exclude=/opt/vesoft --exclude scanner.lex
+ ~/.local/bin/fastcov -d build -l -o fastcov.info -p --exclude /usr/include /usr/lib /opt/vesoft build/ tests/ /test .lex .yy
  - uses: codecov/codecov-action@v2
  if: ${{ matrix.compiler == 'gcc-9.3' && matrix.os == 'ubuntu2004' }}
  with:

src/common/expression/ListComprehensionExpression.cpp

@@ -34,10 +34,10 @@ const Value& ListComprehensionExpression::eval(ExpressionContext& ctx) {
  ctx.setVar(innerVar_, v);
  if (filter_ != nullptr) {
  auto& filterVal = filter_->eval(ctx);
- if (!filterVal.empty() && !filterVal.isNull() && !filterVal.isBool()) {
+ if (!filterVal.empty() && !filterVal.isNull() && !filterVal.isImplicitBool()) {
  return Value::kNullBadType;
  }
- if (filterVal.empty() || filterVal.isNull() || !filterVal.getBool()) {
+ if (filterVal.empty() || filterVal.isNull() || !filterVal.implicitBool()) {
  continue;
  }
  }

src/common/expression/PredicateExpression.cpp

@@ -90,10 +90,10 @@ const Value& PredicateExpression::eval(ExpressionContext& ctx) {
  auto& v = list[i];
  ctx.setVar(innerVar_, v);
  auto& filterVal = filter_->eval(ctx);
- if (!filterVal.empty() && !filterVal.isNull() && !filterVal.isBool()) {
+ if (!filterVal.empty() && !filterVal.isNull() && !filterVal.isImplicitBool()) {
  return Value::kNullBadType;
  }
- if (filterVal.empty() || filterVal.isNull() || !filterVal.getBool()) {
+ if (filterVal.empty() || filterVal.isNull() || !filterVal.implicitBool()) {
  result_ = false;
  return result_;
  }
@@ -106,10 +106,10 @@ const Value& PredicateExpression::eval(ExpressionContext& ctx) {
  auto& v = list[i];
  ctx.setVar(innerVar_, v);
  auto& filterVal = filter_->eval(ctx);
- if (!filterVal.empty() && !filterVal.isNull() && !filterVal.isBool()) {
+ if (!filterVal.empty() && !filterVal.isNull() && !filterVal.isImplicitBool()) {
  return Value::kNullBadType;
  }
- if (filterVal.isBool() && filterVal.getBool()) {
+ if (filterVal.isBool() && filterVal.implicitBool()) {
  result_ = true;
  return result_;
  }
@@ -122,10 +122,10 @@ const Value& PredicateExpression::eval(ExpressionContext& ctx) {
  auto& v = list[i];
  ctx.setVar(innerVar_, v);
  auto& filterVal = filter_->eval(ctx);
- if (!filterVal.empty() && !filterVal.isNull() && !filterVal.isBool()) {
+ if (!filterVal.empty() && !filterVal.isNull() && !filterVal.isImplicitBool()) {
  return Value::kNullBadType;
  }
- if (filterVal.isBool() && filterVal.getBool()) {
+ if (filterVal.isBool() && filterVal.implicitBool()) {
  if (result_ == false) {
  result_ = true;
  } else {
@@ -142,10 +142,10 @@ const Value& PredicateExpression::eval(ExpressionContext& ctx) {
  auto& v = list[i];
  ctx.setVar(innerVar_, v);
  auto& filterVal = filter_->eval(ctx);
- if (!filterVal.empty() && !filterVal.isNull() && !filterVal.isBool()) {
+ if (!filterVal.empty() && !filterVal.isNull() && !filterVal.isImplicitBool()) {
  return Value::kNullBadType;
  }
- if (filterVal.isBool() && filterVal.getBool()) {
+ if (filterVal.isBool() && filterVal.implicitBool()) {
  result_ = false;
  return result_;
  }

src/graph/context/ExecutionContext.cpp

@@ -26,6 +26,7 @@ void ExecutionContext::setResult(const std::string& name, Result&& result) {
 }
 
 void ExecutionContext::dropResult(const std::string& name) {
+ DCHECK_EQ(valueMap_.count(name), 1);
  auto& val = valueMap_[name];
  if (FLAGS_enable_async_gc) {
  GC::instance().clear(std::move(val));

src/graph/context/QueryContext.cpp

@@ -38,7 +38,7 @@ void QueryContext::init() {
  }
  }
  idGen_ = std::make_unique<IdGenerator>(0);
- symTable_ = std::make_unique<SymbolTable>(objPool_.get());
+ symTable_ = std::make_unique<SymbolTable>(objPool_.get(), ectx_.get());
  vctx_ = std::make_unique<ValidateContext>(std::make_unique<AnonVarGenerator>(symTable_.get()));
 }
 

src/graph/context/QueryContext.h

@@ -136,8 +136,10 @@ class QueryContext {
  return killed_.load();
  }
 
+ // This is only valid in building stage!
+ // TODO remove parameter from variables map
  bool existParameter(const std::string& param) const {
- return ectx_->exist(param) && (ectx_->getValue(param).type() != Value::Type::DATASET);
+ return !ectx_->getValue(param).empty(); // Really fill value for parameter
  }
 
  private:

src/graph/context/Symbols.cpp

@@ -34,16 +34,14 @@ std::string SymbolTable::toString() const {
  return ss.str();
 }
 
-SymbolTable::SymbolTable(ObjectPool* objPool) {
- DCHECK(objPool != nullptr);
- objPool_ = objPool;
-}
-
-Variable* SymbolTable::newVariable(std::string name) {
+Variable* SymbolTable::newVariable(const std::string& name) {
  VLOG(1) << "New variable for: " << name;
  DCHECK(vars_.find(name) == vars_.end());
  auto* variable = objPool_->makeAndAdd<Variable>(name);
- addVar(std::move(name), variable);
+ addVar(name, variable);
+ // Initialize all variable in variable map (ouput of node, inner variable etc.)
+ // Some variable will be useless after optimizer, maybe we could remove it.
+ ectx_->initVar(name);
  return variable;
 }
 

src/graph/context/Symbols.h

@@ -12,6 +12,7 @@
 #include "common/base/ObjectPool.h"
 #include "common/base/StatusOr.h"
 #include "common/datatypes/Value.h"
+#include "graph/context/ExecutionContext.h"
 
 namespace nebula {
 namespace graph {
@@ -52,11 +53,10 @@ struct Variable {
 
 class SymbolTable final {
  public:
- explicit SymbolTable(ObjectPool* objPool);
+ explicit SymbolTable(ObjectPool* objPool, ExecutionContext* ectx)
+ : objPool_(DCHECK_NOTNULL(objPool)), ectx_(DCHECK_NOTNULL(ectx)) {}
 
- Variable* newVariable(std::string name);
-
- void addVar(std::string varName, Variable* variable);
+ Variable* newVariable(const std::string& name);
 
  bool readBy(const std::string& varName, PlanNode* node);
 
@@ -79,7 +79,10 @@ class SymbolTable final {
  std::string toString() const;
 
  private:
+ void addVar(std::string varName, Variable* variable);
+
  ObjectPool* objPool_{nullptr};
+ ExecutionContext* ectx_{nullptr};
  // var name -> variable
  std::unordered_map<std::string, Variable*> vars_;
  // alias -> first variable that generate the alias

src/graph/executor/Executor.cpp

@@ -571,9 +571,7 @@ Executor::Executor(const std::string &name, const PlanNode *node, QueryContext *
  // Initialize the position in ExecutionContext for each executor before
  // execution plan starting to run. This will avoid lock something for thread
  // safety in real execution
- if (!ectx_->exist(node->outputVar())) {
- ectx_->initVar(node->outputVar());
- }
+ DCHECK(ectx_->exist(node->outputVar()));
 }
 
 Executor::~Executor() {}

src/graph/executor/admin/SubmitJobExecutor.cpp

@@ -131,8 +131,8 @@ nebula::DataSet SubmitJobExecutor::buildShowResultData(
  v.emplace_back(Row({jd.get_job_id(),
  apache::thrift::util::enumNameSafe(meta::cpp2::JobType::DATA_BALANCE),
  apache::thrift::util::enumNameSafe(jd.get_status()),
- convertJobTimestampToDateTime(jd.get_start_time()).toString(),
- convertJobTimestampToDateTime(jd.get_stop_time()).toString(),
+ convertJobTimestampToDateTime(jd.get_start_time()),
+ convertJobTimestampToDateTime(jd.get_stop_time()),
  apache::thrift::util::enumNameSafe(jd.get_code())}));
  for (size_t i = index; i < paras.size() - 1; i++) {
  meta::cpp2::BalanceTask tsk;

src/graph/executor/test/ProjectTest.cpp

@@ -5,11 +5,13 @@
 
 #include <gtest/gtest.h>
 
+#include "common/expression/PropertyExpression.h"
 #include "graph/context/QueryContext.h"
 #include "graph/executor/query/ProjectExecutor.h"
 #include "graph/executor/test/QueryTestBase.h"
 #include "graph/planner/plan/Logic.h"
 #include "graph/planner/plan/Query.h"
+#include "parser/Clauses.h"
 
 namespace nebula {
 namespace graph {
@@ -51,7 +53,9 @@ class ProjectTest : public QueryTestBase {
 
 TEST_F(ProjectTest, Project1Col) {
  std::string input = "input_project";
- auto yieldColumns = getYieldColumns("YIELD $input_project.vid AS vid", qctx_.get());
+ auto yieldColumns = qctx_->objPool()->makeAndAdd<YieldColumns>();
+ yieldColumns->addColumn(new YieldColumn(
+ VariablePropertyExpression::make(qctx_->objPool(), "input_project", "vid"), "vid"));
 
  auto* project = Project::make(qctx_.get(), start_, yieldColumns);
  project->setInputVar(input);
@@ -76,8 +80,11 @@ TEST_F(ProjectTest, Project1Col) {
 
 TEST_F(ProjectTest, Project2Col) {
  std::string input = "input_project";
- auto yieldColumns =
- getYieldColumns("YIELD $input_project.vid AS vid, $input_project.col2 AS num", qctx_.get());
+ auto yieldColumns = qctx_->objPool()->makeAndAdd<YieldColumns>();
+ yieldColumns->addColumn(new YieldColumn(
+ VariablePropertyExpression::make(qctx_->objPool(), "input_project", "vid"), "vid"));
+ yieldColumns->addColumn(new YieldColumn(
+ VariablePropertyExpression::make(qctx_->objPool(), "input_project", "col2"), "num"));
  auto* project = Project::make(qctx_.get(), start_, yieldColumns);
  project->setInputVar(input);
  project->setColNames(std::vector<std::string>{"vid", "num"});
@@ -102,7 +109,9 @@ TEST_F(ProjectTest, Project2Col) {
 
 TEST_F(ProjectTest, EmptyInput) {
  std::string input = "empty";
- auto yieldColumns = getYieldColumns("YIELD $input_project.vid AS vid", qctx_.get());
+ auto yieldColumns = qctx_->objPool()->makeAndAdd<YieldColumns>();
+ yieldColumns->addColumn(new YieldColumn(
+ VariablePropertyExpression::make(qctx_->objPool(), "input_project", "vid"), "vid"));
  auto* project = Project::make(qctx_.get(), start_, std::move(yieldColumns));
  project->setInputVar(input);
  project->setColNames(std::vector<std::string>{"vid"});

src/graph/optimizer/rule/PushFilterDownProjectRule.cpp

@@ -49,7 +49,6 @@ StatusOr<OptRule::TransformResult> PushFilterDownProjectRule::transform(
  auto picker = [&projColumns, &projColNames, &rewriteMap](const Expression* e) -> bool {
  auto varProps = graph::ExpressionUtils::collectAll(e,
  {Expression::Kind::kTagProperty,
- Expression::Kind::kLabelTagProperty,
  Expression::Kind::kEdgeProperty,
  Expression::Kind::kInputProperty,
  Expression::Kind::kVarProperty,
@@ -71,9 +70,7 @@ StatusOr<OptRule::TransformResult> PushFilterDownProjectRule::transform(
  });
  if (iter == propNames.end()) continue;
  if (graph::ExpressionUtils::isPropertyExpr(column->expr())) {
- if (!column->alias().empty()) {
- rewriteMap[colName] = column->expr();
- }
+ rewriteMap[colName] = column->expr();
  continue;
  } else {
  return false;

src/graph/planner/match/MatchSolver.cpp

@@ -6,6 +6,7 @@
 #include "graph/planner/match/MatchSolver.h"
 
 #include "common/expression/UnaryExpression.h"
+#include "graph/context/QueryContext.h"
 #include "graph/context/ast/AstContext.h"
 #include "graph/context/ast/CypherAstContext.h"
 #include "graph/planner/Planner.h"
@@ -230,17 +231,19 @@ static YieldColumn* buildVertexColumn(ObjectPool* pool, const std::string& alias
  return new YieldColumn(InputPropertyExpression::make(pool, alias), alias);
 }
 
-static YieldColumn* buildEdgeColumn(ObjectPool* pool, const EdgeInfo& edge) {
+static YieldColumn* buildEdgeColumn(QueryContext* qctx, const EdgeInfo& edge) {
  Expression* expr = nullptr;
+ auto pool = qctx->objPool();
  if (edge.range == nullptr) {
  expr = SubscriptExpression::make(
  pool, InputPropertyExpression::make(pool, edge.alias), ConstantExpression::make(pool, 0));
  } else {
+ auto innerVar = qctx->vctx()->anonVarGen()->getVar();
  auto* args = ArgumentList::make(pool);
- args->addArgument(VariableExpression::make(pool, "e"));
+ args->addArgument(VariableExpression::make(pool, innerVar));
  auto* filter = FunctionCallExpression::make(pool, "is_edge", args);
  expr = ListComprehensionExpression::make(
- pool, "e", InputPropertyExpression::make(pool, edge.alias), filter);
+ pool, innerVar, InputPropertyExpression::make(pool, edge.alias), filter);
  }
  return new YieldColumn(expr, edge.alias);
 }
@@ -261,7 +264,7 @@ void MatchSolver::buildProjectColumns(QueryContext* qctx, Path& path, SubPlan& p
 
  auto addEdge = [columns, &colNames, qctx](auto& edgeInfo) {
  if (!edgeInfo.alias.empty() && !edgeInfo.anonymous) {
- columns->addColumn(buildEdgeColumn(qctx->objPool(), edgeInfo));
+ columns->addColumn(buildEdgeColumn(qctx, edgeInfo));
  colNames.emplace_back(edgeInfo.alias);
  }
  };

src/graph/service/PermissionManager.cpp

@@ -133,25 +133,26 @@ Status PermissionManager::canWriteRole(ClientSession *session,
  meta::cpp2::RoleType targetRole,
  GraphSpaceID spaceId,
  const std::string &targetUser) {
- if (!FLAGS_enable_authorize) {
- return Status::OK();
+ // Some check should be done no matter FLAGS_enable_authorize is true or false
+ // Check 1. Reject any user grant or revoke role to GOD,
+ if (targetRole == meta::cpp2::RoleType::GOD) {
+ return Status::PermissionError("No permission to grant/revoke god user.");
  }
- // Cloud auth user cannot grant role
+
+ // Check 2. Cloud auth user cannot grant role
  if (FLAGS_auth_type == "cloud") {
  return Status::PermissionError("Cloud authenticate user can't write role.");
  }
+
+ if (!FLAGS_enable_authorize) {
+ return Status::OK();
+ }
  /**
  * Reject grant or revoke to himself.
  */
  if (session->user() == targetUser) {
  return Status::PermissionError("No permission to grant/revoke yourself.");
  }
- /*
- * Reject any user grant or revoke role to GOD
- */
- if (targetRole == meta::cpp2::RoleType::GOD) {
- return Status::PermissionError("No permission to grant/revoke god user.");
- }
  /*
  * God user can be grant or revoke any one.
  */