address review: keep the original code in the public number_format for overlong formats

The display cache hashed the full format code on every lookup, so one
huge code shared by many cells cost its length for every cell drawn.
A code with more than 4 × 255 bytes can't be within Excel's 255
characters and is now formatted as General straight away.

crates/engine/src/cmd/whatif/consolidate.rs

@@ -61,24 +61,6 @@ 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();
@@ -107,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>, index: &mut HashMap<Key, usize>, k: Key| -> usize {
-        *index.entry(k).or_insert_with_key(|k| {
-            keys.push(k.clone());
-            keys.len() - 1
-        })
+    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
+            }
+        }
     };
     for t in &srcs {
         let (si, r) = parse_range(&d.wb, t, CMD)?;
@@ -149,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, &mut col_index, k)));
+            cols.push((col, index_of(&mut col_keys, k)));
         }
         for row in r0..=r.end.row {
             let k = if left {
@@ -163,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, &mut row_index, k);
+            let ri = index_of(&mut row_keys, k);
             for (col, ci) in &cols {
                 aggs.entry((ri, *ci)).or_default().add(&sh.value(CellRef::new(row, *col)));
             }
@@ -175,9 +157,6 @@ 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() {

crates/engine/src/cmd/whatif/tests.rs

@@ -297,16 +297,3 @@ 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"));
-}

crates/engine/src/display.rs

@@ -8,8 +8,13 @@ use gridcraft_model::{Sheet, Workbook};
 use gridcraft_numfmt::{Formatted, NumberFormat, format_value};
 
 /// Parsed number formats are cached by code.
-fn parsed(code: &str) -> NumberFormat {
+pub(crate) fn parsed(code: &str) -> NumberFormat {
     static CACHE: std::sync::LazyLock<Mutex<HashMap<String, NumberFormat>>> = std::sync::LazyLock::new(|| Mutex::new(HashMap::new()));
+    // A code of more bytes than 4 × the character limit can't be within it: it shows as General
+    // without being hashed or scanned again for every cell that shares it.
+    if code.len() > 4 * gridcraft_numfmt::MAX_FORMAT_CODE_CHARS {
+        return NumberFormat::parse("General");
+    }
     let lock = || CACHE.lock().unwrap_or_else(std::sync::PoisonError::into_inner);
     if let Some(f) = lock().get(code) {
         return f.clone();
@@ -24,7 +29,12 @@ fn parsed(code: &str) -> NumberFormat {
     f
 }
 
+/// The parsed format of `code`; one longer than the character limit is a General format that keeps
+/// the code it was made from. The engine itself formats through `parsed`, which doesn't copy it.
 pub fn number_format(code: &str) -> NumberFormat {
+    if code.len() > 4 * gridcraft_numfmt::MAX_FORMAT_CODE_CHARS {
+        return NumberFormat::parse(code);
+    }
     parsed(code)
 }
 
@@ -226,3 +236,31 @@ pub fn stats(sheet: &Sheet, ranges: &[RangeRef]) -> Stats {
     }
     s
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    /// A format code far past Excel's 255 characters shows its values as General, for every cell
+    /// that shares it, without being hashed into the cache each time.
+    #[test]
+    fn an_overlong_code_formats_as_general() {
+        let wb = Workbook::new();
+        let code = format!("0.00{}", "\"x\"".repeat(400_000));
+        for _ in 0..1_000 {
+            assert_eq!(format(&Value::Number(1.5), &code, &wb).text, "1.5");
+        }
+        assert_eq!(format(&Value::Number(1.5), "0.00", &wb).text, "1.50");
+    }
+
+    /// The public `number_format` keeps the code of an overlong format; it is General, but not the
+    /// same format as a real `General`.
+    #[test]
+    fn number_format_keeps_an_overlong_code() {
+        let code = format!("0.00{}", "\"x\"".repeat(2_000));
+        let f = number_format(&code);
+        assert!(f.is_general());
+        assert_eq!(f.code(), code);
+        assert_ne!(f, number_format("General"));
+    }
+}

crates/engine/src/io.rs

@@ -443,7 +443,7 @@ pub fn csv_values_bytes(wb: &Workbook) -> Result<Vec<u8>> {
     let Some(sh) = wb.sheet(wb.active_sheet()) else { return Ok(Vec::new()) };
     gridcraft_xlsx::write_csv(sh, wb.csv_delimiter.unwrap_or(b','), |c| match sh.value(c) {
         gridcraft_core::Value::Number(n) if n.is_finite() => {
-            let shape = crate::display::number_format(wb.styles.get(sh.style_id(c)).num_fmt.as_str()).date_shape(n);
+            let shape = crate::display::parsed(wb.styles.get(sh.style_id(c)).num_fmt.as_str()).date_shape(n);
             match shape.zip(gridcraft_core::date::datetime_from_serial(wb.date_system, n)) {
                 Some(((date, time), d)) => iso(&d, date, time),
                 None => n.to_string(),

crates/engine/src/macros/recorder.rs

@@ -440,7 +440,7 @@ pub fn infer_input(wb: &Workbook, sheet: usize, r: RangeRef) -> NamedInput {
         Some(match v {
             Value::Empty => return None,
             Value::Text(t) if t.is_empty() => return None,
-            Value::Number(_) if crate::display::number_format(wb.styles.get(sh.style_id(c)).num_fmt.as_str()).is_date() => ColType::Date,
+            Value::Number(_) if crate::display::parsed(wb.styles.get(sh.style_id(c)).num_fmt.as_str()).is_date() => ColType::Date,
             Value::Number(_) => ColType::Number,
             Value::Text(_) => ColType::Text,
             Value::Bool(_) => ColType::Bool,

crates/engine/src/macros/run.rs

@@ -335,7 +335,7 @@ fn check_input(wb: &Workbook, input: &gridcraft_model::NamedInput, place: Place)
                 gridcraft_model::ColType::Number => v.is_number(),
                 gridcraft_model::ColType::Text => v.is_text(),
                 gridcraft_model::ColType::Bool => matches!(v, Value::Bool(_)),
-                gridcraft_model::ColType::Date => v.is_number() && crate::display::number_format(&date()).is_date(),
+                gridcraft_model::ColType::Date => v.is_number() && crate::display::parsed(&date()).is_date(),
             };
             if !ok {
                 wrong.push((c, holds(&v, ty)));

crates/engine/src/pivot.rs

@@ -182,7 +182,7 @@ pub fn read_source(wb: &Workbook, text: &str, default_sheet: usize) -> Result<So
     for (n, a) in numeric.iter_mut().zip(any.iter()) {
         *n = *n && *a;
     }
-    let dates = numeric.iter().zip(formats.iter()).map(|(n, f)| *n && crate::display::number_format(f).is_date()).collect();
+    let dates = numeric.iter().zip(formats.iter()).map(|(n, f)| *n && crate::display::parsed(f).is_date()).collect();
     Ok(Source { sheet: si, range: r, headers, formats, numeric, dates, rows })
 }