aleleba/gridcraft
1
0
Fork 0
mirror of https://github.com/storytold/gridcraft.git synced 2026-10-24 10:46:50 -06:00
Code Issues Packages Projects Releases Wiki Activity

calc: bound SUBTOTAL/AGGREGATE input work (#2175)

Browse Source 
Every area, array and value of one SUBTOTAL/AGGREGATE call is charged against MAX_CELLS before any cell is read; past it the call returns #NUM!. A value stored in XFD1048576 no longer makes SUBTOTAL(9,A:XFD) read ~17 billion cells into a vector.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Higgs32584
2026-10-21 20:10:00 -04:00
co-authored by Claude Opus 5.5
parent 912b817abe
commit be94b011c5
3 changed files with 94 additions and 27 deletions
Download Patch File Download Diff File Expand all files Collapse all files
crates/calc/src/eval.rs
+57 -26
View File
@@ -1422,43 +1422,74 @@ impl<'h> Evaluator<'h> {
if name == "AGGREGATE" && func >= 14 {
k = rest.get(1).map(|e| self.value(e));
}
// Every cell visited (even one skipped as hidden, nested or an error) and every value
// counts against one budget for the whole call. All arguments are evaluated (references
// stay lazy) and charged before any cell is read: a value stored in XFD1048576 makes
// `A:XFD` trim to the whole sheet, ~17 billion cells (#2175).
enum Input {
Cells(usize, RangeRef),
Values(Value),
}
let mut inputs = Vec::new();
let mut budget = MAX_CELLS;
for e in refs {
match self.eval(e) {
Ev::R(areas) => {
for a in areas {
let range = self.trim(&a);
for c in range.iter() {
let (hidden, nested) = {
let wb = self.host.workbook();
let s = wb.sheet(a.sheet);
let hidden =
s.is_some_and(|s| if skip_hidden { s.is_row_hidden(c.row) } else { skip_filtered && s.is_row_filtered(c.row) });
let nested = skip_nested
&& s.and_then(|s| s.cell(c)).and_then(|cell| cell.formula.as_ref()).is_some_and(|f| {
let u = f.text.to_ascii_uppercase();
u.contains("SUBTOTAL(") || u.contains("AGGREGATE(")
});
(hidden, nested)
};
if hidden || nested {
continue;
}
let v = self.host.cell_value(a.sheet, c);
if skip_errors && v.is_error() {
continue;
}
values.push(v);
}
budget = match budget.checked_sub(range.count()) {
Some(left) => left,
None => return err(CellError::Num),
};
inputs.push(Input::Cells(a.sheet, range));
}
}
Ev::V(Value::Array(a)) => values.extend(a.data.iter().filter(|v| !(skip_errors && v.is_error())).cloned()),
Ev::V(v) => values.push(v),
Ev::V(v) => {
let n = match &v {
Value::Array(a) => u64::try_from(a.data.len()).unwrap_or(u64::MAX),
_ => 1,
};
budget = match budget.checked_sub(n) {
Some(left) => left,
None => return err(CellError::Num),
};
inputs.push(Input::Values(v));
}
Ev::L(_) => return err(CellError::Value),
}
}
for input in inputs {
match input {
Input::Cells(sheet, range) => {
for c in range.iter() {
let (hidden, nested) = {
let wb = self.host.workbook();
let s = wb.sheet(sheet);
let hidden =
s.is_some_and(|s| if skip_hidden { s.is_row_hidden(c.row) } else { skip_filtered && s.is_row_filtered(c.row) });
let nested = skip_nested
&& s.and_then(|s| s.cell(c)).and_then(|cell| cell.formula.as_ref()).is_some_and(|f| {
let u = f.text.to_ascii_uppercase();
u.contains("SUBTOTAL(") || u.contains("AGGREGATE(")
});
(hidden, nested)
};
if hidden || nested {
continue;
}
let v = self.host.cell_value(sheet, c);
if skip_errors && v.is_error() {
continue;
}
values.push(v);
}
}
Input::Values(Value::Array(a)) => values.extend(a.data.iter().filter(|v| !(skip_errors && v.is_error())).cloned()),
Input::Values(v) => values.push(v),
}
}
if let Some(k) = k {
values.push(Value::Empty);
return Ev::V(gridcraft_functions::aggregate_values_k(func, &values[..values.len() - 1], &k));
return Ev::V(gridcraft_functions::aggregate_values_k(func, &values, &k));
}
Ev::V(gridcraft_functions::aggregate_values(func, &values))
}
crates/calc/src/tests.rs
+34
View File
@@ -738,6 +738,40 @@ fn subtotal_109_skips_nested_subtotal() {
assert_eq!(t.num("B2"), 3.0);
}
/// One value stored in the sheet's last cell makes `A:XFD` trim to ~17 billion cells. SUBTOTAL
/// and AGGREGATE used to read every one of them into a vector (an abort on open, #2175); now
/// every argument is charged against `MAX_CELLS` before any cell is read, and `#NUM!` comes back.
#[test]
fn subtotal_and_aggregate_cap_cells_visited() {
let mut t = T::new();
t.set("XFD1048576", "1");
let formulas = [
"=SUBTOTAL(9,A:XFD)",
"=SUBTOTAL(109,1:1048576)",
"=AGGREGATE(9,6,A:XFD)",
"=AGGREGATE(14,7,A:XFD,1)",
// A:J and K:T are about 10.5 million cells each: under the limit alone, over it together,
// so the second area is refused before the first is read.
"=SUBTOTAL(9,A:J,K:T)",
"=AGGREGATE(9,4,(A:J,K:T))",
];
for (i, f) in formulas.iter().enumerate() {
let at = format!("B{}", i + 1);
t.set(&at, f);
assert_eq!(t.get(&at), Value::Error(CellError::Num), "{f}");
}
// Whole columns and rows still clip to the used range when it is small.
let mut t = T::new();
t.set("A1", "1");
t.set("B2", "2");
t.set("C2", "#N/A");
t.set("E5", "=SUBTOTAL(9,A:B)");
t.set("E6", "=AGGREGATE(9,6,1:2)");
t.set("E7", "=AGGREGATE(14,6,A:C,1)");
assert_eq!((t.num("E5"), t.num("E6"), t.num("E7")), (3.0, 3.0, 2.0));
}
/// SUBTOTAL leaves out rows a filter hides for every function number, while 1-11 keep rows
/// hidden by hand (as in ConditionalFormattingSamples.xlsx, where `SUBTOTAL(9,Table69[Sales])`
/// under a filtered table adds only the visible rows).
docs/function-parity.md
+3 -1
View File
@@ -1,6 +1,6 @@
# Function parity: worksheet functions and formulas
> **Last reviewed:** 2026-10-15 · **Last updated:** 2026-10-15 · **Change:** minor (MEDIAN and AGGREGATE median avoid intermediate overflow) · **Target:** Microsoft Excel (Microsoft 365), Excel for Mac 16.113.4
> **Last reviewed:** 2026-10-15 · **Last updated:** 2026-10-21 · **Change:** minor (SUBTOTAL and AGGREGATE cap the cells they visit, #2175) · **Target:** Microsoft Excel (Microsoft 365), Excel for Mac 16.113.4
Excel's worksheet functions, category by category, against GridCraft's function library. Part of
[target-app-parity.md](target-app-parity.md); gaps feed [gaps.md](gaps.md).
@@ -77,6 +77,7 @@ PIVOTBY, REGEXTEST…) are all present.
| Defined names (workbook and sheet scope), Create from Selection | done |
| Dynamic arrays, spill, `#SPILL!`, spill operator `A1#` | done; stale-spill bugs (#133, #139; PRs #148, #157) |
| Recalculation after clearing large sparse ranges | every stored cell changed by Clear is reported to recalculation, without a million-coordinate cutoff; removed formula spills are cleared, and removing empty merges still unblocks spills (#1032) |
| SUBTOTAL / AGGREGATE input size | references trim to the used range; all areas, arrays and values of one call are charged against the 16-million-cell limit before any cell is read (skipped hidden, nested and error cells count), and `#NUM!` past it (#2175) |
| Implicit intersection `@`, legacy CSE array formulas | done; legacy array range fill (PR #156) |
| Trim references `A1:.E10` (trailing), `A1.:E10` (leading), `A1.:.E10` (both), also on whole rows/columns (`A:.A`) | done; evaluated as `TRIMRANGE(range, code, code)`, adjusted on insert/delete; files store `_xlfn.TRIMRANGE(range,code,code)` (Excel's own file spelling unconfirmed in public docs), `_xlfn._TRO_ALL/_TRO_LEADING/_TRO_TRAILING(...)` read too |
| LET, LAMBDA, named LAMBDAs, ISOMITTED, MAP/REDUCE/SCAN/BYROW/BYCOL/MAKEARRAY | done |
@@ -114,6 +115,7 @@ workbooks; results checked in as data, not Excel files) and fixing what it finds
| Date | Change | Summary |
|---|---|---|
| 2026-10-21 | minor | SUBTOTAL and AGGREGATE charge every argument against the evaluator's 16-million-cell limit before scanning; a value in XFD1048576 no longer makes `SUBTOTAL(9,A:XFD)` read the whole sheet (#2175) |
| 2026-10-15 | minor | MEDIAN and AGGREGATE median use an overflow-safe midpoint (#1579); regressions cover finite extremes, tiny values and dependent recalculation |
| 2026-10-13 | minor | Clear reports changed stored cells sparsely, including those beyond the first million coordinates; dependent formulas and removed spills update, while clearing empty merges still unblocks spills (#1032) |
| 2026-10-12 | minor | USDOLLAR added as a Compatibility function (not on Microsoft's list, so the counts don't change); MIRR with no positive flow is -1, COUNTIF's bare `"<="` / `">="` skip empty strings, INDIRECT with `a1` = FALSE gives `#REF!` for A1-style text (#738) |