From 83596ce40625225b86927c7d76f6fc134ffe14bc Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Tue, 10 Nov 2026 23:34:41 +0000 Subject: [PATCH] address review: bounds-check SORT indices before narrowing; empty criteria rows are not charged --- crates/functions/src/database.rs | 10 +++++++--- crates/functions/src/dynamic.rs | 2 +- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/crates/functions/src/database.rs b/crates/functions/src/database.rs index 31ead3e60..5d77eb350 100644 --- a/crates/functions/src/database.rs +++ b/crates/functions/src/database.rs @@ -100,11 +100,12 @@ fn matching_records(db: &Array, crit: &Array, ctx: &mut dyn Ctx) -> R tests = tests.saturating_add(conds.len().max(1)); rows.push((r, conds)); } + // An empty criteria row matches every record without testing a condition, so nothing is charged. + let all_match = rows.is_empty() || rows.iter().any(|(_, conds)| conds.is_empty()); // Computed criteria (a formula evaluated per record) count like any other test. - if db.rows.saturating_sub(1).saturating_mul(tests) > SELECTION_WORK_BUDGET { + if !all_match && db.rows.saturating_sub(1).saturating_mul(tests) > SELECTION_WORK_BUDGET { return Err(CellError::Num); } - let all_match = rows.is_empty(); Ok((1..db.rows) .filter(|&rec| { all_match @@ -382,7 +383,10 @@ mod tests { let wide = column("Tree", 5_001, tv("=Plum")); is_err(ev("DCOUNTA", vec![rf(db.clone()), t("Tree"), rf(wide)]), CellError::Num); let narrow = column("Tree", 2, tv("=Apple")); - close(ev("DCOUNTA", vec![rf(db), t("Tree"), rf(narrow)]), 100_000.0); + close(ev("DCOUNTA", vec![rf(db.clone()), t("Tree"), rf(narrow)]), 100_000.0); + // Empty criteria rows test no condition: the first one matches every record, so they cost nothing. + let empty_rows = column("Tree", 5_001, Value::Empty); + close(ev("DCOUNTA", vec![rf(db), t("Tree"), rf(empty_rows)]), 100_000.0); } #[test] diff --git a/crates/functions/src/dynamic.rs b/crates/functions/src/dynamic.rs index 6e0d50afa..b9aaf8dc5 100644 --- a/crates/functions/src/dynamic.rs +++ b/crates/functions/src/dynamic.rs @@ -171,7 +171,7 @@ fn sort(args: &[Arg], _c: &mut dyn Ctx) -> R { let a = if by_col { a.transpose() } else { a }; let idx = if has(args, 1) { int_list(&arg(args, 1)?.value)? } else { vec![1] }; let ord = if has(args, 2) { int_list(&arg(args, 2)?.value)? } else { vec![1] }; - if idx.is_empty() || idx.iter().any(|&i| i < 1 || i as usize > a.cols) || ord.iter().any(|&o| o != 1 && o != -1) { + if idx.is_empty() || idx.iter().any(|&i| i < 1 || i > a.cols as i64) || ord.iter().any(|&o| o != 1 && o != -1) { return Err(CellError::Value); } if ord.len() > 1 && ord.len() != idx.len() {