Skip to content

Commit a22a430

Browse files
committed
simplify filter expression storage index bindings (just reuse the ones we made earlier), fix single-ref-per-expr predicate to correctly walk expr tree and yank refs (allowing nesting in fns, etc)
1 parent 9c8c1ed commit a22a430

1 file changed

Lines changed: 27 additions & 32 deletions

File tree

src/function/table/table_scan.cpp

Lines changed: 27 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -500,33 +500,37 @@ bool TryScanIndex(ART &art, const ColumnList &column_list, TableFunctionInitInpu
500500
return false;
501501
}
502502

503-
// ...only if each expression is a column ref (for value assertions)
503+
// ...only if each expression has a single column reference, and
504+
// that single column reference positionally matches that of the index (this is guaranteed? paranoid?)
504505
// NOTE: We do not push down multi-column filters, e.g., 42 = a + b.
505-
for (const auto &expr : index_exprs) {
506-
if (expr->GetExpressionClass() != ExpressionClass::BOUND_COLUMN_REF) {
507-
return false;
508-
}
509-
}
506+
for (idx_t i = 0; i < index_exprs.size(); ++i) {
507+
unordered_set<column_t> referenced_columns;
508+
auto expr = &index_exprs[i];
509+
510+
// Walk the expr in case of nesting (e.g. function)
511+
ExpressionIterator::EnumerateExpression(*expr, [&](Expression &child_expr) {
512+
if (child_expr.GetExpressionClass() == ExpressionClass::BOUND_COLUMN_REF) {
513+
auto &col_ref = child_expr.Cast<BoundColumnRefExpression>();
514+
referenced_columns.insert(col_ref.binding.column_index);
515+
}
516+
});
510517

511-
// TODO: can't tell if this is guaranteed?
512-
for (idx_t i = 0; i < index_exprs.size(); i++) {
513-
auto &expr = index_exprs[i]->Cast<BoundColumnRefExpression>();
514-
if (expr.binding.column_index != indexed_columns[i]) {
518+
if (referenced_columns.size() != 1 || *referenced_columns.begin() != indexed_columns[i]) {
515519
return false;
516520
}
517521
}
518522

519523
// Resolve bound column references in the index_expr against the current input projection
520-
vector<column_t> updated_index_columns;
521-
updated_index_columns.resize(indexed_columns.size(), std::numeric_limits<idx_t>::max());
524+
vector<column_t> index_column_to_proj_pos;
525+
index_column_to_proj_pos.resize(indexed_columns.size(), std::numeric_limits<idx_t>::max());
522526

523527
bool found_index_column_in_input = false;
524528

525529
// Associate indexed columns to input columns
526530
for (idx_t i = 0; i < indexed_columns.size(); ++i) {
527531
for (idx_t j = 0; j < input.column_ids.size(); ++j) {
528532
if (indexed_columns[i] == input.column_ids[j]) {
529-
updated_index_columns.at(i) = j;
533+
index_column_to_proj_pos.at(i) = j;
530534
found_index_column_in_input = true;
531535
}
532536
}
@@ -545,40 +549,31 @@ bool TryScanIndex(ART &art, const ColumnList &column_list, TableFunctionInitInpu
545549
// If the bound column references an indexed column, update it
546550
for (idx_t i = 0; i < indexed_columns.size(); ++i) {
547551
if (bound_column_ref_expr.binding.column_index == indexed_columns[i]) {
548-
bound_column_ref_expr.binding.column_index = updated_index_columns[i];
552+
bound_column_ref_expr.binding.column_index = index_column_to_proj_pos[i];
549553
break;
550554
}
551555
}
552556
});
553557
}
554558
}
555559

556-
// Get ART columns
557-
vector<const ColumnDefinition *> indexed_column_defs;
558-
for (idx_t i = 0; i < indexed_columns.size(); ++i) {
559-
indexed_column_defs.push_back(&column_list.GetColumn(LogicalIndex(indexed_columns[i])));
560-
}
561-
562560
// The indexes of the filters match input.column_indexes, which are: i -> column_index.
563-
// Try to find a filter on the ART column.
561+
// Reuse the index <-> projection mappings from index expr rebinding
564562
vector<vector<unique_ptr<Expression>>> filters;
565563

566-
for (idx_t i = 0; i < input.column_indexes.size(); ++i) {
567-
for (idx_t j = 0; j < indexed_column_defs.size(); ++j) {
568-
if (input.column_indexes[i].ToLogical() == indexed_column_defs[j]->Logical()) {
569-
auto maybe_filter = filter_set.filters.find(i);
570-
if (maybe_filter != filter_set.filters.end()) {
571-
auto filter = &maybe_filter->second;
572-
auto filter_expressions = ExtractFilterExpressions(*indexed_column_defs[j], *filter, i);
564+
for (idx_t i = 0; i < index_column_to_proj_pos.size(); ++i) {
565+
auto column_def = &column_list.GetColumn(LogicalIndex(indexed_columns[i]));
566+
auto maybe_filter = filter_set.filters.find(index_column_to_proj_pos[i]);
567+
if (maybe_filter != filter_set.filters.end()) {
568+
auto filter = &maybe_filter->second;
569+
auto filter_expressions = ExtractFilterExpressions(*column_def, *filter, index_column_to_proj_pos[i]);
573570

574-
filters.push_back(std::move(filter_expressions));
575-
}
576-
}
571+
filters.push_back(std::move(filter_expressions));
577572
}
578573
}
579574

580575
// Filters must match ART columns 1:1
581-
if (filters.size() != indexed_column_defs.size() || filters.empty()) {
576+
if (filters.size() != indexed_columns.size() || filters.empty()) {
582577
return false;
583578
}
584579

0 commit comments

Comments
 (0)