pjfanning opened a new pull request, #1280:
URL: https://github.com/apache/poi/pull/1280
Follow-up to #1275. **Stacked on #1278 and #1279** (it touches the same
`QUOTIENT`, `FACT` and `EVEN`/`ODD` code): their two commits show here until
they merge, after which only the last commit remains — merge those first.
Excel truncates the non-integer arguments of these functions ("if number is
not an integer, it is truncated") the same way `INT` does since #1275: on the
15 significant digits it exposes, not on the raw binary value. POI cast the raw
double, so with the KB's own example value `880000000*0.00849/3` =
`2490399.9999999995`:
| Formula | before | now (= Excel) |
|---|---|---|
| `QUOTIENT(880000000*0.00849,3)` | 2490399 | 2490400 (same as `INT` of that
quotient) |
| `ISEVEN(880000000*0.00849/3)` | FALSE | TRUE |
| `FACT(5-9E-16)` (4.999999999999999, "5" to Excel) | 24 | 120 |
| `FACTDOUBLE(5-9E-16)` | 8 | 15 |
| `COMBIN(5-9E-16,2)` | 6 | 10 |
| `GCD(5-9E-16,10)` / `LCM(5-9E-16,10)` | 2 / 20 | 5 / 10 |
| `MROUND(0.7+0.1,1.6)` (quotient 0.49999999999999994, "0.5" to Excel) | 0 |
1.6 |
- New **`ExcelArithmetic.truncate(double)`**: toward-zero truncation of
`approxValue(d)`, returned as a double so magnitudes beyond the long range
work. Used in `QUOTIENT`, `GCD`, `LCM`, `FACT`, `FACTDOUBLE`, `COMBIN`,
`ISEVEN`, `ISODD`.
- **`MROUND`** rounds the 15-digit view of its number, as
`MathX.floor`/`ceiling` do (`approxValue` on the number before the existing
`BigDecimal` division — `MathX` is package-private so its
`scaledRoundUsingBigDecimal` can't be reused from `atp`).
- `EVEN`/`ODD` need nothing here: they already ceil on the 15-digit view
after #1279. `PERMUT` is not implemented in POI.
Adjacent fixes on the way:
- `FACTDOUBLE` returned `factorial(n).longValue()`, which silently wraps
beyond 2^63 — `FACTDOUBLE(50)` was garbage (Excel: 5.20469842636667E+32). It
now returns the double, and `#NUM!` above 300 where the result overflows a
double (300!! ≈ 8.2E307). Arguments outside the int range no longer throw from
`coerceValueToInt`.
- `COMBIN(-5E9,2)` threw `IllegalArgumentException` from `safeDoubleToInt`
(the existing guard only checked the upper bound); now `#NUM!`.
- `ISEVEN(1E19)` saturated the long cast to `Long.MAX_VALUE` (odd) → FALSE;
the parity is now taken in doubles.
Not touched: the ~25 other `safeDoubleToInt`/`coerceValueToInt` sites for
index-like arguments (`DATE`, `CHOOSE`, `LEFT`, `REPT`, `LARGE`, …) — the same
15-digit rule presumably applies in Excel but those arguments are rarely
produced by fractional arithmetic, and the central
`OperandResolver.coerceValueToInt` floors rather than truncates, so that's a
separate decision.
Tests: `TestExcelArithmetic.testTruncate`, new `TestParityFunction`
(ISEVEN/ISODD had no tests) and `TestFactDouble`, plus cases in `TestQuotient`,
`TestGcd`, `TestLcm`, `TestMRound` and `TestNumericFunction` (FACT, COMBIN).
Ran those with `TestMathX`, `TestQuotientFunctionsFromSpreadsheet`,
`TestFactDoubleFunctionsFromSpreadsheet`, `TestAnalysisToolPak`,
`TestExternalFunctionFormulas`, `TestMultiOperandNumericFunction` and
`TestFormulasFromSpreadsheet` locally: 472 tests, 0 failures.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
