From c20c6f8c4ad8b3f1755c80bed01389077db4fa2c Mon Sep 17 00:00:00 2001 From: Tom Ebergen Date: Mon, 5 Dec 2022 17:41:53 +0100 Subject: [PATCH 01/10] re-organizing some of the optimizer code --- src/include/duckdb/execution/physical_operator.hpp | 2 +- .../{ => join_order}/cardinality_estimator.hpp | 4 ++-- .../{ => join_order}/estimated_properties.hpp | 2 +- .../duckdb/optimizer/{ => join_order}/join_node.hpp | 10 +++++----- .../{ => join_order}/join_order_optimizer.hpp | 8 ++++---- src/include/duckdb/planner/logical_operator.hpp | 6 +++--- src/optimizer/CMakeLists.txt | 3 --- src/optimizer/cardinality_estimator.cpp | 8 ++++---- src/optimizer/join_order/CMakeLists.txt | 2 +- .../{ => join_order}/estimated_properties.cpp | 2 +- src/optimizer/{ => join_order}/join_node.cpp | 3 ++- .../{ => join_order}/join_order_optimizer.cpp | 5 +++-- src/optimizer/optimizer.cpp | 4 ++-- 13 files changed, 29 insertions(+), 30 deletions(-) rename src/include/duckdb/optimizer/{ => join_order}/cardinality_estimator.hpp (97%) rename src/include/duckdb/optimizer/{ => join_order}/estimated_properties.hpp (95%) rename src/include/duckdb/optimizer/{ => join_order}/join_node.hpp (95%) rename src/include/duckdb/optimizer/{ => join_order}/join_order_optimizer.hpp (97%) rename src/optimizer/{ => join_order}/estimated_properties.cpp (89%) rename src/optimizer/{ => join_order}/join_node.cpp (97%) rename src/optimizer/{ => join_order}/join_order_optimizer.cpp (99%) diff --git a/src/include/duckdb/execution/physical_operator.hpp b/src/include/duckdb/execution/physical_operator.hpp index fdaad81e07dd..51871c4b079f 100644 --- a/src/include/duckdb/execution/physical_operator.hpp +++ b/src/include/duckdb/execution/physical_operator.hpp @@ -14,7 +14,7 @@ #include "duckdb/common/enums/physical_operator_type.hpp" #include "duckdb/common/types/data_chunk.hpp" #include "duckdb/execution/execution_context.hpp" -#include "duckdb/optimizer/join_node.hpp" +#include "duckdb/optimizer/join_order/join_node.hpp" namespace duckdb { class Event; diff --git a/src/include/duckdb/optimizer/cardinality_estimator.hpp b/src/include/duckdb/optimizer/join_order/cardinality_estimator.hpp similarity index 97% rename from src/include/duckdb/optimizer/cardinality_estimator.hpp rename to src/include/duckdb/optimizer/join_order/cardinality_estimator.hpp index cb823ca55cca..f9a718d6dc15 100644 --- a/src/include/duckdb/optimizer/cardinality_estimator.hpp +++ b/src/include/duckdb/optimizer/join_order/cardinality_estimator.hpp @@ -1,14 +1,14 @@ //===----------------------------------------------------------------------===// // DuckDB // -// duckdb/optimizer/cardinality_estimator.hpp +// duckdb/optimizer/join_order/cardinality_estimator.hpp // // //===----------------------------------------------------------------------===// #pragma once #include "duckdb/catalog/catalog_entry/table_catalog_entry.hpp" -#include "duckdb/optimizer/join_node.hpp" +#include "duckdb/optimizer/join_order/join_node.hpp" #include "duckdb/planner/column_binding.hpp" #include "duckdb/planner/column_binding_map.hpp" #include "duckdb/planner/filter/conjunction_filter.hpp" diff --git a/src/include/duckdb/optimizer/estimated_properties.hpp b/src/include/duckdb/optimizer/join_order/estimated_properties.hpp similarity index 95% rename from src/include/duckdb/optimizer/estimated_properties.hpp rename to src/include/duckdb/optimizer/join_order/estimated_properties.hpp index 4a43d0f38ce1..31c4b60fb8bb 100644 --- a/src/include/duckdb/optimizer/estimated_properties.hpp +++ b/src/include/duckdb/optimizer/join_order/estimated_properties.hpp @@ -1,7 +1,7 @@ //===----------------------------------------------------------------------===// // DuckDB // -// duckdb/optimizer/estimated_properties.hpp +// duckdb/optimizer/join_order/estimated_properties.hpp // // //===----------------------------------------------------------------------===// diff --git a/src/include/duckdb/optimizer/join_node.hpp b/src/include/duckdb/optimizer/join_order/join_node.hpp similarity index 95% rename from src/include/duckdb/optimizer/join_node.hpp rename to src/include/duckdb/optimizer/join_order/join_node.hpp index c6bec09ea50d..7e8c545bbfa3 100644 --- a/src/include/duckdb/optimizer/join_node.hpp +++ b/src/include/duckdb/optimizer/join_order/join_node.hpp @@ -1,23 +1,23 @@ //===----------------------------------------------------------------------===// // DuckDB // -// duckdb/optimizer/join_node.hpp +// duckdb/optimizer/join_order/join_node.hpp // // //===----------------------------------------------------------------------===// #pragma once +#include "duckdb/catalog/catalog_entry/table_catalog_entry.hpp" #include "duckdb/common/unordered_map.hpp" #include "duckdb/common/unordered_set.hpp" -#include "duckdb/optimizer/join_order/query_graph.hpp" +#include "duckdb/optimizer/join_order/estimated_properties.hpp" #include "duckdb/optimizer/join_order/join_relation.hpp" +#include "duckdb/optimizer/join_order/query_graph.hpp" #include "duckdb/parser/expression_map.hpp" #include "duckdb/planner/logical_operator_visitor.hpp" -#include "duckdb/storage/statistics/distinct_statistics.hpp" #include "duckdb/planner/table_filter.hpp" -#include "duckdb/catalog/catalog_entry/table_catalog_entry.hpp" -#include "duckdb/optimizer/estimated_properties.hpp" +#include "duckdb/storage/statistics/distinct_statistics.hpp" namespace duckdb { diff --git a/src/include/duckdb/optimizer/join_order_optimizer.hpp b/src/include/duckdb/optimizer/join_order/join_order_optimizer.hpp similarity index 97% rename from src/include/duckdb/optimizer/join_order_optimizer.hpp rename to src/include/duckdb/optimizer/join_order/join_order_optimizer.hpp index 9d89758ea276..6a98b7eb994f 100644 --- a/src/include/duckdb/optimizer/join_order_optimizer.hpp +++ b/src/include/duckdb/optimizer/join_order/join_order_optimizer.hpp @@ -1,22 +1,22 @@ //===----------------------------------------------------------------------===// // DuckDB // -// duckdb/optimizer/join_order_optimizer.hpp +// duckdb/optimizer/join_order/join_order_optimizer.hpp // // //===----------------------------------------------------------------------===// #pragma once +#include "cardinality_estimator.hpp" #include "duckdb/common/unordered_map.hpp" #include "duckdb/common/unordered_set.hpp" -#include "duckdb/optimizer/join_order/query_graph.hpp" #include "duckdb/optimizer/join_order/join_relation.hpp" -#include "duckdb/optimizer/join_node.hpp" +#include "duckdb/optimizer/join_order/query_graph.hpp" #include "duckdb/parser/expression_map.hpp" #include "duckdb/planner/logical_operator.hpp" #include "duckdb/planner/logical_operator_visitor.hpp" -#include "duckdb/optimizer/cardinality_estimator.hpp" +#include "join_node.hpp" #include diff --git a/src/include/duckdb/planner/logical_operator.hpp b/src/include/duckdb/planner/logical_operator.hpp index a02f232eacd7..ab7947374ecb 100644 --- a/src/include/duckdb/planner/logical_operator.hpp +++ b/src/include/duckdb/planner/logical_operator.hpp @@ -9,16 +9,16 @@ #pragma once #include "duckdb/catalog/catalog.hpp" -#include "duckdb/optimizer/estimated_properties.hpp" #include "duckdb/common/common.hpp" #include "duckdb/common/enums/logical_operator_type.hpp" +#include "duckdb/optimizer/join_order/estimated_properties.hpp" +#include "duckdb/planner/column_binding.hpp" #include "duckdb/planner/expression.hpp" #include "duckdb/planner/logical_operator_visitor.hpp" -#include "duckdb/planner/column_binding.hpp" #include "duckdb/planner/plan_serialization.hpp" -#include #include +#include namespace duckdb { diff --git a/src/optimizer/CMakeLists.txt b/src/optimizer/CMakeLists.txt index fb9a485b49d1..e4d52a17214b 100644 --- a/src/optimizer/CMakeLists.txt +++ b/src/optimizer/CMakeLists.txt @@ -17,10 +17,7 @@ add_library_unity( filter_pushdown.cpp filter_pullup.cpp in_clause_rewriter.cpp - join_node.cpp - join_order_optimizer.cpp cardinality_estimator.cpp - estimated_properties.cpp optimizer.cpp expression_rewriter.cpp regex_range_filter.cpp diff --git a/src/optimizer/cardinality_estimator.cpp b/src/optimizer/cardinality_estimator.cpp index 9fda8a66d1f1..c30c84017553 100644 --- a/src/optimizer/cardinality_estimator.cpp +++ b/src/optimizer/cardinality_estimator.cpp @@ -1,12 +1,12 @@ +#include "duckdb/function/table/table_scan.hpp" +#include "duckdb/optimizer/join_order/join_node.hpp" +#include "duckdb/optimizer/join_order/join_order_optimizer.hpp" #include "duckdb/planner/filter/conjunction_filter.hpp" #include "duckdb/planner/filter/constant_filter.hpp" -#include "duckdb/optimizer/join_order_optimizer.hpp" -#include "duckdb/optimizer/join_node.hpp" -#include "duckdb/planner/operator/logical_get.hpp" #include "duckdb/planner/operator/logical_comparison_join.hpp" +#include "duckdb/planner/operator/logical_get.hpp" #include "duckdb/storage/data_table.hpp" #include "duckdb/storage/statistics/numeric_statistics.hpp" -#include "duckdb/function/table/table_scan.hpp" namespace duckdb { diff --git a/src/optimizer/join_order/CMakeLists.txt b/src/optimizer/join_order/CMakeLists.txt index c3dd8bb1d2fe..7148ff7c010f 100644 --- a/src/optimizer/join_order/CMakeLists.txt +++ b/src/optimizer/join_order/CMakeLists.txt @@ -1,5 +1,5 @@ add_library_unity(duckdb_optimizer_join_order OBJECT query_graph.cpp - join_relation_set.cpp) + join_relation_set.cpp join_node.cpp estimated_properties.cpp join_order_optimizer.cpp) set(ALL_OBJECT_FILES ${ALL_OBJECT_FILES} $ PARENT_SCOPE) diff --git a/src/optimizer/estimated_properties.cpp b/src/optimizer/join_order/estimated_properties.cpp similarity index 89% rename from src/optimizer/estimated_properties.cpp rename to src/optimizer/join_order/estimated_properties.cpp index e966039adca4..3fb1ba65783f 100644 --- a/src/optimizer/estimated_properties.cpp +++ b/src/optimizer/join_order/estimated_properties.cpp @@ -1,5 +1,5 @@ -#include "duckdb/optimizer/estimated_properties.hpp" +#include "duckdb/optimizer/join_order/estimated_properties.hpp" namespace duckdb { diff --git a/src/optimizer/join_node.cpp b/src/optimizer/join_order/join_node.cpp similarity index 97% rename from src/optimizer/join_node.cpp rename to src/optimizer/join_order/join_node.cpp index 08d5beb4e5e7..182e5d0ee5f5 100644 --- a/src/optimizer/join_node.cpp +++ b/src/optimizer/join_order/join_node.cpp @@ -1,7 +1,8 @@ +#include "duckdb/optimizer/join_order/join_node.hpp" + #include "duckdb/common/limits.hpp" #include "duckdb/planner/expression/list.hpp" #include "duckdb/planner/operator/list.hpp" -#include "duckdb/optimizer/join_node.hpp" namespace duckdb { diff --git a/src/optimizer/join_order_optimizer.cpp b/src/optimizer/join_order/join_order_optimizer.cpp similarity index 99% rename from src/optimizer/join_order_optimizer.cpp rename to src/optimizer/join_order/join_order_optimizer.cpp index 15e422faf9e8..cc5141910576 100644 --- a/src/optimizer/join_order_optimizer.cpp +++ b/src/optimizer/join_order/join_order_optimizer.cpp @@ -1,7 +1,8 @@ -#include "duckdb/optimizer/join_order_optimizer.hpp" +#include "duckdb/optimizer/join_order/join_order_optimizer.hpp" + +#include "duckdb/common/limits.hpp" #include "duckdb/common/pair.hpp" #include "duckdb/main/client_context.hpp" -#include "duckdb/common/limits.hpp" #include "duckdb/planner/expression/list.hpp" #include "duckdb/planner/expression_iterator.hpp" #include "duckdb/planner/operator/list.hpp" diff --git a/src/optimizer/optimizer.cpp b/src/optimizer/optimizer.cpp index bd676fd8a09b..0641a5ab0eeb 100644 --- a/src/optimizer/optimizer.cpp +++ b/src/optimizer/optimizer.cpp @@ -1,5 +1,6 @@ #include "duckdb/optimizer/optimizer.hpp" +#include "duckdb/execution/column_binding_resolver.hpp" #include "duckdb/execution/expression_executor.hpp" #include "duckdb/main/client_context.hpp" #include "duckdb/main/config.hpp" @@ -12,7 +13,7 @@ #include "duckdb/optimizer/filter_pullup.hpp" #include "duckdb/optimizer/filter_pushdown.hpp" #include "duckdb/optimizer/in_clause_rewriter.hpp" -#include "duckdb/optimizer/join_order_optimizer.hpp" +#include "duckdb/optimizer/join_order/join_order_optimizer.hpp" #include "duckdb/optimizer/regex_range_filter.hpp" #include "duckdb/optimizer/remove_unused_columns.hpp" #include "duckdb/optimizer/rule/equal_or_null_simplification.hpp" @@ -22,7 +23,6 @@ #include "duckdb/optimizer/topn_optimizer.hpp" #include "duckdb/planner/binder.hpp" #include "duckdb/planner/planner.hpp" -#include "duckdb/execution/column_binding_resolver.hpp" namespace duckdb { From e3444f871115db23d2eab6b9c3f5b4648e33be6a Mon Sep 17 00:00:00 2001 From: Tom Ebergen Date: Tue, 6 Dec 2022 10:07:53 +0100 Subject: [PATCH 02/10] run format fix to get tests to pass --- src/common/symbols.cpp | 2 +- src/optimizer/join_order/CMakeLists.txt | 10 ++++++++-- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/src/common/symbols.cpp b/src/common/symbols.cpp index 24996ba65cca..bfec975b9c00 100644 --- a/src/common/symbols.cpp +++ b/src/common/symbols.cpp @@ -15,7 +15,7 @@ #include "duckdb/main/query_result.hpp" #include "duckdb/main/relation.hpp" #include "duckdb/main/stream_query_result.hpp" -#include "duckdb/optimizer/join_order_optimizer.hpp" +#include "duckdb/optimizer/join_order/join_order_optimizer.hpp" #include "duckdb/optimizer/rule.hpp" #include "duckdb/parallel/pipeline.hpp" #include "duckdb/parallel/meta_pipeline.hpp" diff --git a/src/optimizer/join_order/CMakeLists.txt b/src/optimizer/join_order/CMakeLists.txt index 7148ff7c010f..8a4066369fb9 100644 --- a/src/optimizer/join_order/CMakeLists.txt +++ b/src/optimizer/join_order/CMakeLists.txt @@ -1,5 +1,11 @@ -add_library_unity(duckdb_optimizer_join_order OBJECT query_graph.cpp - join_relation_set.cpp join_node.cpp estimated_properties.cpp join_order_optimizer.cpp) +add_library_unity( + duckdb_optimizer_join_order + OBJECT + query_graph.cpp + join_relation_set.cpp + join_node.cpp + estimated_properties.cpp + join_order_optimizer.cpp) set(ALL_OBJECT_FILES ${ALL_OBJECT_FILES} $ PARENT_SCOPE) From b048894d076b92d8987fb60b56cdd837617ba711 Mon Sep 17 00:00:00 2001 From: Tom Ebergen Date: Tue, 6 Dec 2022 10:35:36 +0100 Subject: [PATCH 03/10] fix some more issues with reorganization --- src/optimizer/CMakeLists.txt | 1 - src/optimizer/join_order/CMakeLists.txt | 1 + src/optimizer/{ => join_order}/cardinality_estimator.cpp | 0 3 files changed, 1 insertion(+), 1 deletion(-) rename src/optimizer/{ => join_order}/cardinality_estimator.cpp (100%) diff --git a/src/optimizer/CMakeLists.txt b/src/optimizer/CMakeLists.txt index e4d52a17214b..f67158abfa0e 100644 --- a/src/optimizer/CMakeLists.txt +++ b/src/optimizer/CMakeLists.txt @@ -17,7 +17,6 @@ add_library_unity( filter_pushdown.cpp filter_pullup.cpp in_clause_rewriter.cpp - cardinality_estimator.cpp optimizer.cpp expression_rewriter.cpp regex_range_filter.cpp diff --git a/src/optimizer/join_order/CMakeLists.txt b/src/optimizer/join_order/CMakeLists.txt index 8a4066369fb9..57dc2f29a14c 100644 --- a/src/optimizer/join_order/CMakeLists.txt +++ b/src/optimizer/join_order/CMakeLists.txt @@ -2,6 +2,7 @@ add_library_unity( duckdb_optimizer_join_order OBJECT query_graph.cpp + cardinality_estimator.cpp join_relation_set.cpp join_node.cpp estimated_properties.cpp diff --git a/src/optimizer/cardinality_estimator.cpp b/src/optimizer/join_order/cardinality_estimator.cpp similarity index 100% rename from src/optimizer/cardinality_estimator.cpp rename to src/optimizer/join_order/cardinality_estimator.cpp From eb6dde196fb82a134e63b7eb39acd22e8aad16db Mon Sep 17 00:00:00 2001 From: Tom Ebergen Date: Tue, 6 Dec 2022 12:53:56 +0100 Subject: [PATCH 04/10] more changing of file paths --- .../duckdb/optimizer/join_order/join_order_optimizer.hpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/include/duckdb/optimizer/join_order/join_order_optimizer.hpp b/src/include/duckdb/optimizer/join_order/join_order_optimizer.hpp index 6a98b7eb994f..52989fd1bd6e 100644 --- a/src/include/duckdb/optimizer/join_order/join_order_optimizer.hpp +++ b/src/include/duckdb/optimizer/join_order/join_order_optimizer.hpp @@ -8,15 +8,16 @@ #pragma once -#include "cardinality_estimator.hpp" #include "duckdb/common/unordered_map.hpp" #include "duckdb/common/unordered_set.hpp" #include "duckdb/optimizer/join_order/join_relation.hpp" +#include "duckdb/optimizer/join_order/cardinality_estimator.hpp" #include "duckdb/optimizer/join_order/query_graph.hpp" +#include "duckdb/optimizer/join_order/join_node.hpp" #include "duckdb/parser/expression_map.hpp" #include "duckdb/planner/logical_operator.hpp" #include "duckdb/planner/logical_operator_visitor.hpp" -#include "join_node.hpp" + #include From 075c6acb2d86e660093dc07829ecfeb793a8b8e6 Mon Sep 17 00:00:00 2001 From: Tom Ebergen Date: Tue, 6 Dec 2022 14:50:07 +0100 Subject: [PATCH 05/10] adding operator pool class. But first going to look at what happens to the pointers --- .../duckdb/optimizer/operator_pool.hpp | 25 +++++++++++++++++++ src/optimizer/operator_pool.cpp | 15 +++++++++++ src/optimizer/optimizer.cpp | 4 ++- 3 files changed, 43 insertions(+), 1 deletion(-) create mode 100644 src/include/duckdb/optimizer/operator_pool.hpp create mode 100644 src/optimizer/operator_pool.cpp diff --git a/src/include/duckdb/optimizer/operator_pool.hpp b/src/include/duckdb/optimizer/operator_pool.hpp new file mode 100644 index 000000000000..437d5aba2894 --- /dev/null +++ b/src/include/duckdb/optimizer/operator_pool.hpp @@ -0,0 +1,25 @@ +//===----------------------------------------------------------------------===// +// DuckDB +// +// duckdb/optimizer/operator_pool.hpp +// +// +//===----------------------------------------------------------------------===// + +namespace duckdb { + +struct LogicalOperatorIdentifier { + LogicalOperatorType type; + string name; + +}; + +class OperatorPool { +public: + void AddOperator(unique_ptr op); + bool InPool(unique_ptr op); + +private: + unordered_set seen_operators; +}; +} \ No newline at end of file diff --git a/src/optimizer/operator_pool.cpp b/src/optimizer/operator_pool.cpp new file mode 100644 index 000000000000..ac0062e0f249 --- /dev/null +++ b/src/optimizer/operator_pool.cpp @@ -0,0 +1,15 @@ +// +// Created by Tom Ebergen on 06/12/2022. +// + +namespace duckdb { + +void OperatorPool::AddOperator(unique_ptr op) { + +} + +bool OperatorPool::InPool(unique_ptr op) { + return false; +} + +} \ No newline at end of file diff --git a/src/optimizer/optimizer.cpp b/src/optimizer/optimizer.cpp index 0641a5ab0eeb..4a87013d7d92 100644 --- a/src/optimizer/optimizer.cpp +++ b/src/optimizer/optimizer.cpp @@ -74,7 +74,9 @@ unique_ptr Optimizer::Optimize(unique_ptr plan this->plan = move(plan_p); // first we perform expression rewrites using the ExpressionRewriter // this does not change the logical plan structure, but only simplifies the expression trees - RunOptimizer(OptimizerType::EXPRESSION_REWRITER, [&]() { rewriter.VisitOperator(*plan); }); + RunOptimizer(OptimizerType::EXPRESSION_REWRITER, [&]() { + rewriter.VisitOperator(*plan); + }); // perform filter pullup RunOptimizer(OptimizerType::FILTER_PULLUP, [&]() { From d530f0d4e9a7519a57702ed55fc29ffb2dc9986e Mon Sep 17 00:00:00 2001 From: Tom Ebergen Date: Tue, 6 Dec 2022 15:24:46 +0100 Subject: [PATCH 06/10] added a class to keep a pool of logical operator pointers. They are unique though, so dont understand how they should be copied --- .../duckdb/optimizer/filter_pushdown.hpp | 2 ++ src/include/duckdb/optimizer/operator_pool.hpp | 17 ++++++++++------- src/optimizer/CMakeLists.txt | 1 + src/optimizer/operator_pool.cpp | 11 ++++++++--- 4 files changed, 21 insertions(+), 10 deletions(-) diff --git a/src/include/duckdb/optimizer/filter_pushdown.hpp b/src/include/duckdb/optimizer/filter_pushdown.hpp index ee4d40e7d365..aa6a32b41f2b 100644 --- a/src/include/duckdb/optimizer/filter_pushdown.hpp +++ b/src/include/duckdb/optimizer/filter_pushdown.hpp @@ -11,6 +11,7 @@ #include "duckdb/common/unordered_set.hpp" #include "duckdb/optimizer/filter_combiner.hpp" #include "duckdb/optimizer/rule.hpp" +#include "duckdb/optimizer/operator_pool.hpp" namespace duckdb { @@ -37,6 +38,7 @@ class FilterPushdown { private: vector> filters; Optimizer &optimizer; + OperatorPool seen_operators; //! Push down a LogicalAggregate op unique_ptr PushdownAggregate(unique_ptr op); diff --git a/src/include/duckdb/optimizer/operator_pool.hpp b/src/include/duckdb/optimizer/operator_pool.hpp index 437d5aba2894..5874928851bb 100644 --- a/src/include/duckdb/optimizer/operator_pool.hpp +++ b/src/include/duckdb/optimizer/operator_pool.hpp @@ -6,20 +6,23 @@ // //===----------------------------------------------------------------------===// -namespace duckdb { +#pragma once -struct LogicalOperatorIdentifier { - LogicalOperatorType type; - string name; +#include "duckdb/planner/logical_operator.hpp" +#include "duckdb/common/unordered_set.hpp" -}; +namespace duckdb { class OperatorPool { public: - void AddOperator(unique_ptr op); + OperatorPool() { + seen_operators = unordered_set>(); + } + + unique_ptr AddOperator(unique_ptr op); bool InPool(unique_ptr op); private: - unordered_set seen_operators; + unordered_set> seen_operators; }; } \ No newline at end of file diff --git a/src/optimizer/CMakeLists.txt b/src/optimizer/CMakeLists.txt index f67158abfa0e..7f7ce3f68dd1 100644 --- a/src/optimizer/CMakeLists.txt +++ b/src/optimizer/CMakeLists.txt @@ -18,6 +18,7 @@ add_library_unity( filter_pullup.cpp in_clause_rewriter.cpp optimizer.cpp + operator_pool.cpp expression_rewriter.cpp regex_range_filter.cpp remove_unused_columns.cpp diff --git a/src/optimizer/operator_pool.cpp b/src/optimizer/operator_pool.cpp index ac0062e0f249..e6229bd9f876 100644 --- a/src/optimizer/operator_pool.cpp +++ b/src/optimizer/operator_pool.cpp @@ -2,14 +2,19 @@ // Created by Tom Ebergen on 06/12/2022. // -namespace duckdb { +#include "duckdb/optimizer/operator_pool.hpp" -void OperatorPool::AddOperator(unique_ptr op) { +namespace duckdb { +unique_ptr OperatorPool::AddOperator(unique_ptr op) { + D_ASSERT(!InPool(op)); + seen_operators.insert(op); + return move(op); } bool OperatorPool::InPool(unique_ptr op) { - return false; + auto it = seen_operators.find(op); + return it != seen_operators.end(); } } \ No newline at end of file From 33376e867a8d9e3d9c979571494810ecf47904a1 Mon Sep 17 00:00:00 2001 From: Tom Ebergen Date: Tue, 6 Dec 2022 16:12:35 +0100 Subject: [PATCH 07/10] this should be the fix for filter pushdown --- .../duckdb/optimizer/operator_pool.hpp | 8 +- src/optimizer/filter_pushdown.cpp | 9 +- src/optimizer/operator_pool.cpp | 9 +- src/optimizer/pushdown/pushdown_left_join.cpp | 3 + test/optimizer/pushdown/issue_5097.test | 154 ++++++++++++++++++ 5 files changed, 173 insertions(+), 10 deletions(-) create mode 100644 test/optimizer/pushdown/issue_5097.test diff --git a/src/include/duckdb/optimizer/operator_pool.hpp b/src/include/duckdb/optimizer/operator_pool.hpp index 5874928851bb..f8a3359f29b5 100644 --- a/src/include/duckdb/optimizer/operator_pool.hpp +++ b/src/include/duckdb/optimizer/operator_pool.hpp @@ -16,13 +16,13 @@ namespace duckdb { class OperatorPool { public: OperatorPool() { - seen_operators = unordered_set>(); + seen_operators = unordered_set(); } - unique_ptr AddOperator(unique_ptr op); - bool InPool(unique_ptr op); + void AddOperator(LogicalOperator *op); + bool InPool(LogicalOperator *op); private: - unordered_set> seen_operators; + unordered_set seen_operators; }; } \ No newline at end of file diff --git a/src/optimizer/filter_pushdown.cpp b/src/optimizer/filter_pushdown.cpp index b8b51830ab79..5f0ce5a00f34 100644 --- a/src/optimizer/filter_pushdown.cpp +++ b/src/optimizer/filter_pushdown.cpp @@ -5,14 +5,21 @@ #include "duckdb/planner/operator/logical_join.hpp" #include "duckdb/optimizer/optimizer.hpp" +#include "iostream" + namespace duckdb { using Filter = FilterPushdown::Filter; -FilterPushdown::FilterPushdown(Optimizer &optimizer) : optimizer(optimizer), combiner(optimizer.context) { +FilterPushdown::FilterPushdown(Optimizer &optimizer) : optimizer(optimizer), seen_operators(), combiner(optimizer.context) { } unique_ptr FilterPushdown::Rewrite(unique_ptr op) { + if (seen_operators.InPool(op.get())) { + // We've already optimized this operator, so just return. + return move(op); + } + seen_operators.AddOperator(op.get()); D_ASSERT(!combiner.HasFilters()); switch (op->type) { case LogicalOperatorType::LOGICAL_AGGREGATE_AND_GROUP_BY: diff --git a/src/optimizer/operator_pool.cpp b/src/optimizer/operator_pool.cpp index e6229bd9f876..b227c9c339e3 100644 --- a/src/optimizer/operator_pool.cpp +++ b/src/optimizer/operator_pool.cpp @@ -6,14 +6,13 @@ namespace duckdb { -unique_ptr OperatorPool::AddOperator(unique_ptr op) { +void OperatorPool::AddOperator(LogicalOperator *op) { D_ASSERT(!InPool(op)); - seen_operators.insert(op); - return move(op); + seen_operators.insert((idx_t)op); } -bool OperatorPool::InPool(unique_ptr op) { - auto it = seen_operators.find(op); +bool OperatorPool::InPool(LogicalOperator *op) { + auto it = seen_operators.find((idx_t)op); return it != seen_operators.end(); } diff --git a/src/optimizer/pushdown/pushdown_left_join.cpp b/src/optimizer/pushdown/pushdown_left_join.cpp index 6e5d143d1375..331b0e80e033 100644 --- a/src/optimizer/pushdown/pushdown_left_join.cpp +++ b/src/optimizer/pushdown/pushdown_left_join.cpp @@ -8,6 +8,8 @@ #include "duckdb/planner/operator/logical_comparison_join.hpp" #include "duckdb/planner/operator/logical_filter.hpp" +#include "iostream" + namespace duckdb { using Filter = FilterPushdown::Filter; @@ -120,6 +122,7 @@ unique_ptr FilterPushdown::PushdownLeftJoin(unique_ptrchildren[0] = left_pushdown.Rewrite(move(op->children[0])); op->children[1] = right_pushdown.Rewrite(move(op->children[1])); return FinishPushdown(move(op)); diff --git a/test/optimizer/pushdown/issue_5097.test b/test/optimizer/pushdown/issue_5097.test new file mode 100644 index 000000000000..4a150f42a55f --- /dev/null +++ b/test/optimizer/pushdown/issue_5097.test @@ -0,0 +1,154 @@ +# name: test/optimizer/pushdown/issue_5097.test +# description: Test Pushdown with many Left Joins +# group: [pushdown] + +statement ok +CREATE TABLE t1(c1 INTEGER NOT NULL, c2 INTEGER, c3 INTEGER, info VARCHAR); + +statement ok +CREATE TABLE t10(c1 INTEGER NOT NULL, c2 INTEGER, c3 INTEGER, c4 INTEGER, c5 INTEGER, c6 INTEGER, c7 INTEGER, c8 INTEGER, c9 INTEGER, info VARCHAR); + +statement ok +CREATE TABLE t11(c1 INTEGER NOT NULL, c2 INTEGER, c3 INTEGER, c4 INTEGER, c5 INTEGER, c6 INTEGER, c7 INTEGER, c8 INTEGER, c9 INTEGER, info VARCHAR); + +statement ok +CREATE TABLE t12(c1 INTEGER NOT NULL, c2 INTEGER, c3 INTEGER, c4 INTEGER, c5 INTEGER, c6 INTEGER, c7 INTEGER, c8 INTEGER, c9 INTEGER, info VARCHAR); + +statement ok +CREATE TABLE t13(c1 INTEGER NOT NULL, c2 INTEGER, c3 INTEGER, c4 INTEGER, c5 INTEGER, c6 INTEGER, c7 INTEGER, c8 INTEGER, c9 INTEGER, info VARCHAR); + +statement ok +CREATE TABLE t14(c1 INTEGER NOT NULL, c2 INTEGER, c3 INTEGER, c4 INTEGER, c5 INTEGER, c6 INTEGER, c7 INTEGER, c8 INTEGER, c9 INTEGER, info VARCHAR); + +statement ok +CREATE TABLE t15(c1 INTEGER NOT NULL, c2 INTEGER, c3 INTEGER, c4 INTEGER, c5 INTEGER, c6 INTEGER, c7 INTEGER, c8 INTEGER, c9 INTEGER, info VARCHAR); + +statement ok +CREATE TABLE t16(c1 INTEGER NOT NULL, c2 INTEGER, c3 INTEGER, c4 INTEGER, c5 INTEGER, c6 INTEGER, c7 INTEGER, c8 INTEGER, c9 INTEGER, info VARCHAR); + +statement ok +CREATE TABLE t17(c1 INTEGER NOT NULL, c2 INTEGER, c3 INTEGER, c4 INTEGER, c5 INTEGER, c6 INTEGER, c7 INTEGER, c8 INTEGER, c9 INTEGER, info VARCHAR); + +statement ok +CREATE TABLE t18(c1 INTEGER NOT NULL, c2 INTEGER, c3 INTEGER, c4 INTEGER, c5 INTEGER, c6 INTEGER, c7 INTEGER, c8 INTEGER, c9 INTEGER, info VARCHAR); + +statement ok +CREATE TABLE t19(c1 INTEGER NOT NULL, c2 INTEGER, c3 INTEGER, c4 INTEGER, c5 INTEGER, c6 INTEGER, c7 INTEGER, c8 INTEGER, c9 INTEGER, info VARCHAR); + +statement ok +CREATE TABLE t2(c1 INTEGER NOT NULL, c2 INTEGER, c3 INTEGER, c4 INTEGER, c5 INTEGER, c6 INTEGER, c7 INTEGER, c8 INTEGER, c9 INTEGER, info VARCHAR); + +statement ok +CREATE TABLE t20(c1 INTEGER NOT NULL, c2 INTEGER, c3 INTEGER, c4 INTEGER, c5 INTEGER, c6 INTEGER, c7 INTEGER, c8 INTEGER, c9 INTEGER, info VARCHAR); + +statement ok +CREATE TABLE t21(c1 INTEGER NOT NULL, c2 INTEGER, c3 INTEGER, c4 INTEGER, c5 INTEGER, c6 INTEGER, c7 INTEGER, c8 INTEGER, c9 INTEGER, info VARCHAR); + +statement ok +CREATE TABLE t22(c1 INTEGER NOT NULL, c2 INTEGER, c3 INTEGER, c4 INTEGER, c5 INTEGER, c6 INTEGER, c7 INTEGER, c8 INTEGER, c9 INTEGER, info VARCHAR); + +statement ok +CREATE TABLE t23(c1 INTEGER NOT NULL, c2 INTEGER, c3 INTEGER, c4 INTEGER, c5 INTEGER, c6 INTEGER, c7 INTEGER, c8 INTEGER, c9 INTEGER, info VARCHAR); + +statement ok +CREATE TABLE t24(c1 INTEGER NOT NULL, c2 INTEGER, c3 INTEGER, c4 INTEGER, c5 INTEGER, c6 INTEGER, c7 INTEGER, c8 INTEGER, c9 INTEGER, info VARCHAR); + +statement ok +CREATE TABLE t25(c1 INTEGER NOT NULL, c2 INTEGER, c3 INTEGER, c4 INTEGER, c5 INTEGER, c6 INTEGER, c7 INTEGER, c8 INTEGER, c9 INTEGER, info VARCHAR); + +statement ok +CREATE TABLE t26(c1 INTEGER NOT NULL, c2 INTEGER, c3 INTEGER, c4 INTEGER, c5 INTEGER, c6 INTEGER, c7 INTEGER, c8 INTEGER, c9 INTEGER, info VARCHAR); + +statement ok +CREATE TABLE t27(c1 INTEGER NOT NULL, c2 INTEGER, c3 INTEGER, c4 INTEGER, c5 INTEGER, c6 INTEGER, c7 INTEGER, c8 INTEGER, c9 INTEGER, info VARCHAR); + +statement ok +CREATE TABLE t28(c1 INTEGER NOT NULL, c2 INTEGER, c3 INTEGER, c4 INTEGER, c5 INTEGER, c6 INTEGER, c7 INTEGER, c8 INTEGER, c9 INTEGER, info VARCHAR); + +statement ok +CREATE TABLE t29(c1 INTEGER NOT NULL, c2 INTEGER, c3 INTEGER, c4 INTEGER, c5 INTEGER, c6 INTEGER, c7 INTEGER, c8 INTEGER, c9 INTEGER, info VARCHAR); + +statement ok +CREATE TABLE t3(c1 INTEGER NOT NULL, c2 INTEGER, c3 INTEGER, c4 INTEGER, c5 INTEGER, c6 INTEGER, c7 INTEGER, c8 INTEGER, c9 INTEGER, info VARCHAR); + +statement ok +CREATE TABLE t30(c1 INTEGER NOT NULL, c2 INTEGER, c3 INTEGER, c4 INTEGER, c5 INTEGER, c6 INTEGER, c7 INTEGER, c8 INTEGER, c9 INTEGER, info VARCHAR); + +statement ok +CREATE TABLE t31(c1 INTEGER NOT NULL, c2 INTEGER, c3 INTEGER, c4 INTEGER, c5 INTEGER, c6 INTEGER, c7 INTEGER, c8 INTEGER, c9 INTEGER, info VARCHAR); + +statement ok +CREATE TABLE t32(c1 INTEGER NOT NULL, c2 INTEGER, c3 INTEGER, c4 INTEGER, c5 INTEGER, c6 INTEGER, c7 INTEGER, c8 INTEGER, c9 INTEGER, info VARCHAR); + +statement ok +CREATE TABLE t33(c1 INTEGER NOT NULL, c2 INTEGER, c3 INTEGER, c4 INTEGER, c5 INTEGER, c6 INTEGER, c7 INTEGER, c8 INTEGER, c9 INTEGER, info VARCHAR); + +statement ok +CREATE TABLE t34(c1 INTEGER NOT NULL, c2 INTEGER, c3 INTEGER, c4 INTEGER, c5 INTEGER, c6 INTEGER, c7 INTEGER, c8 INTEGER, c9 INTEGER, info VARCHAR); + +statement ok +CREATE TABLE t35(c1 INTEGER NOT NULL, c2 INTEGER, c3 INTEGER, c4 INTEGER, c5 INTEGER, c6 INTEGER, c7 INTEGER, c8 INTEGER, c9 INTEGER, info VARCHAR); + +statement ok +CREATE TABLE t36(c1 INTEGER NOT NULL, c2 INTEGER, c3 INTEGER, c4 INTEGER, c5 INTEGER, c6 INTEGER, c7 INTEGER, c8 INTEGER, c9 INTEGER, info VARCHAR); + +statement ok +CREATE TABLE t37(c1 INTEGER NOT NULL, c2 INTEGER, c3 INTEGER, c4 INTEGER, c5 INTEGER, c6 INTEGER, c7 INTEGER, c8 INTEGER, c9 INTEGER, info VARCHAR); + +statement ok +CREATE TABLE t38(c1 INTEGER NOT NULL, c2 INTEGER, c3 INTEGER, c4 INTEGER, c5 INTEGER, c6 INTEGER, c7 INTEGER, c8 INTEGER, c9 INTEGER, info VARCHAR); + +statement ok +CREATE TABLE t39(c1 INTEGER NOT NULL, c2 INTEGER, c3 INTEGER, c4 INTEGER, c5 INTEGER, c6 INTEGER, c7 INTEGER, c8 INTEGER, c9 INTEGER, info VARCHAR); + +statement ok +CREATE TABLE t4(c1 INTEGER NOT NULL, c2 INTEGER, c3 INTEGER, c4 INTEGER, c5 INTEGER, c6 INTEGER, c7 INTEGER, c8 INTEGER, c9 INTEGER, info VARCHAR); + +statement ok +CREATE TABLE t40(c1 INTEGER NOT NULL, c2 INTEGER, c3 INTEGER, c4 INTEGER, c5 INTEGER, c6 INTEGER, c7 INTEGER, c8 INTEGER, c9 INTEGER, info VARCHAR); + +statement ok +CREATE TABLE t41(c1 INTEGER NOT NULL, c2 INTEGER, c3 INTEGER, c4 INTEGER, c5 INTEGER, c6 INTEGER, c7 INTEGER, c8 INTEGER, c9 INTEGER, info VARCHAR); + +statement ok +CREATE TABLE t42(c1 INTEGER NOT NULL, c2 INTEGER, c3 INTEGER, c4 INTEGER, c5 INTEGER, c6 INTEGER, c7 INTEGER, c8 INTEGER, c9 INTEGER, info VARCHAR); + +statement ok +CREATE TABLE t43(c1 INTEGER NOT NULL, c2 INTEGER, c3 INTEGER, c4 INTEGER, c5 INTEGER, c6 INTEGER, c7 INTEGER, c8 INTEGER, c9 INTEGER, info VARCHAR); + +statement ok +CREATE TABLE t44(c1 INTEGER NOT NULL, c2 INTEGER, c3 INTEGER, c4 INTEGER, c5 INTEGER, c6 INTEGER, c7 INTEGER, c8 INTEGER, c9 INTEGER, info VARCHAR); + +statement ok +CREATE TABLE t45(c1 INTEGER NOT NULL, c2 INTEGER, c3 INTEGER, c4 INTEGER, c5 INTEGER, c6 INTEGER, c7 INTEGER, c8 INTEGER, c9 INTEGER, info VARCHAR); + +statement ok +CREATE TABLE t46(c1 INTEGER NOT NULL, c2 INTEGER, c3 INTEGER, c4 INTEGER, c5 INTEGER, c6 INTEGER, c7 INTEGER, c8 INTEGER, c9 INTEGER, info VARCHAR); + +statement ok +CREATE TABLE t47(c1 INTEGER NOT NULL, c2 INTEGER, c3 INTEGER, c4 INTEGER, c5 INTEGER, c6 INTEGER, c7 INTEGER, c8 INTEGER, c9 INTEGER, info VARCHAR); + +statement ok +CREATE TABLE t48(c1 INTEGER NOT NULL, c2 INTEGER, c3 INTEGER, c4 INTEGER, c5 INTEGER, c6 INTEGER, c7 INTEGER, c8 INTEGER, c9 INTEGER, info VARCHAR); + +statement ok +CREATE TABLE t5(c1 INTEGER NOT NULL, c2 INTEGER, c3 INTEGER, c4 INTEGER, c5 INTEGER, c6 INTEGER, c7 INTEGER, c8 INTEGER, c9 INTEGER, info VARCHAR); + +statement ok +CREATE TABLE t6(c1 INTEGER NOT NULL, c2 INTEGER, c3 INTEGER, c4 INTEGER, c5 INTEGER, c6 INTEGER, c7 INTEGER, c8 INTEGER, c9 INTEGER, info VARCHAR); + +statement ok +CREATE TABLE t7(c1 INTEGER NOT NULL, c2 INTEGER, c3 INTEGER, c4 INTEGER, c5 INTEGER, c6 INTEGER, c7 INTEGER, c8 INTEGER, c9 INTEGER, info VARCHAR); + +statement ok +CREATE TABLE t8(c1 INTEGER NOT NULL, c2 INTEGER, c3 INTEGER, c4 INTEGER, c5 INTEGER, c6 INTEGER, c7 INTEGER, c8 INTEGER, c9 INTEGER, info VARCHAR); + +statement ok +CREATE TABLE t9(c1 INTEGER NOT NULL, c2 INTEGER, c3 INTEGER, c4 INTEGER, c5 INTEGER, c6 INTEGER, c7 INTEGER, c8 INTEGER, c9 INTEGER, info VARCHAR); + +# Multiple column in the root OR node, don't push down +statement ok +select t1.c2 from t1 +inner join t2 on (t1.c2=t2.c1) +left join t4 on (t1.c3=t4.c1) +left join t5 on (t4.c2=t5.c1); \ No newline at end of file From ce5bb0a0cbce5c0b004e5035ad705d93be58daba Mon Sep 17 00:00:00 2001 From: Tom Ebergen Date: Tue, 6 Dec 2022 16:14:07 +0100 Subject: [PATCH 08/10] add proper test that takes forever when nodes are revisited --- test/optimizer/pushdown/issue_5097.test | 52 ++++++++++++++++++++++++- 1 file changed, 51 insertions(+), 1 deletion(-) diff --git a/test/optimizer/pushdown/issue_5097.test b/test/optimizer/pushdown/issue_5097.test index 4a150f42a55f..cbbeccd6c286 100644 --- a/test/optimizer/pushdown/issue_5097.test +++ b/test/optimizer/pushdown/issue_5097.test @@ -151,4 +151,54 @@ statement ok select t1.c2 from t1 inner join t2 on (t1.c2=t2.c1) left join t4 on (t1.c3=t4.c1) -left join t5 on (t4.c2=t5.c1); \ No newline at end of file +left join t5 on (t4.c2=t5.c1) +left join t6 on (t5.c2=t6.c1) +left join t7 on (t6.c2=t7.c1) +left join t8 on (t7.c2=t8.c1) +left join t9 on (t8.c2=t9.c1) +left join t10 on (t9.c2=t10.c1) +left join t11 on (t10.c2=t11.c1) +left join t12 on (t11.c2=t12.c1) +left join t13 on (t12.c2=t13.c1) +left join t14 on (t13.c2=t14.c1) +left join t15 on (t14.c2=t15.c1) +left join t16 on (t15.c2=t16.c1) +left join t17 on (t16.c2=t17.c1) +left join t18 on (t17.c2=t18.c1) +left join t19 on (t18.c2=t19.c1) +left join t20 on (t19.c2=t20.c1) +left join t21 on (t20.c2=t21.c1) +left join t22 on (t21.c2=t22.c1) +left join t23 on (t22.c2=t23.c1) +left join t24 on (t23.c2=t24.c1) +left join t25 on (t24.c2=t25.c1) +left join t26 on (t25.c2=t26.c1) +left join t27 on (t26.c2=t27.c1) +left join t28 on (t27.c2=t28.c1) +left join t29 on (t28.c2=t29.c1) +left join t30 on (t29.c2=t30.c1) +left join t31 on (t30.c2=t31.c1) +left join t32 on (t31.c2=t32.c1) +left join t33 on (t32.c2=t33.c1) +left join t34 on (t33.c2=t34.c1) +left join t35 on (t34.c2=t35.c1) +left join t36 on (t35.c2=t36.c1) +left join t37 on (t36.c2=t37.c1) +left join t38 on (t37.c2=t38.c1) +left join t39 on (t38.c2=t39.c1) +left join t3 ttttt3 on (ttttt3.c6=t33.c5) +left join t40 on (t39.c2=t40.c1) +left join t41 on (t40.c2=t41.c1) +left join t42 on (t41.c2=t42.c1) +left join t43 on (t42.c2=t43.c1) +left join t44 on (t43.c2=t44.c1) +left join t45 on (t44.c2=t45.c1) +left join t46 on (t45.c2=t46.c1) +left join t47 on (t46.c2=t47.c1) +left join t48 on (t47.c2=t48.c1) +left join t5 ttt5 on (t42.c1=ttt5.c3) +left join t5 ttt6 on (ttt5.c4=ttt6.c5) +left join t5 ttt7 on (ttt6.c6=ttt7.c7) +inner join t3 on (t2.c2=t3.c1) +left join t4 tt4 on (t47.c1=tt4.c3) +left join t4 tt5 on (tt4.c4=tt5.c5); \ No newline at end of file From be33b1c124c0923e4a39f27a33996b29ad0e8637 Mon Sep 17 00:00:00 2001 From: Tom Ebergen Date: Tue, 6 Dec 2022 17:03:46 +0100 Subject: [PATCH 09/10] no more unnecessary optimizations of children that have already been optimized, but if a new node is made in the optimization, we still reoptimize, which I guess is good? --- src/include/duckdb/optimizer/filter_pushdown.hpp | 1 - src/include/duckdb/optimizer/operator_pool.hpp | 1 + src/include/duckdb/optimizer/optimizer.hpp | 6 ++++++ src/optimizer/filter_pushdown.cpp | 6 +++--- src/optimizer/operator_pool.cpp | 4 ++++ src/optimizer/optimizer.cpp | 4 +++- src/optimizer/pushdown/pushdown_left_join.cpp | 2 ++ 7 files changed, 19 insertions(+), 5 deletions(-) diff --git a/src/include/duckdb/optimizer/filter_pushdown.hpp b/src/include/duckdb/optimizer/filter_pushdown.hpp index aa6a32b41f2b..b79737aaff0f 100644 --- a/src/include/duckdb/optimizer/filter_pushdown.hpp +++ b/src/include/duckdb/optimizer/filter_pushdown.hpp @@ -38,7 +38,6 @@ class FilterPushdown { private: vector> filters; Optimizer &optimizer; - OperatorPool seen_operators; //! Push down a LogicalAggregate op unique_ptr PushdownAggregate(unique_ptr op); diff --git a/src/include/duckdb/optimizer/operator_pool.hpp b/src/include/duckdb/optimizer/operator_pool.hpp index f8a3359f29b5..b72d0399d6ee 100644 --- a/src/include/duckdb/optimizer/operator_pool.hpp +++ b/src/include/duckdb/optimizer/operator_pool.hpp @@ -21,6 +21,7 @@ class OperatorPool { void AddOperator(LogicalOperator *op); bool InPool(LogicalOperator *op); + void EmptyOperatorPool(); private: unordered_set seen_operators; diff --git a/src/include/duckdb/optimizer/optimizer.hpp b/src/include/duckdb/optimizer/optimizer.hpp index 96bde0ad7012..7b6af447231b 100644 --- a/src/include/duckdb/optimizer/optimizer.hpp +++ b/src/include/duckdb/optimizer/optimizer.hpp @@ -12,6 +12,7 @@ #include "duckdb/planner/logical_operator.hpp" #include "duckdb/planner/logical_operator_visitor.hpp" #include "duckdb/common/enums/optimizer_type.hpp" +#include "duckdb/optimizer/operator_pool.hpp" #include @@ -24,11 +25,16 @@ class Optimizer { unique_ptr Optimize(unique_ptr plan); + + ClientContext &context; Binder &binder; ExpressionRewriter rewriter; + OperatorPool seen_operators; private: + + void RunOptimizer(OptimizerType type, const std::function &callback); void Verify(LogicalOperator &op); diff --git a/src/optimizer/filter_pushdown.cpp b/src/optimizer/filter_pushdown.cpp index 5f0ce5a00f34..e29f4277d625 100644 --- a/src/optimizer/filter_pushdown.cpp +++ b/src/optimizer/filter_pushdown.cpp @@ -11,15 +11,15 @@ namespace duckdb { using Filter = FilterPushdown::Filter; -FilterPushdown::FilterPushdown(Optimizer &optimizer) : optimizer(optimizer), seen_operators(), combiner(optimizer.context) { +FilterPushdown::FilterPushdown(Optimizer &optimizer) : optimizer(optimizer), combiner(optimizer.context) { } unique_ptr FilterPushdown::Rewrite(unique_ptr op) { - if (seen_operators.InPool(op.get())) { + if (optimizer.seen_operators.InPool(op.get())) { // We've already optimized this operator, so just return. return move(op); } - seen_operators.AddOperator(op.get()); + optimizer.seen_operators.AddOperator(op.get()); D_ASSERT(!combiner.HasFilters()); switch (op->type) { case LogicalOperatorType::LOGICAL_AGGREGATE_AND_GROUP_BY: diff --git a/src/optimizer/operator_pool.cpp b/src/optimizer/operator_pool.cpp index b227c9c339e3..c949ff6d323c 100644 --- a/src/optimizer/operator_pool.cpp +++ b/src/optimizer/operator_pool.cpp @@ -16,4 +16,8 @@ bool OperatorPool::InPool(LogicalOperator *op) { return it != seen_operators.end(); } +void OperatorPool::EmptyOperatorPool() { + seen_operators.clear(); +} + } \ No newline at end of file diff --git a/src/optimizer/optimizer.cpp b/src/optimizer/optimizer.cpp index 4a87013d7d92..fda23b304e06 100644 --- a/src/optimizer/optimizer.cpp +++ b/src/optimizer/optimizer.cpp @@ -26,7 +26,8 @@ namespace duckdb { -Optimizer::Optimizer(Binder &binder, ClientContext &context) : context(context), binder(binder), rewriter(context) { +Optimizer::Optimizer(Binder &binder, ClientContext &context) : context(context), binder(binder), rewriter(context), +seen_operators() { rewriter.rules.push_back(make_unique(rewriter)); rewriter.rules.push_back(make_unique(rewriter)); rewriter.rules.push_back(make_unique(rewriter)); @@ -60,6 +61,7 @@ void Optimizer::RunOptimizer(OptimizerType type, const std::function &ca profiler.StartPhase(OptimizerTypeToString(type)); callback(); profiler.EndPhase(); + seen_operators.EmptyOperatorPool(); if (plan) { Verify(*plan); } diff --git a/src/optimizer/pushdown/pushdown_left_join.cpp b/src/optimizer/pushdown/pushdown_left_join.cpp index 331b0e80e033..c33081c0cef4 100644 --- a/src/optimizer/pushdown/pushdown_left_join.cpp +++ b/src/optimizer/pushdown/pushdown_left_join.cpp @@ -125,6 +125,8 @@ unique_ptr FilterPushdown::PushdownLeftJoin(unique_ptrchildren[0] = left_pushdown.Rewrite(move(op->children[0])); op->children[1] = right_pushdown.Rewrite(move(op->children[1])); + // Need to add the operators here because they are optimized with different + // filter_pushdown objects (i.e. left_pushdown and right_pushdown) return FinishPushdown(move(op)); } From eeb3556a0dd1042292628f0c86a80b0f9570d3d6 Mon Sep 17 00:00:00 2001 From: Tom Ebergen Date: Wed, 7 Dec 2022 12:54:48 +0100 Subject: [PATCH 10/10] make format fix and reorganization --- .../duckdb/optimizer/join_order/join_order_optimizer.hpp | 1 - src/include/duckdb/optimizer/operator_pool.hpp | 2 +- src/include/duckdb/optimizer/optimizer.hpp | 4 ---- src/optimizer/operator_pool.cpp | 2 +- src/optimizer/optimizer.cpp | 8 +++----- 5 files changed, 5 insertions(+), 12 deletions(-) diff --git a/src/include/duckdb/optimizer/join_order/join_order_optimizer.hpp b/src/include/duckdb/optimizer/join_order/join_order_optimizer.hpp index 52989fd1bd6e..2c0f44c5c0e3 100644 --- a/src/include/duckdb/optimizer/join_order/join_order_optimizer.hpp +++ b/src/include/duckdb/optimizer/join_order/join_order_optimizer.hpp @@ -18,7 +18,6 @@ #include "duckdb/planner/logical_operator.hpp" #include "duckdb/planner/logical_operator_visitor.hpp" - #include namespace duckdb { diff --git a/src/include/duckdb/optimizer/operator_pool.hpp b/src/include/duckdb/optimizer/operator_pool.hpp index b72d0399d6ee..b98b29bf0b62 100644 --- a/src/include/duckdb/optimizer/operator_pool.hpp +++ b/src/include/duckdb/optimizer/operator_pool.hpp @@ -26,4 +26,4 @@ class OperatorPool { private: unordered_set seen_operators; }; -} \ No newline at end of file +} // namespace duckdb diff --git a/src/include/duckdb/optimizer/optimizer.hpp b/src/include/duckdb/optimizer/optimizer.hpp index 7b6af447231b..f7fdff949d2a 100644 --- a/src/include/duckdb/optimizer/optimizer.hpp +++ b/src/include/duckdb/optimizer/optimizer.hpp @@ -25,16 +25,12 @@ class Optimizer { unique_ptr Optimize(unique_ptr plan); - - ClientContext &context; Binder &binder; ExpressionRewriter rewriter; OperatorPool seen_operators; private: - - void RunOptimizer(OptimizerType type, const std::function &callback); void Verify(LogicalOperator &op); diff --git a/src/optimizer/operator_pool.cpp b/src/optimizer/operator_pool.cpp index c949ff6d323c..7088cf3343db 100644 --- a/src/optimizer/operator_pool.cpp +++ b/src/optimizer/operator_pool.cpp @@ -20,4 +20,4 @@ void OperatorPool::EmptyOperatorPool() { seen_operators.clear(); } -} \ No newline at end of file +} // namespace duckdb diff --git a/src/optimizer/optimizer.cpp b/src/optimizer/optimizer.cpp index fda23b304e06..3dda7b3e4c6a 100644 --- a/src/optimizer/optimizer.cpp +++ b/src/optimizer/optimizer.cpp @@ -26,8 +26,8 @@ namespace duckdb { -Optimizer::Optimizer(Binder &binder, ClientContext &context) : context(context), binder(binder), rewriter(context), -seen_operators() { +Optimizer::Optimizer(Binder &binder, ClientContext &context) + : context(context), binder(binder), rewriter(context), seen_operators() { rewriter.rules.push_back(make_unique(rewriter)); rewriter.rules.push_back(make_unique(rewriter)); rewriter.rules.push_back(make_unique(rewriter)); @@ -76,9 +76,7 @@ unique_ptr Optimizer::Optimize(unique_ptr plan this->plan = move(plan_p); // first we perform expression rewrites using the ExpressionRewriter // this does not change the logical plan structure, but only simplifies the expression trees - RunOptimizer(OptimizerType::EXPRESSION_REWRITER, [&]() { - rewriter.VisitOperator(*plan); - }); + RunOptimizer(OptimizerType::EXPRESSION_REWRITER, [&]() { rewriter.VisitOperator(*plan); }); // perform filter pullup RunOptimizer(OptimizerType::FILTER_PULLUP, [&]() {