XLSX: a Python spill is recorded at its anchor, and one too large keeps its values

A Python cell's spill range was recorded only when the file held a record for
one of its other cells, so a result whose other cells are blank, which a file
may leave out, opened with no spill and no rebuilt result. The range is now
recorded when the anchor is read, if a result can fill it.

A Python range larger than a result may be (more than 1,000,000 cells, but
within what the reader tracks) emptied the cells it covered, although it is
no spill and file-format-parity.md says such cells keep the values the file
shows. They are now kept as plain values; a formula's spilled values are still
dropped, since recalculation fills them again.

Tests: a_python_spill_whose_blank_cells_have_no_records_is_still_a_spill and
a_python_spill_range_larger_than_a_result_keeps_its_values_as_plain_values;
both fail without the fix.
This commit is contained in:
prxnvt
2026-11-02 02:02:07 -05:00
parent b78afc9b3c
commit ad6aca08bf
2 changed files with 61 additions and 4 deletions
+36
View File
@@ -317,6 +317,42 @@ fn a_python_cell_without_its_code_keeps_its_whole_cached_result_as_values() {
}
}
#[test]
fn a_python_spill_whose_blank_cells_have_no_records_is_still_a_spill() {
// A1's result spills over A1:A3, and the file holds no records for the blank A2 and A3.
let data = "<sheetData><row r=\"1\"><c r=\"A1\" cm=\"1\"><f t=\"array\" ref=\"A1:A3\">_xlfn._xlws.PY(0,0)</f><v>1</v></c></row></sheetData>";
let mut s = s();
open(&mut s, "sparse.xlsx", &excel_shaped(data, Some(&["[1, None, None]"])));
let sh = s.doc().unwrap().wb.active().unwrap();
let a1 = CellRef::new(0, 0);
assert_eq!(sh.spill_ranges.get(&a1), Some(&RangeRef::parse("A1:A3").unwrap()), "the range is recorded at its anchor");
assert_eq!(sh.spill_anchor(CellRef::parse("A3").unwrap()), Some(a1));
let kept = sh.cell(a1).and_then(|c| c.py.as_ref()).and_then(|p| p.output.as_ref()).map(|o| o.value.clone());
assert_eq!(kept, Some(array(3, 1, vec![1.0.into(), Value::Empty, Value::Empty])), "the result is rebuilt over the whole range");
// A formula entered in the spill takes the area, as with any spill.
s.execute("cell.set", json!({"cell": "A3", "input": "7"})).unwrap();
assert_eq!(v(&s, "A1"), Value::Error(CellError::Spill));
}
#[test]
fn a_python_spill_range_larger_than_a_result_keeps_its_values_as_plain_values() {
// C1:D600000 is 1,200,000 cells, more than a result may have but within what the reader
// tracks; its cached values are kept as the file shows them, and nothing spills.
let (mut s, _) = build();
let bytes = save(&mut s, "xlsx");
let crafted = with_sheet_xml(&bytes, |xml| xml.replace("ref=\"C1:D2\"", "ref=\"C1:D600000\""));
assert_ne!(crafted, bytes);
open(&mut s, "crafted.xlsx", &crafted);
let sh = s.doc().unwrap().wb.active().unwrap();
assert!(!sh.spill_ranges.contains_key(&CellRef::new(0, 2)), "{:?}", sh.spill_ranges);
assert_eq!(
(v(&s, "C1"), v(&s, "D1"), v(&s, "C2"), v(&s, "D2")),
(Value::Number(1.0), Value::text("two"), Value::Number(3.0), Value::text("four"))
);
let d2 = sh.cell(CellRef::parse("D2").unwrap()).expect("D2 is stored");
assert!(d2.py.is_none() && d2.formula.is_none(), "a plain value");
}
#[test]
fn a_script_is_measured_after_its_escapes_are_decoded() {
let max = gridcraft_model::python::MAX_FORMULA_CHARS;
+25 -4
View File
@@ -93,6 +93,20 @@ fn forget_dynamic(st: &mut RowState, pos: CellRef) {
}
}
/// Records the spill range of the Python cell read at `pos`, its anchor, when the file declares
/// one that a result can fill: the spill then exists even when the file holds no records for its
/// blank cells. Charged like [`dynamic_range`]'s scans.
fn record_python_spill(st: &mut RowState, sheet: &mut Sheet, pos: CellRef) {
if st.dynamic_checks > MAX_DYNAMIC_CHECKS {
return;
}
st.dynamic_checks = st.dynamic_checks.saturating_add(st.dynamic.len() as u64);
let Some(r) = st.dynamic.iter().rev().find(|r| r.start == pos).copied() else { return };
if r.count() > 1 && r.count() <= gridcraft_model::python::MAX_RESULT_CELLS as u64 {
sheet.spill_ranges.insert(pos, r);
}
}
const MAX_HYPERLINK_CELLS: u64 = 10_000;
/// Target and tooltip bytes kept for hyperlinks of one sheet, over all cells: the same URL is
/// copied for every cell of its range, so length × cells must stay bounded whatever the file.
@@ -557,18 +571,25 @@ fn read_cell(cx: &mut Ctx<'_>, sheet: &mut Sheet, st: &mut RowState, c: &El, pos
(None, None)
}
};
if py.is_some() {
record_python_spill(st, sheet, pos);
}
let spilled_from = if formula.is_none() && py.is_none() { dynamic_range(st, pos) } else { None };
let python_anchor = spilled_from.is_some_and(|r| sheet.cells.get(r.start).is_some_and(|a| a.py.is_some()));
let value = match spilled_from {
// A Python cell's spilled values are kept: they are its result until it runs again. A
// range larger than a result may be is a crafted file's: its values are dropped, like a
// formula's spilled values, and no spill is recorded.
Some(r) if r.count() <= gridcraft_model::python::MAX_RESULT_CELLS as u64 && sheet.cells.get(r.start).is_some_and(|a| a.py.is_some()) => {
// A Python cell's spilled values are kept: they are its result until it runs again.
Some(r) if python_anchor && r.count() <= gridcraft_model::python::MAX_RESULT_CELLS as u64 => {
if !value.is_empty() {
sheet.spill.insert(pos, value);
}
sheet.spill_ranges.insert(r.start, r);
Value::Empty
}
// A Python cell's range larger than a result may be is a crafted file's: it is no spill,
// and its cells keep the values the file shows as plain values, as when the cell keeps
// only its own value.
Some(_) if python_anchor => value,
// A formula's spilled values are dropped: recalculation fills them again.
Some(_) => Value::Empty,
None => value,
};