mirror of
https://github.com/storytold/gridcraft.git
synced 2026-11-08 03:40:23 +00:00
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.
This commit is contained in:
@@ -89,20 +89,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 +131,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 +145,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 +157,10 @@ 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()));
|
||||
}
|
||||
// Every row key meets every column key in the output, so the product is what gets written.
|
||||
if (row_keys.len() as u64).saturating_mul(col_keys.len() as u64) > 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() {
|
||||
|
||||
@@ -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"));
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user