From 7ccacd635d1097b51ce9851d69d50d885ea37970 Mon Sep 17 00:00:00 2001 From: dada-yan Date: Fri, 16 Oct 2026 18:43:56 +1100 Subject: [PATCH 1/3] Keep sparse XLSX imports within the grid memory budget Signed-off-by: dada-yan --- crates/utopia-ingest/src/parsers.rs | 79 +++++++++++++++++- crates/utopia-ingest/tests/fixtures/README.md | 9 ++ .../tests/fixtures/sparse-sheets.xlsx | Bin 0 -> 1747 bytes .../tests/spreadsheet_truncation.rs | 35 ++++++++ 4 files changed, 119 insertions(+), 4 deletions(-) create mode 100644 crates/utopia-ingest/tests/fixtures/README.md create mode 100644 crates/utopia-ingest/tests/fixtures/sparse-sheets.xlsx diff --git a/crates/utopia-ingest/src/parsers.rs b/crates/utopia-ingest/src/parsers.rs index 686b59b3c..506bafbdb 100644 --- a/crates/utopia-ingest/src/parsers.rs +++ b/crates/utopia-ingest/src/parsers.rs @@ -7,6 +7,9 @@ use std::io::{Cursor, Read, Write}; use std::process::Command; const SPREADSHEET_ROW_LIMIT: usize = 2_000; +// A distant cell makes an XLSX range dense, including all the empty cells in between. +// Allow 2,000 rows of 100 columns before allocating the grid; wider sheets read fewer rows. +const SPREADSHEET_CELL_LIMIT: usize = 200_000; const CSV_RECORD_LIMIT: usize = 10_000; /// 文本解码：chardetng 探测编码（覆盖 GBK/GB18030/BIG5 等中文常见编码）。 @@ -773,7 +776,17 @@ pub fn spreadsheet(bytes: &[u8]) -> anyhow::Result<(String, Vec xlsx_range(book, &sheet_name), + _ => workbook + .worksheet_range(&sheet_name) + .map(|range| { + let rows_total = range.height(); + (range, rows_total) + }) + .map_err(anyhow::Error::from), + }; + let Ok((range, rows_total)) = read else { continue; }; if range.is_empty() { @@ -790,9 +803,8 @@ pub fn spreadsheet(bytes: &[u8]) -> anyhow::Result<(String, Vec Vec::new(), }; out.push_str(&format!("\n# Sheet: {sheet_name}\n\n")); - let rows_total = range.height(); - if rows_total > SPREADSHEET_ROW_LIMIT { - let rows_read = SPREADSHEET_ROW_LIMIT; + let rows_read = range.height().min(SPREADSHEET_ROW_LIMIT); + if rows_total > rows_read { warnings.push(crate::ParseWarning { kind: crate::ParseWarning::SPREADSHEET_ROWS_TRUNCATED, detail: serde_json::json!({ @@ -822,6 +834,51 @@ pub fn spreadsheet(bytes: &[u8]) -> anyhow::Result<(String, Vec>>, + sheet: &str, +) -> anyhow::Result<(calamine::Range, usize)> { + // worksheet_range() materializes the whole rectangle before our 2,000-row cap: + // two cells at A1 and P1000000 used to allocate 512 MB for a 1.4 KB file. + // Read the actual cells first, without trusting the optional XML dimension hint. + let mut start = (u32::MAX, u32::MAX); + let mut end = (0, 0); + let mut reader = book.worksheet_cells_reader(sheet)?; + while let Some(cell) = reader.next_cell()? { + if cell.get_value() == &calamine::DataRef::Empty { + continue; + } + let (row, col) = cell.get_position(); + start = (start.0.min(row), start.1.min(col)); + end = (end.0.max(row), end.1.max(col)); + } + drop(reader); + if start.0 == u32::MAX { + return Ok((calamine::Range::empty(), 0)); + } + let rows_total = end.0 as usize - start.0 as usize + 1; + let width = end.1 as usize - start.1 as usize + 1; + let rows_read = rows_total + .min(SPREADSHEET_ROW_LIMIT) + .min(SPREADSHEET_CELL_LIMIT / width); + anyhow::ensure!( + rows_read > 0, + "The spreadsheet has too many columns to read" + ); + end.0 = start.0 + rows_read as u32 - 1; + let mut range = calamine::Range::new(start, end); + // A second pass keeps date/style decoding in Calamine and counts rows beyond + // the budget honestly, without retaining every nonempty cell in memory. + let mut reader = book.worksheet_cells_reader(sheet)?; + while let Some(cell) = reader.next_cell()? { + let position = cell.get_position(); + if position.0 <= end.0 && cell.get_value() != &calamine::DataRef::Empty { + range.set_value(position, calamine::Data::from(cell.get_value().clone())); + } + } + Ok((range, rows_total)) +} + #[derive(Clone)] struct GridMerge { start: (usize, usize), @@ -1205,6 +1262,20 @@ fn pptx_xml_to_text(xml: &str) -> anyhow::Result { mod tests { use super::*; + #[test] + fn sparse_xlsx_ranges_are_bounded_before_the_dense_allocation() { + let bytes = include_bytes!("../tests/fixtures/sparse-sheets.xlsx"); + let mut book = calamine::open_workbook_from_rs(Cursor::new(bytes.to_vec())).unwrap(); + for (sheet, rows_total, rows_read, width) in + [("Tall", 1_000_000, 2_000, 16), ("Wide", 201, 200, 1_000)] + { + let (range, total) = xlsx_range(&mut book, sheet).unwrap(); + assert_eq!(total, rows_total); + assert_eq!(range.get_size(), (rows_read, width)); + assert!(range.height() * range.width() <= SPREADSHEET_CELL_LIMIT); + } + } + const DOC: &str = r#" Segment results SegmentRevenue diff --git a/crates/utopia-ingest/tests/fixtures/README.md b/crates/utopia-ingest/tests/fixtures/README.md new file mode 100644 index 000000000..3b35261c8 --- /dev/null +++ b/crates/utopia-ingest/tests/fixtures/README.md @@ -0,0 +1,9 @@ +`sparse-sheets.xlsx` is a small, model-free OOXML workbook with two sparse sheets: + +- `Tall`: A1 = `tall-header`, B2000 = `tall-last-read`, P1000000 = `tall-omitted`. +- `Wide`: A1 = `wide-header`, B200 = `wide-last-read`, ALL201 = `wide-omitted`. + +Tall has an explicit worksheet dimension; Wide omits the optional hint. The +range test checks the retained dense grid dimensions. +The public parser test checks the retained content and complete row warnings. +Run both with `cargo test -p utopia-ingest sparse`. diff --git a/crates/utopia-ingest/tests/fixtures/sparse-sheets.xlsx b/crates/utopia-ingest/tests/fixtures/sparse-sheets.xlsx new file mode 100644 index 0000000000000000000000000000000000000000..8ddd11a052e5ce9e12c333792ea628c79fe9b178 GIT binary patch literal 1747 zcmWIWW@Zs#U|`^2V44~bd(H6tj@7Hk&DlC> zuOG9aKui47CuUvdOBOinG-(kF`oPqkub)2m|C75a$8SG27G&j8P+1Z7?aUkVqj~e< zg$s=<#dm}@d+cIPFrMIk%`i~IOMbJv=J}e2mb4?x2dlR3Vij%sNxw9F3Z;#+>9c9D)gOYEephns!(Xt1${b8h}*`e0JE zRp7a8i_`ie?`utpZI^{ z(Vd*1O5Yw@K9LuW?^T-IHz)c=SWTB+`NXAq8VTti9v^(ii|S-O)8MajfllRPWMJS2 z(iJ)S<@rU~N%{HNpxEo3u%EBVK%hN-=}z_!4F?+;CU@`dEMD54n^C+gJaGINh%|0~g&DPlS2f_pXgE?w~Sfbh|uMg?3}=aY9W_UYUr=sl(R_$rM) zi7nk5q+WgLZ*D*MwdB!1L5*jjlTN&TW6@PQ+2Ohl=clz=uN-t^wtQbO?d;7h0Zm?8 zXU{G%Q+=RcUD{Tbvo(1&e|I!Pp8bn^s6ILHL8^2*&xF6>G)`Vvcx2|T6y3cIg(=o5{|*LaR{x&m(BJOIbz_>n~leW*AKA?!2k3m%VMml(;FbFJ?Vss5~v;ygxPUv5KqQ^01KU z?uX)UT$~a4So78D;|Huo3%V;6+fT5r+T=Rzk+9$%w`lQI4Yf1hoj1B0&9?8Jtqmip zclbBozkC4bjT)eLBp}`ad89ZaHMOKzA50m7VyJe4FITgHKMnZs&i%U4)mfRxkYvB`@<*ApFE3Mh zo|r73Hcuek$##uW`a-?a+!nce0*VayYE2G`w}FiLcsLn z4;X&@{d(+9uuMX;@tGguT=pju<9GZ*%Rxt = parsed + .warnings + .iter() + .map(|warning| { + assert_eq!(warning.kind, "spreadsheet.rows_truncated"); + warning.detail.clone() + }) + .collect(); + assert_eq!( + warnings, + vec![ + serde_json::json!({ + "sheet": "Tall", "rows_read": 2000, + "rows_total": 1000000, "rows_omitted": 998000, + }), + serde_json::json!({ + "sheet": "Wide", "rows_read": 200, + "rows_total": 201, "rows_omitted": 1, + }), + ] + ); +} + #[test] fn a_csv_past_the_record_cap_reports_the_records_it_left_out() { let parsed = utopia_ingest::parse("customers.csv", csv(10_000).as_bytes()).unwrap(); From 0e12a1db13e42818444d91bbbdf79688e07196d2 Mon Sep 17 00:00:00 2001 From: dada-yan Date: Fri, 16 Oct 2026 23:32:19 +1100 Subject: [PATCH 2/3] Preserve ordinary XLSX imports while bounding sparse grid storage Signed-off-by: dada-yan --- crates/utopia-ingest/src/parsers.rs | 405 +++++++++++++----- crates/utopia-ingest/tests/fixtures/README.md | 9 - .../tests/fixtures/sparse-sheets.xlsx | Bin 1747 -> 0 bytes .../tests/spreadsheet_truncation.rs | 210 +++++++-- 4 files changed, 472 insertions(+), 152 deletions(-) delete mode 100644 crates/utopia-ingest/tests/fixtures/README.md delete mode 100644 crates/utopia-ingest/tests/fixtures/sparse-sheets.xlsx diff --git a/crates/utopia-ingest/src/parsers.rs b/crates/utopia-ingest/src/parsers.rs index 506bafbdb..636fb8d3f 100644 --- a/crates/utopia-ingest/src/parsers.rs +++ b/crates/utopia-ingest/src/parsers.rs @@ -7,9 +7,11 @@ use std::io::{Cursor, Read, Write}; use std::process::Command; const SPREADSHEET_ROW_LIMIT: usize = 2_000; -// A distant cell makes an XLSX range dense, including all the empty cells in between. -// Allow 2,000 rows of 100 columns before allocating the grid; wider sheets read fewer rows. -const SPREADSHEET_CELL_LIMIT: usize = 200_000; +// Each retained cell passes through a 40-byte GridRow entry and two 48-byte renderer +// Cells, besides source values, strings and Vec spare capacity (~370 B/cell in a +// full dense import). Bound actual records/segments, not their empty column spans: +// a 2,000 x 150 sheet uses 300,000 entries and fits without losing any rows. +const XLSX_SEGMENT_LIMIT: usize = 1_000_000; const CSV_RECORD_LIMIT: usize = 10_000; /// 文本解码：chardetng 探测编码（覆盖 GBK/GB18030/BIG5 等中文常见编码）。 @@ -776,107 +778,207 @@ pub fn spreadsheet(bytes: &[u8]) -> anyhow::Result<(String, Vec xlsx_range(book, &sheet_name), - _ => workbook - .worksheet_range(&sheet_name) - .map(|range| { - let rows_total = range.height(); - (range, rows_total) - }) - .map_err(anyhow::Error::from), - }; - let Ok((range, rows_total)) = read else { - continue; + let (text, rows_read, rows_total) = match &mut workbook { + calamine::Sheets::Xlsx(book) => xlsx_sheet(book, &sheet_name, XLSX_SEGMENT_LIMIT) + .with_context(|| format!("Could not read worksheet {sheet_name}"))?, + _ => { + let Ok(range) = workbook.worksheet_range(&sheet_name) else { + continue; + }; + let merges = match &mut workbook { + calamine::Sheets::Xls(book) => book + .merge_cells_by_sheet_name(&sheet_name) + .unwrap_or_default(), + _ => Vec::new(), + }; + let (rows, headers) = spreadsheet_grid(&range, &merges); + let text = crate::table::render_grid_with_headers(&rows, headers) + .map(|text| format!("{text}\n")) + .unwrap_or_else(|| { + let mut text = String::new(); + for row in &rows { + let cells: Vec<&str> = row.iter().map(|(s, _, _)| s.as_str()).collect(); + text.push_str(&cells.join("\t")); + text.push('\n'); + } + text + }); + ( + text, + range.height().min(SPREADSHEET_ROW_LIMIT), + range.height(), + ) + } }; - if range.is_empty() { + if rows_read == 0 { continue; } - // xlsx/xls 把 merge 单独给出；xlsb/ods 的 calamine reader 没有这一步，按无 merge 读。 - let merges = match &mut workbook { - calamine::Sheets::Xlsx(book) => book - .merge_cells_by_sheet_name(&sheet_name) - .unwrap_or_default(), - calamine::Sheets::Xls(book) => book - .merge_cells_by_sheet_name(&sheet_name) - .unwrap_or_default(), - _ => Vec::new(), - }; out.push_str(&format!("\n# Sheet: {sheet_name}\n\n")); - let rows_read = range.height().min(SPREADSHEET_ROW_LIMIT); if rows_total > rows_read { warnings.push(crate::ParseWarning { kind: crate::ParseWarning::SPREADSHEET_ROWS_TRUNCATED, detail: serde_json::json!({ - "sheet": sheet_name.clone(), + "sheet": sheet_name, "rows_read": rows_read, "rows_total": rows_total, "rows_omitted": rows_total - rows_read, }), }); } - // 每张表按网格渲染成带列头的 Markdown 表；一格字都没有的表退回制表符分隔 - let (rows, headers) = spreadsheet_grid(&range, &merges); - match crate::table::render_grid_with_headers(&rows, headers) { - Some(md) => { - out.push_str(&md); - out.push('\n'); - } - None => { - for line in &rows { - let cells: Vec<&str> = line.iter().map(|(s, _, _)| s.as_str()).collect(); - out.push_str(&cells.join("\t")); - out.push('\n'); - } - } - } + out.push_str(&text); } Ok((out, warnings)) } -fn xlsx_range( +fn xlsx_sheet( book: &mut calamine::Xlsx>>, sheet: &str, -) -> anyhow::Result<(calamine::Range, usize)> { - // worksheet_range() materializes the whole rectangle before our 2,000-row cap: - // two cells at A1 and P1000000 used to allocate 512 MB for a 1.4 KB file. - // Read the actual cells first, without trusting the optional XML dimension hint. - let mut start = (u32::MAX, u32::MAX); - let mut end = (0, 0); - let mut reader = book.worksheet_cells_reader(sheet)?; + budget: usize, +) -> anyhow::Result<(String, usize, usize)> { + let mut start_row = u32::MAX; + let mut end_row = 0; + let mut row_counts = vec![0usize; SPREADSHEET_ROW_LIMIT]; + let mut reader = match book.worksheet_cells_reader(sheet) { + Ok(reader) => reader, + // Calamine's ordinary worksheet_range also skips chart sheets. + Err(calamine::XlsxError::NotAWorksheet(_)) => return Ok((String::new(), 0, 0)), + Err(error) => return Err(error.into()), + }; + // Keep only the first 2,000 physical rows' counts, even if XML cells arrive + // out of order. The full row bounds survive so truncation remains honest. while let Some(cell) = reader.next_cell()? { if cell.get_value() == &calamine::DataRef::Empty { continue; } let (row, col) = cell.get_position(); - start = (start.0.min(row), start.1.min(col)); - end = (end.0.max(row), end.1.max(col)); + anyhow::ensure!( + row < 1_048_576 && col < 16_384, + "Cell exceeds the Excel worksheet limits" + ); + if row < start_row { + let shift = (start_row - row).min(SPREADSHEET_ROW_LIMIT as u32) as usize; + row_counts.rotate_right(shift); + row_counts[..shift].fill(0); + start_row = row; + } + end_row = end_row.max(row); + let offset = (row - start_row) as usize; + if let Some(count) = row_counts.get_mut(offset) { + *count += 1; + } } drop(reader); - if start.0 == u32::MAX { - return Ok((calamine::Range::empty(), 0)); + if start_row == u32::MAX { + return Ok((String::new(), 0, 0)); + } + let rows_total = (end_row - start_row + 1) as usize; + let mut height = rows_total.min(SPREADSHEET_ROW_LIMIT); + let mut count = 0; + for (row, &cells) in row_counts[..height].iter().enumerate() { + if cells > budget - count { + height = row; + break; + } + count += cells; } - let rows_total = end.0 as usize - start.0 as usize + 1; - let width = end.1 as usize - start.1 as usize + 1; - let rows_read = rows_total - .min(SPREADSHEET_ROW_LIMIT) - .min(SPREADSHEET_CELL_LIMIT / width); anyhow::ensure!( - rows_read > 0, - "The spreadsheet has too many columns to read" + height > 0, + "The first worksheet row exceeds the cell budget" ); - end.0 = start.0 + rows_read as u32 - 1; - let mut range = calamine::Range::new(start, end); - // A second pass keeps date/style decoding in Calamine and counts rows beyond - // the budget honestly, without retaining every nonempty cell in memory. + // Counts bound this allocation before any source values are retained. A + // distant note adds one entry, not thousands of empty Data/String objects. + let mut values = vec![std::collections::BTreeMap::::new(); height]; let mut reader = book.worksheet_cells_reader(sheet)?; while let Some(cell) = reader.next_cell()? { - let position = cell.get_position(); - if position.0 <= end.0 && cell.get_value() != &calamine::DataRef::Empty { - range.set_value(position, calamine::Data::from(cell.get_value().clone())); + let (row, col) = cell.get_position(); + let offset = row.saturating_sub(start_row) as usize; + if row >= start_row && offset < height && cell.get_value() != &calamine::DataRef::Empty { + let text = spreadsheet_cell_text(Some(&calamine::Data::from(cell.get_value().clone()))); + values[offset].insert(col, text); } } - Ok((range, rows_total)) + drop(reader); + let merges = book.merge_cells_by_sheet_name(sheet)?; + loop { + let start_col = values + .iter() + .filter_map(|row| row.keys().next()) + .min() + .copied() + .unwrap(); + let end_col = values + .iter() + .filter_map(|row| row.keys().next_back()) + .max() + .copied() + .unwrap(); + let width = (end_col - start_col + 1) as usize; + let text_at = |row: usize, col: usize| { + values[row] + .get(&(start_col + col as u32)) + .cloned() + .unwrap_or_default() + }; + let regions = spreadsheet_regions( + (start_row, start_col), + (start_row + height as u32 - 1, end_col), + &merges, + text_at, + ); + let mut owner = vec![usize::MAX; width]; + let mut rows = Vec::with_capacity(height); + let mut segments = 0; + for row in 0..height { + let cells = spreadsheet_row( + row, + width, + ®ions, + &mut owner, + |col| text_at(row, col), + true, + ); + if cells.len() > budget - segments { + break; + } + segments += cells.len(); + rows.push(cells); + } + anyhow::ensure!( + !rows.is_empty(), + "The first worksheet row exceeds the segment budget" + ); + if rows.len() != height { + height = rows.len(); + values.truncate(height); + // Dropped rows cannot leave a distant column in the kept window's + // geometry. Reproject merges too, retaining their physical spans. + continue; + } + let headers = spreadsheet_headers(®ions, &rows); + let text = match crate::table::render_grid_with_headers(&rows, headers) { + Some(text) => format!("{text}\n"), + None => { + // Preserve the old TSV fallback exactly, but materialize only + // one dense row at a time rather than the whole rectangle. + let mut text = String::new(); + for row in 0..height { + let cells = spreadsheet_row( + row, + width, + ®ions, + &mut owner, + |col| text_at(row, col), + false, + ); + let cells: Vec<&str> = cells.iter().map(|(s, _, _)| s.as_str()).collect(); + text.push_str(&cells.join("\t")); + text.push('\n'); + } + text + } + }; + return Ok((text, height, rows_total)); + } } #[derive(Clone)] @@ -900,9 +1002,36 @@ fn spreadsheet_grid( if height == 0 || width == 0 { return (Vec::new(), None); } - let end_row = start_row + height as u32 - 1; - let end_col = start_col + width as u32 - 1; + let text_at = |row, col| spreadsheet_cell_text(range.get((row, col))); + let regions = spreadsheet_regions( + (start_row, start_col), + (start_row + height as u32 - 1, start_col + width as u32 - 1), + merges, + text_at, + ); + let mut owner = vec![usize::MAX; width]; + let rows: Vec<_> = (0..height) + .map(|row| { + spreadsheet_row( + row, + width, + ®ions, + &mut owner, + |col| text_at(row, col), + false, + ) + }) + .collect(); + let headers = spreadsheet_headers(®ions, &rows); + (rows, headers) +} +fn spreadsheet_regions( + (start_row, start_col): (u32, u32), + (end_row, end_col): (u32, u32), + merges: &[calamine::Dimensions], + text_at: impl Fn(usize, usize) -> String, +) -> Vec { let mut regions = Vec::new(); for merge in merges { if merge.start.0 > merge.end.0 @@ -925,30 +1054,40 @@ fn spreadsheet_grid( regions.push(GridMerge { start, end, - text: spreadsheet_cell_text(range.get(start)), + text: text_at(start.0, start.1), }); } + regions +} - let mut owner = vec![usize::MAX; height * width]; +fn spreadsheet_row( + row: usize, + width: usize, + regions: &[GridMerge], + owner: &mut [usize], + text_at: impl Fn(usize) -> String, + compact_empty: bool, +) -> GridRow { + // One owner row keeps the old last-merge-wins projection and anchor jumps, + // without allocating height * width indices. XLSX bounds width at XFD. + owner.fill(usize::MAX); for (index, region) in regions.iter().enumerate() { - for row in region.start.0..=region.end.0 { - for col in region.start.1..=region.end.1 { - owner[row * width + col] = index; - } + if region.start.0 <= row && row <= region.end.0 { + owner[region.start.1..=region.end.1].fill(index); } } - - let mut rows: Vec = Vec::with_capacity(height); - for row in 0..height { - let mut cells = GridRow::new(); - let mut col = 0usize; - while col < width { - let index = owner[row * width + col]; - if index == usize::MAX { - cells.push((spreadsheet_cell_text(range.get((row, col))), 1, 0)); - col += 1; - continue; + let mut cells: GridRow = Vec::new(); + let mut col = 0; + while col < width { + let index = owner[col]; + if index == usize::MAX { + let text = text_at(col); + match (compact_empty && text.is_empty(), cells.last_mut()) { + (true, Some((previous, span, _))) if previous.is_empty() => *span += 1, + _ => cells.push((text, 1, 0)), } + col += 1; + } else { let region = ®ions[index]; if col == region.start.1 { cells.push((region.text.clone(), region.end.1 - region.start.1 + 1, 0)); @@ -957,11 +1096,8 @@ fn spreadsheet_grid( col += 1; } } - rows.push(cells); } - - let headers = spreadsheet_headers(®ions, &rows); - (rows, headers) + cells } fn spreadsheet_cell_text(cell: Option<&calamine::Data>) -> String { @@ -1262,20 +1398,83 @@ fn pptx_xml_to_text(xml: &str) -> anyhow::Result { mod tests { use super::*; + fn budget_book() -> calamine::Xlsx>> { + let mut zip = zip::ZipWriter::new(Cursor::new(Vec::new())); + for (name, xml) in [ + ( + "[Content_Types].xml", + r#""#, + ), + ( + "_rels/.rels", + r#""#, + ), + ( + "xl/workbook.xml", + r#""#, + ), + ( + "xl/_rels/workbook.xml.rels", + r#""#, + ), + ( + "xl/worksheets/sheet1.xml", + r#"leftright12"#, + ), + ] { + zip.start_file(name, zip::write::SimpleFileOptions::default()) + .unwrap(); + zip.write_all(xml.as_bytes()).unwrap(); + } + calamine::open_workbook_from_rs(Cursor::new(zip.finish().unwrap().into_inner())).unwrap() + } + #[test] - fn sparse_xlsx_ranges_are_bounded_before_the_dense_allocation() { - let bytes = include_bytes!("../tests/fixtures/sparse-sheets.xlsx"); - let mut book = calamine::open_workbook_from_rs(Cursor::new(bytes.to_vec())).unwrap(); - for (sheet, rows_total, rows_read, width) in - [("Tall", 1_000_000, 2_000, 16), ("Wide", 201, 200, 1_000)] - { - let (range, total) = xlsx_range(&mut book, sheet).unwrap(); - assert_eq!(total, rows_total); - assert_eq!(range.get_size(), (rows_read, width)); - assert!(range.height() * range.width() <= SPREADSHEET_CELL_LIMIT); + fn a_segment_budget_keeps_only_complete_rows() { + // Four source cells fit; two rows of three emitted segments do not. + let (_, rows_read, rows_total) = xlsx_sheet(&mut budget_book(), "Budget", 5).unwrap(); + assert_eq!((rows_read, rows_total), (1, 2)); + } + + #[test] + fn a_first_row_that_exceeds_the_budget_is_reported() { + for budget in [1, 2] { + let error = xlsx_sheet(&mut budget_book(), "Budget", budget).unwrap_err(); + assert!(error.to_string().contains("first worksheet row")); } } + #[test] + fn empty_runs_keep_the_physical_cell_and_merge_coordinates() { + let regions = [GridMerge { + start: (0, 2), + end: (0, 4), + text: "Merged".into(), + }]; + let row = spreadsheet_row( + 0, + 10, + ®ions, + &mut [usize::MAX; 10], + |col| match col { + 0 => "Item".into(), + 7 => "3".into(), + _ => String::new(), + }, + true, + ); + let expected = [ + ("Item", 1), + ("", 1), + ("Merged", 3), + ("", 2), + ("3", 1), + ("", 2), + ] + .map(|(text, span)| (text.to_owned(), span, 0)); + assert_eq!(row, expected); + } + const DOC: &str = r#" Segment results SegmentRevenue diff --git a/crates/utopia-ingest/tests/fixtures/README.md b/crates/utopia-ingest/tests/fixtures/README.md deleted file mode 100644 index 3b35261c8..000000000 --- a/crates/utopia-ingest/tests/fixtures/README.md +++ /dev/null @@ -1,9 +0,0 @@ -`sparse-sheets.xlsx` is a small, model-free OOXML workbook with two sparse sheets: - -- `Tall`: A1 = `tall-header`, B2000 = `tall-last-read`, P1000000 = `tall-omitted`. -- `Wide`: A1 = `wide-header`, B200 = `wide-last-read`, ALL201 = `wide-omitted`. - -Tall has an explicit worksheet dimension; Wide omits the optional hint. The -range test checks the retained dense grid dimensions. -The public parser test checks the retained content and complete row warnings. -Run both with `cargo test -p utopia-ingest sparse`. diff --git a/crates/utopia-ingest/tests/fixtures/sparse-sheets.xlsx b/crates/utopia-ingest/tests/fixtures/sparse-sheets.xlsx deleted file mode 100644 index 8ddd11a052e5ce9e12c333792ea628c79fe9b178..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 1747 zcmWIWW@Zs#U|`^2V44~bd(H6tj@7Hk&DlC> zuOG9aKui47CuUvdOBOinG-(kF`oPqkub)2m|C75a$8SG27G&j8P+1Z7?aUkVqj~e< zg$s=<#dm}@d+cIPFrMIk%`i~IOMbJv=J}e2mb4?x2dlR3Vij%sNxw9F3Z;#+>9c9D)gOYEephns!(Xt1${b8h}*`e0JE zRp7a8i_`ie?`utpZI^{ z(Vd*1O5Yw@K9LuW?^T-IHz)c=SWTB+`NXAq8VTti9v^(ii|S-O)8MajfllRPWMJS2 z(iJ)S<@rU~N%{HNpxEo3u%EBVK%hN-=}z_!4F?+;CU@`dEMD54n^C+gJaGINh%|0~g&DPlS2f_pXgE?w~Sfbh|uMg?3}=aY9W_UYUr=sl(R_$rM) zi7nk5q+WgLZ*D*MwdB!1L5*jjlTN&TW6@PQ+2Ohl=clz=uN-t^wtQbO?d;7h0Zm?8 zXU{G%Q+=RcUD{Tbvo(1&e|I!Pp8bn^s6ILHL8^2*&xF6>G)`Vvcx2|T6y3cIg(=o5{|*LaR{x&m(BJOIbz_>n~leW*AKA?!2k3m%VMml(;FbFJ?Vss5~v;ygxPUv5KqQ^01KU z?uX)UT$~a4So78D;|Huo3%V;6+fT5r+T=Rzk+9$%w`lQI4Yf1hoj1B0&9?8Jtqmip zclbBozkC4bjT)eLBp}`ad89ZaHMOKzA50m7VyJe4FITgHKMnZs&i%U4)mfRxkYvB`@<*ApFE3Mh zo|r73Hcuek$##uW`a-?a+!nce0*VayYE2G`w}FiLcsLn z4;X&@{d(+9uuMX;@tGguT=pju<9GZ*%Rxt Vec { +fn ledger_rows(data_rows: usize) -> String { let mut sheet_data = String::from( r#"idname"#, ); @@ -14,17 +14,65 @@ fn workbook(data_rows: usize) -> Vec { r#"{id}customer-{id}"# )); } - let worksheet = format!( - r#"{sheet_data}"# + sheet_data +} + +fn workbook(data_rows: usize) -> Vec { + workbook_xml(&ledger_rows(data_rows)) +} + +fn workbook_xml(sheet_data: &str) -> Vec { + workbook_sheets(&[("Ledger", sheet_data)]) +} + +fn workbook_sheets(sheets: &[(&str, &str)]) -> Vec { + workbook_sheets_with_chart(sheets, false) +} + +fn workbook_sheets_with_chart(sheets: &[(&str, &str)], chart: bool) -> Vec { + let mut content_types = String::from( + r#""#, ); + let mut sheet_entries = String::new(); + let mut sheet_rels = String::new(); + let mut parts = Vec::new(); + for (i, (name, sheet_data)) in sheets.iter().enumerate() { + let id = i + 1; + write!(content_types, r#""#).unwrap(); + write!( + sheet_entries, + r#""# + ) + .unwrap(); + write!(sheet_rels, r#""#).unwrap(); + parts.push((format!("xl/worksheets/sheet{id}.xml"), format!(r#"{sheet_data}"#))); + } + if chart { + let id = sheets.len() + 1; + write!(content_types, r#""#).unwrap(); + write!( + sheet_entries, + r#""# + ) + .unwrap(); + write!(sheet_rels, r#""#).unwrap(); + parts.extend([ + (format!("xl/chartsheets/sheet{id}.xml"), r#""#.into()), + (format!("xl/chartsheets/_rels/sheet{id}.xml.rels"), r#""#.into()), + ("xl/drawings/drawing1.xml".into(), r#""#.into()), + ("xl/drawings/_rels/drawing1.xml.rels".into(), r#""#.into()), + ("xl/charts/chart1.xml".into(), r#"Series 1ItemGeneral1"#.into()), + ]); + } + content_types.push_str(""); + parts.extend([ + ("[Content_Types].xml".into(), content_types), + ("_rels/.rels".into(), r#""#.into()), + ("xl/workbook.xml".into(), format!(r#"{sheet_entries}"#)), + ("xl/_rels/workbook.xml.rels".into(), format!(r#"{sheet_rels}"#)), + ]); let mut zip = zip::ZipWriter::new(Cursor::new(Vec::new())); - for (part, xml) in [ - ("[Content_Types].xml", r#""#.to_string()), - ("_rels/.rels", r#""#.to_string()), - ("xl/workbook.xml", r#""#.to_string()), - ("xl/_rels/workbook.xml.rels", r#""#.to_string()), - ("xl/worksheets/sheet1.xml", worksheet), - ] { + for (part, xml) in parts { zip.start_file(part, zip::write::SimpleFileOptions::default()) .unwrap(); zip.write_all(xml.as_bytes()).unwrap(); @@ -32,6 +80,23 @@ fn workbook(data_rows: usize) -> Vec { zip.finish().unwrap().into_inner() } +fn words(cell: &str, text: &str) -> String { + format!(r#"{text}"#) +} + +fn column(mut col: usize) -> String { + let mut letters = Vec::new(); + loop { + letters.push(b'A' + (col % 26) as u8); + if col < 26 { + break; + } + col = col / 26 - 1; + } + letters.reverse(); + String::from_utf8(letters).unwrap() +} + fn csv(records: usize) -> String { let mut csv = String::from("id,name\n"); for id in 1..=records { @@ -79,40 +144,105 @@ fn a_spreadsheet_at_the_row_cap_has_no_warning() { } #[test] -fn sparse_sheets_keep_the_budget_boundary_and_report_every_omitted_row() { - let parsed = utopia_ingest::parse( - "sparse-sheets.xlsx", - include_bytes!("fixtures/sparse-sheets.xlsx"), - ) - .unwrap(); - for sheet in ["tall", "wide"] { - assert!(parsed.text.contains(&format!("{sheet}-header"))); - assert!(parsed.text.contains(&format!("{sheet}-last-read"))); - assert!(!parsed.text.contains(&format!("{sheet}-omitted"))); - } - let warnings: Vec<_> = parsed - .warnings - .iter() - .map(|warning| { - assert_eq!(warning.kind, "spreadsheet.rows_truncated"); - warning.detail.clone() - }) - .collect(); +fn a_distant_cell_past_the_row_window_does_not_shrink_the_retained_ledger() { + let mut rows = ledger_rows(1_999); + rows.push_str(&format!( + r#"{}"#, + words("P1000000", "omitted-distant-note") + )); + let parsed = utopia_ingest::parse("ledger.xlsx", &workbook_xml(&rows)).unwrap(); + assert!(parsed.text.contains("| 1999 | customer-1999 |")); + assert!(!parsed.text.contains("omitted-distant-note")); + assert_eq!(parsed.warnings.len(), 1); + assert_eq!(parsed.warnings[0].kind, "spreadsheet.rows_truncated"); assert_eq!( - warnings, - vec![ - serde_json::json!({ - "sheet": "Tall", "rows_read": 2000, - "rows_total": 1000000, "rows_omitted": 998000, - }), - serde_json::json!({ - "sheet": "Wide", "rows_read": 200, - "rows_total": 201, "rows_omitted": 1, - }), - ] + parsed.warnings[0].detail, + serde_json::json!({ + "sheet": "Ledger", "rows_read": 2000, + "rows_total": 1000000, "rows_omitted": 998000, + }) ); } +#[test] +fn a_header_note_beyond_empty_columns_does_not_truncate_a_ledger() { + let rows = ledger_rows(1_500); + let expected = utopia_ingest::parse("ledger.xlsx", &workbook_xml(&rows)).unwrap(); + assert!(expected.text.contains("| 1500 | customer-1500 |")); + for reference in ["AZZ1", "XFD1"] { + let with_note = rows.replacen( + "", + &format!("{}", words(reference, "header-only-note")), + 1, + ); + let parsed = utopia_ingest::parse("ledger.xlsx", &workbook_xml(&with_note)).unwrap(); + assert!( + parsed.text == expected.text, + "note at {reference} changed the ledger" + ); + assert!( + parsed.warnings.is_empty(), + "{reference}: {:?}", + parsed.warnings + ); + } +} + +#[test] +fn an_ordinary_dense_sheet_retains_two_thousand_rows_and_one_hundred_fifty_columns() { + let columns: Vec<_> = (0..150).map(column).collect(); + let mut rows = String::from(r#""#); + for (col, name) in columns.iter().enumerate() { + rows.push_str(&words(&format!("{name}1"), &format!("column-{col}"))); + } + rows.push_str(""); + for row in 2..=2_000 { + write!( + rows, + r#"{}"#, + words(&format!("A{row}"), &format!("customer-{}", row - 1)) + ) + .unwrap(); + for (col, name) in columns.iter().enumerate().skip(1) { + write!( + rows, + r#"{}"#, + row * 1_000 + col + ) + .unwrap(); + } + rows.push_str(""); + } + let parsed = utopia_ingest::parse("ledger.xlsx", &workbook_xml(&rows)).unwrap(); + assert!(parsed.warnings.is_empty(), "{:?}", parsed.warnings); + let last = parsed + .text + .lines() + .find(|line| line.starts_with("| customer-1999 |")) + .expect("the final row is retained"); + assert!(last.ends_with("| 2000149 |"), "the last column is retained"); + assert_eq!(last.split('|').count(), 152, "all 150 columns are present"); +} + +#[test] +fn a_broken_sheet_is_reported_even_when_another_sheet_is_readable() { + let good = ledger_rows(1); + let broken = r#"not-a-number"#; + let bytes = workbook_sheets(&[("Ledger", &good), ("Broken", broken)]); + let error = utopia_ingest::parse("ledger.xlsx", &bytes).unwrap_err(); + assert!(format!("{error:#}").contains("Broken"), "{error:#}"); +} + +#[test] +fn a_chartsheet_does_not_prevent_reading_the_worksheets() { + let rows = ledger_rows(1); + let expected = utopia_ingest::parse("ledger.xlsx", &workbook_xml(&rows)).unwrap(); + let bytes = workbook_sheets_with_chart(&[("Ledger", &rows)], true); + let parsed = utopia_ingest::parse("ledger.xlsx", &bytes).unwrap(); + assert_eq!(parsed.text, expected.text); + assert!(parsed.warnings.is_empty(), "{:?}", parsed.warnings); +} + #[test] fn a_csv_past_the_record_cap_reports_the_records_it_left_out() { let parsed = utopia_ingest::parse("customers.csv", csv(10_000).as_bytes()).unwrap(); From bf839b5bd07e06ba1add710a4a2666ce041c498e Mon Sep 17 00:00:00 2001 From: dada-yan Date: Sat, 17 Oct 2026 10:14:36 +1100 Subject: [PATCH 3/3] Preserve readable worksheets when another sheet cannot be decoded Signed-off-by: dada-yan --- crates/utopia-ingest/src/parsers.rs | 63 ++++++++++--------- .../tests/spreadsheet_truncation.rs | 38 ++++++++--- 2 files changed, 65 insertions(+), 36 deletions(-) diff --git a/crates/utopia-ingest/src/parsers.rs b/crates/utopia-ingest/src/parsers.rs index 636fb8d3f..a80d11ce5 100644 --- a/crates/utopia-ingest/src/parsers.rs +++ b/crates/utopia-ingest/src/parsers.rs @@ -779,8 +779,12 @@ pub fn spreadsheet(bytes: &[u8]) -> anyhow::Result<(String, Vec xlsx_sheet(book, &sheet_name, XLSX_SEGMENT_LIMIT) - .with_context(|| format!("Could not read worksheet {sheet_name}"))?, + calamine::Sheets::Xlsx(book) => { + let Ok(sheet) = xlsx_sheet(book, &sheet_name, XLSX_SEGMENT_LIMIT) else { + continue; + }; + sheet + } _ => { let Ok(range) = workbook.worksheet_range(&sheet_name) else { continue; @@ -838,12 +842,7 @@ fn xlsx_sheet( let mut start_row = u32::MAX; let mut end_row = 0; let mut row_counts = vec![0usize; SPREADSHEET_ROW_LIMIT]; - let mut reader = match book.worksheet_cells_reader(sheet) { - Ok(reader) => reader, - // Calamine's ordinary worksheet_range also skips chart sheets. - Err(calamine::XlsxError::NotAWorksheet(_)) => return Ok((String::new(), 0, 0)), - Err(error) => return Err(error.into()), - }; + let mut reader = book.worksheet_cells_reader(sheet)?; // Keep only the first 2,000 physical rows' counts, even if XML cells arrive // out of order. The full row bounds survive so truncation remains honest. while let Some(cell) = reader.next_cell()? { @@ -864,7 +863,9 @@ fn xlsx_sheet( end_row = end_row.max(row); let offset = (row - start_row) as usize; if let Some(count) = row_counts.get_mut(offset) { - *count += 1; + // Repeated coordinates overwrite values below, so even malformed + // XML cannot retain more than one physical row's worth of cells. + *count = (*count + 1).min(16_384); } } drop(reader); @@ -881,10 +882,6 @@ fn xlsx_sheet( } count += cells; } - anyhow::ensure!( - height > 0, - "The first worksheet row exceeds the cell budget" - ); // Counts bound this allocation before any source values are retained. A // distant note adds one entry, not thousands of empty Data/String objects. let mut values = vec![std::collections::BTreeMap::::new(); height]; @@ -898,7 +895,7 @@ fn xlsx_sheet( } } drop(reader); - let merges = book.merge_cells_by_sheet_name(sheet)?; + let merges = book.merge_cells_by_sheet_name(sheet).unwrap_or_default(); loop { let start_col = values .iter() @@ -943,10 +940,6 @@ fn xlsx_sheet( segments += cells.len(); rows.push(cells); } - anyhow::ensure!( - !rows.is_empty(), - "The first worksheet row exceeds the segment budget" - ); if rows.len() != height { height = rows.len(); values.truncate(height); @@ -1398,7 +1391,10 @@ fn pptx_xml_to_text(xml: &str) -> anyhow::Result { mod tests { use super::*; - fn budget_book() -> calamine::Xlsx>> { + fn budget_book(sheet_data: &str, merges: &str) -> calamine::Xlsx>> { + let worksheet = format!( + r#"{sheet_data}{merges}"# + ); let mut zip = zip::ZipWriter::new(Cursor::new(Vec::new())); for (name, xml) in [ ( @@ -1417,10 +1413,7 @@ mod tests { "xl/_rels/workbook.xml.rels", r#""#, ), - ( - "xl/worksheets/sheet1.xml", - r#"leftright12"#, - ), + ("xl/worksheets/sheet1.xml", &worksheet), ] { zip.start_file(name, zip::write::SimpleFileOptions::default()) .unwrap(); @@ -1432,16 +1425,28 @@ mod tests { #[test] fn a_segment_budget_keeps_only_complete_rows() { // Four source cells fit; two rows of three emitted segments do not. - let (_, rows_read, rows_total) = xlsx_sheet(&mut budget_book(), "Budget", 5).unwrap(); + let rows = r#"leftright12"#; + let (_, rows_read, rows_total) = + xlsx_sheet(&mut budget_book(rows, ""), "Budget", 5).unwrap(); assert_eq!((rows_read, rows_total), (1, 2)); } #[test] - fn a_first_row_that_exceeds_the_budget_is_reported() { - for budget in [1, 2] { - let error = xlsx_sheet(&mut budget_book(), "Budget", budget).unwrap_err(); - assert!(error.to_string().contains("first worksheet row")); - } + fn dropping_a_row_reprojects_the_retained_title_and_merged_span() { + let title = r#"Title"#; + let rest = r#" note"#; + let merges = r#""#; + // Four source cells fit, but the first two rows use 3 + 4 segments. + let (text, rows_read, rows_total) = xlsx_sheet( + &mut budget_book(&format!("{title}{rest}"), merges), + "Budget", + 5, + ) + .unwrap(); + let (expected, _, _) = xlsx_sheet(&mut budget_book(title, merges), "Budget", 5).unwrap(); + assert_eq!((rows_read, rows_total), (1, 3)); + assert!(text.contains("Title")); + assert_eq!(text, expected); } #[test] diff --git a/crates/utopia-ingest/tests/spreadsheet_truncation.rs b/crates/utopia-ingest/tests/spreadsheet_truncation.rs index e2cbf0bdd..d4df2fd24 100644 --- a/crates/utopia-ingest/tests/spreadsheet_truncation.rs +++ b/crates/utopia-ingest/tests/spreadsheet_truncation.rs @@ -26,10 +26,14 @@ fn workbook_xml(sheet_data: &str) -> Vec { } fn workbook_sheets(sheets: &[(&str, &str)]) -> Vec { - workbook_sheets_with_chart(sheets, false) + workbook_sheets_with_chart(sheets, false, "") } -fn workbook_sheets_with_chart(sheets: &[(&str, &str)], chart: bool) -> Vec { +fn workbook_sheets_with_chart( + sheets: &[(&str, &str)], + chart: bool, + worksheet_suffix: &str, +) -> Vec { let mut content_types = String::from( r#""#, ); @@ -45,7 +49,7 @@ fn workbook_sheets_with_chart(sheets: &[(&str, &str)], chart: bool) -> Vec { ) .unwrap(); write!(sheet_rels, r#""#).unwrap(); - parts.push((format!("xl/worksheets/sheet{id}.xml"), format!(r#"{sheet_data}"#))); + parts.push((format!("xl/worksheets/sheet{id}.xml"), format!(r#"{sheet_data}{worksheet_suffix}"#))); } if chart { let id = sheets.len() + 1; @@ -225,19 +229,39 @@ fn an_ordinary_dense_sheet_retains_two_thousand_rows_and_one_hundred_fifty_colum } #[test] -fn a_broken_sheet_is_reported_even_when_another_sheet_is_readable() { +fn a_broken_sheet_does_not_prevent_reading_the_other_sheets() { let good = ledger_rows(1); + let expected = utopia_ingest::parse("ledger.xlsx", &workbook_xml(&good)).unwrap(); let broken = r#"not-a-number"#; let bytes = workbook_sheets(&[("Ledger", &good), ("Broken", broken)]); - let error = utopia_ingest::parse("ledger.xlsx", &bytes).unwrap_err(); - assert!(format!("{error:#}").contains("Broken"), "{error:#}"); + let parsed = utopia_ingest::parse("ledger.xlsx", &bytes).unwrap(); + assert_eq!(parsed.text, expected.text); + assert!(parsed.warnings.is_empty(), "{:?}", parsed.warnings); +} + +#[test] +fn unreadable_merges_do_not_prevent_reading_the_worksheet_cells() { + use calamine::Reader as _; + + let rows = ledger_rows(1); + let expected = utopia_ingest::parse("ledger.xlsx", &workbook_xml(&rows)).unwrap(); + let merges = r#""#; + let bytes = workbook_sheets_with_chart(&[("Ledger", &rows)], false, merges); + let mut book: calamine::Xlsx<_> = + calamine::open_workbook_from_rs(Cursor::new(bytes.clone())).unwrap(); + assert_eq!(book.worksheet_range("Ledger").unwrap().height(), 2); + assert!(book.merge_cells_by_sheet_name("Ledger").is_err()); + + let parsed = utopia_ingest::parse("ledger.xlsx", &bytes).unwrap(); + assert_eq!(parsed.text, expected.text); + assert!(parsed.warnings.is_empty(), "{:?}", parsed.warnings); } #[test] fn a_chartsheet_does_not_prevent_reading_the_worksheets() { let rows = ledger_rows(1); let expected = utopia_ingest::parse("ledger.xlsx", &workbook_xml(&rows)).unwrap(); - let bytes = workbook_sheets_with_chart(&[("Ledger", &rows)], true); + let bytes = workbook_sheets_with_chart(&[("Ledger", &rows)], true, ""); let parsed = utopia_ingest::parse("ledger.xlsx", &bytes).unwrap(); assert_eq!(parsed.text, expected.text); assert!(parsed.warnings.is_empty(), "{:?}", parsed.warnings);