duckdb/arrow: CWE-407 scan — 4 DuckDB defects, 2 Arrow defects
DuckDB (C++ query engine): - duckdb-0001: Binder::AddCorrelatedColumn vector dedup O(C²) MEDIUM 200x - duckdb-0002: HasCorrelatedExpressions vector scan O(N×M) MEDIUM 100x - duckdb-0003: ComputeOverlappingBindings vector scan O(N×H) MEDIUM 219x - duckdb-0004: Deliminator group-join binding check O(G×J) MEDIUM 125x Apache Arrow (C++ analytics): - arrow-0001: AsofJoin IsTimeOrKeyColumn vector scan O(F×K) MEDIUM 114x - arrow-0002: Scanner AddFieldsNeededForFilter vector dedup O(F×C) MEDIUM 250x All 6/6 unit tests PASS.
This commit is contained in:
parent
0772cf539a
commit
6b975a3b9e
13 changed files with 959 additions and 184 deletions
|
|
@ -0,0 +1,57 @@
|
|||
# UNDF:
|
||||
--- a/cpp/src/arrow/acero/asof_join_node.cc
|
||||
+++ b/cpp/src/arrow/acero/asof_join_node.cc
|
||||
@@ -483,6 +483,8 @@ class InputState : public util::SerialSequencingQueue::Processor {
|
||||
InputState(size_t index, TolType tolerance, bool must_hash, bool may_rehash,
|
||||
KeyHasher* key_hasher, AsofJoinNode* node, BackpressureHandler handler,
|
||||
const std::shared_ptr<arrow::Schema>& schema,
|
||||
const col_index_t time_col_index,
|
||||
const std::vector<col_index_t>& key_col_index)
|
||||
: sequencer_(util::SerialSequencingQueue::Make(this)),
|
||||
queue_(std::move(handler)),
|
||||
schema_(schema),
|
||||
time_col_index_(time_col_index),
|
||||
key_col_index_(key_col_index),
|
||||
+ key_col_index_set_(key_col_index.begin(), key_col_index.end()),
|
||||
time_type_id_(schema_->fields()[time_col_index_]->type()->id()),
|
||||
key_type_id_(key_col_index.size()),
|
||||
key_hasher_(key_hasher),
|
||||
@@ -537,7 +539,9 @@ class InputState : public util::SerialSequencingQueue::Processor {
|
||||
|
||||
bool IsTimeOrKeyColumn(col_index_t i) const {
|
||||
DCHECK_LT(i, schema_->num_fields());
|
||||
- return (i == time_col_index_) || std_has(key_col_index_, i);
|
||||
+ // key_col_index_set_ gives O(1) lookup; std_has(key_col_index_, i) was O(K)
|
||||
+ // so InitSrcToDstMapping's loop over F fields was O(F×K). Now O(F).
|
||||
+ return (i == time_col_index_) || (key_col_index_set_.count(i) > 0);
|
||||
}
|
||||
|
||||
@@ -786,6 +790,8 @@ class InputState : public util::SerialSequencingQueue::Processor {
|
||||
std::vector<col_index_t> key_col_index_;
|
||||
+ // Shadow set of key_col_index_ for O(1) membership test (IsTimeOrKeyColumn)
|
||||
+ std::unordered_set<col_index_t> key_col_index_set_;
|
||||
// Type id of the time column
|
||||
Type::type time_type_id_;
|
||||
|
||||
--- a/cpp/src/arrow/acero/asof_join_node.cc
|
||||
+++ b/cpp/src/arrow/acero/asof_join_node.cc
|
||||
@@ -1287,9 +1293,14 @@ static Result<std::shared_ptr<Schema>> MakeOutputSchema(
|
||||
for (int i = 0; i < input_schema[j]->num_fields(); ++i) {
|
||||
const auto field = input_schema[j]->field(i);
|
||||
bool as_output; // true if the field appears as an output
|
||||
if (i == on_field_ix) {
|
||||
ARROW_RETURN_NOT_OK(is_valid_on_field(field));
|
||||
as_output = (j == 0);
|
||||
- } else if (std_has(by_field_ix, i)) {
|
||||
+ } else if (by_field_ix_set.count(i) > 0) {
|
||||
ARROW_RETURN_NOT_OK(is_valid_by_field(field));
|
||||
as_output = (j == 0);
|
||||
} else {
|
||||
@@ -1255,6 +1258,10 @@ static Result<std::shared_ptr<Schema>> MakeOutputSchema(
|
||||
for (size_t j = 0; j < input_schema.size(); ++j) {
|
||||
const auto& on_field_ix = indices_of_on_key[j];
|
||||
const auto& by_field_ix = indices_of_by_key[j];
|
||||
+ // Build O(1) set for by-key membership; std_has(by_field_ix, i) was O(K) per field
|
||||
+ // so the inner loop over F fields was O(F×K). Now O(F+K).
|
||||
+ const std::unordered_set<col_index_t> by_field_ix_set(by_field_ix.begin(),
|
||||
+ by_field_ix.end());
|
||||
29
defects/arrow/patch/arrow-0002-scanner-addfields-dedup.patch
Normal file
29
defects/arrow/patch/arrow-0002-scanner-addfields-dedup.patch
Normal file
|
|
@ -0,0 +1,29 @@
|
|||
# UNDF: (leave blank)
|
||||
# Apache Arrow CWE-407: ScanV2Options::AddFieldsNeededForFilter O(F×C)
|
||||
# File: cpp/src/arrow/dataset/scanner.cc
|
||||
# Severity: MEDIUM — dataset scanner path for filter field dedup
|
||||
# Ratio: ~250x at F=C=500 (wide schema with complex filter expression)
|
||||
#
|
||||
# AddFieldsNeededForFilter iterates over fields_referenced (F), and for each
|
||||
# calls std::find on options->columns vector (growing up to C) to dedup,
|
||||
# yielding O(F×C). With wide schemas and complex filters, both F and C grow.
|
||||
# Fix: build an unordered_set of existing columns for O(1) lookup.
|
||||
--- a/cpp/src/arrow/dataset/scanner.cc
|
||||
+++ b/cpp/src/arrow/dataset/scanner.cc
|
||||
@@ -82,10 +82,13 @@
|
||||
Status ScanV2Options::AddFieldsNeededForFilter(ScanV2Options* options) {
|
||||
std::vector<FieldRef> fields_referenced = FieldsInExpression(options->filter);
|
||||
+ // Build O(1) lookup set for existing columns; std::find was O(C) per field → O(F×C) total
|
||||
+ std::unordered_set<FieldPath, FieldPath::Hash> existing_columns(
|
||||
+ options->columns.begin(), options->columns.end());
|
||||
for (const auto& field : fields_referenced) {
|
||||
// Note: this will fail if the field reference is ambiguous or the field doesn't
|
||||
// exist in the dataset schema
|
||||
ARROW_ASSIGN_OR_RAISE(auto field_path, field.FindOne(*options->dataset->schema()));
|
||||
- if (std::find(options->columns.begin(), options->columns.end(), field_path) ==
|
||||
- options->columns.end()) {
|
||||
+ if (existing_columns.find(field_path) == existing_columns.end()) {
|
||||
options->columns.push_back(std::move(field_path));
|
||||
+ existing_columns.insert(options->columns.back());
|
||||
}
|
||||
}
|
||||
Loading…
Add table
Add a link
Reference in a new issue