refactor: move common code related to expressions to ExpressionUtil

src/include/duckdb/parser/expression_util.hpp

@@ -0,0 +1,33 @@
+//===----------------------------------------------------------------------===//
+//                         DuckDB
+//
+// duckdb/parser/expression_util.hpp
+//
+//
+//===----------------------------------------------------------------------===//
+
+#pragma once
+
+#include "duckdb/parser/base_expression.hpp"
+
+namespace duckdb {
+class ParsedExpression;
+class Expression;
+
+class ExpressionUtil {
+public:
+	//! ListEquals: check if a list of two expressions is equal (order is important)
+	static bool ListEquals(const vector<unique_ptr<ParsedExpression>> &a, const vector<unique_ptr<ParsedExpression>> &b);
+	static bool ListEquals(const vector<unique_ptr<Expression>> &a, const vector<unique_ptr<Expression>> &b);
+	//! SetEquals: check if two sets of expressions are equal (order is not important)
+	static bool SetEquals(const vector<unique_ptr<ParsedExpression>> &a, const vector<unique_ptr<ParsedExpression>> &b);
+	static bool SetEquals(const vector<unique_ptr<Expression>> &a, const vector<unique_ptr<Expression>> &b);
+private:
+	template<class T>
+	static bool ExpressionListEquals(const vector<unique_ptr<T>> &a, const vector<unique_ptr<T>> &b);
+	template<class T>
+	static bool ExpressionSetEquals(const vector<unique_ptr<T>> &a, const vector<unique_ptr<T>> &b);
+
+};
+
+} // namespace duckdb

src/parser/CMakeLists.txt

@@ -12,6 +12,7 @@ add_subdirectory(transform)
 add_library(duckdb_parser OBJECT
             base_expression.cpp
             constraint.cpp
+            expression_util.cpp
             parsed_expression.cpp
             parsed_expression_iterator.cpp
             parser.cpp

src/parser/expression/conjunction_expression.cpp

@@ -1,7 +1,7 @@
 #include "duckdb/parser/expression/conjunction_expression.hpp"
 #include "duckdb/common/exception.hpp"
 #include "duckdb/common/serializer.hpp"
-#include "duckdb/parser/expression_map.hpp"
+#include "duckdb/parser/expression_util.hpp"
 
 using namespace duckdb;
 using namespace std;
@@ -47,36 +47,7 @@ string ConjunctionExpression::ToString() const {
 }
 
 bool ConjunctionExpression::Equals(const ConjunctionExpression *a, const ConjunctionExpression *b) {
-	if (a->children.size() != b->children.size()) {
-		return false;
-	}
-	// conjunctions are commutative, check if all children have an equivalent expression on the other side
-
-	// we create a map of expression -> count for the left side
-	// we keep the count because the same expression can occur multiple times (e.g. "1 AND 1" is legal)
-	// in this case we track the following value: map["Constant(1)"] = 2
-	expression_map_t<index_t> map;
-	for(index_t i = 0; i < a->children.size(); i++) {
-		map[a->children[i].get()]++;
-	}
-	// now on the right side we reduce the counts again
-	// if the conjunctions are identical, all the counts will be 0 after the
-	for(auto &expr : b->children) {
-		auto entry = map.find(expr.get());
-		// first we check if we can find the expression in the map at all
-		if (entry == map.end()) {
-			return false;
-		}
-		// if we found it we check the count; if the count is already 0 we return false
-		// this happens if e.g. the left side contains "1 AND X", and the right side contains "1 AND 1"
-		// "1" is contained in the map, however, the right side contains the expression twice
-		// hence we know the children are not identical in this case because the LHS and RHS have a different count for the Constant(1) expression
-		if (entry->second == 0) {
-			return false;
-		}
-		entry->second--;
-	}
-	return true;
+	return ExpressionUtil::SetEquals(a->children, b->children);
 }
 
 unique_ptr<ParsedExpression> ConjunctionExpression::Copy() const {

src/parser/expression_util.cpp

@@ -0,0 +1,68 @@
+#include "duckdb/parser/expression_util.hpp"
+#include "duckdb/planner/expression.hpp"
+#include "duckdb/parser/parsed_expression.hpp"
+#include "duckdb/parser/expression_map.hpp"
+
+using namespace duckdb;
+using namespace std;
+
+template<class T>
+bool ExpressionUtil::ExpressionListEquals(const vector<unique_ptr<T>> &a, const vector<unique_ptr<T>> &b) {
+	if (a.size() != b.size()) {
+		return false;
+	}
+	for(index_t i = 0; i < a.size(); i++) {
+		if (!(*a[i] == *b[i])) {
+			return false;
+		}
+	}
+	return true;
+}
+
+template<class T>
+bool ExpressionUtil::ExpressionSetEquals(const vector<unique_ptr<T>> &a, const vector<unique_ptr<T>> &b) {
+	if (a.size() != b.size()) {
+		return false;
+	}
+	// we create a map of expression -> count for the left side
+	// we keep the count because the same expression can occur multiple times (e.g. "1 AND 1" is legal)
+	// in this case we track the following value: map["Constant(1)"] = 2
+	expression_map_t<index_t> map;
+	for(index_t i = 0; i < a.size(); i++) {
+		map[a[i].get()]++;
+	}
+	// now on the right side we reduce the counts again
+	// if the conjunctions are identical, all the counts will be 0 after the
+	for(auto &expr : b) {
+		auto entry = map.find(expr.get());
+		// first we check if we can find the expression in the map at all
+		if (entry == map.end()) {
+			return false;
+		}
+		// if we found it we check the count; if the count is already 0 we return false
+		// this happens if e.g. the left side contains "1 AND X", and the right side contains "1 AND 1"
+		// "1" is contained in the map, however, the right side contains the expression twice
+		// hence we know the children are not identical in this case because the LHS and RHS have a different count for the Constant(1) expression
+		if (entry->second == 0) {
+			return false;
+		}
+		entry->second--;
+	}
+	return true;
+}
+
+bool ExpressionUtil::ListEquals(const vector<unique_ptr<ParsedExpression>> &a, const vector<unique_ptr<ParsedExpression>> &b) {
+	return ExpressionListEquals<ParsedExpression>(a, b);
+}
+
+bool ExpressionUtil::ListEquals(const vector<unique_ptr<Expression>> &a, const vector<unique_ptr<Expression>> &b) {
+	return ExpressionListEquals<Expression>(a, b);
+}
+
+bool ExpressionUtil::SetEquals(const vector<unique_ptr<ParsedExpression>> &a, const vector<unique_ptr<ParsedExpression>> &b) {
+	return ExpressionSetEquals<ParsedExpression>(a, b);
+}
+
+bool ExpressionUtil::SetEquals(const vector<unique_ptr<Expression>> &a, const vector<unique_ptr<Expression>> &b) {
+	return ExpressionSetEquals<Expression>(a, b);
+}

src/parser/query_node/select_node.cpp

@@ -1,4 +1,5 @@
 #include "duckdb/parser/query_node/select_node.hpp"
+#include "duckdb/parser/expression_util.hpp"
 
 using namespace duckdb;
 using namespace std;
@@ -13,22 +14,17 @@ bool SelectNode::Equals(const QueryNode *other_) const {
 	auto other = (SelectNode *)other_;
 
 	// first check counts of all lists and such
-	if (select_list.size() != other->select_list.size() || select_distinct != other->select_distinct ||
-	    orders.size() != other->orders.size() || groups.size() != other->groups.size() ||
-	    distinct_on_targets.size() != other->distinct_on_targets.size()) {
+	if (select_distinct != other->select_distinct ||
+	    orders.size() != other->orders.size()) {
 		return false;
 	}
 	// SELECT
-	for (index_t i = 0; i < select_list.size(); i++) {
-		if (!select_list[i]->Equals(other->select_list[i].get())) {
-			return false;
-		}
+	if (!ExpressionUtil::ListEquals(select_list, other->select_list)) {
+		return false;
 	}
 	// DISTINCT ON
-	for (index_t i = 0; i < distinct_on_targets.size(); i++) {
-		if (!distinct_on_targets[i]->Equals(other->distinct_on_targets[i].get())) {
-			return false;
-		}
+	if (!ExpressionUtil::ListEquals(distinct_on_targets, other->distinct_on_targets)) {
+		return false;
 	}
 	// FROM
 	if (from_table) {
@@ -46,10 +42,8 @@ bool SelectNode::Equals(const QueryNode *other_) const {
 		return false;
 	}
 	// GROUP BY
-	for (index_t i = 0; i < groups.size(); i++) {
-		if (!groups[i]->Equals(other->groups[i].get())) {
-			return false;
-		}
+	if (!ExpressionUtil::ListEquals(groups, other->groups)) {
+		return false;
 	}
 
 	// HAVING

src/planner/expression/bound_conjunction_expression.cpp

@@ -1,5 +1,5 @@
 #include "duckdb/planner/expression/bound_conjunction_expression.hpp"
-#include "duckdb/parser/expression_map.hpp"
+#include "duckdb/parser/expression_util.hpp"
 
 using namespace duckdb;
 using namespace std;
@@ -32,34 +32,7 @@ bool BoundConjunctionExpression::Equals(const BaseExpression *other_) const {
 		return false;
 	}
 	auto other = (BoundConjunctionExpression *)other_;
-	// FIXME: duplicate from ConjunctionExpression, move to ExpressionUtil
-	// conjunctions are commutative, check if all children have an equivalent expression on the other side
-
-	// we create a map of expression -> count for the left side
-	// we keep the count because the same expression can occur multiple times (e.g. "1 AND 1" is legal)
-	// in this case we track the following value: map["Constant(1)"] = 2
-	expression_map_t<index_t> map;
-	for(index_t i = 0; i < children.size(); i++) {
-		map[children[i].get()]++;
-	}
-	// now on the right side we reduce the counts again
-	// if the conjunctions are identical, all the counts will be 0 after the
-	for(auto &expr : other->children) {
-		auto entry = map.find(expr.get());
-		// first we check if we can find the expression in the map at all
-		if (entry == map.end()) {
-			return false;
-		}
-		// if we found it we check the count; if the count is already 0 we return false
-		// this happens if e.g. the left side contains "1 AND X", and the right side contains "1 AND 1"
-		// "1" is contained in the map, however, the right side contains the expression twice
-		// hence we know the children are not identical in this case because the LHS and RHS have a different count for the Constant(1) expression
-		if (entry->second == 0) {
-			return false;
-		}
-		entry->second--;
-	}
-	return true;
+	return ExpressionUtil::SetEquals(children, other->children);
 }
 
 unique_ptr<Expression> BoundConjunctionExpression::Copy() {