Merge branch 'master' into listener

README.md

@@ -37,6 +37,7 @@ Compared with other graph database solutions, **NebulaGraph** has the following
 * OpenCypher-compatible query language
 * Role-based access control for higher-level security
 
+<!--
 ## Notice of Release
 
 NebulaGraph used to be split into three repositories: [Nebula-Graph](https://github.com/vesoft-inc/nebula-graph), [Nebula-Storage,](https://github.com/vesoft-inc/nebula-storage) and [Nebula-Common](https://github.com/vesoft-inc/nebula-common) for versions between v2.0.0 and v2.5.x, which will be archived.
@@ -45,7 +46,6 @@ The one and only codebase of NebulaGraph is now [github.com/vesoft-inc/nebula](h
 
 Please check the latest release via the documentation: https://docs.nebula-graph.io/.
 
-<!--
 
 NebulaGraph 1.x is not actively maintained. Please move to NebulaGraph 2.x.  <br/>
 The data format, rpc protocols, clients, etc. are not compatible between NebulaGraph v1.x and v2.x,  but we do offer [upgrade guide](https://docs.nebula-graph.io/2.5.0/4.deployment-and-installation/3.upgrade-nebula-graph/upgrade-nebula-graph-to-250/).

src/common/context/ExpressionContext.h

@@ -29,6 +29,13 @@ class ExpressionContext {
   // Get the latest version value for the given variable name, such as $a, $b
   virtual const Value& getVar(const std::string& var) const = 0;
 
+  // Set the value of innerVar. The innerVar is a variable defined in an expression.
+  // e.g. ListComprehension
+  virtual void setInnerVar(const std::string& var, Value val) = 0;
+
+  // Get the value of innerVar.
+  virtual const Value& getInnerVar(const std::string& var) const = 0;
+
   // Get the given version value for the given variable name, such as $a, $b
   virtual const Value& getVersionedVar(const std::string& var, int64_t version) const = 0;
 

src/common/expression/ListComprehensionExpression.cpp

@@ -31,7 +31,7 @@ const Value& ListComprehensionExpression::eval(ExpressionContext& ctx) {
 
   for (size_t i = 0; i < list.size(); ++i) {
     auto& v = list[i];
-    ctx.setVar(innerVar_, v);
+    ctx.setInnerVar(innerVar_, v);
     if (filter_ != nullptr) {
       auto& filterVal = filter_->eval(ctx);
       if (!filterVal.empty() && !filterVal.isNull() && !filterVal.isImplicitBool()) {

src/common/expression/PredicateExpression.cpp

@@ -88,7 +88,7 @@ const Value& PredicateExpression::eval(ExpressionContext& ctx) {
       result_ = true;
       for (size_t i = 0; i < list.size(); ++i) {
         auto& v = list[i];
-        ctx.setVar(innerVar_, v);
+        ctx.setInnerVar(innerVar_, v);
         auto& filterVal = filter_->eval(ctx);
         if (!filterVal.empty() && !filterVal.isNull() && !filterVal.isImplicitBool()) {
           return Value::kNullBadType;
@@ -104,7 +104,7 @@ const Value& PredicateExpression::eval(ExpressionContext& ctx) {
       result_ = false;
       for (size_t i = 0; i < list.size(); ++i) {
         auto& v = list[i];
-        ctx.setVar(innerVar_, v);
+        ctx.setInnerVar(innerVar_, v);
         auto& filterVal = filter_->eval(ctx);
         if (!filterVal.empty() && !filterVal.isNull() && !filterVal.isImplicitBool()) {
           return Value::kNullBadType;
@@ -120,7 +120,7 @@ const Value& PredicateExpression::eval(ExpressionContext& ctx) {
       result_ = false;
       for (size_t i = 0; i < list.size(); ++i) {
         auto& v = list[i];
-        ctx.setVar(innerVar_, v);
+        ctx.setInnerVar(innerVar_, v);
         auto& filterVal = filter_->eval(ctx);
         if (!filterVal.empty() && !filterVal.isNull() && !filterVal.isImplicitBool()) {
           return Value::kNullBadType;
@@ -140,7 +140,7 @@ const Value& PredicateExpression::eval(ExpressionContext& ctx) {
       result_ = true;
       for (size_t i = 0; i < list.size(); ++i) {
         auto& v = list[i];
-        ctx.setVar(innerVar_, v);
+        ctx.setInnerVar(innerVar_, v);
         auto& filterVal = filter_->eval(ctx);
         if (!filterVal.empty() && !filterVal.isNull() && !filterVal.isImplicitBool()) {
           return Value::kNullBadType;

src/common/expression/ReduceExpression.cpp

@@ -23,15 +23,15 @@ const Value& ReduceExpression::eval(ExpressionContext& ctx) {
   }
   auto& list = listVal.getList();
 
-  ctx.setVar(accumulator_, initVal);
+  ctx.setInnerVar(accumulator_, initVal);
   for (size_t i = 0; i < list.size(); ++i) {
     auto& v = list[i];
-    ctx.setVar(innerVar_, v);
+    ctx.setInnerVar(innerVar_, v);
     auto& mappingVal = mapping_->eval(ctx);
-    ctx.setVar(accumulator_, mappingVal);
+    ctx.setInnerVar(accumulator_, mappingVal);
   }
 
-  result_ = ctx.getVar(accumulator_);
+  result_ = ctx.getInnerVar(accumulator_);
   return result_;
 }
 

src/common/expression/VariableExpression.cpp

@@ -10,6 +10,9 @@
 
 namespace nebula {
 const Value& VariableExpression::eval(ExpressionContext& ctx) {
+  if (isInner_) {
+    return ctx.getInnerVar(var_);
+  }
   return ctx.getVar(var_);
 }
 

src/common/expression/VariableExpression.h

@@ -11,10 +11,15 @@
 namespace nebula {
 class VariableExpression final : public Expression {
  public:
-  static VariableExpression* make(ObjectPool* pool,
-                                  const std::string& var = "",
-                                  bool isInner = false) {
-    return pool->makeAndAdd<VariableExpression>(pool, var, isInner);
+  // Make a non-inner variable expression
+  static VariableExpression* make(ObjectPool* pool, const std::string& var = "") {
+    return pool->makeAndAdd<VariableExpression>(pool, var, false);
+  }
+
+  // Make a inner variable expression. Inner variable is a variable defined in an expression.
+  // e.g. ListComprehensionExpression [i IN range(1, 10) | i+1]
+  static VariableExpression* makeInner(ObjectPool* pool, const std::string& var = "") {
+    return pool->makeAndAdd<VariableExpression>(pool, var, true);
   }
 
   const std::string& var() const {
@@ -39,7 +44,7 @@ class VariableExpression final : public Expression {
   void accept(ExprVisitor* visitor) override;
 
   Expression* clone() const override {
-    return VariableExpression::make(pool_, var(), isInner_);
+    return pool_->makeAndAdd<VariableExpression>(pool_, var(), isInner_);
   }
 
  private:

src/common/expression/test/ExpressionContextMock.h

@@ -23,6 +23,16 @@ class ExpressionContextMock final : public ExpressionContext {
     }
   }
 
+  void setInnerVar(const std::string& var, Value val) override {
+    exprValueMap_[var] = std::move(val);
+  }
+
+  const Value& getInnerVar(const std::string& var) const override {
+    auto it = exprValueMap_.find(var);
+    DCHECK(it != exprValueMap_.end());
+    return it->second;
+  }
+
   const Value& getVersionedVar(const std::string& var, int64_t version) const override {
     auto found = indices_.find(var);
     if (found == indices_.end()) {
@@ -143,5 +153,7 @@ class ExpressionContextMock final : public ExpressionContext {
   static std::unordered_map<std::string, std::size_t> indices_;
   static std::vector<Value> vals_;
   std::unordered_map<std::string, std::regex> regex_;
+  // Expression value map that stores the value of innerVar
+  std::unordered_map<std::string, Value> exprValueMap_;
 };
 }  // namespace nebula

src/common/expression/test/ListComprehensionExpressionTest.cpp

@@ -23,9 +23,9 @@ TEST_F(ListComprehensionExpressionTest, ListComprehensionEvaluate) {
         "n",
         ListExpression::make(&pool, listItems),
         RelationalExpression::makeGE(
-            &pool, VariableExpression::make(&pool, "n"), ConstantExpression::make(&pool, 2)),
+            &pool, VariableExpression::makeInner(&pool, "n"), ConstantExpression::make(&pool, 2)),
         ArithmeticExpression::makeAdd(
-            &pool, VariableExpression::make(&pool, "n"), ConstantExpression::make(&pool, 10)));
+            &pool, VariableExpression::makeInner(&pool, "n"), ConstantExpression::make(&pool, 10)));
 
     auto value = Expression::eval(expr, gExpCtxt);
     List expected;
@@ -57,7 +57,7 @@ TEST_F(ListComprehensionExpressionTest, ListComprehensionEvaluate) {
         ArithmeticExpression::makeAdd(
             &pool,
             AttributeExpression::make(&pool,
-                                      VariableExpression::make(&pool, "n"),
+                                      VariableExpression::makeInner(&pool, "n"),
                                       ConstantExpression::make(&pool, "age")),
             ConstantExpression::make(&pool, 5)));
 

src/common/expression/test/PredicateExpressionTest.cpp

@@ -24,7 +24,7 @@ TEST_F(PredicateExpressionTest, PredicateEvaluate) {
         "n",
         ListExpression::make(&pool, listItems),
         RelationalExpression::makeGE(
-            &pool, VariableExpression::make(&pool, "n"), ConstantExpression::make(&pool, 2)));
+            &pool, VariableExpression::makeInner(&pool, "n"), ConstantExpression::make(&pool, 2)));
 
     auto value = Expression::eval(expr, gExpCtxt);
     ASSERT_TRUE(value.isBool());
@@ -51,7 +51,7 @@ TEST_F(PredicateExpressionTest, PredicateEvaluate) {
         RelationalExpression::makeGE(
             &pool,
             AttributeExpression::make(&pool,
-                                      VariableExpression::make(&pool, "n"),
+                                      VariableExpression::makeInner(&pool, "n"),
                                       ConstantExpression::make(&pool, "age")),
             ConstantExpression::make(&pool, 19)));
 
@@ -74,7 +74,7 @@ TEST_F(PredicateExpressionTest, PredicateEvaluate) {
         "n",
         ListExpression::make(&pool, listItems),
         RelationalExpression::makeEQ(
-            &pool, VariableExpression::make(&pool, "n"), ConstantExpression::make(&pool, 2)));
+            &pool, VariableExpression::makeInner(&pool, "n"), ConstantExpression::make(&pool, 2)));
 
     auto value = Expression::eval(expr, gExpCtxt);
     ASSERT_TRUE(value.isBool());
@@ -101,7 +101,7 @@ TEST_F(PredicateExpressionTest, PredicateEvaluate) {
         RelationalExpression::makeGE(
             &pool,
             AttributeExpression::make(&pool,
-                                      VariableExpression::make(&pool, "n"),
+                                      VariableExpression::makeInner(&pool, "n"),
                                       ConstantExpression::make(&pool, "age")),
             ConstantExpression::make(&pool, 19)));
 
@@ -117,7 +117,7 @@ TEST_F(PredicateExpressionTest, PredicateEvaluate) {
         "n",
         ConstantExpression::make(&pool, Value(NullType::__NULL__)),
         RelationalExpression::makeEQ(
-            &pool, VariableExpression::make(&pool, "n"), ConstantExpression::make(&pool, 1)));
+            &pool, VariableExpression::makeInner(&pool, "n"), ConstantExpression::make(&pool, 1)));
 
     auto value = Expression::eval(expr, gExpCtxt);
     ASSERT_EQ(Value::kNullValue, value.getNull());

src/common/expression/test/ReduceExpressionTest.cpp

@@ -23,9 +23,10 @@ TEST_F(ReduceExpressionTest, ReduceEvaluate) {
         FunctionCallExpression::make(&pool, "range", argList),
         ArithmeticExpression::makeAdd(
             &pool,
-            VariableExpression::make(&pool, "totalNum"),
-            ArithmeticExpression::makeMultiply(
-                &pool, VariableExpression::make(&pool, "n"), ConstantExpression::make(&pool, 2))));
+            VariableExpression::makeInner(&pool, "totalNum"),
+            ArithmeticExpression::makeMultiply(&pool,
+                                               VariableExpression::makeInner(&pool, "n"),
+                                               ConstantExpression::make(&pool, 2))));
 
     auto value = Expression::eval(expr, gExpCtxt);
     ASSERT_EQ(Value::Type::INT, value.type());

src/common/meta/GflagsManager.cpp

@@ -196,11 +196,14 @@ std::string GflagsManager::ValueToGflagString(const Value& val) {
       std::transform(kvs.begin(), kvs.end(), values.begin(), [](const auto& iter) -> std::string {
         std::stringstream out;
         out << "\"" << iter.first << "\""
-            << ":"
-            << "\"" << iter.second << "\"";
+            << ":";
+        if (iter.second.isStr() || iter.second.isBool()) {
+          out << iter.second;
+        } else {
+          out << "\"" << iter.second << "\"";
+        }
         return out.str();
       });
-
       std::stringstream os;
       os << "{" << folly::join(",", values) << "}";
       return os.str();

src/common/utils/DefaultValueContext.h

@@ -18,6 +18,15 @@ class DefaultValueContext final : public ExpressionContext {
     return Value::kEmpty;
   }
 
+  void setInnerVar(const std::string&, Value) override {
+    LOG(FATAL) << "Not allowed to call";
+  }
+
+  const Value& getInnerVar(const std::string&) const override {
+    LOG(FATAL) << "Not allowed to call";
+    return Value::kEmpty;
+  }
+
   const Value& getVersionedVar(const std::string&, int64_t) const override {
     LOG(FATAL) << "Not allowed to call";
     return Value::kEmpty;

src/graph/context/QueryExpressionContext.cpp

@@ -14,6 +14,16 @@ const Value& QueryExpressionContext::getVar(const std::string& var) const {
   return ectx_->getValue(var);
 }
 
+void QueryExpressionContext::setInnerVar(const std::string& var, Value val) {
+  exprValueMap_[var] = std::move(val);
+}
+
+const Value& QueryExpressionContext::getInnerVar(const std::string& var) const {
+  auto it = exprValueMap_.find(var);
+  if (it == exprValueMap_.end()) return Value::kEmpty;
+  return it->second;
+}
+
 const Value& QueryExpressionContext::getVersionedVar(const std::string& var,
                                                      int64_t version) const {
   if (ectx_ == nullptr) {

src/graph/context/QueryExpressionContext.h

@@ -24,6 +24,13 @@ class QueryExpressionContext final : public ExpressionContext {
   // Get the latest version value for the given variable name, such as $a, $b
   const Value& getVar(const std::string& var) const override;
 
+  // Set the value of innerVar. The innerVar is a variable defined in an expression.
+  // e.g. ListComprehension
+  void setInnerVar(const std::string& var, Value val) override;
+
+  // Get the value of innerVar
+  const Value& getInnerVar(const std::string& var) const override;
+
   // Get the given version value for the given variable name, such as $a, $b
   const Value& getVersionedVar(const std::string& var, int64_t version) const override;
 
@@ -75,6 +82,9 @@ class QueryExpressionContext final : public ExpressionContext {
   // could be evaluated as constant value.
   ExecutionContext* ectx_{nullptr};
   Iterator* iter_{nullptr};
+
+  // Expression value map that stores the value of innerVar
+  std::unordered_map<std::string, Value> exprValueMap_;
 };
 
 }  // namespace graph

src/graph/optimizer/rule/PushFilterDownAggregateRule.cpp

@@ -59,6 +59,7 @@ StatusOr<OptRule::TransformResult> PushFilterDownAggregateRule::transform(
   }
   std::unordered_map<std::string, Expression*> rewriteMap;
   auto colNames = newAggNode->colNames();
+  DCHECK_EQ(newAggNode->colNames().size(), newAggNode->groupItems().size());
   for (size_t i = 0; i < colNames.size(); ++i) {
     auto& colName = colNames[i];
     auto iter = std::find_if(propNames.begin(), propNames.end(), [&colName](const auto& name) {
@@ -89,11 +90,17 @@ StatusOr<OptRule::TransformResult> PushFilterDownAggregateRule::transform(
   newFilterNode->setCondition(newCondition);
 
   // Exchange planNode
+  // newAggNode shall inherit the output of the oldFilterNode
   newAggNode->setOutputVar(oldFilterNode->outputVar());
+  // as the new agg node now inherits the output var ptr from a filter node, the action of
+  // which alters its own colNames, its colNames need to be explicitly preserved.
+  newAggNode->setColNames(oldAggNode->colNames());
+  // newFilterNode shall inherit the input of the oldAggNode
   newFilterNode->setInputVar(oldAggNode->inputVar());
   DCHECK_EQ(oldAggNode->outputVar(), oldFilterNode->inputVar());
-  newAggNode->setInputVar(oldAggNode->outputVar());
-  newFilterNode->setOutputVar(oldAggNode->outputVar());
+  // newAggNode shall inherit oldFilterNode's inputs
+  newAggNode->setInputVar(oldFilterNode->inputVar());
+  newFilterNode->setOutputVar(newAggNode->inputVar());
 
   // Push down filter's optGroup and embed newAggGroupNode into old filter's
   // Group

src/graph/optimizer/rule/PushFilterDownProjectRule.cpp

@@ -136,13 +136,18 @@ StatusOr<OptRule::TransformResult> PushFilterDownProjectRule::transform(
     newAboveFilterNode->setInputVar(newProjNode->outputVar());
     result.newGroupNodes.emplace_back(newAboveFilterGroupNode);
   } else {
+    // newProjNode shall inherit the output from oldFilterNode
+    // which is the output of the opt group
     newProjNode->setOutputVar(oldFilterNode->outputVar());
+    // newProjNode's col names, on the hand, should inherit those of the oldProjNode
+    // since they are the same project.
+    newProjNode->setColNames(oldProjNode->outputVarPtr()->colNames);
     auto newProjGroupNode = OptGroupNode::create(octx, newProjNode, filterGroupNode->group());
     newProjGroupNode->setDeps({newBelowFilterGroup});
     newProjNode->setInputVar(newBelowFilterNode->outputVar());
     result.newGroupNodes.emplace_back(newProjGroupNode);
   }
-
+  DCHECK_EQ(newProjNode->colNames().size(), newProjNode->columns()->columns().size());
   return result;
 }
 

src/graph/planner/match/MatchSolver.cpp

@@ -240,7 +240,7 @@ static YieldColumn* buildEdgeColumn(QueryContext* qctx, const EdgeInfo& edge) {
   } else {
     auto innerVar = qctx->vctx()->anonVarGen()->getVar();
     auto* args = ArgumentList::make(pool);
-    args->addArgument(VariableExpression::make(pool, innerVar));
+    args->addArgument(VariableExpression::makeInner(pool, innerVar));
     auto* filter = FunctionCallExpression::make(pool, "is_edge", args);
     expr = ListComprehensionExpression::make(
         pool, innerVar, InputPropertyExpression::make(pool, edge.alias), filter);

src/graph/planner/plan/PlanNode.cpp

@@ -396,6 +396,12 @@ SingleInputNode::SingleInputNode(QueryContext* qctx, Kind kind, const PlanNode*
   }
 }
 
+void SingleInputNode::copyInputColNames(const PlanNode* input) {
+  if (input != nullptr) {
+    setColNames(input->colNames());
+  }
+}
+
 std::unique_ptr<PlanNodeDescription> SingleDependencyNode::explain() const {
   auto desc = PlanNode::explain();
   DCHECK(desc->dependencies == nullptr);

src/graph/planner/plan/PlanNode.h

@@ -376,11 +376,7 @@ class SingleInputNode : public SingleDependencyNode {
     SingleDependencyNode::cloneMembers(node);
   }
 
-  void copyInputColNames(const PlanNode* input) {
-    if (input != nullptr) {
-      setColNames(input->colNames());
-    }
-  }
+  void copyInputColNames(const PlanNode* input);
 
   SingleInputNode(QueryContext* qctx, Kind kind, const PlanNode* dep);
 };