diff --git a/crates/engine/src/cmd/analysis.rs b/crates/engine/src/cmd/analysis.rs index 9cd10e50f..92699b75f 100644 --- a/crates/engine/src/cmd/analysis.rs +++ b/crates/engine/src/cmd/analysis.rs @@ -152,26 +152,32 @@ fn input_range(s: &Session, p: &Json, keys: &[&str]) -> Result<(usize, RangeRef) /// Splits a range into variables (columns, or rows when `by_rows`); with `labels` the first /// cell of each names it, else `default(i)` does. fn variables(s: &Session, si: usize, r: RangeRef, by_rows: bool, labels: bool, default: &dyn Fn(usize) -> String) -> Result> { - let d = s.doc()?; - let sh = d.wb.sheet(si).ok_or(EngineError::NoDocument)?; - let (outer, inner) = if by_rows { (r.height(), r.width()) } else { (r.width(), r.height()) }; + let outer = if by_rows { r.height() } else { r.width() }; let mut out = Vec::with_capacity(outer as usize); for i in 0..outer { - let cell = |j: u32| if by_rows { CellRef::new(r.start.row + i, r.start.col + j) } else { CellRef::new(r.start.row + j, r.start.col + i) }; - let mut vals: Vec = (0..inner).map(|j| sh.value(cell(j))).collect(); - let mut name = default(i as usize); - if labels && !vals.is_empty() { - let first = vals.remove(0); - let t = first.display(); - if !t.trim().is_empty() { - name = t; - } - } - out.push(Var { name, vals }); + out.push(variable(s, si, r, by_rows, labels, default, i)?); } Ok(out) } +/// Variable `i` of [`variables`], read alone so a caller that may stop early needn't build them all. +fn variable(s: &Session, si: usize, r: RangeRef, by_rows: bool, labels: bool, default: &dyn Fn(usize) -> String, i: u32) -> Result { + let d = s.doc()?; + let sh = d.wb.sheet(si).ok_or(EngineError::NoDocument)?; + let inner = if by_rows { r.width() } else { r.height() }; + let cell = |j: u32| if by_rows { CellRef::new(r.start.row + i, r.start.col + j) } else { CellRef::new(r.start.row + j, r.start.col + i) }; + let mut vals: Vec = (0..inner).map(|j| sh.value(cell(j))).collect(); + let mut name = default(i as usize); + if labels && !vals.is_empty() { + let first = vals.remove(0); + let t = first.display(); + if !t.trim().is_empty() { + name = t; + } + } + Ok(Var { name, vals }) +} + /// The numbers of a variable; blanks are skipped when `skip_blank`, anything else that isn't a /// number is an error. fn numbers(v: &[Value], skip_blank: bool) -> Result> { @@ -272,11 +278,25 @@ struct Grid { cells: BTreeMap<(u32, u32), Value>, /// Cells shown as percentages. pct: Vec<(u32, u32)>, + /// Rows and columns spanned by `cells`. + rows: u32, + cols: u32, + /// A cell was dropped because the report outgrew [`MAX_OUTPUT_CELLS`]: the report is refused. + too_large: bool, } impl Grid { + /// Stores a value. Past [`MAX_OUTPUT_CELLS`] (the most a report writes) nothing more is kept, + /// so a builder that would produce millions of entries stops allocating them, and + /// [`write_report`] refuses the report. fn put(&mut self, r: u32, c: u32, v: Value) { if !v.is_empty() { + let (rows, cols) = (self.rows.max(r.saturating_add(1)), self.cols.max(c.saturating_add(1))); + if self.too_large || u64::from(rows) * u64::from(cols) > MAX_OUTPUT_CELLS { + self.too_large = true; + return; + } + (self.rows, self.cols) = (rows, cols); self.cells.insert((r, c), v); } } @@ -296,9 +316,7 @@ impl Grid { } /// (rows, cols) spanned. fn extent(&self) -> (u32, u32) { - let rows = self.cells.keys().map(|(r, _)| r + 1).max().unwrap_or(0); - let cols = self.cells.keys().map(|(_, c)| c + 1).max().unwrap_or(0); - (rows, cols) + (self.rows, self.cols) } } @@ -310,7 +328,7 @@ fn idx(i: usize) -> u32 { fn descriptive(s: &Session, p: &Json) -> Result { let (si, r) = input_range(s, p, &["range", "input"])?; - let vars = variables(s, si, r, by_rows(p), flag(p, "labels"), &|i| format!("Column{}", i + 1))?; + let (by_rows, labels) = (by_rows(p), flag(p, "labels")); let summary = p.get("summary").map(|_| flag(p, "summary")).unwrap_or(true); let conf = confidence(p, "confidence")?; let kl = num(p, "kthLargest").filter(|k| *k >= 1.0).map(|k| k as usize); @@ -319,7 +337,14 @@ fn descriptive(s: &Session, p: &Json) -> Result { return Err(bad(CMD, "choose at least one of summary, confidence, kthLargest or kthSmallest")); } let mut g = Grid::default(); - for (i, v) in vars.iter().enumerate() { + // Variables are read one at a time: a million-row range grouped by rows is a million variables. + for i in 0..(if by_rows { r.height() } else { r.width() }) { + // The report is refused once the grid is too large: the rest would only build discarded labels. + if g.too_large { + break; + } + let v = variable(s, si, r, by_rows, labels, &|i| format!("Column{}", i + 1), i)?; + let i = i as usize; let x = numbers(&v.vals, true)?; let c = idx(i).saturating_mul(2); g.txt(0, c, &v.name); @@ -1749,7 +1774,7 @@ fn sampling(s: &Session, p: &Json, seed: Option) -> Result { fn write_report(s: &mut Session, p: &Json, tool: &str, g: Grid) -> Result { let (rows, cols) = g.extent(); let (rows, cols) = (rows.max(1), cols.max(1)); - if u64::from(rows) * u64::from(cols) > MAX_OUTPUT_CELLS { + if g.too_large || u64::from(rows) * u64::from(cols) > MAX_OUTPUT_CELLS { return Err(bad(CMD, "the report is too large")); } let d = s.doc()?; @@ -1914,6 +1939,28 @@ mod tests { x } + #[test] + fn a_report_stops_growing_past_the_output_limit() { + // Entries past the limit are not kept, and the report is refused. + let mut g = super::Grid::default(); + g.num(1999, 1999, 1.0); + assert!(!g.too_large); + g.num(0, 2000, 1.0); + assert!(g.too_large); + assert_eq!(g.cells.len(), 1); + assert_eq!(g.extent(), (2000, 2000)); + // Row-grouped statistics of a tall, almost empty column: one variable per row would make + // millions of report entries; the report is refused instead of built. + let mut s = s(); + s.execute("range.setValues", json!({"range": "A1", "values": [[1]]})).unwrap(); + s.execute("range.setValues", json!({"range": "A1000000", "values": [[2]]})).unwrap(); + let err = s + .execute("data.analysisToolPak", json!({"tool": "descriptive", "range": "A1:A1000000", "groupedBy": "rows", "output": "C1"})) + .expect_err("report too large"); + assert!(err.to_string().contains("too large"), "{err}"); + assert_eq!(v(&s, "C1"), Value::Empty); + } + #[test] fn forecast_standard_errors_match_the_full_window_rescan() { // #1043: the running sum must give what re-adding every window gave, gaps included.