Remove common aggregate optimizer and instead eliminate duplicate agg…

…regates during the binding phase

src/include/duckdb/planner/query_node/bound_select_node.hpp

@@ -10,6 +10,7 @@
 
 #include "duckdb/planner/bound_query_node.hpp"
 #include "duckdb/planner/bound_tableref.hpp"
+#include "duckdb/parser/expression_map.hpp"
 
 namespace duckdb {
 
@@ -42,6 +43,10 @@ class BoundSelectNode : public BoundQueryNode {
 	index_t aggregate_index;
 	//! Aggregate functions to compute (only used if HasAggregation is true)
 	vector<unique_ptr<Expression>> aggregates;
+	//! Map from aggregate function to aggregate index (used to eliminate duplicate aggregates)
+	expression_map_t<index_t> aggregate_map;
+
+
 
 	//! Window index used by the LogicalWindow (only used if HasWindow is true)
 	index_t window_index;

src/optimizer/CMakeLists.txt

@@ -5,7 +5,6 @@ add_subdirectory(rule)
 
 add_library_unity(duckdb_optimizer
                   OBJECT
-                  ca_optimizer.cpp
                   cse_optimizer.cpp
                   filter_combiner.cpp
                   filter_pushdown.cpp

src/optimizer/optimizer.cpp

@@ -2,7 +2,6 @@
 
 #include "duckdb/execution/expression_executor.hpp"
 #include "duckdb/main/client_context.hpp"
-#include "duckdb/optimizer/ca_optimizer.hpp"
 #include "duckdb/optimizer/cse_optimizer.hpp"
 #include "duckdb/optimizer/filter_pushdown.hpp"
 #include "duckdb/optimizer/index_scan.hpp"
@@ -94,11 +93,6 @@ unique_ptr<LogicalOperator> Optimizer::Optimize(unique_ptr<LogicalOperator> plan
 	JoinOrderOptimizer optimizer;
 	plan = optimizer.Optimize(move(plan));
 	context.profiler.EndPhase();
-	// next we make sure that multiple occurences of the same aggregation are only computed once
-	// context.profiler.StartPhase("common_aggregate_expressions");
-	// CommonAggregateOptimizer ca_optimizer;
-	// ca_optimizer.VisitOperator(*plan);
-	// context.profiler.EndPhase();
 
 	// then we extract common subexpressions inside the different operators
 	// context.profiler.StartPhase("common_subexpressions");

src/planner/binder/expression/bind_aggregate_expression.cpp

@@ -57,11 +57,24 @@ BindResult SelectBinder::BindAggregate(FunctionExpression &aggr, AggregateFuncti
 	// create the aggregate
 	auto aggregate = make_unique<BoundAggregateExpression>(GetInternalType(return_type), bound_function, aggr.distinct);
 	aggregate->children = move(children);
-	// now create a column reference referring to this aggregate
+
+	// check for all the aggregates if this aggregate already exists
+	index_t aggr_index;
+	auto entry = node.aggregate_map.find(aggregate.get());
+	if (entry == node.aggregate_map.end()) {
+		// new aggregate: insert into aggregate list
+		aggr_index = node.aggregates.size();
+		node.aggregate_map.insert(make_pair(aggregate.get(), aggr_index));
+		node.aggregates.push_back(move(aggregate));
+	} else {
+		// duplicate aggregate: simplify refer to this aggregate
+		aggr_index = entry->second;
+	}
+
+	// now create a column reference referring to the aggregate
 	auto colref = make_unique<BoundColumnRefExpression>(
-	    aggr.alias.empty() ? aggregate->ToString() : aggr.alias, aggregate->return_type,
-	    ColumnBinding(node.aggregate_index, node.aggregates.size()), depth);
+	    aggr.alias.empty() ? node.aggregates[aggr_index]->ToString() : aggr.alias, node.aggregates[aggr_index]->return_type,
+	    ColumnBinding(node.aggregate_index, aggr_index), depth);
 	// move the aggregate expression into the set of bound aggregates
-	node.aggregates.push_back(move(aggregate));
 	return BindResult(move(colref), return_type);
 }

test/optimizer/CMakeLists.txt

@@ -1,7 +1,6 @@
 add_library_unity(test_optimizer
                   OBJECT
                   test_arithmetic_simplification.cpp
-                  test_ca_optimizer.cpp
                   test_case_simplification.cpp
                   test_comparison_simplification.cpp
                   test_conjunction_simplification.cpp