https://bugs.documentfoundation.org/show_bug.cgi?id=171969

--- Comment #3 from [email protected] ---
A possible minimal fix for the "md" calculation would be to clamp the day to
the last valid day of the target month instead of calling Date::Normalize() on
a potentially invalid date.

Current code in ScInterpreter::ScGetDateDif(),
sc/source/core/tool/interpr2.cxx:

```cpp
else
{
    if (m2 == 1)
    {
        aDate1.SetYear( y2 == 1 ? -1 : y2 - 1 );
        aDate1.SetMonth( 12 );
    }
    else
    {
        aDate1.SetYear( y2 );
        aDate1.SetMonth( m2 - 1 );
    }
    aDate1.Normalize();
    nd = aDate2 - aDate1;
}
```

The problem is that, for example, this can produce an intermediate 2023-02-31.
Normalize() does not clamp that date to the end of February; it rolls it
forward to 2023-03-03.

The Date API already provides Date::GetDaysInMonth(), and date.hxx specifically
recommends the static method when working with potentially non-normalized
dates.

A minimal patch could therefore be:

```diff
 else
 {
     if (m2 == 1)
     {
         aDate1.SetYear( y2 == 1 ? -1 : y2 - 1 );
         aDate1.SetMonth( 12 );
     }
     else
     {
         aDate1.SetYear( y2 );
         aDate1.SetMonth( m2 - 1 );
     }
-    aDate1.Normalize();
+
+    // Clamp the original day to the last valid day of the
+    // preceding month instead of rolling an invalid date
+    // into the following month.
+    const sal_uInt16 nDaysInMonth =
+        Date::GetDaysInMonth(aDate1.GetMonth(), aDate1.GetYear());
+    if (aDate1.GetDay() > nDaysInMonth)
+        aDate1.SetDay(nDaysInMonth);
+
     nd = aDate2 - aDate1;
 }
```

This would give, for example:

```
DATEDIF(2023-01-31; 2023-03-01; "md") = 1
DATEDIF(2024-01-31; 2024-03-01; "md") = 1
DATEDIF(2024-03-31; 2024-05-01; "md") = 1
```

instead of creating invalid intermediate dates and obtaining negative or zero
results as a consequence of Normalize() rollover.

Regression tests could also be added to the existing testFuncDATEDIF() in:

sc/qa/unit/ucalc_formula2.cxx

For example, immediately after the existing DATEDIF test cases:

```cpp
{ "2023-01-31", "2023-03-01", "md", "1",
  "=DATEDIF(A16;B16;C16)" },
{ "2024-01-31", "2024-03-01", "md", "1",
  "=DATEDIF(A17;B17;C17)" },
{ "2024-03-31", "2024-05-01", "md", "1",
  "=DATEDIF(A18;B18;C18)" },
{ "2024-04-30", "2024-06-01", "md", "2",
  "=DATEDIF(A19;B19;C19)" },
```

The last test is useful to verify that a case that already works correctly is
not changed unnecessarily.

Of course, this minimal change intentionally stops reproducing Excel's rollover
bug for "md". If exact Excel compatibility must be preserved for OOXML, I think
the preferable long-term solution would be to keep this corrected behavior for
the ODF/OpenFormula DATEDIF implementation and route Excel-compatible DATEDIF
through a separate compatibility implementation, as Calc already does for other
functions whose Excel and ODF semantics differ.

This is only a suggested patch; I have not compiled the LibreOffice source tree
with this modification.

-- 
You are receiving this mail because:
You are the assignee for the bug.

Reply via email to