clean

kuzudb · Jul 1, 2023 · 2754a3c · 2754a3c
1 parent da999a9
commit 2754a3c
Show file tree

Hide file tree

Showing 6 changed files with 20 additions and 34 deletions.
diff --git a/src/binder/expression/expression.cpp b/src/binder/expression/expression.cpp
@@ -64,18 +64,5 @@ expression_vector ExpressionUtil::excludeExpressions(
     return result;
 }
 
-expression_vector ExpressionUtil::removeDuplication(const expression_vector& expressions) {
-    expression_set excludeSet;
-    expression_vector result;
-    for (auto& expression : expressions) {
-        if (excludeSet.contains(expression)) {
-            continue;
-        }
-        excludeSet.insert(expression);
-        result.push_back(expression);
-    }
-    return result;
-}
-
 } // namespace binder
 } // namespace kuzu
diff --git a/src/include/binder/expression/expression.h b/src/include/binder/expression/expression.h
@@ -121,8 +121,6 @@ struct ExpressionUtil {
     static expression_vector excludeExpressions(
         const expression_vector& expressions, const expression_vector& expressionsToExclude);
 
-    static expression_vector removeDuplication(const expression_vector& expressions);
-
     inline static bool isNodeVariable(const Expression& expression) {
         return expression.expressionType == common::ExpressionType::VARIABLE &&
                expression.dataType.getLogicalTypeID() == common::LogicalTypeID::NODE;

diff --git a/src/include/planner/logical_plan/logical_operator/schema.h b/src/include/planner/logical_plan/logical_operator/schema.h
@@ -75,12 +75,13 @@ class Schema {
     f_group_pos createGroup();
 
     void insertToScope(const std::shared_ptr<binder::Expression>& expression, uint32_t groupPos);
-
     void insertToGroupAndScope(
         const std::shared_ptr<binder::Expression>& expression, uint32_t groupPos);
-    // Use this function only if the operator may work with duplicate expressions.
+    // Use these unsafe insert functions only if the operator may work with duplicate expressions.
     // E.g. group by a.age, a.age
-    void insertToGroupAndScopeUnsafe(
+    void insertToScopeMayRepeat(
+        const std::shared_ptr<binder::Expression>& expression, uint32_t groupPos);
+    void insertToGroupAndScopeMayRepeat(
         const std::shared_ptr<binder::Expression>& expression, uint32_t groupPos);
 
     void insertToGroupAndScope(const binder::expression_vector& expressions, uint32_t groupPos);

diff --git a/src/planner/operator/logical_aggregate.cpp b/src/planner/operator/logical_aggregate.cpp
@@ -76,13 +76,13 @@ bool LogicalAggregate::hasDistinctAggregate() {
 
 void LogicalAggregate::insertAllExpressionsToGroupAndScope(f_group_pos groupPos) {
     for (auto& expression : keyExpressions) {
-        schema->insertToGroupAndScopeUnsafe(expression, groupPos);
+        schema->insertToGroupAndScopeMayRepeat(expression, groupPos);
     }
     for (auto& expression : dependentKeyExpressions) {
-        schema->insertToGroupAndScopeUnsafe(expression, groupPos);
+        schema->insertToGroupAndScopeMayRepeat(expression, groupPos);
     }
     for (auto& expression : aggregateExpressions) {
-        schema->insertToGroupAndScopeUnsafe(expression, groupPos);
+        schema->insertToGroupAndScopeMayRepeat(expression, groupPos);
     }
 }
 

diff --git a/src/planner/operator/logical_projection.cpp b/src/planner/operator/logical_projection.cpp
@@ -8,15 +8,10 @@ void LogicalProjection::computeFactorizedSchema() {
     schema = childSchema->copy();
     schema->clearExpressionsInScope();
     for (auto& expression : expressions) {
-        // E.g. RETURN MIN(a.age), MAX(a.age). We first project each expression in aggregate. So
-        // a.age might be projected twice.
-        if (schema->isExpressionInScope(*expression)) {
-            continue;
-        }
         auto groupPos = INVALID_F_GROUP_POS;
         if (childSchema->isExpressionInScope(*expression)) { // expression to reference
             groupPos = childSchema->getGroupPos(*expression);
-            schema->insertToScope(expression, groupPos);
+            schema->insertToScopeMayRepeat(expression, groupPos);
         } else { // expression to evaluate
             auto dependentGroupPos = childSchema->getDependentGroupsPos(expression);
             SchemaUtils::validateAtMostOneUnFlatGroup(dependentGroupPos, *childSchema);
@@ -26,7 +21,7 @@ void LogicalProjection::computeFactorizedSchema() {
             } else {
                 groupPos = SchemaUtils::getLeadingGroupPos(dependentGroupPos, *childSchema);
             }
-            schema->insertToGroupAndScope(expression, groupPos);
+            schema->insertToGroupAndScopeMayRepeat(expression, groupPos);
         }
     }
 }
@@ -36,13 +31,10 @@ void LogicalProjection::computeFlatSchema() {
     auto childSchema = children[0]->getSchema();
     schema->clearExpressionsInScope();
     for (auto& expression : expressions) {
-        if (schema->isExpressionInScope(*expression)) {
-            continue;
-        }
         if (childSchema->isExpressionInScope(*expression)) {
-            schema->insertToScope(expression, 0);
+            schema->insertToScopeMayRepeat(expression, 0);
         } else {
-            schema->insertToGroupAndScope(expression, 0);
+            schema->insertToGroupAndScopeMayRepeat(expression, 0);
         }
     }
 }

diff --git a/src/planner/operator/schema.cpp b/src/planner/operator/schema.cpp
@@ -29,7 +29,15 @@ void Schema::insertToGroupAndScope(
     expressionsInScope.push_back(expression);
 }
 
-void Schema::insertToGroupAndScopeUnsafe(
+void Schema::insertToScopeMayRepeat(
+    const std::shared_ptr<binder::Expression>& expression, uint32_t groupPos) {
+    if (expressionNameToGroupPos.contains(expression->getUniqueName())) {
+        return;
+    }
+    insertToScope(expression, groupPos);
+}
+
+void Schema::insertToGroupAndScopeMayRepeat(
     const std::shared_ptr<binder::Expression>& expression, uint32_t groupPos) {
     if (expressionNameToGroupPos.contains(expression->getUniqueName())) {
         return;