From 53690bace0aec7d5091b2fb7a2ba9a705a447e78 Mon Sep 17 00:00:00 2001 From: neilcawse <8085959+neilcawse@users.noreply.github.com> Date: Sun, 11 Oct 2026 16:39:31 -0400 Subject: [PATCH] Functions: oracle fixes - MODE/VALUE/SUMPRODUCT text args, ADDRESS [Book]Sheet, Excel-style lookup bisection, SUBTOTAL skips filtered rows (table filter criteria kept) Co-Authored-By: Claude Opus 5.5 --- crates/calc/src/eval.rs | 6 +- crates/calc/src/tests.rs | 47 +++++++ crates/engine/src/cmd/data.rs | 5 + crates/engine/src/cmd/forecast.rs | 1 + crates/engine/src/cmd/insert.rs | 8 +- crates/functions/src/lookup.rs | 173 +++++++++++++++++++++----- crates/functions/src/math.rs | 33 ++++- crates/functions/src/stat.rs | 63 +++++++++- crates/functions/src/text.rs | 28 ++++- crates/model/src/features.rs | 11 ++ crates/model/src/sheet.rs | 11 ++ crates/xlsx/src/sheet_read.rs | 8 +- crates/xlsx/src/sheet_write.rs | 28 +++-- crates/xlsx/src/tests/array_export.rs | 2 + crates/xlsx/src/tests/pivot_tests.rs | 1 + crates/xlsx/src/tests/roundtrip.rs | 6 + docs/file-format-parity.md | 3 +- docs/function-parity.md | 3 +- 18 files changed, 376 insertions(+), 61 deletions(-) diff --git a/crates/calc/src/eval.rs b/crates/calc/src/eval.rs index 3d4a743..9ce05be 100644 --- a/crates/calc/src/eval.rs +++ b/crates/calc/src/eval.rs @@ -1201,6 +1201,9 @@ impl<'h> Evaluator<'h> { return err(CellError::Value); } let skip_hidden = matches!(options, 1 | 3 | 5 | 7); + // SUBTOTAL leaves out rows a filter hides whatever the function number (Microsoft's + // SUBTOTAL documentation); 101-111 also leave out rows hidden by hand. + let skip_filtered = name == "SUBTOTAL"; let skip_errors = matches!(options, 2 | 3 | 6 | 7); let skip_nested = options <= 3; let mut values = Vec::new(); @@ -1218,7 +1221,8 @@ impl<'h> Evaluator<'h> { let (hidden, nested) = { let wb = self.host.workbook(); let s = wb.sheet(a.sheet); - let hidden = skip_hidden && s.is_some_and(|s| s.is_row_hidden(c.row)); + let hidden = + s.is_some_and(|s| if skip_hidden { s.is_row_hidden(c.row) } else { skip_filtered && s.is_row_filtered(c.row) }); let nested = skip_nested && s.and_then(|s| s.cell(c)).and_then(|cell| cell.formula.as_ref()).is_some_and(|f| { let u = f.text.to_ascii_uppercase(); diff --git a/crates/calc/src/tests.rs b/crates/calc/src/tests.rs index 2205a1e..ffeccb8 100644 --- a/crates/calc/src/tests.rs +++ b/crates/calc/src/tests.rs @@ -195,6 +195,7 @@ fn arrays_do_not_spill_in_tables() { first_col: false, last_col: false, filter_button: false, + filter: vec![], }); t.set("C4", "=SEQUENCE(2)"); t.set("E1", "=SEQUENCE(4)"); @@ -385,6 +386,52 @@ fn subtotal_skips_hidden() { assert_eq!(t.num("B2"), 6.0); } +/// SUBTOTAL leaves out rows a filter hides for every function number, while 1-11 keep rows +/// hidden by hand (as in ConditionalFormattingSamples.xlsx, where `SUBTOTAL(9,Table69[Sales])` +/// under a filtered table adds only the visible rows). +#[test] +fn subtotal_skips_filtered_rows() { + use gridcraft_model::{AutoFilter, FilterCriterion, LineInfo}; + let mut t = T::new(); + for (i, v) in [1, 2, 4, 8, 16, 32].iter().enumerate() { + t.set(&format!("A{}", i + 2), &v.to_string()); + t.set(&format!("C{}", i + 2), &v.to_string()); + } + let crit = || vec![(0, FilterCriterion::Custom { a: (">".into(), "1".into()), b: None, and: false })]; + let sh = t.wb.sheet_mut(0).unwrap(); + sh.tables.push(gridcraft_model::Table { + id: 1, + name: "Sales".into(), + range: gridcraft_core::RangeRef::parse("A1:A7").unwrap(), + header_row: true, + totals_row: false, + columns: vec![gridcraft_model::TableColumn { name: "Amount".into(), totals: Default::default(), totals_label: None, formula: None }], + style: String::new(), + banded_rows: false, + banded_cols: false, + first_col: false, + last_col: false, + filter_button: true, + filter: crit(), + }); + sh.autofilter = Some(AutoFilter { range: gridcraft_core::RangeRef::parse("C1:C7").unwrap(), criteria: crit() }); + // Rows 3 and 5 are filtered out; row 9 (outside both filters) is hidden by hand. + for row in [2, 4, 8] { + sh.rows.insert(row, LineInfo { hidden: true, ..Default::default() }); + } + t.set("A9", "64"); + t.set("E1", "=SUBTOTAL(9,Sales[Amount])"); + t.set("E2", "=SUBTOTAL(109,Sales[Amount])"); + t.set("E3", "=SUBTOTAL(9,C2:C7)"); + t.set("E4", "=SUBTOTAL(9,A2:A9)"); + t.set("E5", "=SUBTOTAL(109,A2:A9)"); + t.set("E6", "=SUBTOTAL(2,A2:A7)"); + t.set("E7", "=AGGREGATE(9,4,A2:A7)"); + for (at, want) in [("E1", 53.0), ("E2", 53.0), ("E3", 53.0), ("E4", 117.0), ("E5", 53.0), ("E6", 4.0), ("E7", 63.0)] { + assert_eq!(t.num(at), want, "{at}"); + } +} + #[test] fn recalc_all_after_load() { let mut t = T::new(); diff --git a/crates/engine/src/cmd/data.rs b/crates/engine/src/cmd/data.rs index 32ec866..9abbbb0 100644 --- a/crates/engine/src/cmd/data.rs +++ b/crates/engine/src/cmd/data.rs @@ -416,6 +416,9 @@ fn toggle_filter(s: &mut Session, p: &Json) -> Result { let sh = cx.sheet_mut(sheet)?; if let Some(t) = sh.tables.iter_mut().find(|t| t.name == name) { t.filter_button = !t.filter_button; + if !t.filter_button { + t.filter.clear(); + } } Ok(Json::Null) }); @@ -596,6 +599,8 @@ pub(crate) fn apply_filter(wb: &mut Workbook, sheet: usize) { }, FilterCriterion::FillColor(col) => wb.styles.get(sh.style_id(c)).fill.fg == *col, FilterCriterion::FontColor(col) => wb.styles.get(sh.style_id(c)).font.color == *col, + // Icons are not evaluated here: the row keeps its current visibility. + FilterCriterion::Icon { .. } => !sh.is_row_hidden(row), }; if !ok { visible = false; diff --git a/crates/engine/src/cmd/forecast.rs b/crates/engine/src/cmd/forecast.rs index f53ab66..14abe10 100644 --- a/crates/engine/src/cmd/forecast.rs +++ b/crates/engine/src/cmd/forecast.rs @@ -358,6 +358,7 @@ fn forecast_sheet(s: &mut Session, p: &Json) -> Result { first_col: false, last_col: false, filter_button: true, + filter: vec![], }); let q = gridcraft_formula::quote_sheet(&sheet_name); let col_ref = |c: u32| format!("{q}!${0}$2:${0}${end_row}", col_letters(c)); diff --git a/crates/engine/src/cmd/insert.rs b/crates/engine/src/cmd/insert.rs index 6a9137a..64d9a28 100644 --- a/crates/engine/src/cmd/insert.rs +++ b/crates/engine/src/cmd/insert.rs @@ -296,6 +296,7 @@ fn insert_table(s: &mut Session, p: &Json) -> Result { first_col: false, last_col: false, filter_button: true, + filter: vec![], }); cx.structural = true; Ok(json!({"table": name, "range": r.a1()})) @@ -355,7 +356,12 @@ fn table_flag(s: &mut Session, p: &Json, which: &str) -> Result { "bandedCols" => t.banded_cols = on.unwrap_or(!t.banded_cols), "firstCol" => t.first_col = on.unwrap_or(!t.first_col), "lastCol" => t.last_col = on.unwrap_or(!t.last_col), - "header" => t.filter_button = on.unwrap_or(!t.filter_button), + "header" => { + t.filter_button = on.unwrap_or(!t.filter_button); + if !t.filter_button { + t.filter.clear(); + } + } _ => {} } Ok(Json::Null) diff --git a/crates/functions/src/lookup.rs b/crates/functions/src/lookup.rs index 21294d4..56739c9 100644 --- a/crates/functions/src/lookup.rs +++ b/crates/functions/src/lookup.rs @@ -33,43 +33,77 @@ fn find_exact(x: &Value, hay: &[Value], wild: bool, reverse: bool) -> Option= x` (descending), like Excel's approximate match. +/// Approximate match of VLOOKUP, HLOOKUP, LOOKUP and MATCH: the position of the largest entry +/// `<= x` in data sorted ascending (`desc = false`), or of the smallest entry `>= x` in data +/// sorted descending. Like Excel this is a bisection over the whole vector, so on unsorted data +/// it answers what Excel answers rather than a linear "best" entry: +/// - the probe is the middle entry of the remaining window; a probe whose type differs from the +/// needle (text vs number vs logical, blanks, errors) is replaced by the nearest entry of the +/// needle's type below it in the window; when there is none the window moves up past it, so +/// entries of other types are skipped (`LOOKUP(9.9E+307, range)` finds the last number); +/// - a probe equal to the needle ends the search at once (the run of equal entries that follows +/// it is taken, so sorted duplicates give the last one). +/// +/// Observed in Excel: `VLOOKUP(534, {"INTEGRAL";0;1;2;534;9999999999;-9999999999}, …)` finds 534, +/// `HLOOKUP("STRING", {"INTEGRAL","DOUBLE","BLANK","STRING","REF","AREA (rows)",}, …)` +/// finds "STRING" and `LOOKUP("DOUBLE", {"DOUBLE","BLANK","STRING","REF","AREA (rows)"}, …)` +/// finds "DOUBLE". Work is linear in the vector length at worst. fn bsearch_last(x: &Value, hay: &[Value], desc: bool) -> Option { - let idx: Vec = hay.iter().enumerate().filter(|(_, v)| same_type(x, v)).map(|(i, _)| i).collect(); - let (mut lo, mut hi) = (0usize, idx.len()); - // Find the first filtered position where the "passes" predicate fails. - while lo < hi { + let mut hi = hay.len().checked_sub(1)?; + let mut lo = 0usize; + let mut best = None; + while lo <= hi { let mid = lo + (hi - lo) / 2; - let v = idx.get(mid).and_then(|&i| hay.get(i))?; - let o = compare(v, x); - let ok = if desc { o != Ordering::Less } else { o != Ordering::Greater }; - if ok { + let Some(j) = (lo..=mid).rev().find(|&j| hay.get(j).is_some_and(|v| same_type(x, v))) else { lo = mid + 1; - } else { - hi = mid; - } - } - if lo == 0 { None } else { idx.get(lo - 1).copied() } -} - -/// Binary search returning the first entry `>= x` (ascending) or `<= x` (descending). -fn bsearch_first(x: &Value, hay: &[Value], desc: bool) -> Option { - let idx: Vec = hay.iter().enumerate().filter(|(_, v)| same_type(x, v)).map(|(i, _)| i).collect(); - let (mut lo, mut hi) = (0usize, idx.len()); - while lo < hi { - let mid = lo + (hi - lo) / 2; - let v = idx.get(mid).and_then(|&i| hay.get(i))?; + continue; + }; + let v = hay.get(j)?; let o = compare(v, x); + if o == Ordering::Equal { + let mut k = j; + while hay.get(k + 1).is_some_and(|w| same_type(x, w) && compare(w, x) == Ordering::Equal) { + k += 1; + } + return Some(k); + } let before = if desc { o == Ordering::Greater } else { o == Ordering::Less }; if before { + best = Some(j); lo = mid + 1; + } else if j == 0 { + break; } else { - hi = mid; + hi = j - 1; } } - idx.get(lo).copied() + best +} + +/// Order of an entry relative to the needle for XLOOKUP / XMATCH binary search (search mode ±2): +/// Excel's sort order over all types (numbers < text < logicals < errors), reversed for +/// descending data, with blanks always last like a sort leaves them. Unlike the legacy +/// approximate match no entry is skipped (observed: `XLOOKUP(46523, {9700;"xyz";"xyz";160726; +/// 204100;510300}, …, 0, 1, 2)` returns the row of the first "xyz"). +fn binary_order(v: &Value, x: &Value, desc: bool) -> Ordering { + match v { + Value::Empty => Ordering::Greater, + _ if desc => compare(v, x).reverse(), + _ => compare(v, x), + } +} + +/// Binary search for the first entry not ordered before the needle (`>= x` ascending, `<= x` +/// descending). +fn bsearch_first(x: &Value, hay: &[Value], desc: bool) -> Option { + let p = hay.partition_point(|v| binary_order(v, x, desc) == Ordering::Less); + (p < hay.len()).then_some(p) +} + +/// Binary search for the last entry not ordered after the needle (`<= x` ascending, `>= x` +/// descending). +fn bsearch_upto(x: &Value, hay: &[Value], desc: bool) -> Option { + hay.partition_point(|v| binary_order(v, x, desc) != Ordering::Greater).checked_sub(1) } /// A 1-D view of an array (row or column), `None` when it is 2-D. @@ -199,12 +233,13 @@ fn xsearch(x: &Value, hay: &[Value], match_mode: i64, search_mode: i64) -> R= x (ascending) / last >= x (descending). _ => { if desc { - bsearch_last(x, hay, true) + bsearch_upto(x, hay, true) } else { bsearch_first(x, hay, false) } @@ -248,7 +283,7 @@ fn xsearch(x: &Value, hay: &[Value], match_mode: i64, search_mode: i64) -> R Option { let p = bsearch_first(x, hay, desc)?; let v = hay.get(p)?; - if compare(v, x) == Ordering::Equal { Some(p) } else { None } + if binary_order(v, x, desc) == Ordering::Equal { Some(p) } else { None } } fn xlookup(args: &[Arg], _c: &mut dyn Ctx) -> R { @@ -378,9 +413,16 @@ fn transpose(args: &[Arg], _c: &mut dyn Ctx) -> R { } } +/// Sheet text for ADDRESS, quoted when it needs to be. A `[Book]Sheet` prefix is judged by its +/// parts (Microsoft's ADDRESS examples: `"[Book1]Sheet1"` stays bare, `"EXCEL SHEET"` becomes +/// `'EXCEL SHEET'`). fn quote_sheet(s: &str) -> String { - let plain = - !s.is_empty() && s.chars().all(|c| c.is_alphanumeric() || c == '_' || c == '.') && !s.chars().next().is_some_and(|c| c.is_ascii_digit()); + let word = |p: &str| p.chars().all(|c| c.is_alphanumeric() || c == '_' || c == '.'); + let bare_sheet = |p: &str| !p.is_empty() && word(p) && !p.chars().next().is_some_and(|c| c.is_ascii_digit()); + let plain = match s.strip_prefix('[').and_then(|r| r.split_once(']')) { + Some((book, sheet)) => !book.is_empty() && word(book) && bare_sheet(sheet), + None => bare_sheet(s), + }; if plain { s.to_string() } else { format!("'{}'", s.replace('\'', "''")) } } @@ -574,6 +616,45 @@ mod tests { close(ev("LOOKUP", vec![t("zz"), av(mixed), av(row(&[1.0, 2.0, 3.0, 4.0]))]), 4.0); } + /// Approximate matches over unsorted, mixed-type data as observed in Excel + /// (FormulaEvalTestData rows "Vlookup", "Hlookup", "Lookup"): Excel bisects the whole + /// vector and stops at an equal entry, so answers differ from a "best entry" scan. + #[test] + fn approximate_match_follows_bisection() { + // B6:H12 of the test data: header row, then one row per data kind. + let data = arr(vec![ + vec![tv("INTEGRAL"), tv("DOUBLE"), tv("BLANK"), tv("STRING"), tv("REF"), tv("AREA (rows)"), Value::Empty], + vec![nv(0.0), nv(0.0), Value::Empty, tv(""), nv(0.0), nv(1.0), nv(2.0)], + vec![nv(1.0), nv(0.00001), Value::Empty, tv("nospaces"), nv(1.0), nv(1.001), nv(2.999)], + vec![nv(2.0), nv(1.1), Value::Empty, tv("sp ace s"), nv(9999999999.0), nv(2.3456), nv(3.4567)], + vec![nv(534.0), nv(1.00001), Value::Empty, tv("endsWithSpace "), nv(0.00001), tv("alphanum"), nv(3.0)], + vec![nv(9999999999.0), nv(2.99999), Value::Empty, tv(" beginsWithSpace"), nv(0.0), tv("blanksIncl"), Value::Empty], + vec![nv(-9999999999.0), nv(999999.99999), Value::Empty, tv("non-numeric"), tv("sp ace s"), tv("E8"), nv(545.0)], + ]); + close(ev("VLOOKUP", vec![n(534.0), rf(data.clone()), n(2.0)]), 1.00001); + is_text(ev("VLOOKUP", vec![n(534.0), rf(data.clone()), n(4.0)]), "endsWithSpace "); + is_text(ev("VLOOKUP", vec![n(534.0), rf(data.clone()), n(4.0), b(true)]), "endsWithSpace "); + is_text(ev("HLOOKUP", vec![t("STRING"), rf(data.clone()), n(6.0), b(true)]), " beginsWithSpace"); + let keys = arr(vec![vec![tv("DOUBLE"), tv("BLANK"), tv("STRING"), tv("REF"), tv("AREA (rows)")]]); + let results = arr(vec![vec![nv(1.1), Value::Empty, tv("sp ace s"), nv(9999999999.0), nv(2.3456)]]); + close(ev("LOOKUP", vec![t("DOUBLE"), rf(keys), rf(results)]), 1.1); + let b6_b15 = col(&[0.0, 1.0, 2.0, 534.0, 9999999999.0, -9999999999.0, -534.0, -2.0, -1.0]); + let mut keys = vec![tv("INTEGRAL")]; + keys.extend(rows_of(&b6_b15).into_iter().flatten()); + let words = ["STRING", "", "nospaces", "sp ace s", "endsWithSpace ", " beginsWithSpace", "non-numeric", "0", "0.000001", "1"]; + let e_col = arr(words.iter().map(|w| vec![tv(w)]).collect()); + is_text(ev("LOOKUP", vec![n(534.0), rf(arr(keys.into_iter().map(|k| vec![k]).collect())), rf(e_col)]), "endsWithSpace "); + // Sorted duplicates give the last one; descending data with MATCH -1 too. + close(ev("MATCH", vec![n(2.0), av(row(&[1.0, 2.0, 2.0, 2.0, 3.0])), n(1.0)]), 4.0); + close(ev("MATCH", vec![n(20.0), av(row(&[40.0, 20.0, 20.0, 10.0])), n(-1.0)]), 3.0); + // Only text and blanks: no number to find. + is_err(ev("MATCH", vec![n(5.0), av(arr(vec![vec![tv("a"), Value::Empty, tv("b")]]))]), CellError::NA); + // A long run of trailing blanks (whole-column lookups) still finds the last number. + let mut tall = vec![vec![nv(1.0)], vec![nv(2.0)], vec![nv(3.0)]]; + tall.extend((0..10_000).map(|_| vec![Value::Empty])); + close(ev("MATCH", vec![n(9.0), rf(arr(tall))]), 3.0); + } + #[test] fn match_fn() { let v = col(&[10.0, 20.0, 30.0, 40.0]); @@ -632,6 +713,23 @@ mod tests { is_err(ev("XLOOKUP", vec![n(2.0), rf(table()), rf(vals)]), CellError::Value); } + /// XLOOKUP binary search (search mode ±2). Observed in Excel (xlookup.xlsx, Sheet2): next + /// larger over ascending data with text mixed in (0.22) and over descending data (0.24); the + /// bisection compares across types in sort order (numbers < text) instead of skipping text. + /// The other modes follow from the same order. + #[test] + fn xlookup_binary_search_observed() { + let rates = col(&[0.1, 0.22, 0.24, 0.32, 0.35, 0.37]); + let incomes = arr(vec![vec![nv(9700.0)], vec![tv("xyz")], vec![tv("xyz")], vec![nv(160726.0)], vec![nv(204100.0)], vec![nv(510300.0)]]); + close(ev("XLOOKUP", vec![n(46523.0), rf(incomes.clone()), rf(rates.clone()), n(0.0), n(1.0), n(2.0)]), 0.22); + close(ev("XLOOKUP", vec![n(46523.0), rf(incomes), rf(rates), n(0.0), n(-1.0), n(2.0)]), 0.1); + let desc_incomes = col(&[510300.0, 204100.0, 160726.0, 84200.0, 39475.0, 9700.0]); + let desc_rates = col(&[0.37, 0.35, 0.32, 0.24, 0.22, 0.1]); + close(ev("XLOOKUP", vec![n(46523.0), rf(desc_incomes.clone()), rf(desc_rates.clone()), n(0.0), n(1.0), n(-2.0)]), 0.24); + close(ev("XLOOKUP", vec![n(46523.0), rf(desc_incomes.clone()), rf(desc_rates.clone()), n(0.0), n(-1.0), n(-2.0)]), 0.22); + close(ev("XLOOKUP", vec![n(84200.0), rf(desc_incomes), rf(desc_rates), n(0.0), n(0.0), n(-2.0)]), 0.24); + } + fn table_rows3() -> Value { arr(vec![vec![nv(1.0), tv("one"), nv(10.0)], vec![nv(2.0), tv("two"), nv(20.0)], vec![nv(3.0), tv("three"), nv(30.0)]]) } @@ -682,6 +780,17 @@ mod tests { is_text(ev("ADDRESS", vec![n(2.0), n(3.0), n(4.0), b(false)]), "R[2]C[3]"); is_text(ev("ADDRESS", vec![n(1.0), n(1.0), n(1.0), b(true), t("Sheet1")]), "Sheet1!$A$1"); is_text(ev("ADDRESS", vec![n(1.0), n(1.0), n(1.0), b(true), t("My Sheet")]), "'My Sheet'!$A$1"); + // Observed in Excel (FormulaEvalTestData) and Microsoft's ADDRESS examples; the last + // case follows from the quoting rule for sheet names. + let cases: [(f64, f64, f64, bool, &str, &str); 4] = [ + (2.0, 3.0, 3.0, true, "[Book1]Sheet1", "[Book1]Sheet1!$C2"), + (2.0, 3.0, 1.0, false, "[Book1]Sheet1", "[Book1]Sheet1!R2C3"), + (2.0, 3.0, 1.0, false, "EXCEL SHEET", "'EXCEL SHEET'!R2C3"), + (1.0, 1.0, 1.0, true, "[My Book]Sheet1", "'[My Book]Sheet1'!$A$1"), + ]; + for (r, c, abs, a1, sheet, want) in cases { + is_text(ev("ADDRESS", vec![n(r), n(c), n(abs), b(a1), t(sheet)]), want); + } is_text(ev("ADDRESS", vec![n(1048576.0), n(16384.0)]), "$XFD$1048576"); is_err(ev("ADDRESS", vec![n(0.0), n(1.0)]), CellError::Value); is_err(ev("ADDRESS", vec![n(1.0), n(16385.0)]), CellError::Value); diff --git a/crates/functions/src/math.rs b/crates/functions/src/math.rs index 2f6931d..bab219e 100644 --- a/crates/functions/src/math.rs +++ b/crates/functions/src/math.rs @@ -109,20 +109,24 @@ fn product(a: &[Arg], _c: &mut dyn Ctx) -> R { num_val(v.iter().product()) } +/// SUMPRODUCT: non-numeric entries of arrays and multi-cell ranges count as 0, but a scalar +/// argument holding text (a literal or a single-cell reference) is `#VALUE!` (observed: +/// `SUMPRODUCT(G1330:G1330,I1330:I1330)` with G1330 = the text "4" is `#VALUE!`). fn sumproduct(a: &[Arg], _c: &mut dyn Ctx) -> R { - let arrays: Vec<_> = a.iter().map(|x| as_array(&x.value)).collect(); - let Some(first) = arrays.first() else { return Err(CellError::Value) }; + let arrays: Vec<_> = a.iter().map(|x| (as_array(&x.value), !matches!(x.value, Value::Array(_)))).collect(); + let Some((first, _)) = arrays.first() else { return Err(CellError::Value) }; let (rows, cols) = (first.rows, first.cols); - if arrays.iter().any(|x| x.rows != rows || x.cols != cols) { + if arrays.iter().any(|(x, _)| x.rows != rows || x.cols != cols) { return Err(CellError::Value); } let mut total = 0.0; for i in 0..rows.saturating_mul(cols) { let mut p = 1.0; - for arr in &arrays { + for (arr, scalar) in &arrays { match arr.data.get(i) { Some(Value::Number(n)) => p *= n, Some(Value::Error(e)) => return Err(*e), + Some(Value::Text(_)) if *scalar => return Err(CellError::Value), _ => p = 0.0, } } @@ -1336,6 +1340,27 @@ mod tests { is_err(ev("SUMXMY2", vec![av(x), av(row(&[1.0]))]), NA); } + /// SUMPRODUCT cases observed in Excel (FormulaEvalTestData, row "Sumproduct"): scalars and + /// single-cell references multiply in, text in a scalar argument is `#VALUE!`, sizes must match. + #[test] + fn sumproduct_scalar_arguments() { + let cases: Vec<(Vec, Result)> = vec![ + (vec![n(3.0), rf(nv(534.0)), rf(nv(1.1))], Ok(1762.2)), + (vec![rf(nv(1.1)), n(3.0), rf(nv(534.0)), rf(nv(1.1))], Ok(1938.42)), + (vec![n(2.0), rf(col(&[1.00001, 2.99999]))], Err(CellError::Value)), + (vec![rf(tv("4")), rf(nv(2.0))], Err(CellError::Value)), + (vec![t("4"), n(2.0)], Err(CellError::Value)), + (vec![rf(nv(3.0)), rf(Value::Error(CellError::Ref))], Err(CellError::Ref)), + (vec![av(arr(vec![vec![tv("4")]])), n(2.0)], Ok(0.0)), + ]; + for (args, want) in cases { + match want { + Ok(x) => close(ev("SUMPRODUCT", args), x), + Err(err) => is_err(ev("SUMPRODUCT", args), err), + } + } + } + #[test] fn rounding() { close(ev2("ROUND", 2.675, 2.0), 2.68); diff --git a/crates/functions/src/stat.rs b/crates/functions/src/stat.rs index 648241d..25449bd 100644 --- a/crates/functions/src/stat.rs +++ b/crates/functions/src/stat.rs @@ -78,6 +78,34 @@ fn mode_sngl(v: &[f64]) -> R { modes(v).first().copied().ok_or(CellError::NA) } +/// Numbers for MODE / MODE.SNGL / MODE.MULT. Inside multi-cell ranges and arrays only numbers +/// count, like [`numbers`]. Scalar arguments are stricter than in SUM: Excel answers `#VALUE!` +/// for direct text (even numeric text such as `"1"`) and for a single-cell reference to text or +/// a blank cell (observed: `MODE(B8,"1",2,3)` and `MODE(B9,2,D7)` with D7 blank are `#VALUE!`). +/// Arguments are processed left to right, so the first error wins. +fn mode_numbers(args: &[Arg]) -> R> { + let mut out = Vec::new(); + for a in args { + match &a.value { + Value::Array(arr) => { + for v in arr.iter() { + match v { + Value::Number(n) => out.push(*n), + Value::Error(e) => return Err(*e), + _ => {} + } + } + } + Value::Number(n) => out.push(*n), + Value::Error(e) => return Err(*e), + Value::Text(_) | Value::Empty => return Err(CellError::Value), + Value::Bool(_) if a.from_ref => {} + v => out.push(v.to_number()?), + } + } + Ok(out) +} + fn product(v: &[f64]) -> f64 { if v.is_empty() { 0.0 } else { v.iter().product() } } @@ -277,7 +305,7 @@ fn countblank(a: &[Arg], _: C) -> R { } fn mode_mult(a: &[Arg], _: C) -> R { - let v = numbers(a)?; + let v = mode_numbers(a)?; let m = modes(&v); if m.is_empty() { return Err(CellError::NA); @@ -1566,10 +1594,10 @@ pub(crate) fn specs() -> Vec { numbers(a)? )?)), f!("MODE", 1, VAR, Compatibility, A, "MODE(number1, [number2], ...)", "Most frequently occurring number.", |a: &[Arg], _c: C| num_val( - mode_sngl(&numbers(a)?)? + mode_sngl(&mode_numbers(a)?)? )), f!("MODE.SNGL", 1, VAR, Statistical, A, "MODE.SNGL(number1, [number2], ...)", "Most frequently occurring number.", |a: &[Arg], _c: C| { - num_val(mode_sngl(&numbers(a)?)?) + num_val(mode_sngl(&mode_numbers(a)?)?) }), f!("MODE.MULT", 1, VAR, Statistical, A, "MODE.MULT(number1, [number2], ...)", "Vertical array of all the most frequent numbers.", mode_mult), f!("STDEV", 1, VAR, Compatibility, A, "STDEV(number1, [number2], ...)", "Sample standard deviation.", |a: &[Arg], _c: C| num_val( @@ -2251,6 +2279,35 @@ mod tests { assert_eq!(rows_of(&ev("MODE.MULT", vec![c(&[1.0, 2.0, 3.0, 4.0, 3.0, 2.0, 1.0, 2.0, 3.0])])), vec![vec![nv(2.0)], vec![nv(3.0)]]); } + /// MODE argument semantics observed in Excel (FormulaEvalTestData, row "Mode"): multi-cell + /// ranges skip text and blanks, but direct text and single-cell references to text or blank + /// cells are `#VALUE!`; the first error from the left wins. + #[test] + fn mode_scalar_arguments() { + let data = arr(vec![vec![tv(""), nv(0.0), nv(1.0)], vec![tv("x"), nv(1.0), nv(1.001)], vec![tv("y"), nv(9e9), nv(2.3456)]]); + let cases: Vec<(Vec, Result)> = vec![ + (vec![n(1.0), n(1.0), n(2.0), n(3.0)], Ok(1.0)), + (vec![rf(data)], Ok(1.0)), + (vec![rf(arr(vec![vec![nv(-1.1), Value::Empty, tv("1.1")]])), n(-1.1)], Ok(-1.1)), + // MODE(C9,E16,"1",B8): E16 holds the text "1.1". + (vec![rf(nv(1.1)), rf(tv("1.1")), t("1"), rf(nv(1.0))], Err(CellError::Value)), + (vec![rf(nv(1.0)), t("1"), n(2.0), n(3.0)], Err(CellError::Value)), + (vec![t("1"), n(1.0), e(CellError::Div0), n(2.0), n(3.0)], Err(CellError::Value)), + // MODE(B9,2,D7) with D7 blank. + (vec![rf(nv(2.0)), n(2.0), rf(Value::Empty)], Err(CellError::Value)), + (vec![n(1.0), n(1.0), e(CellError::Div0), t("1")], Err(CellError::Div0)), + ]; + for name in ["MODE", "MODE.SNGL"] { + for (args, want) in &cases { + match want { + Ok(x) => close(ev(name, args.clone()), *x), + Err(err) => is_err(ev(name, args.clone()), *err), + } + } + } + is_err(ev("MODE.MULT", vec![rf(nv(1.0)), t("1"), n(1.0)]), CellError::Value); + } + #[test] fn spread() { let d = [1345.0, 1301.0, 1368.0, 1322.0, 1310.0, 1370.0, 1318.0, 1350.0, 1303.0, 1299.0]; diff --git a/crates/functions/src/text.rs b/crates/functions/src/text.rs index 2eeba27..723cd4a 100644 --- a/crates/functions/src/text.rs +++ b/crates/functions/src/text.rs @@ -292,17 +292,14 @@ fn unicode(a: &[Arg], _c: &mut dyn Ctx) -> R { num_val(ch as u32 as f64) } +/// VALUE: a blank cell reads as 0, but empty or blank text is not a number (observed: +/// `VALUE("")` is `#VALUE!`). fn value(a: &[Arg], _c: &mut dyn Ctx) -> R { match scalar(arg(a, 0)?) { Value::Number(n) => num_val(n), Value::Empty => num_val(0.0), Value::Error(e) => Err(e), - Value::Text(t) => { - if t.trim().is_empty() { - return num_val(0.0); - } - parse_number_text(&t).ok_or(CellError::Value).and_then(num_val) - } + Value::Text(t) => parse_number_text(&t).ok_or(CellError::Value).and_then(num_val), _ => Err(CellError::Value), } } @@ -1054,6 +1051,25 @@ mod tests { is_err(ev("UNICHAR", vec![n(55296.0)]), CellError::NA); } + /// VALUE cases observed in Excel (ValueFunctionOfBlank, FormulaEvalTestData); a blank cell + /// still reads as 0. + #[test] + fn value_observed() { + let cases: Vec<(crate::Arg, Result)> = vec![ + (t(""), Err(CellError::Value)), + (t("ABC"), Err(CellError::Value)), + (t("0.33"), Ok(0.33)), + (t("$1,000"), Ok(1000.0)), + (rf(Value::Empty), Ok(0.0)), + ]; + for (arg, want) in cases { + match want { + Ok(x) => close(ev("VALUE", vec![arg]), x), + Err(err) => is_err(ev("VALUE", vec![arg]), err), + } + } + } + #[test] fn numbers_from_text() { close(ev("VALUE", vec![t("1,234.5")]), 1234.5); diff --git a/crates/model/src/features.rs b/crates/model/src/features.rs index 317476e..4f37c8f 100644 --- a/crates/model/src/features.rs +++ b/crates/model/src/features.rs @@ -65,6 +65,11 @@ pub struct Table { pub first_col: bool, pub last_col: bool, pub filter_button: bool, + /// Criteria of the table's AutoFilter (column offset within the table → criterion). With + /// criteria present, hidden data rows are filtered out rather than hidden by hand, which + /// SUBTOTAL distinguishes. + #[serde(default)] + pub filter: Vec<(u32, FilterCriterion)>, } impl Table { @@ -490,6 +495,12 @@ pub enum FilterCriterion { AboveAverage(bool), FillColor(Color), FontColor(Color), + /// Rows whose cell shows icon `id` (0-based; `None` = no icon) of conditional-format icon set + /// `set` (`iconFilter`). Kept from files; re-applying the filter leaves those rows as they are. + Icon { + set: String, + id: Option, + }, } #[derive(Clone, Debug, PartialEq, Serialize, Deserialize)] diff --git a/crates/model/src/sheet.rs b/crates/model/src/sheet.rs index 696c9ef..33eb4c1 100644 --- a/crates/model/src/sheet.rs +++ b/crates/model/src/sheet.rs @@ -238,6 +238,17 @@ impl Sheet { pub fn is_row_hidden(&self, row: u32) -> bool { self.rows.get(&row).is_some_and(|r| r.hidden) } + /// Whether `row` is hidden by an applied filter: a hidden row below the header of the + /// sheet AutoFilter while it has criteria, or a hidden data row of a filtered table. + /// Other hidden rows were hidden by hand. + pub fn is_row_filtered(&self, row: u32) -> bool { + if !self.is_row_hidden(row) { + return false; + } + let in_sheet_filter = + self.autofilter.as_ref().is_some_and(|af| !af.criteria.is_empty() && row > af.range.start.row && row <= af.range.end.row); + in_sheet_filter || self.tables.iter().any(|t| !t.filter.is_empty() && t.data_range().is_some_and(|r| row >= r.start.row && row <= r.end.row)) + } pub fn is_col_hidden(&self, col: u32) -> bool { self.cols.get(&col).is_some_and(|r| r.hidden) } diff --git a/crates/xlsx/src/sheet_read.rs b/crates/xlsx/src/sheet_read.rs index 07ee340..40cc23d 100644 --- a/crates/xlsx/src/sheet_read.rs +++ b/crates/xlsx/src/sheet_read.rs @@ -172,7 +172,7 @@ pub fn read_sheet(cx: &mut Ctx<'_>, part: &str, name: &str) -> Result, e: &El) -> Option { } else if let Some(c) = fc.child("colorFilter") { let Some(dxf) = c.attr_u32("dxfId").and_then(|i| cx.dxfs.get(i as usize)) else { continue }; if c.flag("cellColor", true) { FilterCriterion::FillColor(dxf.fill.fg) } else { FilterCriterion::FontColor(dxf.font.color) } + } else if let Some(i) = fc.child("iconFilter") { + let Some(set) = i.attr("iconSet") else { continue }; + FilterCriterion::Icon { set: set.to_string(), id: i.attr_u32("iconId") } } else { continue; }; @@ -753,7 +756,7 @@ pub fn read_autofilter(cx: &mut Ctx<'_>, e: &El) -> Option { Some(AutoFilter { range, criteria }) } -pub fn read_table(x: &El) -> Option { +pub fn read_table(cx: &mut Ctx<'_>, x: &El) -> Option
{ let range = x.attr("ref").and_then(RangeRef::parse)?; let name = x.attr("displayName").or_else(|| x.attr("name")).unwrap_or("Table").to_string(); let header_row = x.attr_u32("headerRowCount").unwrap_or(1) > 0; @@ -804,6 +807,7 @@ pub fn read_table(x: &El) -> Option
{ first_col: si.is_some_and(|s| s.flag("showFirstColumn", false)), last_col: si.is_some_and(|s| s.flag("showLastColumn", false)), filter_button: x.child("autoFilter").is_some(), + filter: x.child("autoFilter").and_then(|e| read_autofilter(cx, e)).map(|af| af.criteria).unwrap_or_default(), }) } diff --git a/crates/xlsx/src/sheet_write.rs b/crates/xlsx/src/sheet_write.rs index fee804c..289c236 100644 --- a/crates/xlsx/src/sheet_write.rs +++ b/crates/xlsx/src/sheet_write.rs @@ -5,8 +5,8 @@ use std::fmt::Write as _; use gridcraft_core::{CellError, CellRef, RangeRef, Value}; use gridcraft_model::{ - AutoFilter, Cell, CfOperator, CfRule, CfValueKind, Color, CondFormat, ErrorStyle, FilterCriterion, LineInfo, Orientation, Sheet, SparklineKind, - Style, Table, TotalsFn, Validation, ValidationKind, Workbook, default_icon_thresholds, icon_count, + Cell, CfOperator, CfRule, CfValueKind, Color, CondFormat, ErrorStyle, FilterCriterion, LineInfo, Orientation, Sheet, SparklineKind, Style, Table, + TotalsFn, Validation, ValidationKind, Workbook, default_icon_thresholds, icon_count, }; use crate::drawing::{Obj, image_content_type, image_ext}; @@ -339,7 +339,7 @@ pub fn write_sheet(wb: &Workbook, si: usize, selected: bool, out: &mut Out) -> ( if let Some(af) = &sheet.autofilter && !sheet.tables.iter().any(|t| t.range.intersects(&af.range)) { - s.push_str(&autofilter_xml(af, out)); + s.push_str(&autofilter_xml(af.range, &af.criteria, out)); } if !sheet.merges.is_empty() { let merges: Vec<&RangeRef> = sheet.merges.iter().filter(|m| !m.is_single()).collect(); @@ -544,7 +544,8 @@ pub fn write_sheet(wb: &Workbook, si: usize, selected: bool, out: &mut Out) -> ( out.table_id += 1; let n = out.tables; let name = table_name(&t.name, &mut used_names, out.table_id); - out.part(&format!("xl/tables/table{n}.xml"), Some(CT_TABLE), table_xml(t, cols, &name, out.table_id).into_bytes()); + let xml = table_xml(t, cols, &name, out.table_id, out); + out.part(&format!("xl/tables/table{n}.xml"), Some(CT_TABLE), xml.into_bytes()); let rid = rels.add("table", &format!("../tables/table{n}.xml")); let _ = write!(parts, ""); } @@ -886,14 +887,14 @@ fn filter_operator(op: &str) -> &'static str { } } -fn autofilter_xml(af: &AutoFilter, out: &mut Out) -> String { - let mut s = format!(" String { + let mut s = format!(""); return s; } s.push('>'); - for (col, c) in &af.criteria { + for (col, c) in criteria { let _ = write!(s, ""); match c { FilterCriterion::Values { values, blanks } => { @@ -926,6 +927,13 @@ fn autofilter_xml(af: &AutoFilter, out: &mut Out) -> String { st.font.color = *c; let _ = write!(s, "", dxf_id(&mut out.dxfs, &st)); } + FilterCriterion::Icon { set, id } => { + let _ = write!(s, ""); + } } s.push_str(""); } @@ -933,7 +941,7 @@ fn autofilter_xml(af: &AutoFilter, out: &mut Out) -> String { s } -fn table_xml(t: &Table, cols: &[String], name: &str, id: u32) -> String { +fn table_xml(t: &Table, cols: &[String], name: &str, id: u32, out: &mut Out) -> String { let mut s = format!("{XML_DECL}
String { if t.header_row && t.filter_button { let end_row = if t.totals_row { t.range.end.row.saturating_sub(1).max(t.range.start.row) } else { t.range.end.row }; let r = RangeRef::new(t.range.start, CellRef::new(end_row, t.range.end.col)); - let _ = write!(s, "", r.a1()); + s.push_str(&autofilter_xml(r, &t.filter, out)); } let _ = write!(s, "", cols.len()); for (i, name) in cols.iter().enumerate() { diff --git a/crates/xlsx/src/tests/array_export.rs b/crates/xlsx/src/tests/array_export.rs index 8e276e3..0491efd 100644 --- a/crates/xlsx/src/tests/array_export.rs +++ b/crates/xlsx/src/tests/array_export.rs @@ -183,6 +183,7 @@ fn calculated_table_column_keeps_formula_and_cell_value() { first_col: false, last_col: false, filter_button: true, + filter: vec![], }); let bytes = write_xlsx(&wb).unwrap(); let mut pkg = Package::open(&bytes).unwrap(); @@ -330,6 +331,7 @@ fn implicit_intersection_metadata_distinguishes_table_rows_and_columns() { first_col: false, last_col: false, filter_button: true, + filter: vec![], }); sh.cells.set(at("A2"), Cell::value(Value::Number(3.0))); sh.cells.set(at("B2"), formula("[@Input]*2", 6.0)); diff --git a/crates/xlsx/src/tests/pivot_tests.rs b/crates/xlsx/src/tests/pivot_tests.rs index a3818fc..c854432 100644 --- a/crates/xlsx/src/tests/pivot_tests.rs +++ b/crates/xlsx/src/tests/pivot_tests.rs @@ -70,6 +70,7 @@ fn workbook() -> Workbook { first_col: false, last_col: false, filter_button: true, + filter: vec![], }); wb.names.push(DefinedName { name: "SalesData".into(), scope: None, formula: "Data!$A$1:$F$9".into(), comment: String::new(), hidden: false }); diff --git a/crates/xlsx/src/tests/roundtrip.rs b/crates/xlsx/src/tests/roundtrip.rs index 9e14d19..0a2ebf5 100644 --- a/crates/xlsx/src/tests/roundtrip.rs +++ b/crates/xlsx/src/tests/roundtrip.rs @@ -284,6 +284,10 @@ fn sample() -> Workbook { first_col: true, last_col: false, filter_button: true, + filter: vec![ + (0, FilterCriterion::Values { values: vec!["East".into()], blanks: false }), + (1, FilterCriterion::Icon { set: "3TrafficLights1".into(), id: Some(1) }), + ], }); let png = b"\x89PNG\r\n\x1a\n0000IHDRfake".to_vec(); s.images.push(Image { @@ -463,6 +467,7 @@ fn full_roundtrip() { assert_eq!(tb.name, "Sales_Table"); assert_eq!((tb.range, tb.header_row, tb.totals_row, &tb.columns, &tb.style), (ta.range, ta.header_row, ta.totals_row, &ta.columns, &ta.style)); assert_eq!((tb.banded_rows, tb.banded_cols, tb.first_col, tb.last_col, tb.filter_button), (true, true, true, false, true)); + assert_eq!(tb.filter, ta.filter, "the table's filter criteria survive (SUBTOTAL tells filtered rows apart)"); assert_eq!(b.value(at("P1")), Value::text("Region"), "header cells carry the column names"); // Drawings. assert_eq!(b.images.len(), 1); @@ -609,6 +614,7 @@ fn writing_repairs_invalid_models() { first_col: false, last_col: false, filter_button: true, + filter: vec![], }); s.cells.set(at("B2"), Cell { value: Value::Number(1.0), formula: None, style: StyleId(9999) }); let (back, _) = read_xlsx(&write_xlsx(&wb).unwrap()).unwrap(); diff --git a/docs/file-format-parity.md b/docs/file-format-parity.md index 06dd452..841f89a 100644 --- a/docs/file-format-parity.md +++ b/docs/file-format-parity.md @@ -1,6 +1,6 @@ # File-format parity -> **Last reviewed:** 2026-10-11 · **Last updated:** 2026-10-11 · **Change:** minor (`.xls` BIFF8 import, #121) · **Target:** Microsoft Excel (Microsoft 365), Excel for Mac 16.113.4 +> **Last reviewed:** 2026-10-11 · **Last updated:** 2026-10-11 · **Change:** minor (table AutoFilter criteria and icon filters kept on round trip; `.xls` BIFF8 import, #121) · **Target:** Microsoft Excel (Microsoft 365), Excel for Mac 16.113.4 Every format Excel opens or saves, with GridCraft's read and write support, fidelity and tests. Part of [target-app-parity.md](target-app-parity.md). **File formats: ~50% ready, 80–120 h** @@ -76,6 +76,7 @@ refresh. Counted under features (Get & Transform, ~10% ready), not here. | Date | Change | Summary | |---|---|---| +| 2026-10-11 | minor | Table AutoFilter criteria are read and written (they were dropped, leaving only the filter button), and `iconFilter` criteria are kept on table and sheet AutoFilters | | 2026-10-11 | minor | `.xls` (BIFF8) import: values, formulas, number formats, merges, sizes, names (#121) | | 2026-10-11 | minor | XLSX pass-through: custom XML, external links, connections, query tables, printer settings, form/ActiveX controls, OLE objects, sheet backgrounds kept on save (#330, #324) | | 2026-10-11 | minor | XLSX: threaded comments and persons parts written (they came back as notes before); note visibility read from VML (#300) | diff --git a/docs/function-parity.md b/docs/function-parity.md index 901507e..8cfcbd9 100644 --- a/docs/function-parity.md +++ b/docs/function-parity.md @@ -1,6 +1,6 @@ # Function parity: worksheet functions and formulas -> **Last reviewed:** 2026-10-11 · **Last updated:** 2026-10-11 · **Change:** minor (ODDFPRICE, ODDFYIELD, ODDLPRICE, ODDLYIELD added) · **Target:** Microsoft Excel (Microsoft 365), Excel for Mac 16.113.4 +> **Last reviewed:** 2026-10-11 · **Last updated:** 2026-10-11 · **Change:** minor (oracle fixes: MODE, VALUE, SUMPRODUCT, ADDRESS, approximate and binary-search lookups, SUBTOTAL over filtered rows) · **Target:** Microsoft Excel (Microsoft 365), Excel for Mac 16.113.4 Excel's worksheet functions, category by category, against GridCraft's function library. Part of [target-app-parity.md](target-app-parity.md); gaps feed [gaps.md](gaps.md). @@ -103,6 +103,7 @@ workbooks; results checked in as data, not Excel files) and fixing what it finds | Date | Change | Summary | |---|---|---| +| 2026-10-11 | minor | Fixes from the cached-value oracle (`gridcraft-cli check`): MODE / MODE.SNGL / MODE.MULT give `#VALUE!` for text in a direct argument or single-cell reference; `VALUE("")` is `#VALUE!`; SUMPRODUCT with a text scalar argument is `#VALUE!`; ADDRESS leaves `[Book]Sheet` unquoted; VLOOKUP / HLOOKUP / LOOKUP / MATCH approximate match bisect like Excel on unsorted mixed-type data; XLOOKUP / XMATCH binary search compares across types in sort order; SUBTOTAL leaves out filtered rows for every function number (table and sheet AutoFilters) | | 2026-10-11 | minor | ODDFPRICE, ODDFYIELD, ODDLPRICE, ODDLYIELD added: Financial 55 / 55; 505 / 523 by name, 496 working | | 2026-10-11 | minor | PERCENTOF and EUROCONVERT added: 503 / 523 by name, 494 working | | 2026-10-11 | minor | BAHTTEXT added: 502 / 523 by name, 493 working |