Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

First Php8.4 Problem - ROUNDUP and ROUNDDOWN #3896

Closed
1 of 8 tasks
oleibman opened this issue Feb 6, 2024 · 4 comments · Fixed by #3897
Closed
1 of 8 tasks

First Php8.4 Problem - ROUNDUP and ROUNDDOWN #3896

oleibman opened this issue Feb 6, 2024 · 4 comments · Fixed by #3897

Comments

@oleibman
Copy link
Collaborator

oleibman commented Feb 6, 2024

This is:

- [x] a bug report
- [ ] a feature request
- [ ] **not** a usage question (ask them on https://stackoverflow.com/questions/tagged/phpspreadsheet or https://gitter.im/PHPOffice/PhpSpreadsheet)

What is the expected behavior?

Under Php7 and Php8 <= 8.3

var_dump(2..26 + 2.94);

double(5.2)

What is the current behavior?

Under Php8.4 nightly:

float(5.199999999999999)

I don't know if the difference between double and float is significant. But the difference in the result can affect, at the very least, ROUNDUP and ROUNDDOWN.

What are the steps to reproduce?

Please provide a Minimal, Complete, and Verifiable example of code that exhibits the issue without relying on an external Excel file or a web server:

<?php

require __DIR__ . '/vendor/autoload.php';

// Create new Spreadsheet object
$spreadsheet = new \PhpOffice\PhpSpreadsheet\Spreadsheet();
$sheet = $spreadsheet->getActiveSheet();
$sheet->getCell('A1')->setValue('=ROUNDDOWN(2.26 + 2.94, 2)');
$result1 = $sheet->getCell('A1')->getCalculatedValue();
var_dump($result1);

The result is 5.19 in Php8.4, 5.20 in earlier releases.

If this is an issue with reading a specific spreadsheet file, then it may be appropriate to provide a sample file that demonstrates the problem; but please keep it as small as possible, and sanitize any confidential information before uploading.

What features do you think are causing the issue

  • Reader
  • Writer
  • Styles
  • Data Validations
  • Formula Calculations
  • Charts
  • AutoFilter
  • Form Elements

Does an issue affect all spreadsheet file formats? If not, which formats are affected?

All

Which versions of PhpSpreadsheet and PHP are affected?

Any version of PhpSpreadsheet using Php8.4 is affected.

@oleibman
Copy link
Collaborator Author

oleibman commented Feb 6, 2024

One AMORDEGRC test is also affected.

@oleibman
Copy link
Collaborator Author

oleibman commented Feb 6, 2024

According to https://stackoverflow.com/questions/3280892/difference-between-float-and-double-in-php, the difference between float and double in the output depends merely on the availability of xdebug.

oleibman added a commit to oleibman/PhpSpreadsheet that referenced this issue Feb 6, 2024
Fix PHPOffice#3896. It appears that floating-point arithmetic will give different results in Php 8.4 vs. all earlier releases. This causes tests to fail in the nightly run for ROUNDDOWN, ROUNDUP, and AMORDEGRC. I imagine this won't be the last we hear of this. The failures are a distraction when reviewing PR's. This PR eliminates the distraction by adding in a fudge factor for Php 8.4+ while not changing Php 8.3-. It is not a particularly robust solution, but it should be stable for Php 8.3-, and good enough for Php 8.4+ while I study if a better solution is available.
@lucasnetau
Copy link
Contributor

Hi @oleibman, PHP8.4 has a change to round which extends the precision by one digit php/php-src@703ead5 which is the likely cause of the issue.

FWIW float(5.199999999999999) is the output value since PHP8 per https://github.com/php/php-src/blob/PHP-8.0/UPGRADING#L584#L587 because var_dump moved to using serialize_precision instead of precision. In 8.0 serialize_precision is defaulted to -1. If you ini_set precision to -1 in PHP 7.1 to 7.4 you also get float(5.199999999999999) as the output. See https://3v4l.org/duI8M

@oleibman
Copy link
Collaborator Author

@lucasnetau Thank you for the research. It is always nicer to be able to point to the smoking gun rather than fall back on "something changed".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

2 participants