shifathosn
/
gridcraft
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Merge pull request #686 from neilcawse:fix/criteria-over-255

This commit is contained in:
Brandon Thomas 2026-10-12 23:48:47 -04:00
parent decc64a59b e5b12dab06
commit 1d875bdf45
2 changed files with 163 additions and 21 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

101
crates/functions/src/criteria.rs
View File

@ -5,6 +5,10 @@ use std::cmp::Ordering;
use gridcraft_core::parse::parse_number_text;
use gridcraft_core::{CellError, Value, compare_numbers, compare_text};
/// Longest text criterion COUNTIF and its relatives accept, in UTF-16 code units as Excel
/// counts them (😀 is two units); a longer one is `#VALUE!`.
const MAX_CRITERION_UTF16_UNITS: usize = 255;
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
enum Op {
Eq,
@ -22,6 +26,15 @@ enum Operand {
Error(CellError),
/// Text (lower-cased chars kept for wildcard matching).
Text(String),
/// A criterion cut to its first 255 UTF-16 units in the middle of a surrogate pair: the
/// complete characters before the cut, followed (in Excel) by a lone high surrogate.
///
/// No cell text holds a lone surrogate, so `=` matches nothing and `<>` everything, as
/// Excel gives 0 for SUMIFS with a criterion of 128 emoji. For `<`, `<=`, `>` and `>=` the
/// criterion sorts just above the kept text: a cell matches `>`/`>=` when it sorts above
/// that text, and `<`/`<=` otherwise. This is an approximation (Excel wasn't checked) and
/// only matters for comparison criteria over 255 units.
CutText(String),
/// Nothing after the operator (`"="`, `"<>"`) or an empty criterion.
Blank,
}
@ -48,25 +61,52 @@ impl Criterion {
}
}
fn parse_text(t: &str) -> Criterion {
let (op, rest) = if let Some(r) = t.strip_prefix(">=") {
(Op::Ge, r)
} else if let Some(r) = t.strip_prefix("<=") {
(Op::Le, r)
} else if let Some(r) = t.strip_prefix("<>") {
(Op::Ne, r)
} else if let Some(r) = t.strip_prefix('=') {
(Op::Eq, r)
} else if let Some(r) = t.strip_prefix('>') {
(Op::Gt, r)
} else if let Some(r) = t.strip_prefix('<') {
(Op::Lt, r)
} else {
if t.is_empty() {
return Criterion { op: Op::Eq, operand: Operand::Blank, bare_empty: true };
/// Parses the criteria argument of COUNTIF, SUMIF, AVERAGEIF and the single-pair *IFS
/// functions: like `parse`, but text longer than 255 UTF-16 code units is `#VALUE!`, as in
/// Excel (checked on Excel 2016 and Microsoft 365: `"a"×254 & "😀"` is 255 characters but
/// 256 units, and is `#VALUE!`).
pub(crate) fn parse_arg(v: &Value) -> Result<Criterion, CellError> {
match v {
Value::Array(a) => Criterion::parse_arg(a.data.first().unwrap_or(&Value::Empty)),
// A UTF-8 string has at least as many bytes as UTF-16 units, so short text skips the count.
Value::Text(t) if t.len() > MAX_CRITERION_UTF16_UNITS && t.encode_utf16().nth(MAX_CRITERION_UTF16_UNITS).is_some() => {
Err(CellError::Value)
}
(Op::Eq, t)
};
other => Ok(Criterion::parse(other)),
}
}
/// Parses a criteria argument of an *IFS function with two or more criteria pairs. Excel
/// doesn't reject a longer text criterion there: it matches its first 255 UTF-16 code units
/// (checked on Excel 2016 and Microsoft 365 with COUNTIFS, SUMIFS, AVERAGEIFS, MAXIFS and
/// MINIFS). A cut through a surrogate pair matches nothing; see `Operand::CutText`.
pub(crate) fn parse_arg_truncated(v: &Value) -> Criterion {
match v {
Value::Array(a) => Criterion::parse_arg_truncated(a.data.first().unwrap_or(&Value::Empty)),
Value::Text(t) if t.len() > MAX_CRITERION_UTF16_UNITS => {
let mut units = 0;
for (i, c) in t.char_indices() {
if units + c.len_utf16() > MAX_CRITERION_UTF16_UNITS {
if units == MAX_CRITERION_UTF16_UNITS {
return Criterion::parse_text(&t[..i]);
}
// The cut falls between the two halves of `c`.
let (op, rest) = split_op(&t[..i]);
return Criterion { op, operand: Operand::CutText(rest.to_string()), bare_empty: false };
}
units += c.len_utf16();
}
Criterion::parse_text(t)
}
other => Criterion::parse(other),
}
}
fn parse_text(t: &str) -> Criterion {
if t.is_empty() {
return Criterion { op: Op::Eq, operand: Operand::Blank, bare_empty: true };
}
let (op, rest) = split_op(t);
let operand = if rest.is_empty() {
Operand::Blank
} else if let Some(n) = parse_number_text(rest) {
@ -140,10 +180,35 @@ impl Criterion {
_ => false,
},
},
Operand::CutText(kept) => match self.op {
Op::Eq => false,
Op::Ne => true,
Op::Lt | Op::Le => matches!(&v, Value::Text(t) if compare_text(t, kept) != Ordering::Greater),
Op::Gt | Op::Ge => matches!(&v, Value::Text(t) if compare_text(t, kept) == Ordering::Greater),
},
}
}
}
/// Splits a leading comparison operator (`=` when there is none) off a text criterion.
fn split_op(t: &str) -> (Op, &str) {
if let Some(r) = t.strip_prefix(">=") {
(Op::Ge, r)
} else if let Some(r) = t.strip_prefix("<=") {
(Op::Le, r)
} else if let Some(r) = t.strip_prefix("<>") {
(Op::Ne, r)
} else if let Some(r) = t.strip_prefix('=') {
(Op::Eq, r)
} else if let Some(r) = t.strip_prefix('>') {
(Op::Gt, r)
} else if let Some(r) = t.strip_prefix('<') {
(Op::Lt, r)
} else {
(Op::Eq, t)
}
}
fn cmp_op(op: Op, o: Ordering) -> bool {
match op {
Op::Eq => o == Ordering::Equal,

83
crates/functions/src/math.rs
View File

@ -958,6 +958,9 @@ fn criteria_mask(a: &[Arg], first_pair: usize, rows: usize, cols: usize) -> R<Ve
if rest.is_empty() || rest.len() % 2 != 0 {
return Err(CellError::Value);
}
// A text criterion over 255 characters is `#VALUE!` only when there is a single criteria
// pair. With two or more pairs Excel matches its first 255 characters instead.
let single_pair = rest.len() == 2;
let mut mask = vec![true; rows.saturating_mul(cols)];
for pair in rest.chunks(2) {
let (Some(range), Some(crit)) = (pair.first(), pair.get(1)) else { return Err(CellError::Value) };
@ -965,7 +968,8 @@ fn criteria_mask(a: &[Arg], first_pair: usize, rows: usize, cols: usize) -> R<Ve
if r.rows != rows || r.cols != cols {
return Err(CellError::Value);
}
let c = Criterion::parse(&crit.value.scalar());
let scalar = crit.value.scalar();
let c = if single_pair { Criterion::parse_arg(&scalar)? } else { Criterion::parse_arg_truncated(&scalar) };
for (m, v) in mask.iter_mut().zip(r.iter()) {
if *m && !c.matches(v) {
*m = false;
@ -1025,7 +1029,7 @@ fn minifs(a: &[Arg], _c: &mut dyn Ctx) -> R<Value> {
fn countif(a: &[Arg], _c: &mut dyn Ctx) -> R<Value> {
let range = as_array(&crate::util::arg(a, 0)?.value);
let c = Criterion::parse(&crate::util::arg(a, 1)?.value.scalar());
let c = Criterion::parse_arg(&crate::util::arg(a, 1)?.value.scalar())?;
num_val(range.iter().filter(|v| c.matches(v)).count() as f64)
}
@ -1034,7 +1038,7 @@ fn countif(a: &[Arg], _c: &mut dyn Ctx) -> R<Value> {
/// the evaluator does that resizing (it has the references), so cells line up one to one here.
fn if_numbers(a: &[Arg]) -> R<Vec<f64>> {
let range = as_array(&crate::util::arg(a, 0)?.value);
let c = Criterion::parse(&crate::util::arg(a, 1)?.value.scalar());
let c = Criterion::parse_arg(&crate::util::arg(a, 1)?.value.scalar())?;
let sum_range = if has(a, 2) { Some(as_array(&crate::util::arg(a, 2)?.value)) } else { None };
let mut out = Vec::new();
for r in 0..range.rows {
@ -1658,4 +1662,77 @@ mod tests {
is_err(ev("COUNTIFS", vec![rf(qty.clone()), t(">1"), rf(qty)]), CellError::Value);
is_err(ev("SUMIF", vec![rf(col(&[1.0, 2.0])), t(">0"), rf(arr(vec![vec![Value::Error(NA)], vec![nv(1.0)]]))]), NA);
}
#[test]
fn criteria_over_255_chars_are_value_errors_with_one_pair_and_truncated_with_more() {
let ok = "x".repeat(255);
let long = "x".repeat(256);
let cells = arr(vec![vec![tv(&ok)], vec![tv(&long)]]);
let qty = col(&[1.0, 2.0]);
close(ev("COUNTIF", vec![rf(cells.clone()), t(&ok)]), 1.0);
is_err(ev("COUNTIF", vec![rf(cells.clone()), t(&long)]), CellError::Value);
close(ev("SUMIFS", vec![rf(qty.clone()), rf(cells.clone()), t(&ok)]), 1.0);
is_err(ev("SUMIFS", vec![rf(qty.clone()), rf(cells.clone()), t(&long)]), CellError::Value);
is_err(ev("COUNTIFS", vec![rf(cells.clone()), t(&long)]), CellError::Value);
// With two or more pairs Excel matches the first 255 characters: the 255-character cell
// (quantity 1), not the 256-character one (quantity 2). Checked on Excel 2016 and 365.
let over = format!("{ok}y");
close(ev("COUNTIFS", vec![rf(cells.clone()), t(&long), rf(qty.clone()), t(">0")]), 1.0);
close(ev("SUMIFS", vec![rf(qty.clone()), rf(cells.clone()), t(&long), rf(qty.clone()), t(">0")]), 1.0);
close(ev("SUMIFS", vec![rf(qty.clone()), rf(qty.clone()), t(">0"), rf(cells.clone()), t(&over)]), 1.0);
close(ev("AVERAGEIFS", vec![rf(qty.clone()), rf(cells.clone()), t(&long), rf(qty.clone()), t(">0")]), 1.0);
close(ev("MAXIFS", vec![rf(qty.clone()), rf(cells.clone()), t(&"x".repeat(300)), rf(qty), t(">0")]), 1.0);
}
#[test]
fn criteria_over_255_utf16_units_follow_excel() {
// The tester's sheet, identical on Excel 2016 and Microsoft 365: A1:A6 hold these
// strings, B is the row number, and each row uses its own string as the criterion.
let e = "😀";
let strings =
[e.repeat(127), e.repeat(128), format!("{}{e}", "a".repeat(254)), format!("{}{e}", "a".repeat(253)), e.repeat(200), "я".repeat(256)];
let cells = arr(strings.iter().map(|s| vec![tv(s)]).collect());
let rows = col(&[1.0, 2.0, 3.0, 4.0, 5.0, 6.0]);
// COUNTIF, SUMIF, COUNTIFS with one pair, SUMIFS with a second pair `B,">0"`.
let expected: [Option<[f64; 4]>; 6] = [Some([1.0, 1.0, 1.0, 1.0]), None, None, Some([1.0, 4.0, 1.0, 4.0]), None, None];
for (s, want) in strings.iter().zip(expected) {
let countif = ev("COUNTIF", vec![rf(cells.clone()), t(s)]);
let sumif = ev("SUMIF", vec![rf(cells.clone()), t(s), rf(rows.clone())]);
let countifs = ev("COUNTIFS", vec![rf(cells.clone()), t(s)]);
let sumifs = ev("SUMIFS", vec![rf(rows.clone()), rf(cells.clone()), t(s), rf(rows.clone()), t(">0")]);
match want {
Some([a, b, c, d]) => {
close(countif, a);
close(sumif, b);
close(countifs, c);
close(sumifs, d);
}
// Over 255 units: `#VALUE!` with one pair; cut to 255 units with two, which
// matches no cell (for emoji it splits a surrogate pair).
None => {
is_err(countif, CellError::Value);
is_err(sumif, CellError::Value);
is_err(countifs, CellError::Value);
close(sumifs, 0.0);
}
}
}
}
#[test]
fn criteria_cut_inside_a_surrogate_pair() {
let e = "😀";
let cells = arr(vec![vec![tv(&e.repeat(127))], vec![tv(&e.repeat(128))], vec![tv("zzz")], vec![Value::Empty]]);
let rows = col(&[1.0, 2.0, 3.0, 4.0]);
let sumifs = |crit: String| ev("SUMIFS", vec![rf(rows.clone()), rf(cells.clone()), t(&crit), rf(rows.clone()), t(">0")]);
// "=" + 😀×128 is 257 units: the cut after 255 keeps "=" and 127 whole emoji.
close(sumifs(format!("={}", e.repeat(128))), 1.0);
// With a two-unit prefix the cut keeps 126 emoji and half of the 127th. Such a criterion
// equals no cell text: "=" matches nothing and "<>" everything, blanks included.
close(sumifs(format!("=a{}", e.repeat(128))), 0.0);
close(sumifs(format!("<>{}", e.repeat(128))), 10.0);
// Comparisons treat it as sorting just above the 126 kept emoji.
close(sumifs(format!(">={}", e.repeat(128))), 1.0 + 2.0);
close(sumifs(format!("<={}", e.repeat(128))), 3.0);
}
}