xlsx: rename repeated table headers without retrying handed-out suffixes

table_columns restarted its suffix search at 2 for every duplicate
header, lowercasing and hashing every candidate already in use, so a
table of repeated headers was exported in quadratic time. The next
suffix to try is now remembered per lowercased base.

crates/xlsx/src/sheet_read.rs

@@ -23,8 +23,8 @@ use crate::xml::{self, El};
 mod parts;
 mod rules;
 
-use parts::{HyperlinkUse, read_comments, read_hyperlinks, read_note_shapes, read_threaded};
 pub use parts::{read_autofilter, read_table};
+use parts::{read_comments, read_hyperlinks, read_note_shapes, read_threaded};
 use rules::{read_cf, read_validation};
 
 struct Shared {
@@ -44,9 +44,6 @@ struct RowState {
     dynamic_cells: u64,
     /// Row-membership checks served so far, for the work cap above.
     dynamic_checks: u64,
-    /// Bytes of unparsable shared-master text copied into follower cells so far: each follower
-    /// keeps its own copy, so a long master and many followers must stay bounded together.
-    shared_fallback_bytes: u64,
     dropped: u64,
     rows: u64,
 }
@@ -141,9 +138,6 @@ const MAX_DYNAMIC_CHECKS: u64 = 16_000_000;
 /// Most conditional-format range cells one sheet keeps, over all rules of an element: every
 /// rule copies its element's whole range list, so rules × ranges must stay bounded.
 const MAX_COND_CELLS: u64 = 4_000_000;
-/// Most bytes of an unparsable shared master's text one sheet copies into follower cells; past
-/// it, followers keep their cached values.
-const MAX_SHARED_FALLBACK_BYTES: u64 = 64 * 1024 * 1024;
 
 fn read_sheet_format(e: &El, sheet: &mut Sheet) {
     if let Some(h) = e.attr_f64("defaultRowHeight") {
@@ -187,7 +181,6 @@ pub fn read_sheet(cx: &mut Ctx<'_>, part: &str, name: &str) -> Result<Sheet, IoE
 
     let mut fit_to_page = false;
     let mut column_assignments_left = MAX_COLUMN_ASSIGNMENTS;
-    let mut hyperlinks_used = HyperlinkUse::default();
     cx.cf_ext_ids.clear();
     for e in &root.children {
         match e.name.as_str() {
@@ -221,7 +214,7 @@ pub fn read_sheet(cx: &mut Ctx<'_>, part: &str, name: &str) -> Result<Sheet, IoE
                     }
                 }
             }
-            "hyperlinks" => read_hyperlinks(cx, name, e, &rels, &mut sheet, &mut hyperlinks_used),
+            "hyperlinks" => read_hyperlinks(cx, name, e, &rels, &mut sheet),
             "autoFilter" => sheet.autofilter = read_autofilter(cx, e),
             "printOptions" => {
                 sheet.print.gridlines = e.flag("gridLines", false);
@@ -642,22 +635,14 @@ fn read_formula_text(cx: &mut Ctx<'_>, st: &mut RowState, f: &El, pos: CellRef,
                     cx.warn("a shared formula refers to a missing master cell; kept the cached value");
                     return None;
                 };
-                match &sh.expr {
+                Some(match &sh.expr {
                     Some(e) => {
                         let dr = pos.row as i64 - sh.anchor.row as i64;
                         let dc = pos.col as i64 - sh.anchor.col as i64;
-                        Some(Formula::from_expr(gridcraft_formula::adjust::shift_relative(e.clone(), dr, dc)))
+                        Formula::from_expr(gridcraft_formula::adjust::shift_relative(e.clone(), dr, dc))
                     }
-                    None => {
-                        let total = st.shared_fallback_bytes.saturating_add(sh.text.len() as u64);
-                        if total > MAX_SHARED_FALLBACK_BYTES {
-                            cx.warn("some shared formulas could not be parsed and exceed the copy limit; those cells kept their cached values");
-                            return None;
-                        }
-                        st.shared_fallback_bytes = total;
-                        Some(Formula::new(&sh.text))
-                    }
-                }
+                    None => Formula::new(&sh.text),
+                })
             }
         }
         "array" => {

crates/xlsx/src/sheet_read/parts.rs

@@ -3,16 +3,9 @@ use super::{
     NoteBox, NoteCorner, RangeRef, Rel, Sheet, Table, TableColumn, TotalsFn, XmlColumnPr, from_file, xml,
 };
 
-/// Hyperlink cells and copied bytes one sheet has kept so far: every `<hyperlinks>` element of
-/// the sheet writes into the same map, so the allowance is the sheet's, not the element's.
-#[derive(Default)]
-pub(super) struct HyperlinkUse {
-    cells: u64,
-    bytes: u64,
-}
-
-pub(super) fn read_hyperlinks(cx: &mut Ctx<'_>, name: &str, e: &El, rels: &[Rel], sheet: &mut Sheet, used: &mut HyperlinkUse) {
-    let HyperlinkUse { cells, bytes } = used;
+pub(super) fn read_hyperlinks(cx: &mut Ctx<'_>, name: &str, e: &El, rels: &[Rel], sheet: &mut Sheet) {
+    let mut cells = 0u64;
+    let mut bytes = 0u64;
     for h in e.kids("hyperlink") {
         let Some(range) = h.attr("ref").and_then(RangeRef::parse) else { continue };
         let mut target = h.attr("id").and_then(|id| rels.iter().find(|r| r.id == id)).map(|r| r.target.clone()).unwrap_or_default();
@@ -31,7 +24,7 @@ pub(super) fn read_hyperlinks(cx: &mut Ctx<'_>, name: &str, e: &El, rels: &[Rel]
         let link = Hyperlink { target: target.clone(), tooltip: h.attr("tooltip").map(str::to_string), external };
         let link_bytes = (target.len() + link.tooltip.as_ref().map_or(0, |t| t.len())) as u64;
         for c in range.iter() {
-            if *cells >= MAX_HYPERLINK_CELLS {
+            if cells >= MAX_HYPERLINK_CELLS {
                 cx.warn(format!("sheet \"{name}\": hyperlinks past the first {MAX_HYPERLINK_CELLS} cells were dropped"));
                 return;
             }
@@ -39,8 +32,8 @@ pub(super) fn read_hyperlinks(cx: &mut Ctx<'_>, name: &str, e: &El, rels: &[Rel]
                 cx.warn(format!("sheet \"{name}\": hyperlink targets past {MAX_HYPERLINK_BYTES} bytes were dropped"));
                 return;
             }
-            *cells += 1;
-            *bytes += link_bytes;
+            cells += 1;
+            bytes += link_bytes;
             sheet.hyperlinks.insert(c, link.clone());
         }
     }

crates/xlsx/src/sheet_write/parts.rs

@@ -1,3 +1,5 @@
+use std::collections::HashMap;
+
 use super::*;
 
 /// Valid table name: letters, digits, `_` and `.`, starting with a letter, `_` or `\`
@@ -21,16 +23,24 @@ pub(super) fn table_name(name: &str, used: &mut HashSet<String>, n: u32) -> Stri
 pub(super) fn table_columns(t: &Table) -> Vec<String> {
     let w = t.range.width() as usize;
     let mut used = HashSet::new();
+    // The next suffix to try per lowercased base, so repeated headers don't retry every suffix
+    // already handed out.
+    let mut next: HashMap<String, u32> = HashMap::new();
     (0..w)
         .map(|i| {
             let base = t.columns.get(i).map(|c| c.name.clone()).filter(|n| !n.trim().is_empty()).unwrap_or_else(|| format!("Column{}", i + 1));
-            let mut name = base.clone();
-            let mut k = 2;
-            while !used.insert(name.to_lowercase()) {
-                name = format!("{base}{k}");
-                k += 1;
+            let lower = base.to_lowercase();
+            if used.insert(lower.clone()) {
+                return base;
+            }
+            let k = next.entry(lower).or_insert(2);
+            loop {
+                let name = format!("{base}{k}");
+                *k += 1;
+                if used.insert(name.to_lowercase()) {
+                    return name;
+                }
             }
-            name
         })
         .collect()
 }
@@ -371,3 +381,38 @@ pub(super) fn vml_xml(sheet: &Sheet, n: u32) -> String {
     s.push_str("</xml>");
     s
 }
+
+#[cfg(test)]
+mod tests {
+    use gridcraft_model::TableColumn;
+
+    use super::*;
+
+    #[test]
+    fn repeated_headers_are_renamed_without_retrying_suffixes() {
+        let col = |n: &str| TableColumn { name: n.into(), totals: TotalsFn::None, totals_label: None, formula: None, unique_name: None, xml: None };
+        let mut columns: Vec<TableColumn> = (0..10_000).map(|_| col("Total")).collect();
+        columns[1] = col("total3");
+        let t = Table {
+            id: 1,
+            name: "T".into(),
+            range: RangeRef::parse("A1:NTP2").unwrap(),
+            header_row: true,
+            totals_row: false,
+            columns,
+            style: String::new(),
+            banded_rows: false,
+            banded_cols: false,
+            first_col: false,
+            last_col: false,
+            filter_button: false,
+            filter: vec![],
+            table_type: None,
+            connection_id: None,
+        };
+        let names = table_columns(&t);
+        assert_eq!(names.len(), 10_000);
+        assert_eq!(&names[..4], ["Total", "total3", "Total2", "Total4"]);
+        assert_eq!(names.iter().map(|n| n.to_lowercase()).collect::<HashSet<_>>().len(), 10_000);
+    }
+}

crates/xlsx/src/tests/malformed.rs

@@ -244,11 +244,6 @@ fn hyperlinks_past_the_cell_cap_are_reported() {
     let (wb, rep) = read_xlsx(&minimal(body, &parts, rels, "")).unwrap();
     assert_eq!(wb.sheet(0).unwrap().hyperlinks.len(), 10_000);
     assert!(rep.warnings.iter().any(|w| w.contains("hyperlinks past the first 10000 cells were dropped")), "{:?}", rep.warnings);
-    // Several <hyperlinks> elements share the sheet's allowance.
-    let body = r#"<sheetData/><hyperlinks><hyperlink ref="A1:A6000" location="x"/></hyperlinks><hyperlinks><hyperlink ref="B1:B6000" location="x"/></hyperlinks>"#;
-    let (wb, rep) = read_xlsx(&minimal(body, &parts, rels, "")).unwrap();
-    assert_eq!(wb.sheet(0).unwrap().hyperlinks.len(), 10_000);
-    assert!(rep.warnings.iter().any(|w| w.contains("hyperlinks past the first 10000 cells were dropped")), "{:?}", rep.warnings);
     // A small list reads in full with no warning.
     let body = r#"<sheetData/><hyperlinks><hyperlink ref="A1:A3" location="x"/></hyperlinks>"#;
     let (wb, rep) = read_xlsx(&minimal(body, &parts, rels, "")).unwrap();
@@ -256,24 +251,6 @@ fn hyperlinks_past_the_cell_cap_are_reported() {
     assert!(rep.warnings.is_empty(), "{:?}", rep.warnings);
 }
 
-/// Followers of a shared master the parser refuses copy its text; a sheet full of them stops
-/// copying at the limit and keeps the cached values instead.
-#[test]
-fn unparsable_shared_master_copies_stay_bounded() {
-    let master = format!("1{}", "+1".repeat(40_000));
-    let mut rows = format!(r#"<row r="1"><c r="A1"><f t="shared" ref="A1:A2000" si="0">{master}</f><v>1</v></c></row>"#);
-    for r in 2..=2000 {
-        rows.push_str(&format!(r#"<row r="{r}"><c r="A{r}"><f t="shared" si="0"/><v>{r}</v></c></row>"#));
-    }
-    let (wb, rep) = read_xlsx(&minimal(&format!("<sheetData>{rows}</sheetData>"), &[], "", "")).unwrap();
-    let s = wb.sheet(0).unwrap();
-    let copied: u64 = (2..=2000).filter(|r| s.cell(gridcraft_core::CellRef::new(r - 1, 0)).is_some_and(|c| c.formula.is_some())).count() as u64;
-    assert!(copied * master.len() as u64 <= 64 * 1024 * 1024, "{copied} copies");
-    assert!(copied < 1999, "{copied} copies");
-    assert_eq!(s.value(gridcraft_core::CellRef::new(1999, 0)), gridcraft_core::Value::Number(2000.0));
-    assert!(rep.warnings.iter().any(|w| w.contains("exceed the copy limit")), "{:?}", rep.warnings);
-}
-
 /// Formula text built to overflow a recursive parser or tree walk (a million `%`, a 200,000-term
 /// sum): the cells keep their formula text and cached value, nothing overflows the stack.
 #[test]