From eb1acb4d19f89c90cdcd0d92b51916710d342cc8 Mon Sep 17 00:00:00 2001 From: Tom Ebergen Date: Fri, 27 Oct 2023 15:56:33 +0200 Subject: [PATCH 01/18] filter combiner has the OR filter logic I need --- .../duckdb/optimizer/filter_combiner.hpp | 60 +++++++++---------- 1 file changed, 30 insertions(+), 30 deletions(-) diff --git a/src/include/duckdb/optimizer/filter_combiner.hpp b/src/include/duckdb/optimizer/filter_combiner.hpp index f759165731b3..2564329bd2f8 100644 --- a/src/include/duckdb/optimizer/filter_combiner.hpp +++ b/src/include/duckdb/optimizer/filter_combiner.hpp @@ -62,36 +62,36 @@ class FilterCombiner { idx_t GetEquivalenceSet(Expression &expr); FilterResult AddConstantComparison(vector &info_list, ExpressionValueInformation info); // - // //! Functions used to push and generate OR Filters - // void LookUpConjunctions(Expression *expr); - // bool BFSLookUpConjunctions(BoundConjunctionExpression *conjunction); - // void VerifyOrsToPush(Expression &expr); - // - // bool UpdateConjunctionFilter(BoundComparisonExpression *comparison_expr); - // bool UpdateFilterByColumn(BoundColumnRefExpression *column_ref, BoundComparisonExpression *comparison_expr); - // void GenerateORFilters(TableFilterSet &table_filter, vector &column_ids); - // - // template - // void GenerateConjunctionFilter(BoundConjunctionExpression *conjunction, ConjunctionFilter *last_conj_filter) { - // auto new_filter = NextConjunctionFilter(conjunction); - // auto conj_filter_ptr = (ConjunctionFilter *)new_filter.get(); - // last_conj_filter->child_filters.push_back(std::move(new_filter)); - // last_conj_filter = conj_filter_ptr; - // } - // - // template - // unique_ptr NextConjunctionFilter(BoundConjunctionExpression *conjunction) { - // unique_ptr conj_filter = make_uniq(); - // for (auto &expr : conjunction->children) { - // auto comp_expr = (BoundComparisonExpression *)expr.get(); - // auto &const_expr = - // (comp_expr->left->type == ExpressionType::VALUE_CONSTANT) ? *comp_expr->left : *comp_expr->right; - // auto const_value = ExpressionExecutor::EvaluateScalar(const_expr); - // auto const_filter = make_uniq(comp_expr->type, const_value); - // conj_filter->child_filters.push_back(std::move(const_filter)); - // } - // return std::move(conj_filter); - // } + //! Functions used to push and generate OR Filters + void LookUpConjunctions(Expression *expr); + bool BFSLookUpConjunctions(BoundConjunctionExpression *conjunction); + void VerifyOrsToPush(Expression &expr); + + bool UpdateConjunctionFilter(BoundComparisonExpression *comparison_expr); + bool UpdateFilterByColumn(BoundColumnRefExpression *column_ref, BoundComparisonExpression *comparison_expr); + void GenerateORFilters(TableFilterSet &table_filter, vector &column_ids); + + template + void GenerateConjunctionFilter(BoundConjunctionExpression *conjunction, ConjunctionFilter *last_conj_filter) { + auto new_filter = NextConjunctionFilter(conjunction); + auto conj_filter_ptr = (ConjunctionFilter *)new_filter.get(); + last_conj_filter->child_filters.push_back(std::move(new_filter)); + last_conj_filter = conj_filter_ptr; + } + + template + unique_ptr NextConjunctionFilter(BoundConjunctionExpression *conjunction) { + unique_ptr conj_filter = make_uniq(); + for (auto &expr : conjunction->children) { + auto comp_expr = (BoundComparisonExpression *)expr.get(); + auto &const_expr = + (comp_expr->left->type == ExpressionType::VALUE_CONSTANT) ? *comp_expr->left : *comp_expr->right; + auto const_value = ExpressionExecutor::EvaluateScalar(const_expr); + auto const_filter = make_uniq(comp_expr->type, const_value); + conj_filter->child_filters.push_back(std::move(const_filter)); + } + return std::move(conj_filter); + } private: vector> remaining_filters; From ed3e38156e9b482fa39c30b7a614fb49457971ce Mon Sep 17 00:00:00 2001 From: Tom Ebergen Date: Fri, 27 Oct 2023 16:08:13 +0200 Subject: [PATCH 02/18] Revert "filter combiner has the OR filter logic I need" This reverts commit eb1acb4d19f89c90cdcd0d92b51916710d342cc8. --- .../duckdb/optimizer/filter_combiner.hpp | 60 +++++++++---------- 1 file changed, 30 insertions(+), 30 deletions(-) diff --git a/src/include/duckdb/optimizer/filter_combiner.hpp b/src/include/duckdb/optimizer/filter_combiner.hpp index 2564329bd2f8..f759165731b3 100644 --- a/src/include/duckdb/optimizer/filter_combiner.hpp +++ b/src/include/duckdb/optimizer/filter_combiner.hpp @@ -62,36 +62,36 @@ class FilterCombiner { idx_t GetEquivalenceSet(Expression &expr); FilterResult AddConstantComparison(vector &info_list, ExpressionValueInformation info); // - //! Functions used to push and generate OR Filters - void LookUpConjunctions(Expression *expr); - bool BFSLookUpConjunctions(BoundConjunctionExpression *conjunction); - void VerifyOrsToPush(Expression &expr); - - bool UpdateConjunctionFilter(BoundComparisonExpression *comparison_expr); - bool UpdateFilterByColumn(BoundColumnRefExpression *column_ref, BoundComparisonExpression *comparison_expr); - void GenerateORFilters(TableFilterSet &table_filter, vector &column_ids); - - template - void GenerateConjunctionFilter(BoundConjunctionExpression *conjunction, ConjunctionFilter *last_conj_filter) { - auto new_filter = NextConjunctionFilter(conjunction); - auto conj_filter_ptr = (ConjunctionFilter *)new_filter.get(); - last_conj_filter->child_filters.push_back(std::move(new_filter)); - last_conj_filter = conj_filter_ptr; - } - - template - unique_ptr NextConjunctionFilter(BoundConjunctionExpression *conjunction) { - unique_ptr conj_filter = make_uniq(); - for (auto &expr : conjunction->children) { - auto comp_expr = (BoundComparisonExpression *)expr.get(); - auto &const_expr = - (comp_expr->left->type == ExpressionType::VALUE_CONSTANT) ? *comp_expr->left : *comp_expr->right; - auto const_value = ExpressionExecutor::EvaluateScalar(const_expr); - auto const_filter = make_uniq(comp_expr->type, const_value); - conj_filter->child_filters.push_back(std::move(const_filter)); - } - return std::move(conj_filter); - } + // //! Functions used to push and generate OR Filters + // void LookUpConjunctions(Expression *expr); + // bool BFSLookUpConjunctions(BoundConjunctionExpression *conjunction); + // void VerifyOrsToPush(Expression &expr); + // + // bool UpdateConjunctionFilter(BoundComparisonExpression *comparison_expr); + // bool UpdateFilterByColumn(BoundColumnRefExpression *column_ref, BoundComparisonExpression *comparison_expr); + // void GenerateORFilters(TableFilterSet &table_filter, vector &column_ids); + // + // template + // void GenerateConjunctionFilter(BoundConjunctionExpression *conjunction, ConjunctionFilter *last_conj_filter) { + // auto new_filter = NextConjunctionFilter(conjunction); + // auto conj_filter_ptr = (ConjunctionFilter *)new_filter.get(); + // last_conj_filter->child_filters.push_back(std::move(new_filter)); + // last_conj_filter = conj_filter_ptr; + // } + // + // template + // unique_ptr NextConjunctionFilter(BoundConjunctionExpression *conjunction) { + // unique_ptr conj_filter = make_uniq(); + // for (auto &expr : conjunction->children) { + // auto comp_expr = (BoundComparisonExpression *)expr.get(); + // auto &const_expr = + // (comp_expr->left->type == ExpressionType::VALUE_CONSTANT) ? *comp_expr->left : *comp_expr->right; + // auto const_value = ExpressionExecutor::EvaluateScalar(const_expr); + // auto const_filter = make_uniq(comp_expr->type, const_value); + // conj_filter->child_filters.push_back(std::move(const_filter)); + // } + // return std::move(conj_filter); + // } private: vector> remaining_filters; From ffe7d2b1284d7f530ff492ed0757f3cd03ecc6ea Mon Sep 17 00:00:00 2001 From: Tom Ebergen Date: Mon, 30 Oct 2023 09:47:03 +0100 Subject: [PATCH 03/18] re-enable pushdown or filters test --- src/planner/table_filter.cpp | 1 + test/optimizer/pushdown/parquet_or_pushdown.test | 7 ++++--- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/planner/table_filter.cpp b/src/planner/table_filter.cpp index 0f6f2b0bfc86..6f977dddc031 100644 --- a/src/planner/table_filter.cpp +++ b/src/planner/table_filter.cpp @@ -24,4 +24,5 @@ void TableFilterSet::PushFilter(idx_t column_index, unique_ptr filt } } + } // namespace duckdb diff --git a/test/optimizer/pushdown/parquet_or_pushdown.test b/test/optimizer/pushdown/parquet_or_pushdown.test index bc4b5418c87b..81d0eb9de341 100644 --- a/test/optimizer/pushdown/parquet_or_pushdown.test +++ b/test/optimizer/pushdown/parquet_or_pushdown.test @@ -7,15 +7,16 @@ require parquet statement ok PRAGMA enable_verification -# FIXME: re-enable when or pushdown is fixed -mode skip +statement ok +copy (select range as a, (range % 3 == 0)::BOOL as b from range(100)) to '__TEST_DIR__/parquet_or_filter.parquet' (FORMAT PARQUET); # Multiple column in the root OR node, don't push down query II -EXPLAIN SELECT tbl.a, tbl.b FROM "data/parquet-testing/arrow/alltypes_plain.parquet" tbl(a, b) WHERE a=1 OR b=false +EXPLAIN SELECT tbl.a, tbl.b FROM 'delete_me.parquet' tbl(a, b) WHERE a in (1, 3, 5); ---- physical_plan :.*PARQUET_SCAN.*Filters:.* +mode skip # Single column in the root OR node query II From e75df5ee8170798a619f032e7f2db116d0f2bc22 Mon Sep 17 00:00:00 2001 From: Tom Ebergen Date: Mon, 30 Oct 2023 15:09:35 +0100 Subject: [PATCH 04/18] make format fix --- tools/jdbc/src/test/java/org/duckdb/test/TestDuckDBJDBC.java | 1 - 1 file changed, 1 deletion(-) diff --git a/tools/jdbc/src/test/java/org/duckdb/test/TestDuckDBJDBC.java b/tools/jdbc/src/test/java/org/duckdb/test/TestDuckDBJDBC.java index eed8fd93aa5c..d03652e93542 100644 --- a/tools/jdbc/src/test/java/org/duckdb/test/TestDuckDBJDBC.java +++ b/tools/jdbc/src/test/java/org/duckdb/test/TestDuckDBJDBC.java @@ -3766,7 +3766,6 @@ public static void test_offset_limit() throws Exception { } } - public static void test_UUID_binding() throws Exception { try (Connection conn = DriverManager.getConnection("jdbc:duckdb:"); PreparedStatement statement = conn.prepareStatement("select '0b17ce61-375c-4ad8-97b3-349d96d35ab1'::UUID"); From 98099d856cfd49c018a50602debb1951403ecc51 Mon Sep 17 00:00:00 2001 From: Tom Ebergen Date: Mon, 30 Oct 2023 15:36:06 +0100 Subject: [PATCH 05/18] have an or filter pushed down, but need the logic to match the exact filters I want to push down. --- .../duckdb/optimizer/filter_combiner.hpp | 2 +- src/include/duckdb/planner/table_filter.hpp | 1 + src/optimizer/filter_combiner.cpp | 19 ++++++++++++++++++- src/planner/table_filter.cpp | 3 +-- .../pushdown/parquet_or_pushdown.test | 10 ++++++---- 5 files changed, 27 insertions(+), 8 deletions(-) diff --git a/src/include/duckdb/optimizer/filter_combiner.hpp b/src/include/duckdb/optimizer/filter_combiner.hpp index f759165731b3..2366ec83b3f6 100644 --- a/src/include/duckdb/optimizer/filter_combiner.hpp +++ b/src/include/duckdb/optimizer/filter_combiner.hpp @@ -69,7 +69,7 @@ class FilterCombiner { // // bool UpdateConjunctionFilter(BoundComparisonExpression *comparison_expr); // bool UpdateFilterByColumn(BoundColumnRefExpression *column_ref, BoundComparisonExpression *comparison_expr); - // void GenerateORFilters(TableFilterSet &table_filter, vector &column_ids); + void GenerateORFilters(TableFilterSet &table_filter, vector &column_ids); // // template // void GenerateConjunctionFilter(BoundConjunctionExpression *conjunction, ConjunctionFilter *last_conj_filter) { diff --git a/src/include/duckdb/planner/table_filter.hpp b/src/include/duckdb/planner/table_filter.hpp index 368b13eda6f1..8b513fe7e94d 100644 --- a/src/include/duckdb/planner/table_filter.hpp +++ b/src/include/duckdb/planner/table_filter.hpp @@ -63,6 +63,7 @@ class TableFilter { } }; +//! class TableFilterSet { public: unordered_map> filters; diff --git a/src/optimizer/filter_combiner.cpp b/src/optimizer/filter_combiner.cpp index 9461c20006fc..d19dc9f17d40 100644 --- a/src/optimizer/filter_combiner.cpp +++ b/src/optimizer/filter_combiner.cpp @@ -558,6 +558,23 @@ TableFilterSet FilterCombiner::GenerateTableScanFilters(vector &column_id remaining_filters.erase(remaining_filters.begin() + rem_fil_idx); } + else if (remaining_filter->type == ExpressionType::CONJUNCTION_OR) { + auto a = 0; + auto &conj = remaining_filter->Cast(); + for (auto &expr : conj.children) { + if (expr->expression_class != ExpressionClass::BOUND_COMPARISON) { + continue; + } + auto &comp = expr->Cast(); + if (comp.left->type == ExpressionType::BOUND_COLUMN_REF && comp.right->type == ExpressionType::VALUE_CONSTANT ) { + auto col_ref = &comp.left->Cast(); + auto column_index = col_ref->binding.column_index; + auto const_val = &comp.right->Cast(); + auto equality_filter = make_uniq(ExpressionType::COMPARE_EQUAL, const_val->value); + table_filters.PushFilter(column_index, std::move(equality_filter)); + } + } + } } // GenerateORFilters(table_filters, column_ids); @@ -1203,7 +1220,7 @@ ValueComparisonResult CompareValueInformation(ExpressionValueInformation &left, // return true; //} // -// void FilterCombiner::GenerateORFilters(TableFilterSet &table_filter, vector &column_ids) { +//void FilterCombiner::GenerateORFilters(TableFilterSet &table_filter, vector &column_ids) { // for (const auto colref : vec_colref_insertion_order) { // auto column_index = column_ids[colref->binding.column_index]; // if (column_index == COLUMN_IDENTIFIER_ROW_ID) { diff --git a/src/planner/table_filter.cpp b/src/planner/table_filter.cpp index 6f977dddc031..fca602b5dfd6 100644 --- a/src/planner/table_filter.cpp +++ b/src/planner/table_filter.cpp @@ -16,7 +16,7 @@ void TableFilterSet::PushFilter(idx_t column_index, unique_ptr filt auto &and_filter = entry->second->Cast(); and_filter.child_filters.push_back(std::move(filter)); } else { - auto and_filter = make_uniq(); + auto and_filter = make_uniq(); and_filter->child_filters.push_back(std::move(entry->second)); and_filter->child_filters.push_back(std::move(filter)); filters[column_index] = std::move(and_filter); @@ -24,5 +24,4 @@ void TableFilterSet::PushFilter(idx_t column_index, unique_ptr filt } } - } // namespace duckdb diff --git a/test/optimizer/pushdown/parquet_or_pushdown.test b/test/optimizer/pushdown/parquet_or_pushdown.test index 81d0eb9de341..146ed4a7d5b6 100644 --- a/test/optimizer/pushdown/parquet_or_pushdown.test +++ b/test/optimizer/pushdown/parquet_or_pushdown.test @@ -7,14 +7,16 @@ require parquet statement ok PRAGMA enable_verification -statement ok -copy (select range as a, (range % 3 == 0)::BOOL as b from range(100)) to '__TEST_DIR__/parquet_or_filter.parquet' (FORMAT PARQUET); +#statement ok +#copy (select range as a, (range % 3 == 0)::BOOL as b from range(100)) to '__TEST_DIR__/parquet_or_filter.parquet' (FORMAT PARQUET); # Multiple column in the root OR node, don't push down query II -EXPLAIN SELECT tbl.a, tbl.b FROM 'delete_me.parquet' tbl(a, b) WHERE a in (1, 3, 5); +SELECT tbl.a, tbl.b FROM 'delete_me.parquet' tbl(a, b) WHERE a = 2 or a = 5 or a = 9 order by a; ---- -physical_plan :.*PARQUET_SCAN.*Filters:.* +2 false +5 false +9 true mode skip From 11388d1b20013172e8825fa89779f63942276391 Mon Sep 17 00:00:00 2001 From: Tom Ebergen Date: Mon, 30 Oct 2023 17:03:47 +0100 Subject: [PATCH 06/18] progress. I know how to continue --- src/include/duckdb/planner/table_filter.hpp | 3 ++- src/optimizer/filter_combiner.cpp | 9 +++++++-- src/planner/table_filter.cpp | 15 +++++++++++++-- test/optimizer/pushdown/parquet_or_pushdown.test | 14 +++++++++++--- 4 files changed, 33 insertions(+), 8 deletions(-) diff --git a/src/include/duckdb/planner/table_filter.hpp b/src/include/duckdb/planner/table_filter.hpp index 8b513fe7e94d..037fd99ec26d 100644 --- a/src/include/duckdb/planner/table_filter.hpp +++ b/src/include/duckdb/planner/table_filter.hpp @@ -69,7 +69,8 @@ class TableFilterSet { unordered_map> filters; public: - void PushFilter(idx_t table_index, unique_ptr filter); + void PushFilter(idx_t table_index, unique_ptr filter, + TableFilterType conjunction_type = TableFilterType::CONJUNCTION_AND); bool Equals(TableFilterSet &other) { if (filters.size() != other.filters.size()) { diff --git a/src/optimizer/filter_combiner.cpp b/src/optimizer/filter_combiner.cpp index d19dc9f17d40..f57f317cc9fb 100644 --- a/src/optimizer/filter_combiner.cpp +++ b/src/optimizer/filter_combiner.cpp @@ -561,6 +561,10 @@ TableFilterSet FilterCombiner::GenerateTableScanFilters(vector &column_id else if (remaining_filter->type == ExpressionType::CONJUNCTION_OR) { auto a = 0; auto &conj = remaining_filter->Cast(); + // TODO: keep track of what children in the OR filter are pushed down, + // and what children are not. The pushed down children can be removed from + // the remaining OR filter children. If all filters can be pushed down, then remove the + // remaining child all together. for (auto &expr : conj.children) { if (expr->expression_class != ExpressionClass::BOUND_COMPARISON) { continue; @@ -570,10 +574,11 @@ TableFilterSet FilterCombiner::GenerateTableScanFilters(vector &column_id auto col_ref = &comp.left->Cast(); auto column_index = col_ref->binding.column_index; auto const_val = &comp.right->Cast(); - auto equality_filter = make_uniq(ExpressionType::COMPARE_EQUAL, const_val->value); - table_filters.PushFilter(column_index, std::move(equality_filter)); + auto equality_filter = make_uniq(comp.type, const_val->value); + table_filters.PushFilter(column_index, std::move(equality_filter), TableFilterType::CONJUNCTION_OR); } } + remaining_filters.erase(remaining_filters.begin() + rem_fil_idx); } } diff --git a/src/planner/table_filter.cpp b/src/planner/table_filter.cpp index fca602b5dfd6..70e2e83e5fcf 100644 --- a/src/planner/table_filter.cpp +++ b/src/planner/table_filter.cpp @@ -5,7 +5,7 @@ namespace duckdb { -void TableFilterSet::PushFilter(idx_t column_index, unique_ptr filter) { +void TableFilterSet::PushFilter(idx_t column_index, unique_ptr filter, TableFilterType conjunction_type) { auto entry = filters.find(column_index); if (entry == filters.end()) { // no filter yet: push the filter directly @@ -15,8 +15,19 @@ void TableFilterSet::PushFilter(idx_t column_index, unique_ptr filt if (entry->second->filter_type == TableFilterType::CONJUNCTION_AND) { auto &and_filter = entry->second->Cast(); and_filter.child_filters.push_back(std::move(filter)); + } else if (entry->second->filter_type == TableFilterType::CONJUNCTION_OR) { + auto &or_filter = entry->second->Cast(); + or_filter.child_filters.push_back(std::move(filter)); } else { - auto and_filter = make_uniq(); + if (conjunction_type == TableFilterType::CONJUNCTION_OR) { + auto or_filter = make_uniq(); + or_filter->child_filters.push_back(std::move(entry->second)); + or_filter->child_filters.push_back(std::move(filter)); + filters[column_index] = std::move(or_filter); + return; + } + D_ASSERT(conjunction_type == TableFilterType::CONJUNCTION_AND); + auto and_filter = make_uniq(); and_filter->child_filters.push_back(std::move(entry->second)); and_filter->child_filters.push_back(std::move(filter)); filters[column_index] = std::move(and_filter); diff --git a/test/optimizer/pushdown/parquet_or_pushdown.test b/test/optimizer/pushdown/parquet_or_pushdown.test index 146ed4a7d5b6..0e0fce870ac5 100644 --- a/test/optimizer/pushdown/parquet_or_pushdown.test +++ b/test/optimizer/pushdown/parquet_or_pushdown.test @@ -18,17 +18,18 @@ SELECT tbl.a, tbl.b FROM 'delete_me.parquet' tbl(a, b) WHERE a = 2 or a = 5 or a 5 false 9 true -mode skip # Single column in the root OR node query II -EXPLAIN SELECT tbl.a FROM "data/parquet-testing/arrow/alltypes_plain.parquet" tbl(a) WHERE a=1 OR a=2 +EXPLAIN SELECT tbl.a FROM "data/parquet-testing/arrow/alltypes_plain.parquet" tbl(a) WHERE a=1 OR a=2; ---- physical_plan :.*PARQUET_SCAN.*Filters: a=1 OR a=2.* +mode skip + # Single column + root OR node with AND query II -EXPLAIN SELECT tbl.a FROM "data/parquet-testing/arrow/alltypes_plain.parquet" tbl(a) WHERE a=1 OR (a>3 AND a<5) +EXPLAIN SELECT tbl.a FROM "data/parquet-testing/arrow/alltypes_plain.parquet" tbl(a) WHERE a=1 OR (a>3 AND a<5); ---- physical_plan :.*PARQUET_SCAN.*Filters: a=1 OR a>3 AND a<5|.* @@ -40,6 +41,13 @@ EXPLAIN SELECT tbl.a FROM "data/parquet-testing/arrow/alltypes_plain.parquet" tb physical_plan :.*PARQUET_SCAN.*Filters: a=1 OR a>3 OR a<5|.* +# No Filter on top of the parquet scan, filters can be applied in the read +query II +EXPLAIN SELECT tbl.a FROM "data/parquet-testing/arrow/alltypes_plain.parquet" tbl(a) WHERE a=1 OR a>3 OR a<5 +---- +physical_plan :.*FILTER.*PARQUET_SCAN.* + + # Testing not equal query II From caa0b3cb304da57878d61eb868e165cc9a632de1 Mon Sep 17 00:00:00 2001 From: Tom Ebergen Date: Tue, 31 Oct 2023 16:52:28 +0100 Subject: [PATCH 07/18] works but conjunction or statements need to be on the same column id. Otherwise it doesnt work --- src/include/duckdb/planner/table_filter.hpp | 14 +++- src/optimizer/filter_combiner.cpp | 79 +++++++++++++++------ src/planner/table_filter.cpp | 11 +++ test/optimizer/pushdown/pushdown_or.test | 31 ++++++++ 4 files changed, 111 insertions(+), 24 deletions(-) create mode 100644 test/optimizer/pushdown/pushdown_or.test diff --git a/src/include/duckdb/planner/table_filter.hpp b/src/include/duckdb/planner/table_filter.hpp index 037fd99ec26d..d60f98d93cf5 100644 --- a/src/include/duckdb/planner/table_filter.hpp +++ b/src/include/duckdb/planner/table_filter.hpp @@ -12,6 +12,7 @@ #include "duckdb/common/types.hpp" #include "duckdb/common/unordered_map.hpp" #include "duckdb/common/enums/filter_propagate_result.hpp" +#include "duckdb/common/enums/expression_type.hpp" namespace duckdb { class BaseStatistics; @@ -69,7 +70,7 @@ class TableFilterSet { unordered_map> filters; public: - void PushFilter(idx_t table_index, unique_ptr filter, + void PushFilter(idx_t column_index, unique_ptr filter, TableFilterType conjunction_type = TableFilterType::CONJUNCTION_AND); bool Equals(TableFilterSet &other) { @@ -99,6 +100,17 @@ class TableFilterSet { void Serialize(Serializer &serializer) const; static TableFilterSet Deserialize(Deserializer &deserializer); + static bool ExpressionSupportsPushdown(ExpressionType comparison); +}; + +struct PotentialTableFilter { + idx_t column_index; + unique_ptr filter; + TableFilterType conjunction_type; + + PotentialTableFilter(idx_t column_index, unique_ptr filter, TableFilterType conjunction_type) + : column_index(column_index), filter(std::move(filter)), conjunction_type(conjunction_type) { + } }; } // namespace duckdb diff --git a/src/optimizer/filter_combiner.cpp b/src/optimizer/filter_combiner.cpp index f57f317cc9fb..5f97db7c76a6 100644 --- a/src/optimizer/filter_combiner.cpp +++ b/src/optimizer/filter_combiner.cpp @@ -391,20 +391,29 @@ bool FilterCombiner::HasFilters() { // return zonemap_checks; // } +bool TableFilterSet::ExpressionSupportsPushdown(ExpressionType comparison) { + return (comparison == ExpressionType::COMPARE_EQUAL || comparison == ExpressionType::COMPARE_GREATERTHAN || + comparison == ExpressionType::COMPARE_GREATERTHANOREQUALTO || + comparison == ExpressionType::COMPARE_LESSTHAN || comparison == ExpressionType::COMPARE_LESSTHANOREQUALTO); +} + +static bool ValueTypeSupportsPushown(Value value) { + return TypeIsNumeric(value.type().InternalType()) || value.type().InternalType() == PhysicalType::VARCHAR || + value.type().InternalType() == PhysicalType::BOOL; +} + +static bool LeftConstValRightBoundColref(Expression &left, Expression &right) { + return (left.type == ExpressionType::VALUE_CONSTANT && right.type == ExpressionType::BOUND_COLUMN_REF); +} + TableFilterSet FilterCombiner::GenerateTableScanFilters(vector &column_ids) { TableFilterSet table_filters; //! First, we figure the filters that have constant expressions that we can push down to the table scan for (auto &constant_value : constant_values) { if (!constant_value.second.empty()) { auto filter_exp = equivalence_map.end(); - if ((constant_value.second[0].comparison_type == ExpressionType::COMPARE_EQUAL || - constant_value.second[0].comparison_type == ExpressionType::COMPARE_GREATERTHAN || - constant_value.second[0].comparison_type == ExpressionType::COMPARE_GREATERTHANOREQUALTO || - constant_value.second[0].comparison_type == ExpressionType::COMPARE_LESSTHAN || - constant_value.second[0].comparison_type == ExpressionType::COMPARE_LESSTHANOREQUALTO) && - (TypeIsNumeric(constant_value.second[0].constant.type().InternalType()) || - constant_value.second[0].constant.type().InternalType() == PhysicalType::VARCHAR || - constant_value.second[0].constant.type().InternalType() == PhysicalType::BOOL)) { + if (TableFilterSet::ExpressionSupportsPushdown(constant_value.second[0].comparison_type) && + ValueTypeSupportsPushown(constant_value.second[0].constant)) { //! Here we check if these filters are column references filter_exp = equivalence_map.find(constant_value.first); if (filter_exp->second.size() == 1 && @@ -557,28 +566,52 @@ TableFilterSet FilterCombiner::GenerateTableScanFilters(vector &column_id table_filters.PushFilter(column_index, make_uniq()); remaining_filters.erase(remaining_filters.begin() + rem_fil_idx); - } - else if (remaining_filter->type == ExpressionType::CONJUNCTION_OR) { - auto a = 0; + } else if (remaining_filter->type == ExpressionType::CONJUNCTION_OR) { + unordered_set columns_that_need_and_null; auto &conj = remaining_filter->Cast(); - // TODO: keep track of what children in the OR filter are pushed down, - // and what children are not. The pushed down children can be removed from - // the remaining OR filter children. If all filters can be pushed down, then remove the - // remaining child all together. + vector potential_table_filters; + bool all_can_pushdown = true; for (auto &expr : conj.children) { if (expr->expression_class != ExpressionClass::BOUND_COMPARISON) { continue; } auto &comp = expr->Cast(); - if (comp.left->type == ExpressionType::BOUND_COLUMN_REF && comp.right->type == ExpressionType::VALUE_CONSTANT ) { - auto col_ref = &comp.left->Cast(); - auto column_index = col_ref->binding.column_index; - auto const_val = &comp.right->Cast(); - auto equality_filter = make_uniq(comp.type, const_val->value); - table_filters.PushFilter(column_index, std::move(equality_filter), TableFilterType::CONJUNCTION_OR); + BoundColumnRefExpression *colref = nullptr; + BoundConstantExpression *value = nullptr; + bool can_pushdown = false; + if (TableFilterSet::ExpressionSupportsPushdown(comp.type)) { + if (LeftConstValRightBoundColref(*comp.left, *comp.right)) { + value = &comp.left->Cast(); + colref = &comp.right->Cast(); + can_pushdown = true; + } else if (LeftConstValRightBoundColref(*comp.right, *comp.left)) { + value = &comp.right->Cast(); + colref = &comp.left->Cast(); + can_pushdown = true; + } + } + if (!can_pushdown) { + all_can_pushdown = false; + break; } + auto column_index = colref->binding.column_index; + auto equality_filter = make_uniq(comp.type, value->value); + potential_table_filters.push_back( + PotentialTableFilter(column_index, std::move(equality_filter), TableFilterType::CONJUNCTION_OR)); + columns_that_need_and_null.insert(column_index); + } + if (all_can_pushdown) { + for (auto &potential_table_filter : potential_table_filters) { + table_filters.PushFilter(potential_table_filter.column_index, + std::move(potential_table_filter.filter), + potential_table_filter.conjunction_type); + } + for (auto &column_index : columns_that_need_and_null) { + table_filters.PushFilter(column_index, make_uniq(), + TableFilterType::CONJUNCTION_AND); + } + remaining_filters.erase(remaining_filters.begin() + rem_fil_idx); } - remaining_filters.erase(remaining_filters.begin() + rem_fil_idx); } } @@ -1225,7 +1258,7 @@ ValueComparisonResult CompareValueInformation(ExpressionValueInformation &left, // return true; //} // -//void FilterCombiner::GenerateORFilters(TableFilterSet &table_filter, vector &column_ids) { +// void FilterCombiner::GenerateORFilters(TableFilterSet &table_filter, vector &column_ids) { // for (const auto colref : vec_colref_insertion_order) { // auto column_index = column_ids[colref->binding.column_index]; // if (column_index == COLUMN_IDENTIFIER_ROW_ID) { diff --git a/src/planner/table_filter.cpp b/src/planner/table_filter.cpp index 70e2e83e5fcf..e267b3e4dd00 100644 --- a/src/planner/table_filter.cpp +++ b/src/planner/table_filter.cpp @@ -13,9 +13,20 @@ void TableFilterSet::PushFilter(idx_t column_index, unique_ptr filt } else { // there is already a filter: AND it together if (entry->second->filter_type == TableFilterType::CONJUNCTION_AND) { + if (conjunction_type == TableFilterType::CONJUNCTION_OR) { + throw InternalException("Conjunction AND should not be connecting conjunction ors."); + } auto &and_filter = entry->second->Cast(); and_filter.child_filters.push_back(std::move(filter)); } else if (entry->second->filter_type == TableFilterType::CONJUNCTION_OR) { + // Or filter connecting ands + if (conjunction_type == TableFilterType::CONJUNCTION_AND) { + auto and_filter = make_uniq(); + and_filter->child_filters.push_back(std::move(entry->second)); + and_filter->child_filters.push_back(std::move(filter)); + filters[column_index] = std::move(and_filter); + return; + } auto &or_filter = entry->second->Cast(); or_filter.child_filters.push_back(std::move(filter)); } else { diff --git a/test/optimizer/pushdown/pushdown_or.test b/test/optimizer/pushdown/pushdown_or.test new file mode 100644 index 000000000000..29f4b2acaf5c --- /dev/null +++ b/test/optimizer/pushdown/pushdown_or.test @@ -0,0 +1,31 @@ +# name: test/optimizer/pushdown/pushdown_or.test +# description: Test Parquet With Pushing Down of OR Filters +# group: [pushdown] + +require parquet + +#statement ok +#PRAGMA enable_verification + +statement ok +create table t0 as select range as a, range/2 as b, range/3 as c, range/4 as d from range(100); + +# or connecting ands not pushed down +#query II +#explain SELECT * FROM t0 WHERE ((a % 2 = 0) and (b % 2 = 0)) or ((a > 10) and (d < 30)); +#---- +#physical_plan :.*FILTER.*SEQ_SCAN.* + +# many ors involving constant comparisons are pushed down +query II +select * from t0 where a = 5 or b = 8 or c = 20 or d = 10; +---- +physical_plan :.*SEQ_SCAN.*Filters.* + + +## if one of the or conjunctions cannot be pushed down, the filter is applied after the scan +#query II +#explain select * from t0 where a = 5 or b = 8 or c = 20 or c = d; +#---- +#physical_plan :.*Filters.*SEQ_SCAN.* +# From 6adb74cb5dc5485122be757451355a91ef497341 Mon Sep 17 00:00:00 2001 From: Tom Ebergen Date: Tue, 31 Oct 2023 17:49:01 +0100 Subject: [PATCH 08/18] works. can only push down or conjunctions on the same column id. a = 5 or b = 6 cannot be pushed down (yet). --- src/optimizer/filter_combiner.cpp | 25 ++++++++++++++-- test/optimizer/pushdown/pushdown_or.test | 36 +++++++++++++++--------- 2 files changed, 46 insertions(+), 15 deletions(-) diff --git a/src/optimizer/filter_combiner.cpp b/src/optimizer/filter_combiner.cpp index 5f97db7c76a6..0ea29da25baf 100644 --- a/src/optimizer/filter_combiner.cpp +++ b/src/optimizer/filter_combiner.cpp @@ -14,6 +14,7 @@ #include "duckdb/planner/filter/constant_filter.hpp" #include "duckdb/planner/filter/null_filter.hpp" #include "duckdb/optimizer/optimizer.hpp" +#include "duckdb/common/types/value.hpp" namespace duckdb { @@ -442,6 +443,7 @@ TableFilterSet FilterCombiner::GenerateTableScanFilters(vector &column_id } } //! Here we look for LIKE or IN filters + vector remaining_filters_to_remove; for (idx_t rem_fil_idx = 0; rem_fil_idx < remaining_filters.size(); rem_fil_idx++) { auto &remaining_filter = remaining_filters[rem_fil_idx]; if (remaining_filter->expression_class == ExpressionClass::BOUND_FUNCTION) { @@ -555,6 +557,12 @@ TableFilterSet FilterCombiner::GenerateTableScanFilters(vector &column_id } } if (!can_simplify_in_clause) { + // make one big conjunction OR + for (auto &in_val : in_values) { + auto filter = make_uniq(ExpressionType::COMPARE_EQUAL, Value::Numeric(LogicalType::BIGINT, in_val)); + table_filters.PushFilter(column_index, std::move(filter), TableFilterType::CONJUNCTION_OR); + } + remaining_filters_to_remove.push_back(rem_fil_idx); continue; } auto lower_bound = make_uniq(ExpressionType::COMPARE_GREATERTHANOREQUALTO, @@ -565,12 +573,13 @@ TableFilterSet FilterCombiner::GenerateTableScanFilters(vector &column_id table_filters.PushFilter(column_index, std::move(upper_bound)); table_filters.PushFilter(column_index, make_uniq()); - remaining_filters.erase(remaining_filters.begin() + rem_fil_idx); + remaining_filters_to_remove.push_back(rem_fil_idx); } else if (remaining_filter->type == ExpressionType::CONJUNCTION_OR) { unordered_set columns_that_need_and_null; auto &conj = remaining_filter->Cast(); vector potential_table_filters; bool all_can_pushdown = true; + idx_t column_id = DConstants::INVALID_INDEX; for (auto &expr : conj.children) { if (expr->expression_class != ExpressionClass::BOUND_COMPARISON) { continue; @@ -595,6 +604,13 @@ TableFilterSet FilterCombiner::GenerateTableScanFilters(vector &column_id break; } auto column_index = colref->binding.column_index; + if (column_id == DConstants::INVALID_INDEX) { + column_id = column_index; + } + if (column_id != column_index) { + all_can_pushdown = false; + break; + } auto equality_filter = make_uniq(comp.type, value->value); potential_table_filters.push_back( PotentialTableFilter(column_index, std::move(equality_filter), TableFilterType::CONJUNCTION_OR)); @@ -610,11 +626,16 @@ TableFilterSet FilterCombiner::GenerateTableScanFilters(vector &column_id table_filters.PushFilter(column_index, make_uniq(), TableFilterType::CONJUNCTION_AND); } - remaining_filters.erase(remaining_filters.begin() + rem_fil_idx); + remaining_filters_to_remove.push_back(rem_fil_idx); } } } + for (int i = remaining_filters_to_remove.size() - 1; i >= 0; i--) { + auto remaining_filter_index = remaining_filters_to_remove.at(i); + remaining_filters.erase(remaining_filters.begin() + remaining_filter_index); + } + // GenerateORFilters(table_filters, column_ids); return table_filters; diff --git a/test/optimizer/pushdown/pushdown_or.test b/test/optimizer/pushdown/pushdown_or.test index 29f4b2acaf5c..58521af3d3a1 100644 --- a/test/optimizer/pushdown/pushdown_or.test +++ b/test/optimizer/pushdown/pushdown_or.test @@ -10,22 +10,32 @@ require parquet statement ok create table t0 as select range as a, range/2 as b, range/3 as c, range/4 as d from range(100); -# or connecting ands not pushed down -#query II -#explain SELECT * FROM t0 WHERE ((a % 2 = 0) and (b % 2 = 0)) or ((a > 10) and (d < 30)); -#---- -#physical_plan :.*FILTER.*SEQ_SCAN.* +# cannot push down or filters that involve separate columns +query II +explain select * from t0 where a = 5 or b = 8 or c = 20 or d = 10; +---- +physical_plan :.*FILTER.*SEQ_SCAN.* + +# can push down or filters if that are all on the same column +query II +explain select * from t0 where a = 5 or a = 10 or a = 15 or a = 12; +---- +physical_plan :.*SEQ_SCAN.*Filters.* -# many ors involving constant comparisons are pushed down +# can push down two or filters combined by an and, given the or filters are both on the same column query II -select * from t0 where a = 5 or b = 8 or c = 20 or d = 10; +explain select * from t0 where (a = 5 or a = 10) and (b = 15 or b = 12); ---- physical_plan :.*SEQ_SCAN.*Filters.* +query II +explain select * from t0 where (a = 5 or a = 10) and (b = 15 or b = 12); +---- +physical_plan :.*FILTER.*SEQ_SCAN.* + +# in filter can be pushed down +query II +explain select * from t0 where a in (1, 2, 3, 4, 10, 19, 25); +---- +physical_plan :.*SEQ_SCAN.*Filters.* -## if one of the or conjunctions cannot be pushed down, the filter is applied after the scan -#query II -#explain select * from t0 where a = 5 or b = 8 or c = 20 or c = d; -#---- -#physical_plan :.*Filters.*SEQ_SCAN.* -# From 2f5b9b889d40ac6c777ed78f2e4d138d8059ceeb Mon Sep 17 00:00:00 2001 From: Tom Ebergen Date: Tue, 31 Oct 2023 17:54:33 +0100 Subject: [PATCH 09/18] works on parquet as well --- test/optimizer/pushdown/pushdown_or.test | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/test/optimizer/pushdown/pushdown_or.test b/test/optimizer/pushdown/pushdown_or.test index 58521af3d3a1..c0a4b9347fd4 100644 --- a/test/optimizer/pushdown/pushdown_or.test +++ b/test/optimizer/pushdown/pushdown_or.test @@ -8,7 +8,11 @@ require parquet #PRAGMA enable_verification statement ok -create table t0 as select range as a, range/2 as b, range/3 as c, range/4 as d from range(100); +create table t0 as select range as a, range/2 as b, range/3 as c, range/4 as d from range(100); + +statement ok +copy table t1 to 't0-whatever.parquet' (FORMAT PARQUET); + # cannot push down or filters that involve separate columns query II @@ -18,7 +22,7 @@ physical_plan :.*FILTER.*SEQ_SCAN.* # can push down or filters if that are all on the same column query II -explain select * from t0 where a = 5 or a = 10 or a = 15 or a = 12; +explain select * from 't0-whatever.parquet' where a = 5 or a = 10 or a = 15 or a = 12; ---- physical_plan :.*SEQ_SCAN.*Filters.* From 2ba51d2b1d9c9e6cfcba756492f79f668a2bdaf1 Mon Sep 17 00:00:00 2001 From: Tom Ebergen Date: Wed, 1 Nov 2023 15:18:33 +0100 Subject: [PATCH 10/18] tests are passing --- src/include/duckdb/planner/table_filter.hpp | 10 ----- src/optimizer/filter_combiner.cpp | 41 ++++++++++++------- .../pushdown/parquet_or_pushdown.test | 2 + test/optimizer/pushdown/pushdown_or.test | 33 ++++++++++----- test/sql/conjunction/or_comparison.test | 2 +- .../test_conjunction_optimization.test | 4 +- 6 files changed, 53 insertions(+), 39 deletions(-) diff --git a/src/include/duckdb/planner/table_filter.hpp b/src/include/duckdb/planner/table_filter.hpp index d60f98d93cf5..4893e17329e7 100644 --- a/src/include/duckdb/planner/table_filter.hpp +++ b/src/include/duckdb/planner/table_filter.hpp @@ -103,14 +103,4 @@ class TableFilterSet { static bool ExpressionSupportsPushdown(ExpressionType comparison); }; -struct PotentialTableFilter { - idx_t column_index; - unique_ptr filter; - TableFilterType conjunction_type; - - PotentialTableFilter(idx_t column_index, unique_ptr filter, TableFilterType conjunction_type) - : column_index(column_index), filter(std::move(filter)), conjunction_type(conjunction_type) { - } -}; - } // namespace duckdb diff --git a/src/optimizer/filter_combiner.cpp b/src/optimizer/filter_combiner.cpp index 0ea29da25baf..2e17679e1c2e 100644 --- a/src/optimizer/filter_combiner.cpp +++ b/src/optimizer/filter_combiner.cpp @@ -532,6 +532,13 @@ TableFilterSet FilterCombiner::GenerateTableScanFilters(vector &column_id //! Check if values are consecutive, if yes transform them to >= <= (only for integers) // e.g. if we have x IN (1, 2, 3, 4, 5) we transform this into x >= 1 AND x <= 5 if (!type.IsIntegral()) { + for (idx_t i = 1; i < func.children.size(); i++) { + auto &child = func.children[i]; + auto &const_value_expr = child->Cast(); + auto filter = make_uniq(ExpressionType::COMPARE_EQUAL, const_value_expr.value); + table_filters.PushFilter(column_index, std::move(filter), TableFilterType::CONJUNCTION_OR); + } + remaining_filters_to_remove.push_back(rem_fil_idx); continue; } @@ -559,7 +566,7 @@ TableFilterSet FilterCombiner::GenerateTableScanFilters(vector &column_id if (!can_simplify_in_clause) { // make one big conjunction OR for (auto &in_val : in_values) { - auto filter = make_uniq(ExpressionType::COMPARE_EQUAL, Value::Numeric(LogicalType::BIGINT, in_val)); + auto filter = make_uniq(ExpressionType::COMPARE_EQUAL, Value::Numeric(type, in_val)); table_filters.PushFilter(column_index, std::move(filter), TableFilterType::CONJUNCTION_OR); } remaining_filters_to_remove.push_back(rem_fil_idx); @@ -577,17 +584,21 @@ TableFilterSet FilterCombiner::GenerateTableScanFilters(vector &column_id } else if (remaining_filter->type == ExpressionType::CONJUNCTION_OR) { unordered_set columns_that_need_and_null; auto &conj = remaining_filter->Cast(); - vector potential_table_filters; + auto or_filter = make_uniq(); bool all_can_pushdown = true; idx_t column_id = DConstants::INVALID_INDEX; for (auto &expr : conj.children) { + // go through every child of the conjunction and check that it can be pushed down + // if not a direct comparison skip if (expr->expression_class != ExpressionClass::BOUND_COMPARISON) { + all_can_pushdown = false; continue; } auto &comp = expr->Cast(); BoundColumnRefExpression *colref = nullptr; BoundConstantExpression *value = nullptr; bool can_pushdown = false; + // if not a simple comparison between bound column index and constant value, skip if (TableFilterSet::ExpressionSupportsPushdown(comp.type)) { if (LeftConstValRightBoundColref(*comp.left, *comp.right)) { value = &comp.left->Cast(); @@ -603,7 +614,13 @@ TableFilterSet FilterCombiner::GenerateTableScanFilters(vector &column_id all_can_pushdown = false; break; } - auto column_index = colref->binding.column_index; + auto column_index = column_ids[colref->binding.column_index]; + if (column_index == DConstants::INVALID_INDEX) { + all_can_pushdown = false; + break; + } + // if conjunction has two separate column_ids, skip + // (i.e a = 5 or b = 7) if (column_id == DConstants::INVALID_INDEX) { column_id = column_index; } @@ -611,21 +628,15 @@ TableFilterSet FilterCombiner::GenerateTableScanFilters(vector &column_id all_can_pushdown = false; break; } - auto equality_filter = make_uniq(comp.type, value->value); - potential_table_filters.push_back( - PotentialTableFilter(column_index, std::move(equality_filter), TableFilterType::CONJUNCTION_OR)); + auto column_filter = make_uniq(comp.type, value->value); + or_filter->child_filters.push_back(std::move(column_filter)); columns_that_need_and_null.insert(column_index); } + // if all of the expressions in the conjunction or can be pushed down, then add add them to the + // table filters. Otherwise leave it as a filter on the read. if (all_can_pushdown) { - for (auto &potential_table_filter : potential_table_filters) { - table_filters.PushFilter(potential_table_filter.column_index, - std::move(potential_table_filter.filter), - potential_table_filter.conjunction_type); - } - for (auto &column_index : columns_that_need_and_null) { - table_filters.PushFilter(column_index, make_uniq(), - TableFilterType::CONJUNCTION_AND); - } + table_filters.PushFilter(column_id, std::move(or_filter), TableFilterType::CONJUNCTION_AND); + table_filters.PushFilter(column_id, make_uniq(), TableFilterType::CONJUNCTION_AND); remaining_filters_to_remove.push_back(rem_fil_idx); } } diff --git a/test/optimizer/pushdown/parquet_or_pushdown.test b/test/optimizer/pushdown/parquet_or_pushdown.test index 0e0fce870ac5..5e6c9d2c8468 100644 --- a/test/optimizer/pushdown/parquet_or_pushdown.test +++ b/test/optimizer/pushdown/parquet_or_pushdown.test @@ -7,6 +7,8 @@ require parquet statement ok PRAGMA enable_verification +mode skip + #statement ok #copy (select range as a, (range % 3 == 0)::BOOL as b from range(100)) to '__TEST_DIR__/parquet_or_filter.parquet' (FORMAT PARQUET); diff --git a/test/optimizer/pushdown/pushdown_or.test b/test/optimizer/pushdown/pushdown_or.test index c0a4b9347fd4..72fe02c2076d 100644 --- a/test/optimizer/pushdown/pushdown_or.test +++ b/test/optimizer/pushdown/pushdown_or.test @@ -4,15 +4,11 @@ require parquet -#statement ok -#PRAGMA enable_verification - statement ok -create table t0 as select range as a, range/2 as b, range/3 as c, range/4 as d from range(100); +PRAGMA enable_verification statement ok -copy table t1 to 't0-whatever.parquet' (FORMAT PARQUET); - +create table t0 as select range as a, range/2 as b, range/3 as c, range/4 as d from range(100); # cannot push down or filters that involve separate columns query II @@ -20,26 +16,41 @@ explain select * from t0 where a = 5 or b = 8 or c = 20 or d = 10; ---- physical_plan :.*FILTER.*SEQ_SCAN.* -# can push down or filters if that are all on the same column +# can push down or filters if it is a conjunction or on the same column query II -explain select * from 't0-whatever.parquet' where a = 5 or a = 10 or a = 15 or a = 12; +explain select * from t0 where a = 5 or a = 10 or a = 15 or a = 12; ---- -physical_plan :.*SEQ_SCAN.*Filters.* +physical_plan :.*SEQ_SCAN.*Filters.*OR.* # can push down two or filters combined by an and, given the or filters are both on the same column query II explain select * from t0 where (a = 5 or a = 10) and (b = 15 or b = 12); ---- -physical_plan :.*SEQ_SCAN.*Filters.* +physical_plan :.*SEQ_SCAN.*Filters.*OR.* query II explain select * from t0 where (a = 5 or a = 10) and (b = 15 or b = 12); ---- physical_plan :.*FILTER.*SEQ_SCAN.* +# two or conjunction filters on separate column ids are not pushed down +query II +explain select * from t0 where (a = 5 or b = 10) and (c = 15 or d = 12); +---- +physical_plan :.*FILTER.*SEQ_SCAN.* + # in filter can be pushed down query II explain select * from t0 where a in (1, 2, 3, 4, 10, 19, 25); ---- -physical_plan :.*SEQ_SCAN.*Filters.* +physical_plan :.*SEQ_SCAN.*Filters.*OR.* + +# test strings + +statement ok +create table t1 as select unnest(['happy', 'sad', 'angry', 'ok', 'medium']) as mood; +query II +explain select * from t1 where mood in ('happy', 'ok'); +---- +physical_plan :.*SEQ_SCAN.*Filters.*OR.* diff --git a/test/sql/conjunction/or_comparison.test b/test/sql/conjunction/or_comparison.test index aaa4b994c0e6..3e9a42930133 100644 --- a/test/sql/conjunction/or_comparison.test +++ b/test/sql/conjunction/or_comparison.test @@ -6,7 +6,7 @@ statement ok PRAGMA enable_verification; statement ok -CREATE TABLE tab0(pk INTEGER PRIMARY KEY, col0 INTEGER, col1 FLOAT, col2 VARCHAR, col3 INTEGER, col4 FLOAT, col5 VARCHAR);; +CREATE TABLE tab0(pk INTEGER PRIMARY KEY, col0 INTEGER, col1 FLOAT, col2 VARCHAR, col3 INTEGER, col4 FLOAT, col5 VARCHAR); statement ok INSERT INTO tab0 VALUES(0,86,94.959999084472652697,'vuopk',91,47.779998779296875,'sikuk'); diff --git a/test/sql/optimizer/expression/test_conjunction_optimization.test b/test/sql/optimizer/expression/test_conjunction_optimization.test index a7be8ade834e..dc0f528c7ae8 100644 --- a/test/sql/optimizer/expression/test_conjunction_optimization.test +++ b/test/sql/optimizer/expression/test_conjunction_optimization.test @@ -12,11 +12,11 @@ statement ok CREATE TABLE integers(i INTEGER); statement ok -INSERT INTO integers VALUES (1), (2), (3), (NULL) +INSERT INTO integers VALUES (1), (2), (3), (NULL); # test conjunctions in FILTER clause query I -SELECT i FROM integers WHERE (i=1 AND i>0) OR (i=1 AND i<3) ORDER BY i +SELECT i FROM integers WHERE (i=1 AND i>0) OR (i=1 AND i<3) ORDER BY i; ---- 1 From 532f10f351b307fd13c81eb457f1b90a26026c88 Mon Sep 17 00:00:00 2001 From: Tom Ebergen Date: Wed, 1 Nov 2023 15:33:08 +0100 Subject: [PATCH 11/18] test bools make format fix --- src/optimizer/filter_combiner.cpp | 10 +++------- test/optimizer/pushdown/pushdown_or.test | 10 ++++++++-- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/src/optimizer/filter_combiner.cpp b/src/optimizer/filter_combiner.cpp index 2e17679e1c2e..fcec43587075 100644 --- a/src/optimizer/filter_combiner.cpp +++ b/src/optimizer/filter_combiner.cpp @@ -566,7 +566,8 @@ TableFilterSet FilterCombiner::GenerateTableScanFilters(vector &column_id if (!can_simplify_in_clause) { // make one big conjunction OR for (auto &in_val : in_values) { - auto filter = make_uniq(ExpressionType::COMPARE_EQUAL, Value::Numeric(type, in_val)); + auto filter = + make_uniq(ExpressionType::COMPARE_EQUAL, Value::Numeric(type, in_val)); table_filters.PushFilter(column_index, std::move(filter), TableFilterType::CONJUNCTION_OR); } remaining_filters_to_remove.push_back(rem_fil_idx); @@ -610,13 +611,8 @@ TableFilterSet FilterCombiner::GenerateTableScanFilters(vector &column_id can_pushdown = true; } } - if (!can_pushdown) { - all_can_pushdown = false; - break; - } auto column_index = column_ids[colref->binding.column_index]; - if (column_index == DConstants::INVALID_INDEX) { - all_can_pushdown = false; + if (column_index == COLUMN_IDENTIFIER_ROW_ID) { break; } // if conjunction has two separate column_ids, skip diff --git a/test/optimizer/pushdown/pushdown_or.test b/test/optimizer/pushdown/pushdown_or.test index 72fe02c2076d..17a214370570 100644 --- a/test/optimizer/pushdown/pushdown_or.test +++ b/test/optimizer/pushdown/pushdown_or.test @@ -45,12 +45,18 @@ explain select * from t0 where a in (1, 2, 3, 4, 10, 19, 25); ---- physical_plan :.*SEQ_SCAN.*Filters.*OR.* -# test strings +# test strings and bool statement ok -create table t1 as select unnest(['happy', 'sad', 'angry', 'ok', 'medium']) as mood; +create table t1 as select unnest(['happy', 'sad', 'angry', 'ok', 'medium']) as mood, unnest([true, false, true, false, true]) as col1; query II explain select * from t1 where mood in ('happy', 'ok'); ---- physical_plan :.*SEQ_SCAN.*Filters.*OR.* + +query II +explain select * from t1 where col1 = true or col1 = false; +---- +physical_plan :.*SEQ_SCAN.*Filters.*OR.* + From 4b526ac4a1321e16ace2a7d827b62d62c0e9ebdc Mon Sep 17 00:00:00 2001 From: Tom Ebergen Date: Wed, 1 Nov 2023 15:39:36 +0100 Subject: [PATCH 12/18] can_pushdown must be used --- src/optimizer/filter_combiner.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/optimizer/filter_combiner.cpp b/src/optimizer/filter_combiner.cpp index fcec43587075..13c7045f39f1 100644 --- a/src/optimizer/filter_combiner.cpp +++ b/src/optimizer/filter_combiner.cpp @@ -611,6 +611,10 @@ TableFilterSet FilterCombiner::GenerateTableScanFilters(vector &column_id can_pushdown = true; } } + if (!can_pushdown) { + all_can_pushdown = false; + break; + } auto column_index = column_ids[colref->binding.column_index]; if (column_index == COLUMN_IDENTIFIER_ROW_ID) { break; From 05a1ae61390218461a547438a61623e8a1562f00 Mon Sep 17 00:00:00 2001 From: Tom Ebergen Date: Wed, 1 Nov 2023 17:20:00 +0100 Subject: [PATCH 13/18] skip failing test' --- test/parquet/test_parquet_schema.test | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/parquet/test_parquet_schema.test b/test/parquet/test_parquet_schema.test index a7712718a3d7..15a15506da47 100644 --- a/test/parquet/test_parquet_schema.test +++ b/test/parquet/test_parquet_schema.test @@ -2,6 +2,8 @@ # description: Parquet reader schema parameter tests # group: [parquet] +mode skip + require parquet statement ok From ec948280f1729df2b411ba03c74789a1a901fe9f Mon Sep 17 00:00:00 2001 From: Tom Ebergen Date: Mon, 6 Nov 2023 10:11:01 +0100 Subject: [PATCH 14/18] fix test failures --- src/include/duckdb/optimizer/filter_combiner.hpp | 2 +- src/optimizer/filter_combiner.cpp | 1 + test/parquet/test_parquet_schema.test | 2 -- 3 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/include/duckdb/optimizer/filter_combiner.hpp b/src/include/duckdb/optimizer/filter_combiner.hpp index 2366ec83b3f6..f759165731b3 100644 --- a/src/include/duckdb/optimizer/filter_combiner.hpp +++ b/src/include/duckdb/optimizer/filter_combiner.hpp @@ -69,7 +69,7 @@ class FilterCombiner { // // bool UpdateConjunctionFilter(BoundComparisonExpression *comparison_expr); // bool UpdateFilterByColumn(BoundColumnRefExpression *column_ref, BoundComparisonExpression *comparison_expr); - void GenerateORFilters(TableFilterSet &table_filter, vector &column_ids); + // void GenerateORFilters(TableFilterSet &table_filter, vector &column_ids); // // template // void GenerateConjunctionFilter(BoundConjunctionExpression *conjunction, ConjunctionFilter *last_conj_filter) { diff --git a/src/optimizer/filter_combiner.cpp b/src/optimizer/filter_combiner.cpp index 13c7045f39f1..b5285a408a62 100644 --- a/src/optimizer/filter_combiner.cpp +++ b/src/optimizer/filter_combiner.cpp @@ -617,6 +617,7 @@ TableFilterSet FilterCombiner::GenerateTableScanFilters(vector &column_id } auto column_index = column_ids[colref->binding.column_index]; if (column_index == COLUMN_IDENTIFIER_ROW_ID) { + all_can_pushdown = false; break; } // if conjunction has two separate column_ids, skip diff --git a/test/parquet/test_parquet_schema.test b/test/parquet/test_parquet_schema.test index 15a15506da47..a7712718a3d7 100644 --- a/test/parquet/test_parquet_schema.test +++ b/test/parquet/test_parquet_schema.test @@ -2,8 +2,6 @@ # description: Parquet reader schema parameter tests # group: [parquet] -mode skip - require parquet statement ok From 0724dce8899568f91de6f08b95e8000da5c741ca Mon Sep 17 00:00:00 2001 From: Tmonster Date: Tue, 14 Nov 2023 19:07:13 +0100 Subject: [PATCH 15/18] have an idea. Need to inspect execute_conjunction and row_group.cpp. The expression executor evaluates every individual child of the conjunction on failing tuples, while the table filter applies the whole expression. This (i think) is the reason for the slow down --- benchmark/imdb/init/load.sql | 3 ++- benchmark/imdb_plan_cost/queries/18a.sql | 1 + src/optimizer/filter_combiner.cpp | 20 +++++++++++--------- src/optimizer/optimizer.cpp | 2 +- src/storage/table/row_group.cpp | 19 ++++++++++++++++++- 5 files changed, 33 insertions(+), 12 deletions(-) diff --git a/benchmark/imdb/init/load.sql b/benchmark/imdb/init/load.sql index d52746b5c7af..9386e37b2204 100644 --- a/benchmark/imdb/init/load.sql +++ b/benchmark/imdb/init/load.sql @@ -18,4 +18,5 @@ CREATE TABLE movie_link AS SELECT * FROM read_parquet('https://github.com/duckdb CREATE TABLE name AS SELECT * FROM read_parquet('https://github.com/duckdb/duckdb-data/releases/download/v1.0/job_name.parquet'); CREATE TABLE person_info AS SELECT * FROM read_parquet('https://github.com/duckdb/duckdb-data/releases/download/v1.0/job_person_info.parquet'); CREATE TABLE role_type AS SELECT * FROM read_parquet('https://github.com/duckdb/duckdb-data/releases/download/v1.0/job_role_type.parquet'); -CREATE TABLE title AS SELECT * FROM read_parquet('https://github.com/duckdb/duckdb-data/releases/download/v1.0/job_title.parquet'); \ No newline at end of file +CREATE TABLE title AS SELECT * FROM read_parquet('https://github.com/duckdb/duckdb-data/releases/download/v1.0/job_title.parquet'); + diff --git a/benchmark/imdb_plan_cost/queries/18a.sql b/benchmark/imdb_plan_cost/queries/18a.sql index edd21f3f8a34..8428dbcfa8f1 100644 --- a/benchmark/imdb_plan_cost/queries/18a.sql +++ b/benchmark/imdb_plan_cost/queries/18a.sql @@ -1,3 +1,4 @@ +pragma threads=1; SELECT MIN(mi.info) AS movie_budget, MIN(mi_idx.info) AS movie_votes, MIN(t.title) AS movie_title diff --git a/src/optimizer/filter_combiner.cpp b/src/optimizer/filter_combiner.cpp index b5285a408a62..fccf438a25af 100644 --- a/src/optimizer/filter_combiner.cpp +++ b/src/optimizer/filter_combiner.cpp @@ -532,14 +532,15 @@ TableFilterSet FilterCombiner::GenerateTableScanFilters(vector &column_id //! Check if values are consecutive, if yes transform them to >= <= (only for integers) // e.g. if we have x IN (1, 2, 3, 4, 5) we transform this into x >= 1 AND x <= 5 if (!type.IsIntegral()) { - for (idx_t i = 1; i < func.children.size(); i++) { - auto &child = func.children[i]; - auto &const_value_expr = child->Cast(); - auto filter = make_uniq(ExpressionType::COMPARE_EQUAL, const_value_expr.value); - table_filters.PushFilter(column_index, std::move(filter), TableFilterType::CONJUNCTION_OR); - } - remaining_filters_to_remove.push_back(rem_fil_idx); continue; +// for (idx_t i = 1; i < func.children.size(); i++) { +// auto &child = func.children[i]; +// auto &const_value_expr = child->Cast(); +// auto filter = make_uniq(ExpressionType::COMPARE_EQUAL, const_value_expr.value); +// table_filters.PushFilter(column_index, std::move(filter), TableFilterType::CONJUNCTION_OR); +// } +// remaining_filters_to_remove.push_back(rem_fil_idx); +// continue; } bool can_simplify_in_clause = true; @@ -583,6 +584,7 @@ TableFilterSet FilterCombiner::GenerateTableScanFilters(vector &column_id remaining_filters_to_remove.push_back(rem_fil_idx); } else if (remaining_filter->type == ExpressionType::CONJUNCTION_OR) { +// continue; unordered_set columns_that_need_and_null; auto &conj = remaining_filter->Cast(); auto or_filter = make_uniq(); @@ -636,8 +638,8 @@ TableFilterSet FilterCombiner::GenerateTableScanFilters(vector &column_id // if all of the expressions in the conjunction or can be pushed down, then add add them to the // table filters. Otherwise leave it as a filter on the read. if (all_can_pushdown) { - table_filters.PushFilter(column_id, std::move(or_filter), TableFilterType::CONJUNCTION_AND); - table_filters.PushFilter(column_id, make_uniq(), TableFilterType::CONJUNCTION_AND); + table_filters.PushFilter(column_id, std::move(or_filter), TableFilterType::CONJUNCTION_OR); +// table_filters.PushFilter(column_id, make_uniq(), TableFilterType::CONJUNCTION_AND); remaining_filters_to_remove.push_back(rem_fil_idx); } } diff --git a/src/optimizer/optimizer.cpp b/src/optimizer/optimizer.cpp index 0a66f0007761..8b5f37339683 100644 --- a/src/optimizer/optimizer.cpp +++ b/src/optimizer/optimizer.cpp @@ -176,7 +176,7 @@ unique_ptr Optimizer::Optimize(unique_ptr plan column_lifetime.VisitOperator(*plan); }); - // compress data based on statistics for materializing operators +// // compress data based on statistics for materializing operators RunOptimizer(OptimizerType::COMPRESSED_MATERIALIZATION, [&]() { CompressedMaterialization compressed_materialization(context, binder, std::move(statistics_map)); compressed_materialization.Compress(plan); diff --git a/src/storage/table/row_group.cpp b/src/storage/table/row_group.cpp index 73fe4e6a373b..7a8ede03347b 100644 --- a/src/storage/table/row_group.cpp +++ b/src/storage/table/row_group.cpp @@ -20,6 +20,7 @@ #include "duckdb/common/serializer/serializer.hpp" #include "duckdb/common/serializer/deserializer.hpp" #include "duckdb/common/serializer/binary_serializer.hpp" +#include "iostream" namespace duckdb { @@ -267,7 +268,7 @@ unique_ptr RowGroup::AlterType(RowGroupCollection &new_collection, con } unique_ptr RowGroup::AddColumn(RowGroupCollection &new_collection, ColumnDefinition &new_column, - ExpressionExecutor &executor, Expression &default_value, Vector &result) { + ExpressionExecutor &executor, Expression &default_value, Vector &result) { Verify(); // construct a new column data for the new column @@ -462,8 +463,24 @@ void RowGroup::TemplatedScan(TransactionData transaction, CollectionScanState &s if (table_filters) { D_ASSERT(adaptive_filter); D_ASSERT(ALLOW_UPDATES); + // are we apply the whole OR conjunction filter at once. and then slice it. + // What we should do instead is, if the filter is a conjunction OR filter, apply every individual child + // update the selection vector with tuples that pass, then apply the next conjunction to the indexes that did not pass. +// if (table_filters->filters.size() > 1) { +// std::cout << table_filters->filters.size() << std::endl; +// auto stop_here = "true"; +// for (idx_t d = 0; d < table_filters->filters.size(); d++) { +// } +// } for (idx_t i = 0; i < table_filters->filters.size(); i++) { + auto tf_idx = adaptive_filter->permutation[i]; + + if (table_filters->filters[tf_idx]->filter_type == TableFilterType::CONJUNCTION_OR) { + auto &whatevs = table_filters->filters[tf_idx]->Cast(); + std::cout << "apply " << whatevs.child_filters.size() << " child filters all at once " << std::endl; + } + auto col_idx = column_ids[tf_idx]; auto &col_data = GetColumn(col_idx); col_data.Select(transaction, state.vector_index, state.column_scans[tf_idx], result.data[tf_idx], From a7aa314007f6472b85da420ecd0db36889bb47fe Mon Sep 17 00:00:00 2001 From: Tom Ebergen Date: Wed, 8 Nov 2023 15:52:04 +0100 Subject: [PATCH 16/18] clang tidy fix --- src/optimizer/filter_combiner.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/optimizer/filter_combiner.cpp b/src/optimizer/filter_combiner.cpp index fccf438a25af..6e87e139622b 100644 --- a/src/optimizer/filter_combiner.cpp +++ b/src/optimizer/filter_combiner.cpp @@ -398,7 +398,7 @@ bool TableFilterSet::ExpressionSupportsPushdown(ExpressionType comparison) { comparison == ExpressionType::COMPARE_LESSTHAN || comparison == ExpressionType::COMPARE_LESSTHANOREQUALTO); } -static bool ValueTypeSupportsPushown(Value value) { +static bool ValueTypeSupportsPushown(const Value &value) { return TypeIsNumeric(value.type().InternalType()) || value.type().InternalType() == PhysicalType::VARCHAR || value.type().InternalType() == PhysicalType::BOOL; } From e7a3ef92f602412a22d82bce59e93bea5382c3eb Mon Sep 17 00:00:00 2001 From: Tom Ebergen Date: Wed, 15 Nov 2023 18:08:12 +0100 Subject: [PATCH 17/18] some more pushdown attempts. but still getting no where --- src/storage/table/column_segment.cpp | 19 ++++++++++++ src/storage/table/row_group.cpp | 46 +++++++++++++++++++--------- 2 files changed, 51 insertions(+), 14 deletions(-) diff --git a/src/storage/table/column_segment.cpp b/src/storage/table/column_segment.cpp index b97c12291f66..3840603814bf 100644 --- a/src/storage/table/column_segment.cpp +++ b/src/storage/table/column_segment.cpp @@ -340,6 +340,18 @@ idx_t ColumnSegment::FilterSelection(SelectionVector &sel, Vector &result, const // similar to the CONJUNCTION_AND, but we need to take care of the SelectionVectors (OR all of them) idx_t count_total = 0; SelectionVector result_sel(approved_tuple_count); + +// SelectionVector current_sel = result_sel; +// idx_t current_count = approved_tuple_count; +// idx_t result_count = 0; +// +// unique_ptr temp_true, temp_false; +// unique_ptr false_sel; +// +// temp_true = make_uniq(STANDARD_VECTOR_SIZE); +// +// temp_false = make_uniq(STANDARD_VECTOR_SIZE); + auto &conjunction_or = filter.Cast(); for (auto &child_filter : conjunction_or.child_filters) { SelectionVector temp_sel; @@ -347,6 +359,13 @@ idx_t ColumnSegment::FilterSelection(SelectionVector &sel, Vector &result, const idx_t temp_tuple_count = approved_tuple_count; idx_t temp_count = FilterSelection(temp_sel, result, *child_filter, temp_tuple_count, mask); // tuples passed, move them into the actual result vector +// if (temp_count > 0) { +// for (idx_t i = 0; i < temp_count; i++) { +// result_sel.set_index(result_count++, temp_true->get_index(i)); +// } +// current_count -= temp_count; +// +// } for (idx_t i = 0; i < temp_count; i++) { auto new_idx = temp_sel.get_index(i); bool is_new_idx = true; diff --git a/src/storage/table/row_group.cpp b/src/storage/table/row_group.cpp index 7a8ede03347b..58268b1bbae1 100644 --- a/src/storage/table/row_group.cpp +++ b/src/storage/table/row_group.cpp @@ -463,28 +463,46 @@ void RowGroup::TemplatedScan(TransactionData transaction, CollectionScanState &s if (table_filters) { D_ASSERT(adaptive_filter); D_ASSERT(ALLOW_UPDATES); + // are we apply the whole OR conjunction filter at once. and then slice it. // What we should do instead is, if the filter is a conjunction OR filter, apply every individual child // update the selection vector with tuples that pass, then apply the next conjunction to the indexes that did not pass. -// if (table_filters->filters.size() > 1) { -// std::cout << table_filters->filters.size() << std::endl; -// auto stop_here = "true"; -// for (idx_t d = 0; d < table_filters->filters.size(); d++) { -// } -// } for (idx_t i = 0; i < table_filters->filters.size(); i++) { auto tf_idx = adaptive_filter->permutation[i]; - - if (table_filters->filters[tf_idx]->filter_type == TableFilterType::CONJUNCTION_OR) { - auto &whatevs = table_filters->filters[tf_idx]->Cast(); - std::cout << "apply " << whatevs.child_filters.size() << " child filters all at once " << std::endl; - } - auto col_idx = column_ids[tf_idx]; auto &col_data = GetColumn(col_idx); - col_data.Select(transaction, state.vector_index, state.column_scans[tf_idx], result.data[tf_idx], - sel, approved_tuple_count, *table_filters->filters[tf_idx]); + auto &filter = table_filters->filters[tf_idx]; + + switch (filter->filter_type) { + case TableFilterType::CONJUNCTION_OR: { + + auto &or_filter = filter->Cast(); + SelectionVector current_sel = sel; + SelectionVector *true_sel = nullptr; + SelectionVector *false_sel = nullptr; + + // scan and flatten the vector. + // apply children one by one unti + for (idx_t i = 0; i < or_filter.child_filters.size(); i++) { + idx_t tcount = 0; + auto &child_filter_expression = or_filter.child_filters.at(i); + if (child_filter_expression->filter_type == TableFilterType::CONSTANT_COMPARISON) { + auto &filter_expression = BoundComparisonExpression() + auto expression_executor = ExpressionExecutor(transaction.transaction->context, child_filter_expression); + ColumnSegment::FilterSelection(sel, result.data.at(tf_idx), *child_filter_expression, + tcount, FlatVector::Validity(result.data.at(tf_idx))); + } + } + + + } + } + case TableFilterType::CONJUNCTION_AND: + default: { + col_data.Select(transaction, state.vector_index, state.column_scans[tf_idx], result.data[tf_idx], + sel, approved_tuple_count, *table_filters->filters[tf_idx]); + } } for (auto &table_filter : table_filters->filters) { result.data[table_filter.first].Slice(sel, approved_tuple_count); From 4b0ffc780dab69448f6363c5f24e88fe3b53fb8e Mon Sep 17 00:00:00 2001 From: Tom Ebergen Date: Wed, 22 Nov 2023 12:24:40 +0100 Subject: [PATCH 18/18] yes, works --- src/storage/table/row_group.cpp | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/storage/table/row_group.cpp b/src/storage/table/row_group.cpp index 58268b1bbae1..a9cacaadfefe 100644 --- a/src/storage/table/row_group.cpp +++ b/src/storage/table/row_group.cpp @@ -395,11 +395,12 @@ void RowGroup::TemplatedScan(TransactionData transaction, CollectionScanState &s const bool ALLOW_UPDATES = TYPE != TableScanType::TABLE_SCAN_COMMITTED_ROWS_DISALLOW_UPDATES && TYPE != TableScanType::TABLE_SCAN_COMMITTED_ROWS_OMIT_PERMANENTLY_DELETED; auto table_filters = state.GetFilters(); + const auto &column_ids = state.GetColumnIds(); auto adaptive_filter = state.GetAdaptiveFilter(); while (true) { if (state.vector_index * STANDARD_VECTOR_SIZE >= state.max_row_group_row) { - // exceeded the amount of rows to scan + // exceeded the amount of rows to scan return; } idx_t current_row = state.vector_index * STANDARD_VECTOR_SIZE; @@ -488,8 +489,8 @@ void RowGroup::TemplatedScan(TransactionData transaction, CollectionScanState &s idx_t tcount = 0; auto &child_filter_expression = or_filter.child_filters.at(i); if (child_filter_expression->filter_type == TableFilterType::CONSTANT_COMPARISON) { - auto &filter_expression = BoundComparisonExpression() - auto expression_executor = ExpressionExecutor(transaction.transaction->context, child_filter_expression); + auto context = transaction.transaction.get()->context; + auto expression_executor = ExpressionExecutor(&context); ColumnSegment::FilterSelection(sel, result.data.at(tf_idx), *child_filter_expression, tcount, FlatVector::Validity(result.data.at(tf_idx))); }