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

Floats rounds down #12075

Closed
sinnbeck opened this issue Aug 29, 2023 · 21 comments
Closed

Floats rounds down #12075

sinnbeck opened this issue Aug 29, 2023 · 21 comments

Comments

@sinnbeck
Copy link

sinnbeck commented Aug 29, 2023

Description

The following code:

<?php
echo number_format($contents['value'], 1);
echo round($contents['value'], 1);

Resulted in this output:

5.7
5.7

But I expected this output instead:

5.8
5.8

This happens after upgrading to php 8.2.9 (I have tested all versions down to 8.2.5)

I believe it is caused by this fix

Fix GH-11587 (After php8.1, when PDO::ATTR_EMULATE_PREPARES is true and PDO::ATTR_STRINGIFY_FETCHES is true, decimal zeros are no longer filled).

PDO::ATTR_EMULATE_PREPARES is true and PDO::ATTR_STRINGIFY_FETCHES is true

image

PHP Version

PHP 8.2.9

Operating System

Ubuntu 20.04

@damianwadley
Copy link
Member

What does var_dump($contents['value']) show?

@sinnbeck
Copy link
Author

sinnbeck commented Aug 29, 2023

Apparently somehow it is below 7.5 in the new version.

float(5.749999999999966)

This is the output in php 8.2.5

float(5.75)

@damianwadley
Copy link
Member

It sounds like normal floating-point problems: before you were receiving something that was processed and resulted in 5.75, while now you're receiving something that hasn't been processed for you. The actual number instead of some stringified/unstringified value. Which means you need (needed) to be the one doing the processing - namely, rounding it to the precision that you expect the data to be first, in order to get an accurate representation of what the data was intended to be (because floating-point numbers are fun like that), and then doing the desired rounding to the precision you want for display.

@sinnbeck
Copy link
Author

Yeah I am in the process of trying to fix these precision errors. This is an old project I inherited and the code-base it huge. I just didn't expect this sort of change in a minor version.

It works if I set ATTR_EMULATE_PREPARES to false.

I assume I'ts not going to be changed back, so I suppose its time to give it another shot at refactoring all calculations :)

@hormus
Copy link

hormus commented Aug 31, 2023

<?php

ini_set('precision', 14);
ini_set('serialize_precision', 17);
var_dump(ini_get('serialize_precision'));
$a = 5.749999999999966;
$b = round($a, 1);
var_dump(number_format($a, 1), $b);
ini_set('precision', 14);
ini_set('serialize_precision', 17);
echo $b;
echo "\n";
var_dump($b);
?>

Expected result and not use more precise float RFC serialize_precision and precision set to -1 (for this example):

string(2) "17"
string(3) "5.7"
float(5.7000000000000002)
5.7
float(5.7000000000000002)

round use serialize_precision (set 17) and $precision (this example is 1).
displaying a float is not always the same as creating it albeit lossy. create it use serialize_precision and php functions use precision or serialize_precision.

@youkidearitai
Copy link
Contributor

I just didn't expect this sort of change in a minor version.

GH-11587 was originally keep backward compatible change before PHP 8.1. Therefore, it was fixed.

@sinnbeck
Copy link
Author

@youkidearitai yeah I can see that, yet all tests passed in php 8.1. But I have rewritten the logic now so it works, but others might run into similar issues

@youkidearitai
Copy link
Contributor

@sinnbeck I see.

@SakiTakamachi Any thoughts?

@SakiTakamachi
Copy link
Member

@sinnbeck
I would like to know the version of mysql and the table definition.

For reference, this is what happens in my environment.

# php -v
PHP 8.2.9 (cli) (built: Aug 17 2023 22:42:21) (NTS)
Copyright (c) The PHP Group
Zend Engine v4.2.9, Copyright (c) Zend Technologies
# mysql -v
Welcome to the MySQL monitor.  Commands end with ; or \g.
Your MySQL connection id is 26
Server version: 8.0.33 MySQL Community Server - GPL
mysql> desc test;
+---------+------------+------+-----+---------+-------+
| Field   | Type       | Null | Key | Default | Extra |
+---------+------------+------+-----+---------+-------+
| value_1 | float(3,2) | YES  |     | NULL    |       |
| value_2 | float      | YES  |     | NULL    |       |
+---------+------------+------+-----+---------+-------+
2 rows in set (0.00 sec)
mysql> select * from test;
+---------+---------+
| value_1 | value_2 |
+---------+---------+
|    5.75 |    5.75 |
+---------+---------+
<?php
$pdo = new PDO("mysql:host=mysql;dbname=test", "root", ""); // my env
$pdo->setAttribute(PDO::ATTR_EMULATE_PREPARES, true);
$pdo->setAttribute(PDO::ATTR_STRINGIFY_FETCHES, true);

$results = $pdo->query("SELECT * from test");
foreach ($results as $result) {
     var_dump($result);
}
// PDO::ATTR_EMULATE_PREPARES = true, PDO::ATTR_STRINGIFY_FETCHES = true
# php float_test.php
array(4) {
  ["value_1"]=>
  string(4) "5.75"
  [0]=>
  string(4) "5.75"
  ["value_2"]=>
  string(4) "5.75"
  [1]=>
  string(4) "5.75"
}

// PDO::ATTR_EMULATE_PREPARES = false, PDO::ATTR_STRINGIFY_FETCHES = false
#  php float_test.php
array(4) {
  ["value_1"]=>
  float(5.75)
  [0]=>
  float(5.75)
  ["value_2"]=>
  float(5.75)
  [1]=>
  float(5.75)
}

@sinnbeck
Copy link
Author

sinnbeck commented Aug 31, 2023

@SakiTakamachi

The column is a double.

COLUMN_NAME COLUMN_TYPE IS_NULLABLE COLUMN_KEY COLUMN_DEFAULT EXTRA
id bigint(20) unsigned NO PRI auto_increment
respondent_id int(11) unsigned NO MUL
answer double NO

And it is doing a SUM() on a bunch of rows. The value 5.75 does not come directly from here, but rather is the output of several php calculations done with the output.

I am currently trying to find a way of make a simple reproducible example.

@hormus
Copy link

hormus commented Aug 31, 2023

it's ok @sinnbeck list all the steps, there is "fixed point" which means write by pen.
I think it's a problem from precision to serialize_precision, the "float point" value with serialize_precision is not 3.75 (maybe with precision 3.75)
They are two php settings precision and serialize_precision from php.ini

@sinnbeck
Copy link
Author

@hormus thanks. That was my first thought as well but it only happens when both ATTR_EMULATE_PREPARES and ATTR_STRINGIFY_FETCHES is set to false. So I assume it's due to the fix of the trailing zeroes in pdo

@SakiTakamachi
Copy link
Member

@sinnbeck

Ah I see.

It may be a bit difficult, but is it possible to perform a similar test with php8.0?

@sinnbeck
Copy link
Author

sinnbeck commented Sep 1, 2023

@SakiTakamachi If I manage to find a way to test it I can easily test it in both 8.1 and 8.0 using docker if needed.

@sinnbeck
Copy link
Author

sinnbeck commented Sep 1, 2023

Ok I think I managed to get something working. Sadly the data is a bit long

Test data:

36.8
19.555555555556
10.3
23.0
7.7222222222222
19.555555555556
4.0
5.6666666666667
5.375
7.7222222222222
5.2222222222222
4.0
5.375
5.375
4.0
9.3333333333333
5.75
5.375
9.8
10.3
5.375
5.75
5.375
36.8
9.3333333333333
19.555555555556
19.555555555556
36.8
36.8
7.7222222222222
7.7222222222222
7.7222222222222
7.7222222222222
7.7222222222222
7.7222222222222
7.7222222222222
7.7222222222222
7.7222222222222
7.7222222222222
7.7222222222222
7.7222222222222
10.3
10.3
10.3
10.3
10.3
10.3
5.2222222222222
5.2222222222222
5.2222222222222
5.2222222222222
5.75
5.75
5.75
5.75
5.75
5.75
5.6666666666667
5.6666666666667
5.6666666666667
5.6666666666667
5.6666666666667
5.6666666666667
5.6666666666667
5.375
5.375
17.666666666667
17.666666666667
17.666666666667
17.666666666667
17.666666666667
17.666666666667
9.3333333333333
9.3333333333333
9.3333333333333
9.3333333333333
9.3333333333333
9.3333333333333
23.0
23.0
23.0
23.0
9.8
9.8
9.8
9.8
9.8
9.8
9.8
4.0
4.0
4.0
4.0
3.6666666666667
3.6666666666667
3.6666666666667
3.6666666666667
3.6666666666667
3.6666666666667
9.8
7.7222222222222
4.0
9.3333333333333
10.3
9.8
19.555555555556
5.6666666666667
5.2222222222222
19.555555555556
19.555555555556
19.555555555556
19.555555555556
36.8
7.7222222222222
7.7222222222222
7.7222222222222
10.3
5.2222222222222
5.2222222222222
5.2222222222222

Do a sum of this and in php 8.2.5 you get 1246
image

while in php 8.2.9 you get 1246.0000000000052
image

This is the same query from Dbeaver
image

@sinnbeck
Copy link
Author

sinnbeck commented Sep 1, 2023

But as I said, I have already solved the issue for the app, so if this is how it should be, I completely understand and the issue can be closed 😊👍

@hormus
Copy link

hormus commented Sep 1, 2023

ini_set('serialize_precision', 14);
var_dump(36.8 +
19.555555555556 +
10.3 +
23.0 +
7.7222222222222 +
19.555555555556 +
4.0 +
5.6666666666667 +
5.375 +
7.7222222222222 +
5.2222222222222 +
4.0 +
5.375 +
5.375 +
4.0 +
9.3333333333333 +
5.75 +
5.375 +
9.8 +
10.3 +
5.375 +
5.75 +
5.375 +
36.8 +
9.3333333333333 +
19.555555555556 +
19.555555555556 +
36.8 +
36.8 +
7.7222222222222 +
7.7222222222222 +
7.7222222222222 +
7.7222222222222 +
7.7222222222222 +
7.7222222222222 +
7.7222222222222 +
7.7222222222222 +
7.7222222222222 +
7.7222222222222 +
7.7222222222222 +
7.7222222222222 +
10.3 +
10.3 +
10.3 +
10.3 +
10.3 +
10.3 +
5.2222222222222 +
5.2222222222222 +
5.2222222222222 +
5.2222222222222 +
5.75 +
5.75 +
5.75 +
5.75 +
5.75 +
5.75 +
5.6666666666667 +
5.6666666666667 +
5.6666666666667 +
5.6666666666667 +
5.6666666666667 +
5.6666666666667 +
5.6666666666667 +
5.375 +
5.375 +
17.666666666667 +
17.666666666667 +
17.666666666667 +
17.666666666667 +
17.666666666667 +
17.666666666667 +
9.3333333333333 +
9.3333333333333 +
9.3333333333333 +
9.3333333333333 +
9.3333333333333 +
9.3333333333333 +
23.0 +
23.0 +
23.0 +
23.0 +
9.8 +
9.8 +
9.8 +
9.8 +
9.8 +
9.8 +
9.8 +
4.0 +
4.0 +
4.0 +
4.0 +
3.6666666666667 +
3.6666666666667 +
3.6666666666667 +
3.6666666666667 +
3.6666666666667 +
3.6666666666667 +
9.8 +
7.7222222222222 +
4.0 +
9.3333333333333 +
10.3 +
9.8 +
19.555555555556 +
5.6666666666667 +
5.2222222222222 +
19.555555555556 +
19.555555555556 +
19.555555555556 +
19.555555555556 +
36.8 +
7.7222222222222 +
7.7222222222222 +
7.7222222222222 +
10.3 +
5.2222222222222 +
5.2222222222222 +
5.2222222222222);

Expected result is 1246, with serialize_precision -1 or 17 is 1246.0000000000052
there is the example in the RFC which sees float 10 but the real number is not 10.

https://wiki.php.net/rfc/precise_float_value for OLD behavior

@sinnbeck
Copy link
Author

sinnbeck commented Sep 1, 2023

@hormus Nice. But as it changes based on whether ATTR_EMULATE_PREPARES is set to true/false I would assume the precision handling is changed in the new PDO change?

https://www.php.net/ChangeLog-8.php

PDO:
Fix GH-11587 (After php8.1, when PDO::ATTR_EMULATE_PREPARES is true and PDO::ATTR_STRINGIFY_FETCHES is true, decimal zeros are no longer filled).

Anyways. I assume it's expected and that I can close the issue ?

@SakiTakamachi
Copy link
Member

Regardless of the values ​​of PDO::ATTR_EMULATE_PREPARES and PDO::ATTR_STRINGIFY_FETCHES, in my environment, the following results are obtained for all four combinations.

float(1246.0000000000052)

So this doesn't seem like a mysql or pdo issue.

@sinnbeck
Copy link
Author

sinnbeck commented Sep 1, 2023

@SakiTakamachi Thanks for trying it out. Its really stange. I am testing on the php:8.2.9-fpm from docker, and I get it every time.. Here I am connecting to the same database twice and second time I set ATTR_EMULATE_PREPARES to false

It is just doing the query and var_dump() the result (same as your original example)

ATTR_EMULATE_PREPARES = true
array(2) {
  ["w"]=>
  string(18) "1246.0000000000052"
  [0]=>
  string(18) "1246.0000000000052"
}
ATTR_EMULATE_PREPARES = false
array(2) {
  ["w"]=>
  string(4) "1246"
  [0]=>
  string(4) "1246"
}

Maybe it is just some weird interoperability between my maridadb database and PDO.

@sinnbeck
Copy link
Author

sinnbeck commented Sep 1, 2023

Ok I just tested on 8.1.22. Apparently my old tests on 8.1 much have been on an earlier version as the that specific version gives the exact same output as 8.2.9. So I think I will close this issue for now, and if others run into similar things, it can perhaps be reopened.

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

No branches or pull requests

6 participants