Compare commits

..
Author SHA1 Message Date
Dylan M. Taylor 6c9f00a335 address review: count label cells in the consolidate output limit 2026-11-04 01:50:44 +00:00
Dylan M. Taylor 7d8c69873d engine: Consolidate finds keys through hash maps and refuses an oversized table
data.consolidate searched the keys collected so far for every row and
column key, so 100,000 positional rows cost about five billion
comparisons. It also built every row × column combination without a
size check. Keys are now indexed, and a table over 5 million cells is
refused before it is built.
2026-11-03 16:35:20 +00:00
5 changed files with 45 additions and 61 deletions
+31 -10
View File
@@ -61,6 +61,24 @@ enum Key {
const MAX_CONSOLIDATE_CELLS: u64 = 5_000_000;
/// The cells a consolidated table writes: every row key meets every column key, plus a label for
/// each key on the sides that have labels.
fn output_cells(rows: usize, cols: usize, top: bool, left: bool) -> u64 {
let (r, c) = (rows as u64, cols as u64);
r.saturating_mul(c).saturating_add(if top { c } else { 0 }).saturating_add(if left { r } else { 0 })
}
#[cfg(test)]
#[test]
fn the_output_limit_counts_label_cells() {
// A million row keys and four column keys are 4 million cells, 5 million with the row labels.
assert_eq!(output_cells(1_000_000, 4, false, false), 4_000_000);
assert_eq!(output_cells(1_000_000, 4, false, true), 5_000_000);
assert!(output_cells(1_000_000, 5, false, true) > MAX_CONSOLIDATE_CELLS);
assert_eq!(output_cells(3, 2, true, true), 6 + 2 + 3);
assert_eq!(output_cells(usize::MAX, usize::MAX, true, true), u64::MAX);
}
pub(super) fn consolidate(s: &mut Session, p: &Json) -> Result<Json> {
const CMD: &str = "data.consolidate";
let func = str_param(p, "function").unwrap_or("sum").to_string();
@@ -89,20 +107,20 @@ pub(super) fn consolidate(s: &mut Session, p: &Json) -> Result<Json> {
}
None => (d.wb.active_sheet(), d.selection.active),
};
// Keys in first-seen order, each with its index: a lookup is one probe, not a scan.
let mut row_keys: Vec<Key> = Vec::new();
let mut col_keys: Vec<Key> = Vec::new();
let mut row_index: HashMap<Key, usize> = HashMap::new();
let mut col_index: HashMap<Key, usize> = HashMap::new();
let mut row_label: HashMap<Key, String> = HashMap::new();
let mut col_label: HashMap<Key, String> = HashMap::new();
let mut aggs: HashMap<(usize, usize), Agg> = HashMap::new();
let mut total: u64 = 0;
let index_of = |keys: &mut Vec<Key>, k: Key| -> usize {
match keys.iter().position(|x| *x == k) {
Some(i) => i,
None => {
keys.push(k);
keys.len() - 1
}
}
let index_of = |keys: &mut Vec<Key>, index: &mut HashMap<Key, usize>, k: Key| -> usize {
*index.entry(k).or_insert_with_key(|k| {
keys.push(k.clone());
keys.len() - 1
})
};
for t in &srcs {
let (si, r) = parse_range(&d.wb, t, CMD)?;
@@ -131,7 +149,7 @@ pub(super) fn consolidate(s: &mut Session, p: &Json) -> Result<Json> {
} else {
Key::Pos(col - c0)
};
cols.push((col, index_of(&mut col_keys, k)));
cols.push((col, index_of(&mut col_keys, &mut col_index, k)));
}
for row in r0..=r.end.row {
let k = if left {
@@ -145,7 +163,7 @@ pub(super) fn consolidate(s: &mut Session, p: &Json) -> Result<Json> {
} else {
Key::Pos(row - r0)
};
let ri = index_of(&mut row_keys, k);
let ri = index_of(&mut row_keys, &mut row_index, k);
for (col, ci) in &cols {
aggs.entry((ri, *ci)).or_default().add(&sh.value(CellRef::new(row, *col)));
}
@@ -157,6 +175,9 @@ pub(super) fn consolidate(s: &mut Session, p: &Json) -> Result<Json> {
{
return Err(EngineError::Other("The consolidated data doesn't fit below and right of the destination.".into()));
}
if output_cells(row_keys.len(), col_keys.len(), top, left) > MAX_CONSOLIDATE_CELLS {
return Err(bad(CMD, "the consolidated table would be too large"));
}
let mut cells: Vec<(CellRef, Value)> = Vec::new();
if top {
for (ci, k) in col_keys.iter().enumerate() {
+13
View File
@@ -297,3 +297,16 @@ fn consolidate_by_position_and_label() {
assert!(s.execute("data.consolidate", json!({"sources": []})).is_err());
assert!(s.execute("data.consolidate", json!({"sources": ["A1"], "function": "median"})).is_err());
}
/// A hundred thousand positional rows take one key lookup each; a tall source and a wide one
/// whose keys would meet in a table of 160 million cells are refused before it is built.
#[test]
fn consolidate_keys_are_indexed_and_the_output_is_bounded() {
let mut s = s();
s.execute("range.setValues", json!({"range": "A1", "values": [[1], [2]]})).unwrap();
let r = s.execute("data.consolidate", json!({"sources": ["A1:A100000"], "destination": "C1"})).unwrap();
assert_eq!(r["rows"], 100_000);
assert_eq!(n(&s, "Sheet1!C2"), 2.0);
let r = s.execute("data.consolidate", json!({"sources": ["A1:A10000", "A1:XFC1"], "destination": "A1"}));
assert!(r.unwrap_err().to_string().contains("too large"));
}
+1 -15
View File
@@ -332,14 +332,6 @@ impl NumberFormat {
None => Formatted { text: t.to_string(), color: None, fill: None, numeric: false },
}
}
/// [`Self::format_text`]'s text, or `None` once it passes `max_units` UTF-16 code units.
fn format_text_within(&self, t: &str, max_units: usize) -> Option<String> {
match self.inner.text_section.and_then(|i| self.inner.sections.get(i)) {
Some(sec) => render::render_text_within(sec, t, max_units).map(|out| out.text),
None => (t.encode_utf16().count() <= max_units).then(|| t.to_string()),
}
}
}
/// Formats a cell value for display.
@@ -433,10 +425,6 @@ impl NumberFormat {
/// The `TEXT()` worksheet function: formats a value with a format code. Numeric text is
/// converted to a number first; other text and booleans (as `TRUE`/`FALSE`) go through the text
/// section, or are returned as is.
/// Longest text a cell holds, in UTF-16 code units (Excel's 32,767 characters, which XLSB also
/// stores that way), and so the longest `TEXT` result.
const MAX_CELL_TEXT_CHARS: usize = 32_767;
pub fn text_function(v: &Value, code: &str, sys: DateSystem) -> Result<String, CellError> {
if code.is_empty() {
return Ok(String::new());
@@ -452,9 +440,7 @@ pub fn text_function(v: &Value, code: &str, sys: DateSystem) -> Result<String, C
Value::Bool(b) => return Ok(fmt.format_text(if b { "TRUE" } else { "FALSE" }).text),
Value::Text(t) => match gridcraft_core::parse::parse_number_text(&t) {
Some(n) => n,
// Every `@` repeats the text: a result past a cell's limit is `#VALUE!`, so nested
// TEXT calls can't multiply it further.
None => return fmt.format_text_within(&t, MAX_CELL_TEXT_CHARS).ok_or(CellError::Value),
None => return Ok(fmt.format_text(&t).text),
},
Value::Array(_) => return Err(CellError::Value),
};
-29
View File
@@ -45,35 +45,6 @@ pub(crate) fn render_text(sec: &Section, text: &str) -> Out {
out
}
/// [`render_text`], but `None` once the output passes `max_units` UTF-16 code units (a cell's
/// limit), checked before each piece is appended, so an oversized result is never built.
pub(crate) fn render_text_within(sec: &Section, text: &str, max_units: usize) -> Option<Out> {
let text_units = text.encode_utf16().count();
let mut out = Out::default();
let mut units = 0usize;
for t in &sec.toks {
match t {
Tok::At => {
units = units.saturating_add(text_units);
if units > max_units {
return None;
}
out.push(text);
}
Tok::Fill(c) => out.fill(*c),
other => {
let literal = other.literal_text();
units = units.saturating_add(literal.encode_utf16().count());
if units > max_units {
return None;
}
out.push(&literal);
}
}
}
Some(out)
}
/// Renders a number through a General section (literals around the General keyword).
pub(crate) fn render_general(sec: &Section, v: f64, width: usize) -> Out {
let mut out = Out::default();
-7
View File
@@ -396,13 +396,6 @@ fn text_fn() {
assert_eq!(text_function(&Value::Number(1234.5), "$#,##0.00", S), Ok("$1,234.50".into()));
assert_eq!(text_function(&Value::from("5"), "0.00", S), Ok("5.00".into()));
assert_eq!(text_function(&Value::from("abc"), "0.00", S), Ok("abc".into()));
// Each `@` repeats the text: up to a cell's 32,767 characters, then #VALUE!.
let at = "@".repeat(255);
assert_eq!(text_function(&Value::from("x".repeat(128).as_str()), &at, S).map(|s| s.len()), Ok(128 * 255));
assert_eq!(text_function(&Value::from("x".repeat(32_000).as_str()), &at, S), Err(CellError::Value));
// The limit counts UTF-16 units: 16,384 emoji are 32,768 of them, one too many.
assert_eq!(text_function(&Value::from("\u{1F600}".repeat(16_383).as_str()), "@", S).map(|s| s.chars().count()), Ok(16_383));
assert_eq!(text_function(&Value::from("\u{1F600}".repeat(16_384).as_str()), "@", S), Err(CellError::Value));
assert_eq!(text_function(&Value::Error(CellError::NA), "0", S), Err(CellError::NA));
assert_eq!(text_function(&Value::Number(46302.0), "yyyy", S), Ok("2026".into()));
assert_eq!(text_function(&Value::Number(0.5), "General", S), Ok("0.5".into()));