Cakephp: 3.x decimal precision lost before saving data

[jonatascabral]

• [x] bug
• [ ] enhancement

• [ ] feature-discussion (RFC)

• CakePHP Version: v3.3.13

• Platform and Target: NGINX v1.11 with MySQL v5.7.17

When I try to save a data in table the precision of decimal value is lost, I checked the code and the class DecimalType, before save the data, convert the value to a float using sprintf function, so why it is converting a decimal value to a float?? It's a decimal value the precision is larger than an float.

The example code with the problem:

$scoreData = [
    'distributor_id' => 1,
    'points' => '19.701592653589',
    'month_ref' => '2017-02-06',
];
...
$score = $this->Distributors->Scores->newEntity($scoreData);
...
$this->Distributors->Scores->save($score);

The value saved on database is rounded to: 19.701593;

In MySQL table the column is DECIMAL(18,12);

[markstory]

We convert to floats on marshalling, as we found most people were expecting float values so they could do math on their entities.

[berarma]

Using floats works well with few decimal positions, not so well when there's many. You could use a custom database type for long decimals.

[jonatascabral]

Thanks for the answers.
@markstory I think the problem is in the 'return sprintf("%F", $value);' in toDatabase method, if is needed to convert to a float just do a cast to float or double instead of "formating" the value.

@berarma I undestood I can create a custom type, but if decimal type already exists I think it must be a "mirror" of the database type decimal and don't do any round or use the precision limit of the column to make this round.

Sorry if I appear to be rude it's not intentional. 😆

@markstory I can do the update and submit a pull request, I will be very glad helping the CakePHP community. 😄

[markstory]

My concern with changing the DecimalType to return strings is around breaking compatibility in existing applications. We could however add a new type that allows decimal values to be mapped to strings.

[inoas]

Maybe we should have DecimalStringType and opt-in via bootstrap.php to be deprecated in 4.x (becomes DecimalType ~and the current DecimalType becomes FloatType~)?

Working with decimals in mysql is usually the way to handle currencies. Sometimes a lot of after-comma digits are required and then things getting imprecise due to CakePHP is really unexpected

[markstory]

A DecimalStringType and documenting how to use it sounds like a good idea. We already have a FloatType and I don't think we need another.

[berarma]

What about using objects instead of strings like dates currently do? Maybe using some kind of library like https://github.com/rtlopez/decimal.

[markstory]

@berarma That is also an option

[markstory]

Moving to enhancement as adding a DecimalStringType is a new feature.

[daywalker313]

Hi,
I just wanted to point out that PHPs floating point precision is not the issue here.
Instead it's the class DecimalType, where sprintf with %F is being used.
That leads to a fixed 6 (+1 rounded) digit precision.

It's a bit confusing as DecimalType effectively leads to a worse precision than FloatType.

It would be smarter to use sprintf with a precision matching the decimal datatype's precision, however for personal use I just created a custom "PrecisionDecimalType" using:
return sprintf('%.15F', $value);

PS: Make sure to adjust the php precision ini setting accordingly, e.g.:
ini_set('precision', 15);

[markstory]

Adding more precision to the DecimalType is an option. Database servers will ignore/truncate the extra precision when they don't need it.

[inoas]

Similar should happen to datetime/datetime(6)

[ADmad]

I think we should change DecimalType itself in 4.0. The whole premise of decimal types is to avoid the precision issue that occur with floats, so it's simply wrong using float in PHP for decimal types.

[TheLevti]

Casting to string only works up to 13 decimals. After that its cut off and in some cases if the number ends with e.g. ..555, the last digit before the cut off, will be round up, which also can lead to wrong results.

[TheLevti]

For large numbers it is this case. For very small numbers, strval() or (string) will return a scientific notation as a string (e.g. "1.2e-3"), which can break the app and maybe fail on database level if the database can not handle it. Usually a scientific number is not a string.

[ADmad]

@TheLevti I am aware my patch is not perfect, but it at least improves the situation. Anyone requiring more than 13 decimal places would have to make their custom type class.

[dereuromark]

We should then document the edge cases of different float type classes.

[dereuromark]

https://github.com/cakephp/cakephp/pull/13081 could be added as an optional Type class into the Shim plugin, documenting the pros and cons then.

[TheLevti]

I am not sure if a cast to string is necessary at all. Let the database/driver handle it. At least mysql can handle float, string, scientific notation just fine for DECIMAL columns. The only real thing needed for that type is that a database DECIMAL type is left as a string and not cast to float when reading from DB to PHP! If you store a lets say DECIMAL (32,18), you will always lose precision when reading from the DB, because of the (float) cast in the toPHP function.

[TheLevti]

For toDatabase() a check for if (!is_scalar($value)) { is just enough in my opinion.

[TheLevti]

If you check other ORMs, e.g. doctrine, it also does no modifications when reading or writing from/to the database, because DECIMAL type should be treated as a string.

See: https://github.com/doctrine/dbal/blob/master/lib/Doctrine/DBAL/Types/DecimalType.php

[ADmad]

I am not sure if a cast to string is necessary at all.

Yup it isn't but we specify the PDO binding type as PDO::PARAM_STR in DecimalType so wouldn't PDO cast to string anyway?

[TheLevti]

Then let pdo handle it. If not doing something fancy, it would also just use a simple string cast.

[TheLevti]

The dev should be aware that if he is using/working with a decimal column, that he should only work with a string value and not try to save a float. And if he does, we should not try to fix his value before going to the db.

[ADmad]

Seems fair. But when marshalling at least it would be best to cast the incoming value to string.

[TheLevti]

yes you are right. because those values should be handled as strings.

[ADmad]

I am working updating DecimalType for 4.x where I don't have to worry about backwards compatibility. Once that's reviewed and has a consensus we can see what parts of it we can backport for 3.x.

[TheLevti]

Please also do not forget to update cake bake to type hint decimal columns as string.

[markstory]

Please also do not forget to update cake bake to type hint decimal columns as string.

Since you've remembered, maybe you could help out in getting that done?

[TheLevti]

Already done by @ADmad in https://github.com/cakephp/bake/commit/20cb431a7c2e499002d8dac645d1b56cb2368139

[arodu]

I had the same problem in 3.x, and managed to fix it for now by adding this in the Entity

protected function _setValue($value) {
  return number_format($value, 8, '.', '');
}

I know it's not the best, but for now it has worked for me.