From ef5a91b4d8bf0a6ca15d76ae37b04661a2ca98b0 Mon Sep 17 00:00:00 2001
From: Aiee <18348405+Aiee@users.noreply.github.com>
Date: Tue, 8 Feb 2022 17:05:25 +0800
Subject: [PATCH 1/4] Add comments to util/ExpressionUtils

---
 src/graph/util/AstUtils.h          |   1 +
 src/graph/util/ContainerConv.h     |  33 ---------
 src/graph/util/ExpressionUtils.cpp | 113 ++++++++---------------------
 src/graph/util/ExpressionUtils.h   |  82 +++++++++++++++------
 4 files changed, 90 insertions(+), 139 deletions(-)
 delete mode 100644 src/graph/util/ContainerConv.h

diff --git a/src/graph/util/AstUtils.h b/src/graph/util/AstUtils.h
index 8262b0c7df8..ae7dee87ea3 100644
--- a/src/graph/util/AstUtils.h
+++ b/src/graph/util/AstUtils.h
@@ -11,6 +11,7 @@
 namespace nebula {
 namespace graph {
 
+// Used to test GQL parser
 class AstUtils final {
  public:
   explicit AstUtils(...) = delete;
diff --git a/src/graph/util/ContainerConv.h b/src/graph/util/ContainerConv.h
deleted file mode 100644
index 817f081d7c9..00000000000
--- a/src/graph/util/ContainerConv.h
+++ /dev/null
@@ -1,33 +0,0 @@
-/* Copyright (c) 2020 vesoft inc. All rights reserved.
- *
- * This source code is licensed under Apache 2.0 License.
- */
-
-#ifndef GRAPH_UTIL_CONTAINERCONV_H_
-#define GRAPH_UTIL_CONTAINERCONV_H_
-
-#include "common/base/Base.h"
-
-namespace nebula {
-namespace graph {
-
-class ContainerConv {
- public:
-  explicit ContainerConv(...) = delete;
-
-  template <template <typename, typename...> class To,
-            typename T,
-            template <typename, typename...>
-            class From>
-  static To<T> to(From<T> &&from) {
-    To<T> to;
-    to.reserve(from.size());
-    to.insert(to.end(), std::make_move_iterator(from.begin()), std::make_move_iterator(from.end()));
-    return to;
-  }
-};
-
-}  // namespace graph
-}  // namespace nebula
-
-#endif  // GRAPH_UTIL_CONTAINERCONV_H_
diff --git a/src/graph/util/ExpressionUtils.cpp b/src/graph/util/ExpressionUtils.cpp
index 42d57d2d3b2..51de2eda947 100644
--- a/src/graph/util/ExpressionUtils.cpp
+++ b/src/graph/util/ExpressionUtils.cpp
@@ -5,10 +5,6 @@
 
 #include "graph/util/ExpressionUtils.h"
 
-#include <memory>
-#include <queue>
-#include <unordered_set>
-
 #include "common/base/ObjectPool.h"
 #include "common/expression/ArithmeticExpression.h"
 #include "common/expression/Expression.h"
@@ -25,16 +21,14 @@ namespace nebula {
 namespace graph {
 
 bool ExpressionUtils::isPropertyExpr(const Expression *expr) {
-  auto kind = expr->kind();
-
-  return std::unordered_set<Expression::Kind>{Expression::Kind::kTagProperty,
-                                              Expression::Kind::kLabelTagProperty,
-                                              Expression::Kind::kEdgeProperty,
-                                              Expression::Kind::kInputProperty,
-                                              Expression::Kind::kVarProperty,
-                                              Expression::Kind::kDstProperty,
-                                              Expression::Kind::kSrcProperty}
-      .count(kind);
+  return isKindOf(expr,
+                  {Expression::Kind::kTagProperty,
+                   Expression::Kind::kLabelTagProperty,
+                   Expression::Kind::kEdgeProperty,
+                   Expression::Kind::kInputProperty,
+                   Expression::Kind::kVarProperty,
+                   Expression::Kind::kDstProperty,
+                   Expression::Kind::kSrcProperty});
 }
 
 const Expression *ExpressionUtils::findAny(const Expression *self,
@@ -57,8 +51,8 @@ const Expression *ExpressionUtils::findAny(const Expression *self,
   return nullptr;
 }
 
-// Find all expression fit any kind
-// Empty for not found any one
+// Finds all expressions fit the exprected list
+// Returns an empty vector if no expression found
 std::vector<const Expression *> ExpressionUtils::collectAll(
     const Expression *self, const std::unordered_set<Expression::Kind> &expected) {
   auto finder = [&expected](const Expression *expr) -> bool {
@@ -83,52 +77,12 @@ bool ExpressionUtils::checkVarExprIfExist(const Expression *expr, const QueryCon
   return false;
 }
 
-std::vector<const Expression *> ExpressionUtils::findAllStorage(const Expression *expr) {
-  return collectAll(expr,
-                    {Expression::Kind::kTagProperty,
-                     Expression::Kind::kLabelTagProperty,
-                     Expression::Kind::kEdgeProperty,
-                     Expression::Kind::kDstProperty,
-                     Expression::Kind::kSrcProperty,
-                     Expression::Kind::kEdgeSrc,
-                     Expression::Kind::kEdgeType,
-                     Expression::Kind::kEdgeRank,
-                     Expression::Kind::kEdgeDst,
-                     Expression::Kind::kVertex,
-                     Expression::Kind::kEdge});
-}
-
-std::vector<const Expression *> ExpressionUtils::findAllInputVariableProp(const Expression *expr) {
-  return collectAll(expr, {Expression::Kind::kInputProperty, Expression::Kind::kVarProperty});
-}
-
-bool ExpressionUtils::isConstExpr(const Expression *expr) {
-  return !hasAny(expr,
-                 {Expression::Kind::kInputProperty,
-                  Expression::Kind::kVarProperty,
-                  Expression::Kind::kVar,
-                  Expression::Kind::kVersionedVar,
-                  Expression::Kind::kLabelAttribute,
-                  Expression::Kind::kLabelTagProperty,
-                  Expression::Kind::kTagProperty,
-                  Expression::Kind::kEdgeProperty,
-                  Expression::Kind::kDstProperty,
-                  Expression::Kind::kSrcProperty,
-                  Expression::Kind::kEdgeSrc,
-                  Expression::Kind::kEdgeType,
-                  Expression::Kind::kEdgeRank,
-                  Expression::Kind::kEdgeDst,
-                  Expression::Kind::kVertex,
-                  Expression::Kind::kEdge});
-}
-
 bool ExpressionUtils::isEvaluableExpr(const Expression *expr, const QueryContext *qctx) {
   EvaluableExprVisitor visitor(qctx);
   const_cast<Expression *>(expr)->accept(&visitor);
   return visitor.ok();
 }
 
-// rewrite Attribute to LabelTagProp
 Expression *ExpressionUtils::rewriteAttr2LabelTagProp(
     const Expression *expr, const std::unordered_map<std::string, AliasType> &aliasTypeMap) {
   ObjectPool *pool = expr->getObjPool();
@@ -166,7 +120,21 @@ Expression *ExpressionUtils::rewriteAttr2LabelTagProp(
   return RewriteVisitor::transform(expr, std::move(matcher), std::move(rewriter));
 }
 
-// rewrite LabelAttr to EdgeProp
+Expression *ExpressionUtils::rewriteLabelAttr2TagProp(const Expression *expr) {
+  ObjectPool *pool = expr->getObjPool();
+  auto matcher = [](const Expression *e) -> bool {
+    return e->kind() == Expression::Kind::kLabelAttribute;
+  };
+  auto rewriter = [pool](const Expression *e) -> Expression * {
+    auto labelAttrExpr = static_cast<const LabelAttributeExpression *>(e);
+    auto leftName = labelAttrExpr->left()->name();
+    auto rightName = labelAttrExpr->right()->value().getStr();
+    return TagPropertyExpression::make(pool, leftName, rightName);
+  };
+
+  return RewriteVisitor::transform(expr, std::move(matcher), std::move(rewriter));
+}
+
 Expression *ExpressionUtils::rewriteLabelAttr2EdgeProp(const Expression *expr) {
   ObjectPool *pool = expr->getObjPool();
   auto matcher = [](const Expression *e) -> bool {
@@ -182,7 +150,6 @@ Expression *ExpressionUtils::rewriteLabelAttr2EdgeProp(const Expression *expr) {
   return RewriteVisitor::transform(expr, std::move(matcher), std::move(rewriter));
 }
 
-// rewrite var in VariablePropExpr to another var
 Expression *ExpressionUtils::rewriteInnerVar(const Expression *expr, std::string newVar) {
   ObjectPool *pool = expr->getObjPool();
   auto matcher = [](const Expression *e) -> bool {
@@ -197,7 +164,6 @@ Expression *ExpressionUtils::rewriteInnerVar(const Expression *expr, std::string
   return RewriteVisitor::transform(expr, std::move(matcher), std::move(rewriter));
 }
 
-// rewrite parameter to Constant
 Expression *ExpressionUtils::rewriteParameter(const Expression *expr, QueryContext *qctx) {
   auto matcher = [qctx](const Expression *e) -> bool {
     return e->kind() == Expression::Kind::kVar &&
@@ -212,23 +178,6 @@ Expression *ExpressionUtils::rewriteParameter(const Expression *expr, QueryConte
   return graph::RewriteVisitor::transform(expr, matcher, rewriter);
 }
 
-// rewrite LabelAttr to tagProp
-Expression *ExpressionUtils::rewriteLabelAttr2TagProp(const Expression *expr) {
-  ObjectPool *pool = expr->getObjPool();
-  auto matcher = [](const Expression *e) -> bool {
-    return e->kind() == Expression::Kind::kLabelAttribute;
-  };
-  auto rewriter = [pool](const Expression *e) -> Expression * {
-    auto labelAttrExpr = static_cast<const LabelAttributeExpression *>(e);
-    auto leftName = labelAttrExpr->left()->name();
-    auto rightName = labelAttrExpr->right()->value().getStr();
-    return TagPropertyExpression::make(pool, leftName, rightName);
-  };
-
-  return RewriteVisitor::transform(expr, std::move(matcher), std::move(rewriter));
-}
-
-// rewrite Agg to VarProp
 Expression *ExpressionUtils::rewriteAgg2VarProp(const Expression *expr) {
   ObjectPool *pool = expr->getObjPool();
   auto matcher = [](const Expression *e) -> bool {
@@ -703,11 +652,11 @@ void ExpressionUtils::splitFilter(const Expression *expr,
   auto filterUnpickedPtr = LogicalExpression::makeAnd(pool);
 
   std::vector<Expression *> &operands = logicExpr->operands();
-  for (auto iter = operands.begin(); iter != operands.end(); ++iter) {
-    if (picker((*iter))) {
-      filterPickedPtr->addOperand((*iter)->clone());
+  for (auto &operand : operands) {
+    if (picker(operand)) {
+      filterPickedPtr->addOperand(operand->clone());
     } else {
-      filterUnpickedPtr->addOperand((*iter)->clone());
+      filterUnpickedPtr->addOperand(operand->clone());
     }
   }
   auto foldLogicalExpr = [](const LogicalExpression *e) -> Expression * {
@@ -850,9 +799,7 @@ std::vector<Expression *> ExpressionUtils::expandImplOr(const Expression *expr)
 }
 
 Status ExpressionUtils::checkAggExpr(const AggregateExpression *aggExpr) {
-  auto func = aggExpr->name();
-  std::transform(func.begin(), func.end(), func.begin(), ::toupper);
-
+  const auto &func = aggExpr->name();
   NG_RETURN_IF_ERROR(AggFunctionManager::find(func));
 
   auto *aggArg = aggExpr->arg();
diff --git a/src/graph/util/ExpressionUtils.h b/src/graph/util/ExpressionUtils.h
index b5d3f642fc3..7f673f3d733 100644
--- a/src/graph/util/ExpressionUtils.h
+++ b/src/graph/util/ExpressionUtils.h
@@ -29,114 +29,146 @@ class ExpressionUtils {
  public:
   explicit ExpressionUtils(...) = delete;
 
+  // ** Expression type check **
+  // Checks if the kind of the given expression is one of the expected kind
   static inline bool isKindOf(const Expression* expr,
                               const std::unordered_set<Expression::Kind>& expected) {
     return expected.find(expr->kind()) != expected.end();
   }
 
+  // Checks if the expression is a property expression (TagProperty or LabelTagProperty or
+  // EdgeProperty or InputProperty or VarProperty or DstProperty or SrcProperty)
   static bool isPropertyExpr(const Expression* expr);
 
+  static bool isEvaluableExpr(const Expression* expr, const QueryContext* qctx = nullptr);
+
+  // Iterate though all sub operands recursively of the given expression, and return the first
+  // expression whose kind mataches the expected list
   static const Expression* findAny(const Expression* self,
                                    const std::unordered_set<Expression::Kind>& expected);
 
+  // Checks if the given expression contains any expression whose kind mataches the expected list
   static bool hasAny(const Expression* expr, const std::unordered_set<Expression::Kind>& expected) {
     return findAny(expr, expected) != nullptr;
   }
 
+  // Iterates though all sub operands recursively of the given expression, and return all
+  // expressions whose kinds mataches the expected list
   static std::vector<const Expression*> collectAll(
       const Expression* self, const std::unordered_set<Expression::Kind>& expected);
 
+  // Determines if the
   static bool checkVarExprIfExist(const Expression* expr, const QueryContext* qctx);
 
-  static std::vector<const Expression*> findAllStorage(const Expression* expr);
-
-  static std::vector<const Expression*> findAllInputVariableProp(const Expression* expr);
-
-  // **Expression type check**
-  static bool isConstExpr(const Expression* expr);
-
-  static bool isEvaluableExpr(const Expression* expr, const QueryContext* qctx = nullptr);
-
+  // ** Expression rewrite **
+  // rewrites Attribute to LabelTagProp
   static Expression* rewriteAttr2LabelTagProp(
       const Expression* expr, const std::unordered_map<std::string, AliasType>& aliasTypeMap);
 
+  // rewrite LabelAttr to tagProp
   static Expression* rewriteLabelAttr2TagProp(const Expression* expr);
 
+  // rewrite LabelAttr to EdgeProp
   static Expression* rewriteLabelAttr2EdgeProp(const Expression* expr);
 
+  // rewrite Agg to VarProp
   static Expression* rewriteAgg2VarProp(const Expression* expr);
 
+  // rewrite var in VariablePropExpr to another var
   static Expression* rewriteInnerVar(const Expression* expr, std::string newVar);
 
+  // Rewrites ParameterExpression to ConstantExpression
   static Expression* rewriteParameter(const Expression* expr, QueryContext* qctx);
 
-  // Rewrite relational expression, gather evaluable expressions to one side
+  // Rewrites relational expression, gather all evaluable expressions in the left operands and move
+  // them to the right
   static Expression* rewriteRelExpr(const Expression* expr);
   static Expression* rewriteRelExprHelper(const Expression* expr, Expression*& relRightOperandExpr);
 
-  // Rewrite IN expression into OR expression or relEQ expression
+  // Rewrites IN expression into OR expression or relEQ expression
   static Expression* rewriteInExpr(const Expression* expr);
 
-  // Rewrite Logical AND expr to Logical OR expr using distributive law
+  // Rewrites Logical AND expr to Logical OR expr using distributive law
   // Examples:
   // A and (B or C)  => (A and B) or (A and C)
   // (A or B) and (C or D)  =>  (A and C) or (A and D) or (B and C) or (B or D)
   static Expression* rewriteLogicalAndToLogicalOr(const Expression* expr);
 
-  // Return the operands of container expressions
+  // Returns the operands of container expressions
   // For list and set, return the operands
   // For map, return the keys
   static std::vector<Expression*> getContainerExprOperands(const Expression* expr);
 
-  // Clone and fold constant expression
+  // Clones and folds constant expression
+  // Examples:
+  // v.age > 40 + 1  =>  v.age > 41
   static StatusOr<Expression*> foldConstantExpr(const Expression* expr);
 
-  // Clone and reduce unaryNot expression
+  // Clones and reduces unaryNot expression
+  // Examples:
+  // !!(A and B)  =>  (A and B)
   static Expression* reduceUnaryNotExpr(const Expression* expr);
 
-  // Transform filter using multiple expression rewrite strategies
+  // Transforms filter using multiple expression rewrite strategies:
   // 1. rewrite relational expressions containing arithmetic operands so that
   //    all constants are on the right side of relExpr.
   // 2. fold constant
   // 3. reduce unary expression e.g. !(A and B) => !A or !B
   static StatusOr<Expression*> filterTransform(const Expression* expr);
 
-  // Negate the given logical expr: (A && B) -> (!A || !B)
+  // Negates the given logical expr: (A && B) -> (!A || !B)
   static LogicalExpression* reverseLogicalExpr(LogicalExpression* expr);
 
-  // Negate the given relational expr: (A > B) -> (A <= B)
+  // Negates the given relational expr: (A > B) -> (A <= B)
   static RelationalExpression* reverseRelExpr(RelationalExpression* expr);
 
-  // Return the negation of the given relational kind
+  // Returns the negation of the given relational kind
   static Expression::Kind getNegatedRelExprKind(const Expression::Kind kind);
 
-  // Return the negation of the given logical kind
+  // Returns the negation of the given logical kind:
+  // for logicalAnd expression returns Expression::Kind::relOR
   static Expression::Kind getNegatedLogicalExprKind(const Expression::Kind kind);
 
-  // Return the negation of the given arithmetic kind: plus -> minus
+  // Returns the negation of the given arithmetic kind: plus -> minus
   static Expression::Kind getNegatedArithmeticType(const Expression::Kind kind);
 
+  // For a logical AND expression, extracts all non-logicalAndExpr from its operands and set them as
+  // the new operands
   static void pullAnds(Expression* expr);
-
   static void pullAndsImpl(LogicalExpression* expr, std::vector<Expression*>& operands);
 
+  // For a logical OR expression, extracts all non-logicalOrExpr from its operands and set them as
+  // the new operands
   static void pullOrs(Expression* expr);
   static void pullOrsImpl(LogicalExpression* expr, std::vector<Expression*>& operands);
 
+  // Constructs a nested logical OR expression
+  // Example:
+  // [expr1, expr2, expr3]  =>  ((expr1 OR expr2) OR expr3)
   static Expression* pushOrs(ObjectPool* pool, const std::vector<Expression*>& rels);
 
+  // Constructs a nested logical AND expression
+  // Example:
+  // [expr1, expr2, expr3]  =>  ((expr1 AND expr2) AND expr3)
   static Expression* pushAnds(ObjectPool* pool, const std::vector<Expression*>& rels);
 
   static Expression* pushImpl(ObjectPool* pool,
                               Expression::Kind kind,
                               const std::vector<Expression*>& rels);
 
+  // Iterates the expression recursively and pulls all out logical AND expressions
   static Expression* flattenInnerLogicalAndExpr(const Expression* expr);
 
+  // Iterates the expression recursively and pulls all out logical OR expressions
   static Expression* flattenInnerLogicalOrExpr(const Expression* expr);
 
+  // Pulls out logical expression recursively
+  // calls flattenInnerLogicalAndExpr() first then executes flattenInnerLogicalOrExpr()
   static Expression* flattenInnerLogicalExpr(const Expression* expr);
 
+  // Uses the picker to split the given experssion expr into two parts: filterPicked and
+  // filterUnpicked If expr is a non-LogicalAnd expression, applies the picker to expr directly If
+  // expr is a logicalAnd expression,  applies the picker to all its operands
   static void splitFilter(const Expression* expr,
                           std::function<bool(const Expression*)> picker,
                           Expression** filterPicked,
@@ -148,11 +180,13 @@ class ExpressionUtils {
 
   static std::vector<Expression*> expandImplOr(const Expression* expr);
 
+  // Check if the aggExpr is valid in yield clause
   static Status checkAggExpr(const AggregateExpression* aggExpr);
 
+  // Checks if expr contains function call expression that generate a random value
   static bool findInnerRandFunction(const Expression* expr);
 
-  // loop condition
+  // ** Loop node condition **
   // ++loopSteps <= steps
   static Expression* stepCondition(ObjectPool* pool, const std::string& loopStep, uint32_t steps);
 
@@ -168,6 +202,8 @@ class ExpressionUtils {
   // TODO(jie) Move it to a better place
   static bool isGeoIndexAcceleratedPredicate(const Expression* expr);
 
+  // Checks if the depth of the expression exceeds the maximum
+  // This is used in the parser to prevent OOM due to highly nested expression
   static bool checkExprDepth(const Expression* expr);
 };
 

From efcf9c3ae90811774434a46f689372303bd93f13 Mon Sep 17 00:00:00 2001
From: Aiee <18348405+Aiee@users.noreply.github.com>
Date: Fri, 11 Feb 2022 14:16:14 +0800
Subject: [PATCH 2/4] Modify copy right header comment style

---
 src/graph/util/AnonColGenerator.h           | 11 ++++-------
 src/graph/util/AnonVarGenerator.h           |  7 +++----
 src/graph/util/AstUtils.h                   |  7 +++----
 src/graph/util/ExpressionUtils.cpp          | 11 +++++------
 src/graph/util/ExpressionUtils.h            | 12 +++++-------
 src/graph/util/FTIndexUtils.cpp             |  7 +++----
 src/graph/util/FTIndexUtils.h               |  7 +++----
 src/graph/util/IdGenerator.cpp              |  7 +++----
 src/graph/util/IdGenerator.h                |  7 +++----
 src/graph/util/IndexUtil.cpp                |  7 +++----
 src/graph/util/IndexUtil.h                  |  7 +++----
 src/graph/util/ParserUtil.cpp               |  7 +++----
 src/graph/util/ParserUtil.h                 |  7 +++----
 src/graph/util/PlannerUtil.cpp              |  7 +++----
 src/graph/util/PlannerUtil.h                |  7 +++----
 src/graph/util/SchemaUtil.cpp               |  7 +++----
 src/graph/util/SchemaUtil.h                 |  7 +++----
 src/graph/util/ToJson.cpp                   |  7 +++----
 src/graph/util/ToJson.h                     |  7 +++----
 src/graph/util/Utils.h                      |  7 +++----
 src/graph/util/ValidateUtil.cpp             |  7 +++----
 src/graph/util/ValidateUtil.h               |  7 +++----
 src/graph/util/ZoneUtil.cpp                 |  7 +++----
 src/graph/util/ZoneUtil.h                   |  7 +++----
 src/graph/util/test/ExpressionUtilsTest.cpp |  7 +++----
 src/graph/util/test/IdGeneratorTest.cpp     |  8 ++++----
 26 files changed, 84 insertions(+), 112 deletions(-)

diff --git a/src/graph/util/AnonColGenerator.h b/src/graph/util/AnonColGenerator.h
index 2cf942d9e89..d12d5525d8b 100644
--- a/src/graph/util/AnonColGenerator.h
+++ b/src/graph/util/AnonColGenerator.h
@@ -1,7 +1,6 @@
-/* Copyright (c) 2020 vesoft inc. All rights reserved.
- *
- * This source code is licensed under Apache 2.0 License.
- */
+// Copyright (c) 2020 vesoft inc. All rights reserved.
+//
+// This source code is licensed under Apache 2.0 License.
 
 #ifndef GRAPH_UTIL_ANONCOLGENERATOR_H_
 #define GRAPH_UTIL_ANONCOLGENERATOR_H_
@@ -19,9 +18,7 @@ constexpr char kEdgesStr[] = "_edges";
 constexpr char kPathStr[] = "_path";
 constexpr char kCostStr[] = "_cost";
 
-/**
- * An utility to generate an anonymous column name.
- */
+// An utility to generate an anonymous column name.
 class AnonColGenerator final {
  public:
   AnonColGenerator() {
diff --git a/src/graph/util/AnonVarGenerator.h b/src/graph/util/AnonVarGenerator.h
index 374e01efaa7..9ebc1fd6a00 100644
--- a/src/graph/util/AnonVarGenerator.h
+++ b/src/graph/util/AnonVarGenerator.h
@@ -1,7 +1,6 @@
-/* Copyright (c) 2020 vesoft inc. All rights reserved.
- *
- * This source code is licensed under Apache 2.0 License.
- */
+// Copyright (c) 2020 vesoft inc. All rights reserved.
+//
+// This source code is licensed under Apache 2.0 License.
 
 #ifndef GRAPH_UTIL_ANONVARGENERATOR_H_
 #define GRAPH_UTIL_ANONVARGENERATOR_H_
diff --git a/src/graph/util/AstUtils.h b/src/graph/util/AstUtils.h
index ae7dee87ea3..4dc3fe63398 100644
--- a/src/graph/util/AstUtils.h
+++ b/src/graph/util/AstUtils.h
@@ -1,7 +1,6 @@
-/* Copyright (c) 2020 vesoft inc. All rights reserved.
- *
- * This source code is licensed under Apache 2.0 License.
- */
+// Copyright (c) 2020 vesoft inc. All rights reserved.
+//
+// This source code is licensed under Apache 2.0 License.
 
 #ifndef GRAPH_UTIL_ASTUTIL_H_
 #define GRAPH_UTIL_ASTUTIL_H_
diff --git a/src/graph/util/ExpressionUtils.cpp b/src/graph/util/ExpressionUtils.cpp
index 51de2eda947..5ab273db0a5 100644
--- a/src/graph/util/ExpressionUtils.cpp
+++ b/src/graph/util/ExpressionUtils.cpp
@@ -1,7 +1,6 @@
-/* Copyright (c) 2021 vesoft inc. All rights reserved.
- *
- * This source code is licensed under Apache 2.0 License.
- */
+// Copyright (c) 2021 vesoft inc. All rights reserved.
+//
+// This source code is licensed under Apache 2.0 License.
 
 #include "graph/util/ExpressionUtils.h"
 
@@ -398,10 +397,10 @@ Expression *ExpressionUtils::rewriteRelExpr(const Expression *expr) {
   // Match relational expressions following these rules:
   // 1. the right operand of rel expr should be evaluable
   // 2. the left operand of rel expr should be:
-  // 2.a an arithmetic expr that does not contains string and has at least one operand that is
+  // - 2.a an arithmetic expr that does not contains string and has at least one operand that is
   // evaluable
   // OR
-  // 2.b an relational expr so that it might could be simplified:
+  // - 2.b an relational expr so that it might could be simplified:
   // ((v.age > 40 == true)  => (v.age > 40))
   auto matcher = [&checkArithmExpr](const Expression *e) -> bool {
     if (!e->isRelExpr()) {
diff --git a/src/graph/util/ExpressionUtils.h b/src/graph/util/ExpressionUtils.h
index 7f673f3d733..741238849d7 100644
--- a/src/graph/util/ExpressionUtils.h
+++ b/src/graph/util/ExpressionUtils.h
@@ -1,7 +1,6 @@
-/* Copyright (c) 2021 vesoft inc. All rights reserved.
- *
- * This source code is licensed under Apache 2.0 License.
- */
+// Copyright (c) 2021 vesoft inc. All rights reserved.
+//
+// This source code is licensed under Apache 2.0 License.
 
 #ifndef _UTIL_EXPRESSION_UTILS_H_
 #define _UTIL_EXPRESSION_UTILS_H_
@@ -100,6 +99,7 @@ class ExpressionUtils {
   static std::vector<Expression*> getContainerExprOperands(const Expression* expr);
 
   // Clones and folds constant expression
+  // Returns an error status if an overflow occurs
   // Examples:
   // v.age > 40 + 1  =>  v.age > 41
   static StatusOr<Expression*> foldConstantExpr(const Expression* expr);
@@ -187,15 +187,13 @@ class ExpressionUtils {
   static bool findInnerRandFunction(const Expression* expr);
 
   // ** Loop node condition **
+  // Generates an expression that is used as the condition of loop node:
   // ++loopSteps <= steps
   static Expression* stepCondition(ObjectPool* pool, const std::string& loopStep, uint32_t steps);
-
   // size(var) == 0
   static Expression* zeroCondition(ObjectPool* pool, const std::string& var);
-
   // size(var) != 0
   static Expression* neZeroCondition(ObjectPool* pool, const std::string& var);
-
   // var == value
   static Expression* equalCondition(ObjectPool* pool, const std::string& var, const Value& value);
 
diff --git a/src/graph/util/FTIndexUtils.cpp b/src/graph/util/FTIndexUtils.cpp
index 3960661c948..1b806f5cb23 100644
--- a/src/graph/util/FTIndexUtils.cpp
+++ b/src/graph/util/FTIndexUtils.cpp
@@ -1,7 +1,6 @@
-/* Copyright (c) 2021 vesoft inc. All rights reserved.
- *
- * This source code is licensed under Apache 2.0 License.
- */
+// Copyright (c) 2021 vesoft inc. All rights reserved.
+//
+// This source code is licensed under Apache 2.0 License.
 
 #include "graph/util/FTIndexUtils.h"
 
diff --git a/src/graph/util/FTIndexUtils.h b/src/graph/util/FTIndexUtils.h
index cbca1bd4444..cd7e1830e61 100644
--- a/src/graph/util/FTIndexUtils.h
+++ b/src/graph/util/FTIndexUtils.h
@@ -1,7 +1,6 @@
-/* Copyright (c) 2021 vesoft inc. All rights reserved.
- *
- * This source code is licensed under Apache 2.0 License.
- */
+// Copyright (c) 2021 vesoft inc. All rights reserved.
+//
+// This source code is licensed under Apache 2.0 License.
 
 #ifndef GRAPH_UTIL_FT_INDEXUTIL_H_
 #define GRAPH_UTIL_FT_INDEXUTIL_H_
diff --git a/src/graph/util/IdGenerator.cpp b/src/graph/util/IdGenerator.cpp
index 019a40920da..feddf2abb5e 100644
--- a/src/graph/util/IdGenerator.cpp
+++ b/src/graph/util/IdGenerator.cpp
@@ -1,7 +1,6 @@
-/* Copyright (c) 2020 vesoft inc. All rights reserved.
- *
- * This source code is licensed under Apache 2.0 License.
- */
+// Copyright (c) 2020 vesoft inc. All rights reserved.
+//
+// This source code is licensed under Apache 2.0 License.
 
 #include "graph/util/IdGenerator.h"
 
diff --git a/src/graph/util/IdGenerator.h b/src/graph/util/IdGenerator.h
index c22c516d908..35b5dc46a6d 100644
--- a/src/graph/util/IdGenerator.h
+++ b/src/graph/util/IdGenerator.h
@@ -1,7 +1,6 @@
-/* Copyright (c) 2020 vesoft inc. All rights reserved.
- *
- * This source code is licensed under Apache 2.0 License.
- */
+// Copyright (c) 2020 vesoft inc. All rights reserved.
+//
+// This source code is licensed under Apache 2.0 License.
 
 #ifndef GRAPH_PLANNER_IDGENERATOR_H_
 #define GRAPH_PLANNER_IDGENERATOR_H_
diff --git a/src/graph/util/IndexUtil.cpp b/src/graph/util/IndexUtil.cpp
index b08461cc6d3..dbd74e29f81 100644
--- a/src/graph/util/IndexUtil.cpp
+++ b/src/graph/util/IndexUtil.cpp
@@ -1,7 +1,6 @@
-/* Copyright (c) 2020 vesoft inc. All rights reserved.
- *
- * This source code is licensed under Apache 2.0 License.
- */
+// Copyright (c) 2020 vesoft inc. All rights reserved.
+//
+// This source code is licensed under Apache 2.0 License.
 
 #include "graph/util/IndexUtil.h"
 
diff --git a/src/graph/util/IndexUtil.h b/src/graph/util/IndexUtil.h
index 22100690363..68d367c4cbf 100644
--- a/src/graph/util/IndexUtil.h
+++ b/src/graph/util/IndexUtil.h
@@ -1,7 +1,6 @@
-/* Copyright (c) 2020 vesoft inc. All rights reserved.
- *
- * This source code is licensed under Apache 2.0 License.
- */
+// Copyright (c) 2020 vesoft inc. All rights reserved.
+//
+// This source code is licensed under Apache 2.0 License.
 
 #ifndef GRAPH_UTIL_INDEXUTIL_H_
 #define GRAPH_UTIL_INDEXUTIL_H_
diff --git a/src/graph/util/ParserUtil.cpp b/src/graph/util/ParserUtil.cpp
index a8f93c671d9..56679d9d668 100644
--- a/src/graph/util/ParserUtil.cpp
+++ b/src/graph/util/ParserUtil.cpp
@@ -1,7 +1,6 @@
-/* Copyright (c) 2021 vesoft inc. All rights reserved.
- *
- * This source code is licensed under Apache 2.0 License.
- */
+// Copyright (c) 2021 vesoft inc. All rights reserved.
+//
+// This source code is licensed under Apache 2.0 License.
 
 #include "graph/util/ParserUtil.h"
 
diff --git a/src/graph/util/ParserUtil.h b/src/graph/util/ParserUtil.h
index f86a46db238..ab4f2d310b3 100644
--- a/src/graph/util/ParserUtil.h
+++ b/src/graph/util/ParserUtil.h
@@ -1,7 +1,6 @@
-/* Copyright (c) 2021 vesoft inc. All rights reserved.
- *
- * This source code is licensed under Apache 2.0 License.
- */
+// Copyright (c) 2021 vesoft inc. All rights reserved.
+//
+// This source code is licensed under Apache 2.0 License.
 
 #ifndef GRAPH_UTIL_PARSERUTIL_H_
 #define GRAPH_UTIL_PARSERUTIL_H_
diff --git a/src/graph/util/PlannerUtil.cpp b/src/graph/util/PlannerUtil.cpp
index d161ede4557..6102cc17924 100644
--- a/src/graph/util/PlannerUtil.cpp
+++ b/src/graph/util/PlannerUtil.cpp
@@ -1,7 +1,6 @@
-/* Copyright (c) 2021 vesoft inc. All rights reserved.
- *
- * This source code is licensed under Apache 2.0 License.
- */
+// Copyright (c) 2021 vesoft inc. All rights reserved.
+//
+// This source code is licensed under Apache 2.0 License.
 
 #include "graph/util/PlannerUtil.h"
 
diff --git a/src/graph/util/PlannerUtil.h b/src/graph/util/PlannerUtil.h
index 25f29450061..017494be55c 100644
--- a/src/graph/util/PlannerUtil.h
+++ b/src/graph/util/PlannerUtil.h
@@ -1,7 +1,6 @@
-/* Copyright (c) 2021 vesoft inc. All rights reserved.
- *
- * This source code is licensed under Apache 2.0 License.
- */
+// Copyright (c) 2021 vesoft inc. All rights reserved.
+//
+// This source code is licensed under Apache 2.0 License.
 
 #ifndef GRAPH_UTIL_PLANNER_UTIL_H_
 #define GRAPH_UTIL_PLANNER_UTIL_H_
diff --git a/src/graph/util/SchemaUtil.cpp b/src/graph/util/SchemaUtil.cpp
index 37ac63e4053..4a6217e26d4 100644
--- a/src/graph/util/SchemaUtil.cpp
+++ b/src/graph/util/SchemaUtil.cpp
@@ -1,7 +1,6 @@
-/* Copyright (c) 2020 vesoft inc. All rights reserved.
- *
- * This source code is licensed under Apache 2.0 License.
- */
+// Copyright (c) 2020 vesoft inc. All rights reserved.
+//
+// This source code is licensed under Apache 2.0 License.
 
 #include "graph/util/SchemaUtil.h"
 
diff --git a/src/graph/util/SchemaUtil.h b/src/graph/util/SchemaUtil.h
index a93755eb444..b6faddcf768 100644
--- a/src/graph/util/SchemaUtil.h
+++ b/src/graph/util/SchemaUtil.h
@@ -1,7 +1,6 @@
-/* Copyright (c) 2020 vesoft inc. All rights reserved.
- *
- * This source code is licensed under Apache 2.0 License.
- */
+// Copyright (c) 2020 vesoft inc. All rights reserved.
+//
+// This source code is licensed under Apache 2.0 License.
 
 #ifndef GRAPH_UTIL_SCHEMAUTIL_H_
 #define GRAPH_UTIL_SCHEMAUTIL_H_
diff --git a/src/graph/util/ToJson.cpp b/src/graph/util/ToJson.cpp
index 6a9bb6634d6..e3652062b55 100644
--- a/src/graph/util/ToJson.cpp
+++ b/src/graph/util/ToJson.cpp
@@ -1,7 +1,6 @@
-/* Copyright (c) 2020 vesoft inc. All rights reserved.
- *
- * This source code is licensed under Apache 2.0 License.
- */
+// Copyright (c) 2020 vesoft inc. All rights reserved.
+//
+// This source code is licensed under Apache 2.0 License.
 
 #include "graph/util/ToJson.h"
 
diff --git a/src/graph/util/ToJson.h b/src/graph/util/ToJson.h
index eed062480a7..4c03abf9989 100644
--- a/src/graph/util/ToJson.h
+++ b/src/graph/util/ToJson.h
@@ -1,7 +1,6 @@
-/* Copyright (c) 2020 vesoft inc. All rights reserved.
- *
- * This source code is licensed under Apache 2.0 License.
- */
+// Copyright (c) 2020 vesoft inc. All rights reserved.
+//
+// This source code is licensed under Apache 2.0 License.
 #ifndef GRAPH_UTIL_TOJSON_H_
 #define GRAPH_UTIL_TOJSON_H_
 
diff --git a/src/graph/util/Utils.h b/src/graph/util/Utils.h
index 583aae3d1b7..1a1db5c54a7 100644
--- a/src/graph/util/Utils.h
+++ b/src/graph/util/Utils.h
@@ -1,7 +1,6 @@
-/* Copyright (c) 2021 vesoft inc. All rights reserved.
- *
- * This source code is licensed under Apache 2.0 License.
- */
+// Copyright (c) 2021 vesoft inc. All rights reserved.
+//
+// This source code is licensed under Apache 2.0 License.
 
 #ifndef GRAPH_UTIL_UTILS_H_
 #define GRAPH_UTIL_UTILS_H_
diff --git a/src/graph/util/ValidateUtil.cpp b/src/graph/util/ValidateUtil.cpp
index b0f6eeaa18d..c71bc7279b5 100644
--- a/src/graph/util/ValidateUtil.cpp
+++ b/src/graph/util/ValidateUtil.cpp
@@ -1,7 +1,6 @@
-/* Copyright (c) 2021 vesoft inc. All rights reserved.
- *
- * This source code is licensed under Apache 2.0 License.
- */
+// Copyright (c) 2021 vesoft inc. All rights reserved.
+//
+// This source code is licensed under Apache 2.0 License.
 
 #include "graph/util/ValidateUtil.h"
 
diff --git a/src/graph/util/ValidateUtil.h b/src/graph/util/ValidateUtil.h
index eb5bb9d3cbf..66621cd7bd1 100644
--- a/src/graph/util/ValidateUtil.h
+++ b/src/graph/util/ValidateUtil.h
@@ -1,7 +1,6 @@
-/* Copyright (c) 2021 vesoft inc. All rights reserved.
- *
- * This source code is licensed under Apache 2.0 License.
- */
+// Copyright (c) 2021 vesoft inc. All rights reserved.
+//
+// This source code is licensed under Apache 2.0 License.
 
 #ifndef GRAPH_UTIL_VALIDATE_UTIL_H_
 #define GRAPH_UTIL_VALIDATE_UTIL_H_
diff --git a/src/graph/util/ZoneUtil.cpp b/src/graph/util/ZoneUtil.cpp
index 3dc91e4deb1..0f4762ce253 100644
--- a/src/graph/util/ZoneUtil.cpp
+++ b/src/graph/util/ZoneUtil.cpp
@@ -1,7 +1,6 @@
-/* Copyright (c) 2020 vesoft inc. All rights reserved.
- *
- * This source code is licensed under Apache 2.0 License.
- */
+// Copyright (c) 2020 vesoft inc. All rights reserved.
+//
+// This source code is licensed under Apache 2.0 License.
 
 #include "graph/util/ZoneUtil.h"
 
diff --git a/src/graph/util/ZoneUtil.h b/src/graph/util/ZoneUtil.h
index 55db72b112e..9a08919f236 100644
--- a/src/graph/util/ZoneUtil.h
+++ b/src/graph/util/ZoneUtil.h
@@ -1,7 +1,6 @@
-/* Copyright (c) 2020 vesoft inc. All rights reserved.
- *
- * This source code is licensed under Apache 2.0 License.
- */
+// Copyright (c) 2020 vesoft inc. All rights reserved.
+//
+// This source code is licensed under Apache 2.0 License.
 
 #ifndef GRAPH_UTIL_ZONEUTIL_H_
 #define GRAPH_UTIL_ZONEUTIL_H_
diff --git a/src/graph/util/test/ExpressionUtilsTest.cpp b/src/graph/util/test/ExpressionUtilsTest.cpp
index 70a96f3d58b..8b349c9c3b3 100644
--- a/src/graph/util/test/ExpressionUtilsTest.cpp
+++ b/src/graph/util/test/ExpressionUtilsTest.cpp
@@ -1,7 +1,6 @@
-/* Copyright (c) 2020 vesoft inc. All rights reserved.
- *
- * This source code is licensed under Apache 2.0 License.
- */
+// Copyright (c) 2020 vesoft inc. All rights reserved.
+//
+// This source code is licensed under Apache 2.0 License.
 
 #include <gtest/gtest.h>
 
diff --git a/src/graph/util/test/IdGeneratorTest.cpp b/src/graph/util/test/IdGeneratorTest.cpp
index b784b6e19c4..8fec40ecc99 100644
--- a/src/graph/util/test/IdGeneratorTest.cpp
+++ b/src/graph/util/test/IdGeneratorTest.cpp
@@ -1,7 +1,7 @@
-/* Copyright (c) 2020 vesoft inc. All rights reserved.
- *
- * This source code is licensed under Apache 2.0 License.
- */
+// Copyright (c) 2020 vesoft inc. All rights reserved.
+//
+// This source code is licensed under Apache 2.0 License.
+
 #include <gtest/gtest.h>
 
 #include <boost/thread/barrier.hpp>

From a5df2c7cb72b719bd87f5ed86ddc12b80826103b Mon Sep 17 00:00:00 2001
From: Aiee <18348405+Aiee@users.noreply.github.com>
Date: Fri, 11 Feb 2022 18:13:50 +0800
Subject: [PATCH 3/4] Finish up to indexUtil.h

---
 src/graph/util/FTIndexUtils.cpp         |  8 +-------
 src/graph/util/FTIndexUtils.h           | 12 ++++++++----
 src/graph/util/IdGenerator.h            |  1 +
 src/graph/util/IndexUtil.cpp            | 13 -------------
 src/graph/util/IndexUtil.h              |  3 +--
 src/graph/validator/LookupValidator.cpp |  2 +-
 6 files changed, 12 insertions(+), 27 deletions(-)

diff --git a/src/graph/util/FTIndexUtils.cpp b/src/graph/util/FTIndexUtils.cpp
index 1b806f5cb23..c6d423c0c1f 100644
--- a/src/graph/util/FTIndexUtils.cpp
+++ b/src/graph/util/FTIndexUtils.cpp
@@ -113,13 +113,7 @@ StatusOr<std::vector<std::string>> FTIndexUtils::textSearch(
     const std::string& index,
     const std::vector<nebula::plugin::HttpClient>& tsClients) {
   auto tsExpr = static_cast<TextSearchExpression*>(expr);
-  // if (*tsExpr->arg()->from() != from_) {
-  //     return Status::SemanticError("Schema name error : %s",
-  //     tsExpr->arg()->from()->c_str());
-  // }
-  // auto index =
-  // plugin::IndexTraits::indexName(*space_.spaceDesc.space_name_ref(),
-  // isEdge_);
+
   nebula::plugin::DocItem doc(index, tsExpr->arg()->prop(), tsExpr->arg()->val());
   nebula::plugin::LimitItem limit(tsExpr->arg()->timeout(), tsExpr->arg()->limit());
   std::vector<std::string> result;
diff --git a/src/graph/util/FTIndexUtils.h b/src/graph/util/FTIndexUtils.h
index cd7e1830e61..55a6f1d235a 100644
--- a/src/graph/util/FTIndexUtils.h
+++ b/src/graph/util/FTIndexUtils.h
@@ -17,17 +17,18 @@ namespace graph {
 class FTIndexUtils final {
  public:
   FTIndexUtils() = delete;
-
+  // Checks if the filter expression is full-text search related
   static bool needTextSearch(const Expression* expr);
-
+  // Checks meta client and returns the full-text search client if there is one
   static StatusOr<std::vector<nebula::plugin::HttpClient>> getTSClients(meta::MetaClient* client);
-
+  // Checks if the full-text index exists
   static StatusOr<bool> checkTSIndex(const std::vector<nebula::plugin::HttpClient>& tsClients,
                                      const std::string& index);
-
+  // Drops the full-text index
   static StatusOr<bool> dropTSIndex(const std::vector<nebula::plugin::HttpClient>& tsClients,
                                     const std::string& index);
 
+  // Converts TextSearchExpression into a relational expresion that could be pushed down
   static StatusOr<Expression*> rewriteTSFilter(
       ObjectPool* pool,
       bool isEdge,
@@ -35,11 +36,14 @@ class FTIndexUtils final {
       const std::string& index,
       const std::vector<nebula::plugin::HttpClient>& tsClients);
 
+  // Performs full-text search using elastic search adapter
+  // Search type is defiend by the expression kind
   static StatusOr<std::vector<std::string>> textSearch(
       Expression* expr,
       const std::string& index,
       const std::vector<nebula::plugin::HttpClient>& tsClients);
 
+  // Picks a random full-text search client from the given list
   static const nebula::plugin::HttpClient& randomFTClient(
       const std::vector<nebula::plugin::HttpClient>& tsClients);
 };
diff --git a/src/graph/util/IdGenerator.h b/src/graph/util/IdGenerator.h
index 35b5dc46a6d..e0578c4248c 100644
--- a/src/graph/util/IdGenerator.h
+++ b/src/graph/util/IdGenerator.h
@@ -10,6 +10,7 @@
 namespace nebula {
 namespace graph {
 
+// TODO(Aiee) Remove class and use snowflake id generater
 class IdGenerator {
  public:
   explicit IdGenerator(int64_t init = 0) : counter_(init) {}
diff --git a/src/graph/util/IndexUtil.cpp b/src/graph/util/IndexUtil.cpp
index dbd74e29f81..fa34fa9374f 100644
--- a/src/graph/util/IndexUtil.cpp
+++ b/src/graph/util/IndexUtil.cpp
@@ -9,19 +9,6 @@
 namespace nebula {
 namespace graph {
 
-Status IndexUtil::validateColumns(const std::vector<std::string> &fields) {
-  std::unordered_set<std::string> fieldSet(fields.begin(), fields.end());
-  if (fieldSet.size() != fields.size()) {
-    return Status::Error("Found duplicate column field");
-  }
-
-  if (fields.empty()) {
-    return Status::Error("Column is empty");
-  }
-
-  return Status::OK();
-}
-
 // static
 Status IndexUtil::validateIndexParams(const std::vector<IndexParamItem *> &params,
                                       meta::cpp2::IndexParams &indexParams) {
diff --git a/src/graph/util/IndexUtil.h b/src/graph/util/IndexUtil.h
index 68d367c4cbf..70161a37222 100644
--- a/src/graph/util/IndexUtil.h
+++ b/src/graph/util/IndexUtil.h
@@ -16,8 +16,6 @@ class IndexUtil final {
  public:
   IndexUtil() = delete;
 
-  static Status validateColumns(const std::vector<std::string> &fields);
-
   static Status validateIndexParams(const std::vector<IndexParamItem *> &params,
                                     meta::cpp2::IndexParams &indexParams);
 
@@ -27,6 +25,7 @@ class IndexUtil final {
                                              const std::string &indexName,
                                              const meta::cpp2::IndexItem &indexItem);
 
+  // TODO(Aiee) Replace with ExpressionUtils::getNegatedRelExprKind()
   static Expression::Kind reverseRelationalExprKind(Expression::Kind kind);
 };
 
diff --git a/src/graph/validator/LookupValidator.cpp b/src/graph/validator/LookupValidator.cpp
index fa97357d71e..128758c96be 100644
--- a/src/graph/validator/LookupValidator.cpp
+++ b/src/graph/validator/LookupValidator.cpp
@@ -194,7 +194,7 @@ Status LookupValidator::validateWhere() {
     auto ret = checkFilter(filter);
     NG_RETURN_IF_ERROR(ret);
     lookupCtx_->filter = std::move(ret).value();
-    // Make sure the type of the rewritted filter expr is right
+    // Make sure the type of the rewritten filter expr is right
     NG_RETURN_IF_ERROR(deduceExprType(lookupCtx_->filter));
   }
   if (lookupCtx_->isEdge) {

From 36100b09f65bb2419008512fecbb652f1021cdd7 Mon Sep 17 00:00:00 2001
From: Aiee <18348405+Aiee@users.noreply.github.com>
Date: Thu, 17 Feb 2022 19:12:28 +0800
Subject: [PATCH 4/4] Add more comments

---
 src/graph/util/ExpressionUtils.h  |  3 +--
 src/graph/util/IdGenerator.h      |  2 +-
 src/graph/util/IndexUtil.h        |  5 +++++
 src/graph/util/ParserUtil.cpp     |  6 ------
 src/graph/util/ParserUtil.h       | 10 +++++++++-
 src/graph/util/PlannerUtil.h      |  8 +++++++-
 src/graph/util/SchemaUtil.h       | 23 +++++++++++++++++------
 src/graph/util/ToJson.h           |  4 ++++
 src/graph/util/Utils.h            |  2 ++
 src/graph/util/ValidateUtil.cpp   |  4 ++--
 src/graph/util/ValidateUtil.h     |  5 +++++
 src/graph/util/ZoneUtil.h         |  1 +
 src/graph/validator/Validator.cpp |  1 +
 13 files changed, 55 insertions(+), 19 deletions(-)

diff --git a/src/graph/util/ExpressionUtils.h b/src/graph/util/ExpressionUtils.h
index 741238849d7..83ca54aa145 100644
--- a/src/graph/util/ExpressionUtils.h
+++ b/src/graph/util/ExpressionUtils.h
@@ -174,10 +174,9 @@ class ExpressionUtils {
                           Expression** filterPicked,
                           Expression** filterUnpicked);
 
+  // TODO(Aiee) These functions are unused. Remove when refactor.
   static Expression* expandExpr(ObjectPool* pool, const Expression* expr);
-
   static Expression* expandImplAnd(ObjectPool* pool, const Expression* expr);
-
   static std::vector<Expression*> expandImplOr(const Expression* expr);
 
   // Check if the aggExpr is valid in yield clause
diff --git a/src/graph/util/IdGenerator.h b/src/graph/util/IdGenerator.h
index e0578c4248c..b99cb1f4f20 100644
--- a/src/graph/util/IdGenerator.h
+++ b/src/graph/util/IdGenerator.h
@@ -10,7 +10,7 @@
 namespace nebula {
 namespace graph {
 
-// TODO(Aiee) Remove class and use snowflake id generater
+// TODO(Aiee) Remove class and use snowflake id generater instead
 class IdGenerator {
  public:
   explicit IdGenerator(int64_t init = 0) : counter_(init) {}
diff --git a/src/graph/util/IndexUtil.h b/src/graph/util/IndexUtil.h
index 70161a37222..2bbcc51021e 100644
--- a/src/graph/util/IndexUtil.h
+++ b/src/graph/util/IndexUtil.h
@@ -16,11 +16,16 @@ class IndexUtil final {
  public:
   IndexUtil() = delete;
 
+  // Checks wether the parameter in the geo s2 index is valid
   static Status validateIndexParams(const std::vector<IndexParamItem *> &params,
                                     meta::cpp2::IndexParams &indexParams);
 
+  // TODO(Aiee) no status will be returned. Change the interface
+  // Extracts the field and type from the indexItem and returns a Dataset to depscribe the index
   static StatusOr<DataSet> toDescIndex(const meta::cpp2::IndexItem &indexItem);
 
+  // TODO(Aiee) no status will be returned. Change the interface
+  // Returns the infomation of the given index
   static StatusOr<DataSet> toShowCreateIndex(bool isTagIndex,
                                              const std::string &indexName,
                                              const meta::cpp2::IndexItem &indexItem);
diff --git a/src/graph/util/ParserUtil.cpp b/src/graph/util/ParserUtil.cpp
index 56679d9d668..de0e8c0132a 100644
--- a/src/graph/util/ParserUtil.cpp
+++ b/src/graph/util/ParserUtil.cpp
@@ -10,12 +10,6 @@
 namespace nebula {
 namespace graph {
 
-// static
-bool ParserUtil::isLabel(const Expression *expr) {
-  return expr->kind() == Expression::Kind::kLabel ||
-         expr->kind() == Expression::Kind::kLabelAttribute;
-}
-
 // static
 void ParserUtil::rewriteLC(QueryContext *qctx,
                            ListComprehensionExpression *lc,
diff --git a/src/graph/util/ParserUtil.h b/src/graph/util/ParserUtil.h
index ab4f2d310b3..e9f843aaa1a 100644
--- a/src/graph/util/ParserUtil.h
+++ b/src/graph/util/ParserUtil.h
@@ -17,15 +17,23 @@ class ParserUtil final {
  public:
   ParserUtil() = delete;
 
-  static bool isLabel(const Expression *expr);
+  // Rewrites the list comprehension expression so that the innerVar_ will be replaced by a newly
+  // generated variable.
+  // This is to prevent duplicate variable names existing both inside and outside the expression.
   static void rewriteLC(QueryContext *qctx,
                         ListComprehensionExpression *lc,
                         const std::string &oldVarName);
 
+  // Rewrites the predicate comprehension expression so that the innerVar_ will be replaced by a
+  // newly generated variable. This is to prevent duplicate variable names existing both inside and
+  // outside the expression.
   static void rewritePred(QueryContext *qctx,
                           PredicateExpression *pred,
                           const std::string &oldVarName);
 
+  // Rewrites the list reduce expression so that the innerVar_ and the accumulator_ will be replaced
+  // by newly generated variables. This is to prevent duplicate variable names existing both inside
+  // and outside the expression.
   static void rewriteReduce(QueryContext *qctx,
                             ReduceExpression *reduce,
                             const std::string &oldAccName,
diff --git a/src/graph/util/PlannerUtil.h b/src/graph/util/PlannerUtil.h
index 017494be55c..bdba5e642a8 100644
--- a/src/graph/util/PlannerUtil.h
+++ b/src/graph/util/PlannerUtil.h
@@ -15,13 +15,19 @@ class PlanNode;
 class PlannerUtil final {
  public:
   PlannerUtil() = delete;
-
+  // Generates a dataset where each row is a vid from starts.vids, and saves the dataset in variable
+  // vidsVar
   static void buildConstantInput(QueryContext* qctx, Starts& starts, std::string& vidsVar);
 
+  // Builds a starts subplan when in the runtime
   static SubPlan buildRuntimeInput(QueryContext* qctx, Starts& starts);
 
+  // Builds a starts subplan from the Starts object.
+  // If starts.vids is not empty and the starts.originalSrc is null, call buildConstantInput(),
+  // otherwise build a runtime input
   static SubPlan buildStart(QueryContext* qctx, Starts& starts, std::string& vidsVar);
 
+  // Deduplicates the dst in the getNeighbor plan node and outputs the result to variable output
   static PlanNode* extractDstFromGN(QueryContext* qctx, PlanNode* gn, const std::string& output);
 };
 
diff --git a/src/graph/util/SchemaUtil.h b/src/graph/util/SchemaUtil.h
index b6faddcf768..424cca2daec 100644
--- a/src/graph/util/SchemaUtil.h
+++ b/src/graph/util/SchemaUtil.h
@@ -24,24 +24,35 @@ class SchemaUtil final {
   SchemaUtil() = delete;
 
  public:
+  // Iterates schemaProps and sets the each shchemaProp into schema.
+  // Returns Status error when when failed to set.
   static Status validateProps(const std::vector<SchemaPropItem*>& schemaProps,
                               meta::cpp2::Schema& schema);
 
+  // Generates a NebulaSchemaProvider that contains schema info from the schema.
   static std::shared_ptr<const meta::NebulaSchemaProvider> generateSchemaProvider(
       const SchemaVer ver, const meta::cpp2::Schema& schema);
 
+  // Sets TTLDuration from shcemaProp into shcema.
   static Status setTTLDuration(SchemaPropItem* schemaProp, meta::cpp2::Schema& schema);
 
+  // Sets TTLCol from shcemaProp into shcema.
   static Status setTTLCol(SchemaPropItem* schemaProp, meta::cpp2::Schema& schema);
 
+  // Sets Comment from shcemaProp into shcema.
   static Status setComment(SchemaPropItem* schemaProp, meta::cpp2::Schema& schema);
 
+  // Calculates the vid value from expr.
+  // If the result value type mismatches vidType, returns Status error.
   static StatusOr<Value> toVertexID(Expression* expr, Value::Type vidType);
 
+  // Iterate exprs and calculate each element's value and return them as a vector.
   static StatusOr<std::vector<Value>> toValueVec(std::vector<Expression*> exprs);
 
+  // Returns the "Field", "Type", "Null", "Default", "Comment" of the schema as a dataset
   static StatusOr<DataSet> toDescSchema(const meta::cpp2::Schema& schema);
 
+  // Returns the schema info of the given tag/edge as a dataset.
   static StatusOr<DataSet> toShowCreateSchema(bool isTag,
                                               const std::string& name,
                                               const meta::cpp2::Schema& schema);
@@ -49,22 +60,22 @@ class SchemaUtil final {
   static std::string typeToString(const meta::cpp2::ColumnTypeDef& col);
   static std::string typeToString(const meta::cpp2::ColumnDef& col);
 
+  // Returns the coresponding Value type of the given PropertyType.
   static Value::Type propTypeToValueType(nebula::cpp2::PropertyType propType);
 
+  // Validates wether the value type matches the ColumnTypeDef.
   static bool isValidVid(const Value& value, const meta::cpp2::ColumnTypeDef& type);
-
   static bool isValidVid(const Value& value, nebula::cpp2::PropertyType type);
-
   static bool isValidVid(const Value& value);
 
-  // Fetch all tags in the space and retrieve props from tags
-  // only take _tag when withProp is false
+  // Fetches all tags in the space and retrieve props from tags.
+  // Only take _tag when withProp is false.
   static StatusOr<std::unique_ptr<std::vector<VertexProp>>> getAllVertexProp(QueryContext* qctx,
                                                                              GraphSpaceID spaceId,
                                                                              bool withProp);
 
-  // retrieve prop from specific edgetypes
-  // only take _src _dst _type _rank when withProp is false
+  // Retrieves prop from specific edgetypes.
+  // Only takes _src _dst _type _rank when withProp is false.
   static StatusOr<std::unique_ptr<std::vector<EdgeProp>>> getEdgeProps(
       QueryContext* qctx,
       const SpaceInfo& space,
diff --git a/src/graph/util/ToJson.h b/src/graph/util/ToJson.h
index 4c03abf9989..67c34326ae3 100644
--- a/src/graph/util/ToJson.h
+++ b/src/graph/util/ToJson.h
@@ -50,6 +50,7 @@ struct Variable;
 }  // namespace graph
 namespace util {
 
+// Converts all elements in arr to JSON objects, returns a dynamic::array
 template <typename T>
 folly::dynamic toJson(const std::vector<T> &arr);
 
@@ -64,6 +65,8 @@ std::string toJson(const List &list);
 std::string toJson(const Value &value);
 std::string toJson(const EdgeKeyRef *ref);
 std::string toJson(const Expression *expr);
+
+// Concerts the given object to a dynamic object
 folly::dynamic toJson(const meta::cpp2::SpaceDesc &desc);
 folly::dynamic toJson(const meta::cpp2::ColumnDef &column);
 folly::dynamic toJson(const meta::cpp2::Schema &schema);
@@ -84,6 +87,7 @@ folly::dynamic toJson(const storage::cpp2::IndexQueryContext &iqc);
 folly::dynamic toJson(const storage::cpp2::IndexColumnHint &hints);
 folly::dynamic toJson(const graph::Variable *var);
 
+// Converts std::pair p into a dynamic object where the key is p.first and value if p.second
 template <typename K, typename V>
 folly::dynamic toJson(const std::pair<K, V> &p) {
   return folly::dynamic::object(toJson(p.first), toJson(p.second));
diff --git a/src/graph/util/Utils.h b/src/graph/util/Utils.h
index 1a1db5c54a7..5c729a720df 100644
--- a/src/graph/util/Utils.h
+++ b/src/graph/util/Utils.h
@@ -13,6 +13,8 @@
 
 namespace nebula::graph::util {
 
+// Iterates the container and for each element, apply the function fn(). Joins the results of the
+// fn() with the delimiter
 template <typename Container, typename Fn>
 std::string join(const Container& container, Fn fn, const std::string& delimiter = ",") {
   std::vector<std::string> strs;
diff --git a/src/graph/util/ValidateUtil.cpp b/src/graph/util/ValidateUtil.cpp
index c71bc7279b5..87095cf27f0 100644
--- a/src/graph/util/ValidateUtil.cpp
+++ b/src/graph/util/ValidateUtil.cpp
@@ -46,7 +46,7 @@ Status ValidateUtil::validateOver(QueryContext* qctx, const OverClause* clause,
     if (edges.empty()) {
       return Status::SemanticError("No edge type found in space `%s'", space.name.c_str());
     }
-    for (auto edge : edges) {
+    for (const auto& edge : edges) {
       auto edgeType = schemaMng->toEdgeType(space.id, edge);
       if (!edgeType.ok()) {
         return Status::SemanticError(
@@ -56,7 +56,7 @@ Status ValidateUtil::validateOver(QueryContext* qctx, const OverClause* clause,
     }
     over.allEdges = std::move(edges);
     over.isOverAll = true;
-  } else {
+  } else {  // Over specific edges
     auto edges = clause->edges();
     for (auto* edge : edges) {
       auto edgeName = *edge->edge();
diff --git a/src/graph/util/ValidateUtil.h b/src/graph/util/ValidateUtil.h
index 66621cd7bd1..330ee417448 100644
--- a/src/graph/util/ValidateUtil.h
+++ b/src/graph/util/ValidateUtil.h
@@ -18,10 +18,15 @@ class ValidateUtil final {
  public:
   ValidateUtil() = delete;
 
+  // Checks the upper bound steps of clause is greater than its lower bound.
+  // TODO(Aiee) Seems this method not only checks the clause but also set the clause into step which
+  // is in the context. Consider seprate it.
   static Status validateStep(const StepClause* clause, StepClause& step);
 
+  // Retrives edges from schema manager and validate the over clause.
   static Status validateOver(QueryContext* qctx, const OverClause* clause, Over& over);
 
+  // Returns a status error if the expr contains any labelExpression.
   static Status invalidLabelIdentifiers(const Expression* expr);
 };
 
diff --git a/src/graph/util/ZoneUtil.h b/src/graph/util/ZoneUtil.h
index 9a08919f236..c44a12a3b8f 100644
--- a/src/graph/util/ZoneUtil.h
+++ b/src/graph/util/ZoneUtil.h
@@ -11,6 +11,7 @@
 namespace nebula {
 namespace graph {
 
+// TODO(Aiee) Remove unused class
 class ZoneUtil final {
  public:
   ZoneUtil() = delete;
diff --git a/src/graph/validator/Validator.cpp b/src/graph/validator/Validator.cpp
index 03f8d48867d..ceef1627b86 100644
--- a/src/graph/validator/Validator.cpp
+++ b/src/graph/validator/Validator.cpp
@@ -419,6 +419,7 @@ Status Validator::checkDuplicateColName() {
   return Status::OK();
 }
 
+// TODO(Aiee) Move to validateUtil
 Status Validator::validateStarts(const VerticesClause* clause, Starts& starts) {
   if (clause == nullptr) {
     return Status::SemanticError("From clause nullptr.");