Compare commits

..
Author SHA1 Message Date
Dylan M. Taylor 3badcb0b74 address review: charge numeric-text periods of lifted CUMIPMT/CUMPRINC to the work budget 2026-11-04 00:24:16 +00:00
Dylan M. Taylor 0fc6a87d87 functions: one work budget across every call of a lifted function
A function lifted over a computed array runs once per element, and only the
six lookups counted their repeated work against LIFTED_SCAN_BUDGET. Criteria
aggregates rescanned their ranges per criterion, CUMIPMT/CUMPRINC stepped
through up to 1e7 periods per element, and array-producing functions such as
MUNIT built a whole result per element before keeping its first cell.

The budget now covers all of it: COUNTIF/SUMIF/AVERAGEIF(S)/MAXIFS/MINIFS
charge their range cells per call, the period loops of every element are
added up before any runs, and array results charge their cells as they are
built. Past the budget the call is #NUM!, as lifted lookups already are.
2026-11-02 22:18:09 -05:00
4 changed files with 157 additions and 134 deletions
+10 -43
View File
@@ -70,42 +70,27 @@ fn cell_criterion(v: &Value) -> Option<Criterion> {
/// when the label is unknown, criterion).
type CriteriaRow = (usize, Vec<(usize, Option<usize>, Criterion)>);
/// Most condition tests one selection may make (records times conditions): every record can be
/// tested against every criteria row when none matches, so a modest database and criteria table
/// multiply into billions of tests. Past it the function is `#NUM!`, as lifted lookups are; 5e8
/// is a few seconds of matching.
const SELECTION_WORK_BUDGET: usize = 500_000_000;
/// Rows (1-based within the database, excluding the header) of matching records.
fn matching_records(db: &Array, crit: &Array, ctx: &mut dyn Ctx) -> R<Vec<usize>> {
// The database column under each criteria label, found once per label rather than per cell.
let columns: Vec<Option<usize>> = (0..crit.cols)
.map(|c| match crit.get(0, c) {
Some(Value::Text(t)) => find_column(db, t),
Some(Value::Number(n)) => find_column(db, &gridcraft_core::number_to_text(*n)),
_ => None,
})
.collect();
fn matching_records(db: &Array, crit: &Array, ctx: &mut dyn Ctx) -> Vec<usize> {
// Per criteria row: its index and a list of (criteria column, database column or None when
// the label is unknown, criterion).
let mut rows: Vec<CriteriaRow> = Vec::new();
let mut tests = 0usize;
for r in 1..crit.rows {
let mut conds = Vec::new();
for c in 0..crit.cols {
let Some(cell) = crit.get(r, c) else { continue };
let Some(cr) = cell_criterion(cell) else { continue };
conds.push((c, columns.get(c).copied().flatten(), cr));
let col = match crit.get(0, c) {
Some(Value::Text(t)) => find_column(db, t),
Some(Value::Number(n)) => find_column(db, &gridcraft_core::number_to_text(*n)),
_ => None,
};
conds.push((c, col, cr));
}
tests = tests.saturating_add(conds.len().max(1));
rows.push((r, conds));
}
// Computed criteria (a formula evaluated per record) count like any other test.
if 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)
(1..db.rows)
.filter(|&rec| {
all_match
|| rows.iter().any(|(r, conds)| {
@@ -117,7 +102,7 @@ fn matching_records(db: &Array, crit: &Array, ctx: &mut dyn Ctx) -> R<Vec<usize>
})
})
})
.collect())
.collect()
}
struct Selection<'a> {
@@ -138,7 +123,7 @@ fn select<'a>(a: &'a [Arg], ctx: &mut dyn Ctx) -> R<Selection<'a>> {
let db = as_array(dbv);
let crit = as_array(critv);
let field = field_index(&db, a)?;
let records = matching_records(&db, &crit, ctx)?;
let records = matching_records(&db, &crit, ctx);
Ok(Selection { db, field, records })
}
@@ -367,24 +352,6 @@ mod tests {
is_err(d("DSUM", n(9.0)), CellError::Value);
}
/// Security: records times criteria conditions past the selection budget is `#NUM!` before
/// any record is tested; a nonmatching criteria table used to test every record against
/// every criteria row.
#[test]
fn selection_work_is_bounded() {
use gridcraft_core::Array;
let column = |label: &str, len: usize, v: Value| {
let mut data = vec![tv(label)];
data.extend(std::iter::repeat_n(v, len));
Value::from(Array::column(data))
};
let db = column("Tree", 100_000, tv("Apple"));
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);
}
#[test]
fn simple_criteria() {
let c = |cells: Vec<Value>| arr(vec![vec![tv("Tree"), tv("Height")], cells]);
+1 -22
View File
@@ -416,17 +416,13 @@ fn workday_core(a: &[Arg], c: &mut dyn Ctx, mask: [bool; 7], hol: Option<&Arg>)
if days == 0 {
return num_val(start as f64);
}
// The holidays on working days, still sorted (`holidays` sorts them), so each step counts
// the ones it passed with two binary searches rather than a scan of every holiday: a run of
// consecutive holidays takes one step per holiday.
let hs: Vec<i64> = hs.into_iter().filter(|&h| !mask[weekday0(sys, h)]).collect();
let dir = days.signum();
let mut cur = start;
let mut rem = days.abs();
while rem > 0 {
let next = advance(sys, cur, rem, dir, &mask, per_week);
let (lo, hi) = if dir > 0 { (cur + 1, next) } else { (next, cur - 1) };
rem = hs.partition_point(|&h| h <= hi).saturating_sub(hs.partition_point(|&h| h < lo)) as i64;
rem = hs.iter().filter(|&&h| h >= lo && h <= hi && !mask[weekday0(sys, h)]).count() as i64;
cur = next;
if !(0..=MAX_SERIAL as i64).contains(&cur) {
return Err(CellError::Num);
@@ -759,23 +755,6 @@ mod tests {
is_err(ev("TIMEVALUE", vec![t("x")]), CellError::Value);
}
/// Security: WORKDAY counts the holidays each step passes by binary search, so a long run of
/// consecutive holidays is linear work, not one scan of every holiday per holiday.
#[test]
fn workday_over_many_consecutive_holidays() {
let start = d(2000.0, 1.0, 3.0);
let hol: Vec<f64> = (1..=200_000).map(|i| start + f64::from(i)).collect();
let out = ev("WORKDAY.INTL", vec![n(start), n(1.0), n(11.0), av(col(&hol))]);
// Sundays are the only weekend (11): 200,000 days of holidays are skipped, landing on the
// first working day after them.
let after = start + 200_001.0;
let sunday = ev("WEEKDAY", vec![n(after), n(2.0)]) == nv(7.0);
close(out, if sunday { after + 1.0 } else { after });
// Holidays on a weekend don't push the result out.
let fri = d(2026.0, 10.0, 9.0);
close(ev("WORKDAY", vec![n(fri), n(1.0), av(col(&[fri + 1.0, fri + 3.0]))]), fri + 4.0);
}
#[test]
fn day_counts() {
close(ev("DAYS", vec![n(d(2021.0, 3.0, 15.0)), n(d(2021.0, 2.0, 1.0))]), 42.0);
+13 -44
View File
@@ -178,10 +178,6 @@ fn sort(args: &[Arg], _c: &mut dyn Ctx) -> R<Value> {
return Err(CellError::Value);
}
let orders: Vec<i64> = (0..idx.len()).map(|i| *ord.get(i).or(ord.first()).unwrap_or(&1)).collect();
// A column repeated in sort_index can't break a tie its first use left, so only first uses
// stay: the keys copied per row are then at most the row itself, however long sort_index is.
let mut seen = std::collections::HashSet::new();
let (idx, orders): (Vec<i64>, Vec<i64>) = idx.into_iter().zip(orders).filter(|(i, _)| seen.insert(*i)).unzip();
let mut lines: Vec<(Vec<Value>, Vec<Value>)> =
rows_of(&a).into_iter().map(|r| (idx.iter().map(|&i| r.get(i as usize - 1).cloned().unwrap_or(Value::Empty)).collect(), r)).collect();
lines.sort_by(|x, y| cmp_keys(&x.0, &y.0, &orders));
@@ -838,16 +834,9 @@ fn pivotby(args: &[Arg], ctx: &mut dyn Ctx) -> R<Value> {
let ckeys: Vec<Vec<Value>> = p.rows.iter().map(|&r| key_row(&cf, r)).collect();
let rk = distinct_keys(&rkeys, if row_sort.unsigned_abs() as usize <= rf.cols { row_sort } else { 0 });
let ck = distinct_keys(&ckeys, if col_sort.unsigned_abs() as usize <= cf.cols { col_sort } else { 0 });
// Source rows of every row key, column key and pair, built in one pass, so a total reads
// its rows instead of rescanning the whole source once per total.
let mut cell: HashMap<(String, String), Vec<usize>> = HashMap::new();
let mut by_row: HashMap<String, Vec<usize>> = HashMap::new();
let mut by_col: HashMap<String, Vec<usize>> = HashMap::new();
for ((r, c), &i) in rkeys.iter().zip(ckeys.iter()).zip(p.rows.iter()) {
let (r, c) = (row_key(r), row_key(c));
by_row.entry(r.clone()).or_default().push(i);
by_col.entry(c.clone()).or_default().push(i);
cell.entry((r, c)).or_default().push(i);
cell.entry((row_key(r), row_key(c))).or_default().push(i);
}
let mut agg = |rows: &[usize], c: usize| -> Value {
if rows.is_empty() {
@@ -857,12 +846,14 @@ fn pivotby(args: &[Arg], ctx: &mut dyn Ctx) -> R<Value> {
aggregate_with(f, &vs, ctx)
};
let rows_for = |r: Option<&Vec<Value>>, c: Option<&Vec<Value>>| -> Vec<usize> {
match (r, c) {
(Some(r), Some(c)) => cell.get(&(row_key(r), row_key(c))).cloned().unwrap_or_default(),
(Some(r), None) => by_row.get(&row_key(r)).cloned().unwrap_or_default(),
(None, Some(c)) => by_col.get(&row_key(c)).cloned().unwrap_or_default(),
(None, None) => p.rows.clone(),
}
let rkey = r.map(|k| row_key(k));
let ckey = c.map(|k| row_key(k));
p.rows
.iter()
.zip(rkeys.iter().zip(ckeys.iter()))
.filter(|(_, (a, b))| rkey.as_ref().is_none_or(|k| *k == row_key(a)) && ckey.as_ref().is_none_or(|k| *k == row_key(b)))
.map(|(&i, _)| i)
.collect()
};
let mut cols: Vec<Option<&Vec<Value>>> = ck.iter().map(Some).collect();
if col_total > 0 {
@@ -912,7 +903,10 @@ fn pivotby(args: &[Arg], ctx: &mut dyn Ctx) -> R<Value> {
}
};
for c in &cols {
let rows = rows_for(r, *c);
let rows = match (r, c) {
(Some(rk), Some(ck)) => cell.get(&(row_key(rk), row_key(ck))).cloned().unwrap_or_default(),
_ => rows_for(r, *c),
};
for vc in 0..vals.cols {
line.push(agg(&rows, vc));
}
@@ -1116,21 +1110,6 @@ mod tests {
assert_eq!(rows_of(&r), vec![vec![nv(0.0), nv(5.0)], vec![nv(1.0), nv(2.0)], vec![nv(1.0), nv(1.0)]]);
}
/// Security: a column repeated in sort_index keys the sort once. The keys copied per row stay
/// within the row, however long sort_index is, and a repeat (even with the other order) can't
/// change the result, since its first use already ordered the rows.
#[test]
fn sort_drops_repeated_sort_indices() {
let m = || av(arr(vec![vec![nv(1.0), nv(2.0)], vec![nv(1.0), nv(1.0)], vec![nv(0.0), nv(5.0)]]));
let want = vec![vec![nv(0.0), nv(5.0)], vec![nv(1.0), nv(2.0)], vec![nv(1.0), nv(1.0)]];
let r = ev("SORT", vec![m(), av(row(&[1.0, 1.0, 2.0, 1.0])), av(row(&[1.0, -1.0, -1.0, 1.0]))]);
assert_eq!(rows_of(&r), want);
let idx = vec![1.0; 1_000_000];
let tall = av(gridcraft_core::Array::filled(2_000, 1, nv(1.0)).into());
let r = ev("SORT", vec![tall, av(row(&idx))]);
assert_eq!(rows_of(&r).len(), 2_000);
}
#[test]
fn sortby() {
let r = ev("SORTBY", vec![av(col(&[1.0, 2.0, 3.0])), av(col(&[30.0, 10.0, 20.0]))]);
@@ -1302,16 +1281,6 @@ mod tests {
);
}
/// Security: PIVOTBY totals read their source rows from indexes built in the grouping pass,
/// rather than rescanning the source once per total, so many distinct keys stay linear.
#[test]
fn pivotby_totals_with_many_groups() {
let keys: Vec<f64> = (0..20_000).map(f64::from).collect();
let r = rows_of(&ev("PIVOTBY", vec![av(col(&keys)), av(col(&vec![1.0; 20_000])), av(col(&vec![2.0; 20_000])), t("SUM")]));
assert_eq!(r.get(1), Some(&vec![nv(0.0), nv(2.0), nv(2.0)]));
assert_eq!(r.last(), Some(&vec![tv("Total"), nv(40_000.0), nv(40_000.0)]));
}
#[test]
fn pivotby_size_check_counts_column_header_rows() {
// One row, n column fields and n value columns: n header rows + value-name row + body row + total row,
+133 -25
View File
@@ -3,7 +3,7 @@
use gridcraft_core::{Array, CellError, Value};
use crate::util::MAX_CELLS;
use crate::util::{MAX_CELLS, num};
use crate::{Arg, Ctx, FnSpec};
/// Value-parameter masks (see [`FnSpec::value_args`]), observed in Excel 365: a reference to more
@@ -68,26 +68,68 @@ const SCANS: &[&str] = &["VLOOKUP", "HLOOKUP", "LOOKUP", "MATCH", "XLOOKUP", "XM
/// `XMATCH(SEQUENCE(1048576), -SEQUENCE(1048576))` runs for hours and hangs opening the file.
/// Past the budget the call is `#NUM!`, as MMULT is past 2e9 multiply-adds. 5e8 keeps the worst
/// case to a few seconds and still admits 5,000 needles over a 100,000-row table.
///
/// The same budget covers all the work of a lifted call added up across its elements: criteria
/// aggregates rescanning their ranges, period loops, and array results built per element.
const LIFTED_SCAN_BUDGET: usize = 500_000_000;
/// The length of the longest vector a lifted call of `spec` searches per needle: 0 unless `spec`
/// is one of the [`SCANS`] lookups and a non-lifted argument is an array.
/// Criteria aggregates: every call tests each cell of its ranges against its criteria, so a call
/// lifted over `n` criteria does `n` x (range cells) work, like the [`SCANS`] lookups.
const CRITERIA: &[&str] = &["COUNTIF", "COUNTIFS", "SUMIF", "SUMIFS", "AVERAGEIF", "AVERAGEIFS", "MAXIFS", "MINIFS"];
/// Cumulative TVM functions that step through every period from `start_period` (argument 3) to
/// `end_period` (argument 4) on each call.
const PERIOD_LOOPS: &[&str] = &["CUMIPMT", "CUMPRINC"];
/// The work every lifted call of `spec` repeats over its non-lifted arguments: the longest vector
/// a [`SCANS`] lookup searches, or every cell a [`CRITERIA`] aggregate tests. 0 for other
/// functions.
fn scan_len(spec: &FnSpec, args: &[Arg]) -> usize {
if !SCANS.contains(&spec.name) {
let scans = SCANS.contains(&spec.name);
if !scans && !CRITERIA.contains(&spec.name) {
return 0;
}
args.iter()
.enumerate()
.filter(|(i, _)| !spec.is_scalar_arg(*i))
.filter_map(|(_, a)| match &a.value {
Value::Array(arr) => Some(arr.rows.max(arr.cols)),
_ => None,
})
.max()
.unwrap_or(0)
let lens = args.iter().enumerate().filter(|(i, _)| !spec.is_scalar_arg(*i)).filter_map(|(_, a)| match &a.value {
Value::Array(arr) => Some(if scans { arr.rows.max(arr.cols) } else { arr.data.len() }),
_ => None,
});
if scans { lens.max().unwrap_or(0) } else { lens.fold(0, usize::saturating_add) }
}
/// The work of one lifted call that depends on its own (lifted) scalar arguments: the number of
/// periods a [`PERIOD_LOOPS`] function steps through. 0 for other functions.
fn call_cost(spec: &FnSpec, cell_args: &[Arg]) -> usize {
if !PERIOD_LOOPS.contains(&spec.name) {
return 0;
}
// The function's own coercion (text like "10000000" counts), so no loop goes uncharged.
let period = |i: usize| num(cell_args, i).ok().filter(|n| n.is_finite()).map(f64::trunc);
match (period(3), period(4)) {
// The function itself refuses spans past 1e7 periods; anything larger costs the cap.
(Some(start), Some(end)) if end >= start => ((end - start) + 1.0).min(1e8) as usize,
_ => 0,
}
}
pub(crate) fn call_lifted(spec: &FnSpec, args: &[Arg], ctx: &mut dyn Ctx) -> Value {
call_lifted_within(spec, args, ctx, LIFTED_SCAN_BUDGET)
}
/// Sets the lifted arguments of `cell_args` to their elements at (`r`, `c`).
fn element_args(spec: &FnSpec, args: &[Arg], r: usize, c: usize, cell_args: &mut [Arg]) {
for (i, a) in args.iter().enumerate() {
if spec.is_scalar_arg(i)
&& let Value::Array(arr) = &a.value
&& let Some(slot) = cell_args.get_mut(i)
{
let v = arr.get_broadcast(r, c);
slot.value = if let Value::Array(inner) = &v { inner.data.first().cloned().unwrap_or(Value::Empty) } else { v };
}
}
}
/// [`call_lifted`] with every call's work added up against `budget` (see [`LIFTED_SCAN_BUDGET`]).
fn call_lifted_within(spec: &FnSpec, args: &[Arg], ctx: &mut dyn Ctx, budget: usize) -> Value {
let rejected = if spec.value_args.is_empty() { None } else { reject_multi_cell_refs(spec, args) };
let args = rejected.as_deref().unwrap_or(args);
let mut rows = 0usize;
@@ -106,25 +148,42 @@ pub(crate) fn call_lifted(spec: &FnSpec, args: &[Arg], ctx: &mut dyn Ctx) -> Val
return (spec.imp)(args, ctx);
}
let needles = rows.saturating_mul(cols);
if needles > MAX_CELLS || needles.saturating_mul(scan_len(spec, args)) > LIFTED_SCAN_BUDGET {
// Every call is charged its repeated scan (at least 1), its own period loop, and the cells of
// an array result (built in full, though only its first cell is kept), so lifting a scalar
// argument can't multiply what one call may do into hours of work:
// `COUNTIF(big_range, SEQUENCE(1e4))`, `CUMIPMT(0.1, 1e7, 1, 1, SEQUENCE(1e4,,1e7,0), 0)`,
// `MUNIT(SEQUENCE(1e4,,3000,0))`.
let per_call = scan_len(spec, args).max(1);
let mut cell_args: Vec<Arg> = args.to_vec();
let mut spent = needles.saturating_mul(per_call);
if needles > MAX_CELLS || spent > budget {
return Value::Error(CellError::Num);
}
let mut cell_args: Vec<Arg> = args.to_vec();
// Period loops are known before any call runs, so a call over budget does none of them.
if PERIOD_LOOPS.contains(&spec.name) {
for r in 0..rows {
for c in 0..cols {
element_args(spec, args, r, c, &mut cell_args);
spent = spent.saturating_add(call_cost(spec, &cell_args));
if spent > budget {
return Value::Error(CellError::Num);
}
}
}
}
let mut data = Vec::with_capacity(rows * cols);
for r in 0..rows {
for c in 0..cols {
for (i, a) in args.iter().enumerate() {
if spec.is_scalar_arg(i)
&& let Value::Array(arr) = &a.value
&& let Some(slot) = cell_args.get_mut(i)
{
let v = arr.get_broadcast(r, c);
slot.value = if let Value::Array(inner) = &v { inner.data.first().cloned().unwrap_or(Value::Empty) } else { v };
}
}
element_args(spec, args, r, c, &mut cell_args);
let out = (spec.imp)(&cell_args, ctx);
data.push(match out {
Value::Array(a) => a.data.first().cloned().unwrap_or(Value::Empty),
Value::Array(a) => {
spent = spent.saturating_add(a.data.len());
if spent > budget {
return Value::Error(CellError::Num);
}
a.data.first().cloned().unwrap_or(Value::Empty)
}
v => v,
});
}
@@ -209,6 +268,55 @@ mod tests {
assert_eq!(rows_of(&out), vec![vec![nv(-1.0)]; 1000]);
}
/// Security: lifting a criteria aggregate rescans its range for every criterion, so criteria
/// x range cells past the budget is `#NUM!` before any scan runs.
#[test]
fn lifted_criteria_aggregates_stop_past_their_work_budget() {
use gridcraft_core::Array;
let tall = |n: usize| av(Array::filled(n, 1, nv(-1.0)).into());
is_err(ev("COUNTIF", vec![tall(1_000_000), tall(501)]), CellError::Num);
is_err(ev("SUMIF", vec![tall(1_000_000), tall(501)]), CellError::Num);
is_err(ev("COUNTIFS", vec![tall(1_000_000), tall(501)]), CellError::Num);
// Two ranges of 250,000 cells each count together.
is_err(ev("SUMIFS", vec![tall(250_000), tall(250_000), tall(1001)]), CellError::Num);
let counts = ev("COUNTIF", vec![av(col(&[1.0, 2.0, 2.0])), av(col(&[2.0, 1.0, 5.0]))]);
assert_eq!(rows_of(&counts), vec![vec![nv(2.0)], vec![nv(1.0)], vec![nv(0.0)]]);
for name in super::CRITERIA.iter().chain(super::PERIOD_LOOPS) {
assert!(crate::lookup(name).is_some(), "{name} isn't registered");
}
}
/// Security: CUMIPMT and CUMPRINC loop over their periods once per lifted element; the
/// periods of all elements are added up first, so an over-budget call runs none of them.
#[test]
fn lifted_period_loops_stop_past_their_work_budget() {
use gridcraft_core::Array;
let ends = |len: usize, end: f64| av(Array::filled(len, 1, nv(end)).into());
let args = |len: usize, end: f64| vec![n(0.01), n(1e7), n(1000.0), n(1.0), ends(len, end), n(0.0)];
// 51 elements of 1e7 periods each is past 5e8.
is_err(ev("CUMIPMT", args(51, 1e7)), CellError::Num);
is_err(ev("CUMPRINC", args(51, 1e7)), CellError::Num);
// Numeric text is coerced by the function, so its periods are charged the same.
let text_ends = av(Array::filled(51, 1, Value::text("10000000")).into());
is_err(ev("CUMIPMT", vec![n(0.01), n(1e7), n(1000.0), n(1.0), text_ends, n(0.0)]), CellError::Num);
let out = ev("CUMIPMT", args(2, 12.0));
assert_eq!(rows_of(&out).len(), 2);
assert!(matches!(rows_of(&out).first().and_then(|r| r.first()), Some(Value::Number(x)) if *x < 0.0));
}
/// Security: an array-producing function lifted over a computed array builds each element's
/// whole result before keeping one cell; their cells count against the budget.
#[test]
fn lifted_array_results_count_against_the_budget() {
use gridcraft_core::Array;
let spec = crate::lookup("MUNIT").unwrap_or_else(|| panic!("MUNIT isn't registered"));
let dims = |n: usize| [av(Array::filled(n, 1, nv(10.0)).into())];
// Ten 10x10 identity matrices are 1,000 cells.
let out = super::call_lifted_within(spec, &dims(10), &mut TestCtx::default(), 1_010);
assert_eq!(rows_of(&out), vec![vec![nv(1.0)]; 10]);
is_err(super::call_lifted_within(spec, &dims(11), &mut TestCtx::default(), 1_010), CellError::Num);
}
#[test]
fn value_params_reject_multi_cell_references() {
let s = FnSpec { value_args: &[true, false], ..spec() };