mirror of
https://github.com/storytold/gridcraft.git
synced 2026-10-26 11:46:49 -06:00
Repeated headers: number pivot fields and table columns in linear time
Each header was checked against every name before it, once per suffix tried, so a source whose headers repeat (a row of thousands of equal labels, such as check boxes) took cubic time: 4,000 columns took 28 s in Recommended PivotTables, and a row of 16,384 hung the app, in PivotTables, in Insert Table and in Get Data from Table/Range. Taken names and the next suffix of each header are now kept in hash maps, in the pivot source, in table creation and in the XLSX pivot cache; the names are the same as before.
This commit is contained in:
@@ -228,17 +228,24 @@ fn insert_table(s: &mut Session, p: &Json) -> Result<Json> {
|
||||
};
|
||||
let sh = cx.sheet_mut(sheet)?;
|
||||
let mut names: Vec<String> = Vec::new();
|
||||
// Lower-case names taken, and the next suffix for each header, so that thousands of equal
|
||||
// headers don't take cubic time.
|
||||
let mut taken = std::collections::HashSet::new();
|
||||
let mut next_suffix = std::collections::HashMap::new();
|
||||
for c in r.start.col..=r.end.col {
|
||||
let mut n = sh.value(CellRef::new(r.start.row, c)).display();
|
||||
if n.is_empty() {
|
||||
n = format!("Column{}", c - r.start.col + 1);
|
||||
}
|
||||
let base = n.clone();
|
||||
let mut k = 2;
|
||||
while names.iter().any(|x| x.eq_ignore_ascii_case(&n)) {
|
||||
let key = base.to_ascii_lowercase();
|
||||
let mut k = next_suffix.get(&key).copied().unwrap_or(2);
|
||||
while taken.contains(&n.to_ascii_lowercase()) {
|
||||
n = format!("{base}{k}");
|
||||
k += 1;
|
||||
}
|
||||
next_suffix.insert(key, k);
|
||||
taken.insert(n.to_ascii_lowercase());
|
||||
// Headers are text.
|
||||
sh.set_value(CellRef::new(r.start.row, c), gridcraft_core::Value::text(n.as_str()));
|
||||
names.push(n);
|
||||
|
||||
@@ -1621,6 +1621,32 @@ mod tests {
|
||||
assert_eq!(val(&s, &CellRef::new(gt, 1).a1()), Value::Number(72.0));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn repeated_headers_get_numbered_names() {
|
||||
let mut s = Session::new();
|
||||
s.new_workbook();
|
||||
let mut header = vec![json!("a"), json!("A"), json!("a"), json!("a2")];
|
||||
// Thousands of equal headers (a row of check boxes, say) used to take cubic time.
|
||||
header.extend((0..3000).map(|_| json!("x")));
|
||||
let data: Vec<_> = (0..header.len()).map(|i| json!(i)).collect();
|
||||
let end = CellRef::new(1, header.len() as u32 - 1).a1();
|
||||
s.execute("range.setValues", json!({"range": "A1", "values": [header, data]})).unwrap();
|
||||
let t = std::time::Instant::now();
|
||||
let src = crate::pivot::read_source(&s.doc().unwrap().wb, &format!("Sheet1!A1:{end}"), 0).unwrap();
|
||||
assert!(t.elapsed().as_secs_f64() < 2.0, "took {:?}", t.elapsed());
|
||||
assert_eq!(&src.headers[..4], ["a", "A2", "a3", "a22"]);
|
||||
assert_eq!(src.headers[4], "x");
|
||||
assert_eq!(src.headers.last().map(String::as_str), Some("x3000"));
|
||||
// Table column names follow the same rule.
|
||||
let t = std::time::Instant::now();
|
||||
s.execute("insert.table", json!({"range": format!("A1:{end}"), "header": true})).unwrap();
|
||||
assert!(t.elapsed().as_secs_f64() < 2.0, "took {:?}", t.elapsed());
|
||||
let text = |s: &mut Session, a: &str| s.execute("cell.get", json!({"cell": a})).unwrap()["text"].clone();
|
||||
assert_eq!(text(&mut s, "B1"), "A2");
|
||||
let last = CellRef::new(0, 3003).a1();
|
||||
assert_eq!(text(&mut s, &last), "x3000");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn hostile_params_never_panic() {
|
||||
let mut s = sales();
|
||||
|
||||
@@ -126,18 +126,25 @@ pub fn read_source(wb: &Workbook, text: &str, default_sheet: usize) -> Result<So
|
||||
return Err("The source range is too large for a PivotTable.".into());
|
||||
}
|
||||
let mut headers: Vec<String> = Vec::new();
|
||||
// Names taken so far (lower-case), and the next suffix to try for each repeated header, so
|
||||
// that thousands of equal headers don't take cubic time.
|
||||
let mut taken: HashSet<String> = HashSet::new();
|
||||
let mut next_suffix: HashMap<String, usize> = HashMap::new();
|
||||
let mut formats = Vec::new();
|
||||
for c in r.start.col..=r.end.col {
|
||||
let h = crate::display::cell_text(wb, sh, CellRef::new(r.start.row, c)).trim().to_string();
|
||||
if h.is_empty() {
|
||||
return Err(FIELD_NAME_ERR.into());
|
||||
}
|
||||
let key = h.to_ascii_lowercase();
|
||||
let mut n = h.clone();
|
||||
let mut k = 2;
|
||||
while headers.iter().any(|x| x.eq_ignore_ascii_case(&n)) {
|
||||
let mut k = next_suffix.get(&key).copied().unwrap_or(2);
|
||||
while taken.contains(&n.to_ascii_lowercase()) {
|
||||
n = format!("{h}{k}");
|
||||
k += 1;
|
||||
}
|
||||
next_suffix.insert(key, k);
|
||||
taken.insert(n.to_ascii_lowercase());
|
||||
headers.push(n);
|
||||
let fmt = if r.end.row > r.start.row {
|
||||
wb.styles.get(sh.style_id(CellRef::new(r.start.row + 1, c))).num_fmt.as_str().to_string()
|
||||
|
||||
@@ -415,17 +415,22 @@ fn build_cache(wb: &Workbook, pt: &PivotTable, pivot_sheet: usize) -> Option<Cac
|
||||
}
|
||||
let sh = wb.sheet(si)?;
|
||||
let mut fields: Vec<CField> = Vec::new();
|
||||
// Lower-case names taken, and the next suffix for each header (linear for repeated headers).
|
||||
let mut taken = std::collections::HashSet::new();
|
||||
let mut next_suffix: HashMap<String, usize> = HashMap::new();
|
||||
for (k, c) in (r.start.col..=r.end.col).enumerate() {
|
||||
let mut h = header_text(&sh.value(CellRef::new(r.start.row, c)).scalar());
|
||||
if h.is_empty() {
|
||||
h = format!("Column{}", k + 1);
|
||||
}
|
||||
let mut n = h.clone();
|
||||
let mut i = 2;
|
||||
while fields.iter().any(|f| same(&f.name, &n)) {
|
||||
let mut i = next_suffix.get(&h.to_lowercase()).copied().unwrap_or(2);
|
||||
while taken.contains(&n.to_lowercase()) {
|
||||
n = format!("{h}{i}");
|
||||
i += 1;
|
||||
}
|
||||
next_suffix.insert(h.to_lowercase(), i);
|
||||
taken.insert(n.to_lowercase());
|
||||
fields.push(CField {
|
||||
name: n,
|
||||
col: Some(k as u32),
|
||||
|
||||
Reference in New Issue
Block a user