diff --git a/crates/core/src/eval/functions/statistical/max/mod.rs b/crates/core/src/eval/functions/statistical/max/mod.rs index 2366cd3ca..d736f5a17 100644 --- a/crates/core/src/eval/functions/statistical/max/mod.rs +++ b/crates/core/src/eval/functions/statistical/max/mod.rs @@ -1,8 +1,9 @@ use crate::types::{ErrorKind, Value}; /// `MAX(value1, ...)` — largest numeric value in the arguments. -/// Direct args: Numbers, Bool (TRUE=1, FALSE=0), parseable text coerced to number. -/// Array elements: Numbers only; text/Bool → skip; errors propagate. +/// Direct args: Numbers, Dates, Bool (TRUE=1, FALSE=0), parseable text coerced +/// to number. +/// Array elements: Numbers and Dates; text/Bool → skip; errors propagate. /// /// "No numbers" is *two* rules, not one: /// @@ -21,11 +22,47 @@ use crate::types::{ErrorKind, Value}; /// `array_had_content` is deliberate: a blank sitting next to something else /// numberless changes nothing. /// -/// This change moves the blank-only array and nothing else. `array_had_content` -/// is still set by text and booleans alone, so it exempts exactly what those -/// captures cover and makes no claim past them. +/// `array_had_content` is still set by text and booleans alone, so it exempts +/// exactly what those captures cover and makes no claim past them. A zoned +/// instant is the remaining numberless case: unprobed, and still `#REF!`. /// /// [`stat_helpers::is_blank_only_array`]: super::stat_helpers::is_blank_only_array +/// +/// **Dates participate as bare serials** and carry their type out. A date-only +/// range answers the latest date, a date beside a plain number is compared on +/// the serial with no special casing, and the result is date-typed whenever a +/// date took part — even when a plain number won the comparison. +/// +/// Captured and extrapolated are not the same thing here, so keep them apart: +/// +/// - **Values** are captured in Google Sheets for every form — a date-only +/// column, a date/number column, array literals of both shapes, and dates +/// passed as direct arguments. +/// - **Typing** is captured for the *range* forms only, read back through the +/// cell that holds the result (`=MAX()` and +/// `=MAX()` both come back `date`). The literal and +/// direct-argument rows report `number`, but that is an artifact of the +/// capture harness reading them through an `INDEX(...,1,1)` wrapper, which +/// drops the cell's date format — it is not a Sheets answer. So the date +/// typing of `=MAX({DATE(...),DATE(...)})` is **extrapolated** from the +/// range forms, not probed. +/// +/// **None of those rows are in this repo yet.** They come off the +/// conformance-fixtures pipeline and land in a separate fixtures-only PR — +/// they fail until this code exists, and CI rejects a PR that mixes fixture +/// TSVs with code. Same arrangement as the blank-only rows described on +/// `stat_helpers::is_blank_only_array`. A reviewer working from this repo +/// alone can check the unit tests and this comment; the Sheets answers +/// themselves have to be taken from that pipeline. +/// +/// Two consequences are filed rather than fixed here: +/// +/// - `COUNT` does not count dates, so `=COUNT(MAX())` answers 0 +/// where it used to answer 1 — a pre-existing `COUNT` gap this change makes +/// reachable. See #780. +/// - `MAXA`/`MINA` silently drop a `Zoned` sitting beside a `Date`, where +/// `MAX`/`MIN` route the same input through `zoned_extreme` and error. +/// Unprobed on both sides. See #781. pub fn max_fn(args: &[Value]) -> Value { if args.is_empty() { return Value::Error(ErrorKind::NA); @@ -39,12 +76,17 @@ pub fn max_fn(args: &[Value]) -> Value { let mut had_array = false; let mut array_had_content = false; let mut skipped_sparkline = false; + let mut saw_date = false; for arg in args { match arg { Value::Sparkline(_) => skipped_sparkline = true, Value::Number(n) => { result = Some(result.map_or(*n, |cur: f64| cur.max(*n))); } + Value::Date(n) => { + saw_date = true; + result = Some(result.map_or(*n, |cur: f64| cur.max(*n))); + } Value::Bool(b) => { let n = if *b { 1.0 } else { 0.0 }; result = Some(result.map_or(n, |cur: f64| cur.max(n))); @@ -72,13 +114,18 @@ pub fn max_fn(args: &[Value]) -> Value { &mut result, &mut skipped_sparkline, &mut array_had_content, + &mut saw_date, ) { return e; } } Value::Error(e) => return Value::Error(e.clone()), Value::ErrorMsg(e, m) => return Value::ErrorMsg(e.clone(), m.clone()), - _ => {} + // Listed rather than a catch-all so a new `Value` variant is a + // compile error here instead of a silent skip. A `Zoned` only + // reaches this loop when no other argument was zone-aware, which + // `zoned_extreme` above has already ruled on. + Value::Zoned(_) => {} } } // A skipped sparkline is not "nothing usable": the aggregate had something @@ -91,7 +138,8 @@ pub fn max_fn(args: &[Value]) -> Value { // An array holding text or booleans answers 0 (`=MAX({"a","b"})` and // `=MAX({TRUE,FALSE})` are both 0). `array_had_content` is set by exactly // those two variants and nothing else, so it exempts what that capture - // covers and makes no claim beyond it. + // covers and makes no claim beyond it. Dates never reach here: they + // contribute a number, so `result` is `Some` whenever one was seen. if had_array && !array_had_content && result.is_none() { // An array of nothing but blanks is a further exemption, and it is // captured: `=MAX(A1:A3)` over empty cells is 0, the same answer MIN, @@ -104,7 +152,13 @@ pub fn max_fn(args: &[Value]) -> Value { } return Value::Error(ErrorKind::Ref); } - Value::Number(result.unwrap_or(0.0)) + match result { + // A date anywhere in scope makes the answer date-typed, whether or not + // the date is the value that won. + Some(n) if saw_date => Value::Date(n), + Some(n) => Value::Number(n), + None => Value::Number(0.0), + } } /// Recursively fold a nested array's numbers into `result` for MAX's @@ -115,31 +169,46 @@ pub fn max_fn(args: &[Value]) -> Value { /// and `=MAX(Data!K1:K1)` are both 0). The flag is what distinguishes "skipped a /// sparkline" from "saw nothing usable at all", which stay different answers. /// +/// A `Date` folds in as its bare serial and raises `saw_date`, which types the +/// answer; it never touches `had_content`, since a date always leaves a number +/// behind and so can never reach the numberless rule. +/// /// `had_content` is set by text and booleans *only* — the two variants the /// capture covers (`=MAX({"a","b"})` and `=MAX({TRUE,FALSE})` are both 0). -/// It is deliberately not a catch-all: every other non-numeric variant, most -/// notably `Date`, leaves it alone and so keeps the `#REF!` MAX has always -/// answered. Listing the variants rather than falling through also stops a -/// future `Value` kind inheriting content-hood by accident. An all-blank array -/// no longer ends at `#REF!` either, but it gets there without this flag — see -/// the blank-only check in `max_fn`. +/// It is deliberately not a catch-all: every other non-numeric variant leaves +/// it alone and so keeps the `#REF!` MAX has always answered. Listing the +/// variants rather than falling through also stops a future `Value` kind +/// inheriting content-hood by accident. Two variants no longer end at `#REF!`, +/// and neither gets there through this flag: an all-blank array is decided by +/// the blank-only check in `max_fn`, and a `Date` contributes a number so the +/// numberless rule is never reached at all. fn max_array_into( elems: &[Value], result: &mut Option, skipped_sparkline: &mut bool, had_content: &mut bool, + saw_date: &mut bool, ) -> Result<(), Value> { for elem in elems { match elem { Value::Number(n) => { *result = Some(result.map_or(*n, |cur: f64| cur.max(*n))); } + Value::Date(n) => { + *saw_date = true; + *result = Some(result.map_or(*n, |cur: f64| cur.max(*n))); + } Value::Text(_) | Value::Bool(_) => *had_content = true, Value::Sparkline(_) => *skipped_sparkline = true, Value::Error(e) => return Err(Value::Error(e.clone())), Value::ErrorMsg(e, m) => return Err(Value::ErrorMsg(e.clone(), m.clone())), - Value::Array(inner) => max_array_into(inner, result, skipped_sparkline, had_content)?, - _ => {} + Value::Array(inner) => { + max_array_into(inner, result, skipped_sparkline, had_content, saw_date)? + } + // Listed rather than a catch-all so a new `Value` variant is a + // compile error here instead of inheriting "skipped, and not + // content either" by accident. + Value::Empty | Value::Zoned(_) => {} } } Ok(()) diff --git a/crates/core/src/eval/functions/statistical/max/tests/edge.rs b/crates/core/src/eval/functions/statistical/max/tests/edge.rs index abb4abfca..3a2a251f3 100644 --- a/crates/core/src/eval/functions/statistical/max/tests/edge.rs +++ b/crates/core/src/eval/functions/statistical/max/tests/edge.rs @@ -66,15 +66,25 @@ fn max_array_of_only_blanks_is_zero() { } #[test] -fn max_blank_beside_something_else_numberless_is_still_ref_error() { +fn max_blank_beside_something_else_does_not_become_the_blank_only_zero() { // The blank-only rule is decided over the arguments as a whole, not by a // per-element flag, so a blank cannot on its own pull an array that holds - // something else into the blank-only 0. Pinned with a date element because - // that is where MAX leaves such an array today; if that ever moves it has - // to move deliberately, not as fallout from this rule. + // something else into the blank-only 0. + // + // This was pinned with a date element and `#REF!`, on the note that "if + // that ever moves it has to move deliberately, not as fallout from this + // rule". It moved deliberately: dates now participate, so this array + // answers the date. The invariant the pin exists for is unchanged and + // still discriminating — the answer is the date, *not* the blank-only 0. + // + // There is no longer any variant that reaches MAX's numberless `#REF!` + // through a populated array: text and booleans set `array_had_content`, + // dates contribute a number, a sparkline sets its own flag, and a zoned + // instant is intercepted by `zoned_extreme` before the loop runs. The + // empty-array assertion below is what still carries the `#REF!` side. assert_eq!( max_fn(&[Value::Array(vec![Value::Empty, Value::Date(43831.0)])]), - Value::Error(ErrorKind::Ref) + Value::Date(43831.0) ); // An empty array argument stays #REF! even alongside an all-blank one. assert_eq!( @@ -84,24 +94,65 @@ fn max_blank_beside_something_else_numberless_is_still_ref_error() { } #[test] -fn max_array_of_only_dates_is_unchanged_at_ref_error() { - // `max_array_into` folds only `Value::Number`, so a date-only array has - // never produced a result and has always answered #REF!. That is almost - // certainly wrong against Sheets — but it is pre-existing, unprobed, and - // must not be quietly turned into a plausible-looking 0 by the - // text-and-boolean carve-out. `had_content` is set by text and booleans - // only, never by a catch-all, and this pins that. +fn max_array_of_only_dates_returns_the_latest_date() { + // Was #REF! until dates were captured: a date-only array now answers the + // largest serial, date-typed. Replaces the pin that recorded the old + // #REF! as unprobed. assert_eq!( max_fn(&[Value::Array(vec![ Value::Date(43831.0), Value::Date(44197.0) ])]), - Value::Error(ErrorKind::Ref) + Value::Date(44197.0) ); // Same for a date arriving through a nested-row range materialization. assert_eq!( max_fn(&[Value::Array(vec![Value::Array(vec![Value::Date(43831.0)])])]), - Value::Error(ErrorKind::Ref) + Value::Date(43831.0) + ); +} + +#[test] +fn max_dates_compare_as_bare_serials_and_type_the_answer() { + // A plain number and a date are compared on the serial with no special + // casing, and the answer is date-typed because a date took part — even + // when the plain number is the one that won (that is the MIN direction; + // pinned in min's tests, mirrored here for the losing-date direction). + assert_eq!( + max_fn(&[Value::Array(vec![Value::Date(43831.0), Value::Number(5.0)])]), + Value::Date(43831.0) + ); + // Direct (non-array) arguments take the same rule. + assert_eq!( + max_fn(&[Value::Date(43831.0), Value::Date(44197.0)]), + Value::Date(44197.0) + ); + assert_eq!( + max_fn(&[Value::Date(43831.0), Value::Number(5.0)]), + Value::Date(43831.0) + ); + // No date in scope: the answer stays a plain number. + assert_eq!( + max_fn(&[Value::Number(5.0), Value::Number(1.0)]), + Value::Number(5.0) + ); +} + +#[test] +fn max_date_beside_a_blank_no_longer_falls_into_the_ref_rule() { + // A date leaves a number behind, so the "populated but numberless" #REF! + // rule can no longer be reached with a date in scope. + assert_eq!( + max_fn(&[Value::Array(vec![Value::Date(43831.0), Value::Empty])]), + Value::Date(43831.0) + ); + // An all-blank array is a separate, captured case and answers 0, not the + // date rule and not #REF! — see `max_array_of_only_blanks_is_zero`. Kept + // here as the contrast: a date in scope types the answer, an array with + // nothing in it but blanks does not. + assert_eq!( + max_fn(&[Value::Array(vec![Value::Empty, Value::Empty])]), + Value::Number(0.0) ); } diff --git a/crates/core/src/eval/functions/statistical/maxa/mod.rs b/crates/core/src/eval/functions/statistical/maxa/mod.rs index 7d299d715..c22533233 100644 --- a/crates/core/src/eval/functions/statistical/maxa/mod.rs +++ b/crates/core/src/eval/functions/statistical/maxa/mod.rs @@ -8,6 +8,42 @@ use crate::types::{ErrorKind, Value}; /// - Empty array argument → `#REF!`. /// - Array of nothing but blanks → 0. /// - No args → `#N/A`. +/// +/// **Dates participate as bare serials** and carry their type out, exactly as +/// they do for MAX: a date-only range answers the latest date, a date beside a +/// plain number is compared on the serial, and the result is date-typed +/// whenever a date took part — even when a plain number won. +/// +/// Captured alongside the MAX/MIN forms, which agree with MAXA on every date +/// input — but captured and extrapolated are not the same thing here: +/// +/// - **Values** are captured in Google Sheets for every form — a date-only +/// column, a date/number column, array literals of both shapes, and dates +/// passed as direct arguments. +/// - **Typing** is captured for the *range* forms only, read back through the +/// cell that holds the result. The literal and direct-argument rows report +/// `number`, but that is an artifact of the capture harness reading them +/// through an `INDEX(...,1,1)` wrapper, which drops the cell's date format — +/// it is not a Sheets answer. So the date typing of +/// `=MAXA({DATE(...),DATE(...)})` is **extrapolated** from the range forms, +/// not probed. +/// +/// **None of those rows are in this repo yet.** They come off the +/// conformance-fixtures pipeline and land in a separate fixtures-only PR — +/// they fail until this code exists, and CI rejects a PR that mixes fixture +/// TSVs with code. Same arrangement as the blank-only rows described on +/// `stat_helpers::is_blank_only_array`. A reviewer working from this repo +/// alone can check the unit tests and this comment; the Sheets answers +/// themselves have to be taken from that pipeline. +/// +/// Two consequences are filed rather than fixed here: +/// +/// - `COUNT` does not count dates, so `=COUNT(MAXA())` answers 0 +/// where it used to answer 1 — a pre-existing `COUNT` gap this change makes +/// reachable. See #780. +/// - A `Zoned` sitting beside a `Date` is silently dropped by the loop below, +/// where `MAX`/`MIN` route the same input through `zoned_extreme` and error. +/// MAXA/MINA never consult that path. Unprobed on both sides. See #781. pub fn maxa_fn(args: &[Value]) -> Value { if args.is_empty() { return Value::Error(ErrorKind::NA); @@ -15,12 +51,17 @@ pub fn maxa_fn(args: &[Value]) -> Value { let mut result: Option = None; // See `fold_array_max` for why this flag exists. let mut skipped_sparkline = false; + let mut saw_date = false; for arg in args { match arg { Value::Sparkline(_) => skipped_sparkline = true, Value::Number(n) => { result = Some(result.map_or(*n, |cur: f64| cur.max(*n))); } + Value::Date(n) => { + saw_date = true; + result = Some(result.map_or(*n, |cur: f64| cur.max(*n))); + } Value::Bool(b) => { let n = if *b { 1.0 } else { 0.0 }; result = Some(result.map_or(n, |cur: f64| cur.max(n))); @@ -36,19 +77,28 @@ pub fn maxa_fn(args: &[Value]) -> Value { if inner.is_empty() { return Value::Error(ErrorKind::Ref); } - // In array context: Numbers included, Bool→1/0, Text→0, Empty→skip. + // In array context: Numbers included, Dates included as their + // bare serial (and they type the answer), Bool→1/0, Text→0, + // Empty→skip. // Recurses into nested arrays (e.g. a vertical range // materializes as nested one-element row arrays). - if let Err(e) = fold_array_max(inner, &mut result, &mut skipped_sparkline) { + if let Err(e) = + fold_array_max(inner, &mut result, &mut skipped_sparkline, &mut saw_date) + { return e; } } Value::Error(e) => return Value::Error(e.clone()), Value::ErrorMsg(e, m) => return Value::ErrorMsg(e.clone(), m.clone()), - _ => {} + // Listed rather than a catch-all so a new `Value` variant is a + // compile error here instead of a silent skip. + Value::Zoned(_) => {} } } match result { + // A date anywhere in scope makes the answer date-typed, whether or not + // the date is the value that won. + Some(n) if saw_date => Value::Date(n), Some(n) => Value::Number(n), None if skipped_sparkline => Value::Number(0.0), // An array of nothing but blanks is 0, not #N/A: `=MAXA(A1:A3)` over @@ -74,6 +124,7 @@ fn fold_array_max( arr: &[Value], result: &mut Option, skipped_sparkline: &mut bool, + saw_date: &mut bool, ) -> Result<(), Value> { for v in arr { let n = match v { @@ -82,16 +133,23 @@ fn fold_array_max( continue; } Value::Number(n) => *n, + // A date folds in as its bare serial and types the answer. + Value::Date(n) => { + *saw_date = true; + *n + } Value::Bool(b) => if *b { 1.0 } else { 0.0 }, Value::Text(_) => 0.0, Value::Empty => continue, Value::Array(inner) => { - fold_array_max(inner, result, skipped_sparkline)?; + fold_array_max(inner, result, skipped_sparkline, saw_date)?; continue; } Value::Error(e) => return Err(Value::Error(e.clone())), Value::ErrorMsg(e, m) => return Err(Value::ErrorMsg(e.clone(), m.clone())), - _ => continue, + // Listed rather than a catch-all so a new `Value` variant is a + // compile error here instead of inheriting "skipped" by accident. + Value::Zoned(_) => continue, }; *result = Some(result.map_or(n, |cur: f64| cur.max(n))); } diff --git a/crates/core/src/eval/functions/statistical/maxa/tests/edge.rs b/crates/core/src/eval/functions/statistical/maxa/tests/edge.rs index f3346f6bc..db9a439f2 100644 --- a/crates/core/src/eval/functions/statistical/maxa/tests/edge.rs +++ b/crates/core/src/eval/functions/statistical/maxa/tests/edge.rs @@ -97,3 +97,44 @@ fn maxa_text_only_array_is_zero() { Value::Number(0.0) ); } + +#[test] +fn dates_participate_and_type_the_answer() { + // MAXA agrees with MAX on every date input: a date-only array answers the + // largest serial (it was #N/A before dates were captured), a date and a + // plain number compare as bare serials, and the answer is date-typed + // whenever a date took part. + assert_eq!( + maxa_fn(&[Value::Array(vec![ + Value::Date(43831.0), + Value::Date(44197.0) + ])]), + Value::Date(44197.0) + ); + assert_eq!( + maxa_fn(&[Value::Date(43831.0), Value::Date(44197.0)]), + Value::Date(44197.0) + ); + assert_eq!( + maxa_fn(&[Value::Array(vec![Value::Date(43831.0), Value::Number(5.0)])]), + Value::Date(43831.0) + ); + // A nested-row range materialization takes the same path. + assert_eq!( + maxa_fn(&[Value::Array(vec![Value::Array(vec![Value::Date(43831.0)])])]), + Value::Date(43831.0) + ); + // No date in scope: still a plain number. + assert_eq!( + maxa_fn(&[Value::Number(5.0), Value::Bool(true)]), + Value::Number(5.0) + ); + // An all-blank array is a separate, captured case and answers 0, not the + // date rule and not #N/A — see `maxa_array_of_only_blanks_is_zero`. Kept + // here as the contrast: a date in scope types the answer, an array with + // nothing in it but blanks does not. + assert_eq!( + maxa_fn(&[Value::Array(vec![Value::Empty, Value::Empty])]), + Value::Number(0.0) + ); +} diff --git a/crates/core/src/eval/functions/statistical/min/mod.rs b/crates/core/src/eval/functions/statistical/min/mod.rs index c29fc6094..952a885b9 100644 --- a/crates/core/src/eval/functions/statistical/min/mod.rs +++ b/crates/core/src/eval/functions/statistical/min/mod.rs @@ -1,8 +1,9 @@ use crate::types::{ErrorKind, Value}; /// `MIN(value1, ...)` — smallest numeric value in the arguments. -/// Direct args: Numbers, Bool (TRUE=1, FALSE=0), parseable text coerced to number. -/// Array elements: Numbers only; text/Bool → skip; errors propagate. +/// Direct args: Numbers, Dates, Bool (TRUE=1, FALSE=0), parseable text coerced +/// to number. +/// Array elements: Numbers and Dates; text/Bool → skip; errors propagate. /// /// "No numbers" is *two* rules, not one: /// @@ -24,6 +25,44 @@ use crate::types::{ErrorKind, Value}; /// called from here at all; MAX, MAXA and MINA were brought to it. /// /// [`stat_helpers::is_blank_only_array`]: super::stat_helpers::is_blank_only_array +/// +/// **Dates participate as bare serials** and carry their type out. A date-only +/// range answers the earliest date, a date beside a plain number is compared on +/// the serial with no special casing (so a small plain number beats every +/// date), and the result is date-typed whenever a date took part — even when +/// the plain number won. +/// +/// Captured and extrapolated are not the same thing here, so keep them apart: +/// +/// - **Values** are captured in Google Sheets for every form — a date-only +/// column, a date/number column, array literals of both shapes, and dates +/// passed as direct arguments. +/// - **Typing** is captured for the *range* forms only, read back through the +/// cell that holds the result (`=MIN()` and +/// `=MIN()` both come back `date` — the second even +/// though a plain 5 won). The literal and direct-argument rows report +/// `number`, but that is an artifact of the capture harness reading them +/// through an `INDEX(...,1,1)` wrapper, which drops the cell's date format — +/// it is not a Sheets answer. So the date typing of +/// `=MIN({DATE(...),DATE(...)})` is **extrapolated** from the range forms, +/// not probed. +/// +/// **None of those rows are in this repo yet.** They come off the +/// conformance-fixtures pipeline and land in a separate fixtures-only PR — +/// they fail until this code exists, and CI rejects a PR that mixes fixture +/// TSVs with code. Same arrangement as the blank-only rows described on +/// `stat_helpers::is_blank_only_array`. A reviewer working from this repo +/// alone can check the unit tests and this comment; the Sheets answers +/// themselves have to be taken from that pipeline. +/// +/// Two consequences are filed rather than fixed here: +/// +/// - `COUNT` does not count dates, so `=COUNT(MIN())` answers 0 +/// where it used to answer 1 — a pre-existing `COUNT` gap this change makes +/// reachable. See #780. +/// - `MAXA`/`MINA` silently drop a `Zoned` sitting beside a `Date`, where +/// `MAX`/`MIN` route the same input through `zoned_extreme` and error. +/// Unprobed on both sides. See #781. pub fn min_fn(args: &[Value]) -> Value { if args.is_empty() { return Value::Error(ErrorKind::NA); @@ -34,11 +73,16 @@ pub fn min_fn(args: &[Value]) -> Value { return r; } let mut result: Option = None; + let mut saw_date = false; for arg in args { match arg { Value::Number(n) => { result = Some(result.map_or(*n, |cur: f64| cur.min(*n))); } + Value::Date(n) => { + saw_date = true; + result = Some(result.map_or(*n, |cur: f64| cur.min(*n))); + } Value::Bool(b) => { let n = if *b { 1.0 } else { 0.0 }; result = Some(result.map_or(n, |cur: f64| cur.min(n))); @@ -64,30 +108,56 @@ pub fn min_fn(args: &[Value]) -> Value { // Recurse into nested arrays (e.g. a vertical range // materializes as nested one-element row arrays) so every // cell is visited. - if let Err(e) = min_array_into(elems, &mut result) { + if let Err(e) = min_array_into(elems, &mut result, &mut saw_date) { return e; } } Value::Error(e) => return Value::Error(e.clone()), Value::ErrorMsg(e, m) => return Value::ErrorMsg(e.clone(), m.clone()), - _ => {} + // Listed rather than a catch-all so a new `Value` variant is a + // compile error here instead of a silent skip. A `Zoned` only + // reaches this loop when no other argument was zone-aware, which + // `zoned_extreme` above has already ruled on. + Value::Zoned(_) | Value::Sparkline(_) => {} } } - Value::Number(result.unwrap_or(0.0)) + match result { + // A date anywhere in scope makes the answer date-typed, whether or not + // the date is the value that won. + Some(n) if saw_date => Value::Date(n), + Some(n) => Value::Number(n), + None => Value::Number(0.0), + } } /// Recursively fold a nested array's numbers into `result` for MIN's /// array-context rules (Bool/Text/Empty skipped, errors propagate). -fn min_array_into(elems: &[Value], result: &mut Option) -> Result<(), Value> { +/// A `Date` folds in as its bare serial and raises `saw_date`, which types the +/// answer; every other variant keeps the arm it already had. +fn min_array_into( + elems: &[Value], + result: &mut Option, + saw_date: &mut bool, +) -> Result<(), Value> { for elem in elems { match elem { Value::Number(n) => { *result = Some(result.map_or(*n, |cur: f64| cur.min(*n))); } + Value::Date(n) => { + *saw_date = true; + *result = Some(result.map_or(*n, |cur: f64| cur.min(*n))); + } Value::Error(e) => return Err(Value::Error(e.clone())), Value::ErrorMsg(e, m) => return Err(Value::ErrorMsg(e.clone(), m.clone())), - Value::Array(inner) => min_array_into(inner, result)?, - _ => {} + Value::Array(inner) => min_array_into(inner, result, saw_date)?, + // Listed rather than a catch-all so a new `Value` variant is a + // compile error here instead of inheriting "skipped" by accident. + Value::Text(_) + | Value::Bool(_) + | Value::Empty + | Value::Zoned(_) + | Value::Sparkline(_) => {} } } Ok(()) diff --git a/crates/core/src/eval/functions/statistical/min/tests/edge.rs b/crates/core/src/eval/functions/statistical/min/tests/edge.rs index 53eedeea4..5f0ab5619 100644 --- a/crates/core/src/eval/functions/statistical/min/tests/edge.rs +++ b/crates/core/src/eval/functions/statistical/min/tests/edge.rs @@ -78,6 +78,59 @@ fn min_array_of_only_blanks_is_zero() { ); } +#[test] +fn min_array_of_only_dates_returns_the_earliest_date() { + // Was a silent 0 — the exact hazard of a numberless fold: it renders as a + // date in 1899 and looks like an answer. Now the smallest serial, typed. + assert_eq!( + min_fn(&[Value::Array(vec![ + Value::Date(43831.0), + Value::Date(44197.0) + ])]), + Value::Date(43831.0) + ); + assert_eq!( + min_fn(&[Value::Array(vec![Value::Array(vec![Value::Date(43831.0)])])]), + Value::Date(43831.0) + ); +} + +#[test] +fn min_dates_compare_as_bare_serials_and_type_the_answer() { + // A small plain number beats every date, because the comparison is on the + // bare serial — and the answer is still date-typed, since a date took part. + assert_eq!( + min_fn(&[Value::Array(vec![Value::Date(43831.0), Value::Number(5.0)])]), + Value::Date(5.0) + ); + assert_eq!( + min_fn(&[Value::Date(43831.0), Value::Number(5.0)]), + Value::Date(5.0) + ); + assert_eq!( + min_fn(&[Value::Date(43831.0), Value::Date(44197.0)]), + Value::Date(43831.0) + ); + // No date in scope: the answer stays a plain number. + assert_eq!( + min_fn(&[Value::Number(5.0), Value::Number(1.0)]), + Value::Number(1.0) + ); +} + +#[test] +fn min_date_beside_a_blank_is_the_date_not_zero() { + assert_eq!( + min_fn(&[Value::Array(vec![Value::Date(43831.0), Value::Empty])]), + Value::Date(43831.0) + ); + // An all-blank array is a separate, still-unprobed case and keeps its 0. + assert_eq!( + min_fn(&[Value::Array(vec![Value::Empty, Value::Empty])]), + Value::Number(0.0) + ); +} + #[test] fn min_negative_numbers() { assert_eq!( diff --git a/crates/core/src/eval/functions/statistical/mina/mod.rs b/crates/core/src/eval/functions/statistical/mina/mod.rs index 594cea4cb..affaa9aa1 100644 --- a/crates/core/src/eval/functions/statistical/mina/mod.rs +++ b/crates/core/src/eval/functions/statistical/mina/mod.rs @@ -8,6 +8,43 @@ use crate::types::{ErrorKind, Value}; /// - Empty array argument → `#REF!`. /// - Array of nothing but blanks → 0. /// - No args → `#N/A`. +/// +/// **Dates participate as bare serials** and carry their type out, exactly as +/// they do for MIN: a date-only range answers the earliest date, a date beside +/// a plain number is compared on the serial (so a small plain number beats +/// every date), and the result is date-typed whenever a date took part — even +/// when the plain number won. +/// +/// Captured alongside the MAX/MIN forms, which agree with MINA on every date +/// input — but captured and extrapolated are not the same thing here: +/// +/// - **Values** are captured in Google Sheets for every form — a date-only +/// column, a date/number column, array literals of both shapes, and dates +/// passed as direct arguments. +/// - **Typing** is captured for the *range* forms only, read back through the +/// cell that holds the result. The literal and direct-argument rows report +/// `number`, but that is an artifact of the capture harness reading them +/// through an `INDEX(...,1,1)` wrapper, which drops the cell's date format — +/// it is not a Sheets answer. So the date typing of +/// `=MINA({DATE(...),DATE(...)})` is **extrapolated** from the range forms, +/// not probed. +/// +/// **None of those rows are in this repo yet.** They come off the +/// conformance-fixtures pipeline and land in a separate fixtures-only PR — +/// they fail until this code exists, and CI rejects a PR that mixes fixture +/// TSVs with code. Same arrangement as the blank-only rows described on +/// `stat_helpers::is_blank_only_array`. A reviewer working from this repo +/// alone can check the unit tests and this comment; the Sheets answers +/// themselves have to be taken from that pipeline. +/// +/// Two consequences are filed rather than fixed here: +/// +/// - `COUNT` does not count dates, so `=COUNT(MINA())` answers 0 +/// where it used to answer 1 — a pre-existing `COUNT` gap this change makes +/// reachable. See #780. +/// - A `Zoned` sitting beside a `Date` is silently dropped by the loop below, +/// where `MAX`/`MIN` route the same input through `zoned_extreme` and error. +/// MAXA/MINA never consult that path. Unprobed on both sides. See #781. pub fn mina_fn(args: &[Value]) -> Value { if args.is_empty() { return Value::Error(ErrorKind::NA); @@ -15,12 +52,17 @@ pub fn mina_fn(args: &[Value]) -> Value { let mut result: Option = None; // See `fold_array_min` for why this flag exists. let mut skipped_sparkline = false; + let mut saw_date = false; for arg in args { match arg { Value::Sparkline(_) => skipped_sparkline = true, Value::Number(n) => { result = Some(result.map_or(*n, |cur: f64| cur.min(*n))); } + Value::Date(n) => { + saw_date = true; + result = Some(result.map_or(*n, |cur: f64| cur.min(*n))); + } Value::Bool(b) => { let n = if *b { 1.0 } else { 0.0 }; result = Some(result.map_or(n, |cur: f64| cur.min(n))); @@ -36,19 +78,28 @@ pub fn mina_fn(args: &[Value]) -> Value { if inner.is_empty() { return Value::Error(ErrorKind::Ref); } - // In array context: Numbers included, Bool→1/0, Text→0, Empty→skip. + // In array context: Numbers included, Dates included as their + // bare serial (and they type the answer), Bool→1/0, Text→0, + // Empty→skip. // Recurses into nested arrays (e.g. a vertical range // materializes as nested one-element row arrays). - if let Err(e) = fold_array_min(inner, &mut result, &mut skipped_sparkline) { + if let Err(e) = + fold_array_min(inner, &mut result, &mut skipped_sparkline, &mut saw_date) + { return e; } } Value::Error(e) => return Value::Error(e.clone()), Value::ErrorMsg(e, m) => return Value::ErrorMsg(e.clone(), m.clone()), - _ => {} + // Listed rather than a catch-all so a new `Value` variant is a + // compile error here instead of a silent skip. + Value::Zoned(_) => {} } } match result { + // A date anywhere in scope makes the answer date-typed, whether or not + // the date is the value that won. + Some(n) if saw_date => Value::Date(n), Some(n) => Value::Number(n), None if skipped_sparkline => Value::Number(0.0), // An array of nothing but blanks is 0, not #N/A: `=MINA(A1:A3)` over @@ -74,6 +125,7 @@ fn fold_array_min( arr: &[Value], result: &mut Option, skipped_sparkline: &mut bool, + saw_date: &mut bool, ) -> Result<(), Value> { for v in arr { let n = match v { @@ -82,16 +134,23 @@ fn fold_array_min( continue; } Value::Number(n) => *n, + // A date folds in as its bare serial and types the answer. + Value::Date(n) => { + *saw_date = true; + *n + } Value::Bool(b) => if *b { 1.0 } else { 0.0 }, Value::Text(_) => 0.0, Value::Empty => continue, Value::Array(inner) => { - fold_array_min(inner, result, skipped_sparkline)?; + fold_array_min(inner, result, skipped_sparkline, saw_date)?; continue; } Value::Error(e) => return Err(Value::Error(e.clone())), Value::ErrorMsg(e, m) => return Err(Value::ErrorMsg(e.clone(), m.clone())), - _ => continue, + // Listed rather than a catch-all so a new `Value` variant is a + // compile error here instead of inheriting "skipped" by accident. + Value::Zoned(_) => continue, }; *result = Some(result.map_or(n, |cur: f64| cur.min(n))); } diff --git a/crates/core/src/eval/functions/statistical/mina/tests/edge.rs b/crates/core/src/eval/functions/statistical/mina/tests/edge.rs index aeb5d8d0a..9c84c2bd0 100644 --- a/crates/core/src/eval/functions/statistical/mina/tests/edge.rs +++ b/crates/core/src/eval/functions/statistical/mina/tests/edge.rs @@ -97,3 +97,44 @@ fn mina_text_only_array_is_zero() { Value::Number(0.0) ); } + +#[test] +fn dates_participate_and_type_the_answer() { + // MINA agrees with MIN on every date input: a date-only array answers the + // smallest serial (it was #N/A before dates were captured), a small plain + // number beats every date, and the answer is date-typed whenever a date + // took part — including when the plain number won. + assert_eq!( + mina_fn(&[Value::Array(vec![ + Value::Date(43831.0), + Value::Date(44197.0) + ])]), + Value::Date(43831.0) + ); + assert_eq!( + mina_fn(&[Value::Date(43831.0), Value::Date(44197.0)]), + Value::Date(43831.0) + ); + assert_eq!( + mina_fn(&[Value::Array(vec![Value::Date(43831.0), Value::Number(5.0)])]), + Value::Date(5.0) + ); + // A nested-row range materialization takes the same path. + assert_eq!( + mina_fn(&[Value::Array(vec![Value::Array(vec![Value::Date(43831.0)])])]), + Value::Date(43831.0) + ); + // No date in scope: still a plain number. + assert_eq!( + mina_fn(&[Value::Number(5.0), Value::Bool(true)]), + Value::Number(1.0) + ); + // An all-blank array is a separate, captured case and answers 0, not the + // date rule and not #N/A — see `mina_array_of_only_blanks_is_zero`. Kept + // here as the contrast: a date in scope types the answer, an array with + // nothing in it but blanks does not. + assert_eq!( + mina_fn(&[Value::Array(vec![Value::Empty, Value::Empty])]), + Value::Number(0.0) + ); +} diff --git a/crates/core/tests/integration.rs b/crates/core/tests/integration.rs index a864df870..ac1683652 100644 --- a/crates/core/tests/integration.rs +++ b/crates/core/tests/integration.rs @@ -536,3 +536,83 @@ fn makearray_larger_result_still_array() { Value::Array(vec![Value::Number(2.0), Value::Number(3.0)]) ); } + +// ── MAX/MIN/MAXA/MINA over dates (#776) ───────────────────────────────────── + +#[test] +fn extremes_over_dates_agree_and_stay_date_typed() { + // All four functions agree on every date input: the extreme serial, typed + // as a date. MAX used to answer #REF! here and MIN a silent 0 that renders + // as a date in 1899. + // + // Evidence split, so these assertions are not all the same strength: + // - the *serials* below are captured Google Sheets answers for exactly + // these array-literal and direct-argument forms; + // - the *date typing* is captured for the range forms only + // (`=MAX()` read back through the cell holding it). The + // literal rows came back reporting `number`, but only because the + // capture harness reads them through an `INDEX(...,1,1)` wrapper that + // drops the cell's date format — not a Sheets answer. The ISDATE + // assertion below is therefore **extrapolated** from the range capture, + // not probed in this shape. + for (f, expected) in [ + ("MAX", 44197.0), + ("MAXA", 44197.0), + ("MIN", 43831.0), + ("MINA", 43831.0), + ] { + assert_eq!( + helpers::eval(&format!("={f}({{DATE(2020,1,1),DATE(2021,1,1)}})")), + Value::Date(expected), + "{f} over a date-only array" + ); + // Same answer whether the dates arrive as array elements or as + // separate arguments. + assert_eq!( + helpers::eval(&format!("={f}(DATE(2020,1,1),DATE(2021,1,1))")), + Value::Date(expected), + "{f} over date arguments" + ); + // Extrapolated from the range capture — see the note above. + assert_eq!( + helpers::eval(&format!("=ISDATE({f}({{DATE(2020,1,1),DATE(2021,1,1)}}))")), + Value::Bool(true), + "{f} result is date-typed" + ); + } +} + +#[test] +fn extremes_compare_dates_and_numbers_as_bare_serials() { + // A plain number and a date are compared on the serial with no special + // casing, so 5 beats every date for MIN/MINA — and the answer is still + // date-typed, because a date took part. + // + // Same evidence split as above: the serials are captured for this literal + // form, while the date typing is captured only for the equivalent *range* + // form (a date/number column, where MIN answers the plain 5 and the cell + // still reads back as `date`). The `Value::Date` expectations here are + // extrapolated from that. + for f in ["MAX", "MAXA"] { + assert_eq!( + helpers::eval(&format!("={f}({{DATE(2020,1,1),5}})")), + Value::Date(43831.0), + "{f} of a date and 5" + ); + } + for f in ["MIN", "MINA"] { + assert_eq!( + helpers::eval(&format!("={f}({{DATE(2020,1,1),5}})")), + Value::Date(5.0), + "{f} of a date and 5" + ); + } + // Without a date in scope the answer stays a plain number. + for f in ["MAX", "MAXA", "MIN", "MINA"] { + assert_eq!( + helpers::eval(&format!("=ISDATE({f}({{1,2,3}}))")), + Value::Bool(false), + "{f} over plain numbers is not date-typed" + ); + } +} diff --git a/crates/wasm/README.md b/crates/wasm/README.md index c5b1d5602..12895b6c0 100644 --- a/crates/wasm/README.md +++ b/crates/wasm/README.md @@ -141,6 +141,18 @@ evaluate('TODAY()') > treating the result as a number, also accept `type === 'date'` (the `value` > encoding is identical — a serial number). +> #### Result-type change: `MAX` / `MIN` / `MAXA` / `MINA` over dates +> +> Dates now take part in these four aggregates, and the result is **date-typed** +> whenever a date took part — including when a plain number won the comparison. +> `evaluate('MAX(A1:A10)')` over a column of dates returns +> `{ type: 'date', value }` where it previously returned `{ type: 'number', value }` +> (`MAX` over a date-only array literal previously returned an `#REF!` error and +> `MIN` a silent `0`). The `value` encoding is unchanged — still a serial number; +> only `type` moved. This matches Google Sheets, which formats the result cell as +> a date. If you branch on `type`, accept `'date'` anywhere you accepted +> `'number'` from these functions. See issue #776. + ### `validate(formula)` Checks whether a formula is syntactically valid without evaluating it.