Cphalcon: Huge DECIMAL number casted to FLOAT/DOUBLE

[dmarkic]

Expected and Actual Behavior

Using bcmath to calculate few things and than storing it into MySQL DECIMAL(30,20) column will cast the number to double/float which causes the loss of decimal places.

Script to reproduce the issue

<?php
/**
 * Phalcon decimal bug (cast to float)
 */
// calculated value
$value = '0.12345678901234567890';

class ModelWithDecimal extends \Phalcon\Mvc\Model
{
    public $id;
    public $value;
}

$di = new \Phalcon\Di\FactoryDefault();

$di->set('db', function () use ($di) {
    return new \Phalcon\Db\Adapter\Pdo\Mysql(array(
        'host'      => 'localhost',
        'username'  => 'test',
        'password'  => 'test',
        'dbname'    => 'test'
    ));
});


$M = new ModelWithDecimal;
$M->value = $value;
$M->save();

echo " value: " . $value . "\n";
echo " M->value: " . $M->value . "\n";

$M = ModelWithDecimal::findFirst(1);
echo " load: M->value: " . $M->value . "\n";

Output

# ./decimal.php 
 value: 0.12345678901234567890
 M->value: 0.12345678901234567890
 load: M->value: 0.12345678901235000000
#

Expected output

# ./decimal.php 
 value: 0.12345678901234567890
 M->value: 0.12345678901234567890
 load: M->value: 0.12345678901234567890
#

Analysis

As far as I could I've traced this cast to this code:
https://github.com/phalcon/cphalcon/blob/107b6f0197b0464d9f6b11ff390a560c000bf448/phalcon/db/adapter/pdo.zep#L242

Solution

I've extended the \Phalcon\Db\Adapter\Pdo\Mysql class and modified this a bit like this:

use \Phalcon\Db\Column;

class Mysql extends \Phalcon\Db\Adapter\Pdo\Mysql
{

    /**
     * Phalcon tries to convert DECIMAL to doubleval() which
     * looses the decimal precision. We change the bind type to string.
     */
    public function executePrepared(\PDOStatement $statement, array $placeholders, $dataTypes) : \PDOStatement
    {
        foreach ($placeholders as $wildcard => $value) {
            if (is_numeric($wildcard)) {
                $parameter = $wildcard + 1;
            } elseif (is_string($wildcard)) {
                $parameter = $wildcard;
            } else {
                throw new \Exception('Invalid bind parameter (me)');
            }
            if (is_array($dataTypes)) {
                $type = ($dataTypes[$wildcard] ?? null);
                if ($type !== null) {
                    if ($type == Column::BIND_PARAM_DECIMAL) {
                        $dataTypes[$wildcard] = Column::BIND_PARAM_STR;
                    }
                }
            }
        }
        return parent::executePrepared($statement, $placeholders, $dataTypes);
    }
}

This does solve my problem, but I guess it could be solved some other way?

Details

• Phalcon version: (php --ri phalcon)

phalcon


Web framework delivered as a C-extension for PHP
phalcon => enabled
Author => Phalcon Team and contributors
Version => 3.2.4
Build Date => Oct 21 2017 16:34:03
Powered by Zephir => Version 0.10.4-11e39849b0

Directive => Local Value => Master Value
phalcon.db.escape_identifiers => On => On
phalcon.db.force_casting => Off => Off
phalcon.orm.events => On => On
phalcon.orm.virtual_foreign_keys => On => On
phalcon.orm.column_renaming => On => On
phalcon.orm.not_null_validations => On => On
phalcon.orm.exception_on_failed_save => Off => Off
phalcon.orm.enable_literals => On => On
phalcon.orm.late_state_binding => Off => Off
phalcon.orm.enable_implicit_joins => On => On
phalcon.orm.cast_on_hydrate => Off => Off
phalcon.orm.ignore_unknown_columns => Off => Off
phalcon.orm.update_snapshot_on_save => On => On
phalcon.orm.disable_assign_setters => Off => Off

• PHP Version: (php -v)

PHP 7.0.19-1 (cli) (built: May 11 2017 14:04:47) ( NTS )
Copyright (c) 1997-2017 The PHP Group
Zend Engine v3.0.0, Copyright (c) 1998-2017 Zend Technologies
    with Zend OPcache v7.0.19-1, Copyright (c) 1999-2017, by Zend Technologies
    with Xdebug v2.5.0, Copyright (c) 2002-2016, by Derick Rethans

• Operating System: Linux debian stretch
• Installation type: installing via package manager: https://packagecloud.io/phalcon/stable/debian/ stretch main
• Zephir version (if any):
• Server: Other
• Other related info (Database, table schema):

CREATE TABLE `model_with_decimal` (
    `id` INT(10) UNSIGNED NOT NULL AUTO_INCREMENT,
    `value` DECIMAL(30,20) UNSIGNED NOT NULL,
    PRIMARY KEY (`id`)
)
COLLATE='utf8_general_ci'
ENGINE=InnoDB

[stale[bot]]

Thank you for contributing to this issue. As it has been 90 days since the last activity, we are automatically closing the issue. This is often because the request was already solved in some way and it just wasn't updated or it's no longer applicable. If that's not the case, please feel free to either reopen this issue or open a new one. We will be more than happy to look at it again! You can read more here: https://blog.phalconphp.com/post/github-closing-old-issues

[crnjakovic]

@sergeyklay @niden this shouldn't be closed as it is still a valid issue. As PHP does not support decimals natively, DECIMAL columns should be casted to STRING. Casting to double/float introduces a fresh hell of imprecisions. Saving longitude (20.44892159999995) this way introduces a lot of issues.

All in all, MySQL's DECIMAL(M,D) does not equal to PHP's double, but string (unfortunately). MySQL's FLOAT and DOUBLE on the other hand, are good candidates for PHP's double

[niden]

Thanks @scrnjakovic

This needs a bit more discussion actually on how we can address this. Decimal should be a double. However in the example above, keeping it as double will not help.

I am wondering if this can be a user initiated change i.e. where the column is defined as string/varchar.

[crnjakovic]

Thanks @niden for reopening this :)

Decimal should be a double.

I would have to disagree with this. Unlike float and double, decimal has fixed precision.

PHP float double docs:

Floating point numbers have limited precision. Although it depends on the system, PHP typically uses the IEEE 754 double precision format, which will give a maximum relative error due to rounding in the order of 1.11e-16. Non elementary arithmetic operations may give larger errors, and, of course, error propagation must be considered when several operations are compounded.

MySQL FLOAT and DOUBLE docs:

Because floating-point values are approximate and not stored as exact values, attempts to treat them as exact in comparisons may lead to problems. They are also subject to platform or implementation dependencies.

MySQL DECIMAL and NUMERIC docs:

The DECIMAL and NUMERIC types store exact numeric data values. These types are used when it is important to preserve exact precision, for example with monetary data.

From this it's very clear that column with DECIMAL data type must not be treated as double in PHP, otherwise we are treating something that is meant and expected to be precise as something that is by definition not precise.

[niden]

I see your point @scrnjakovic. I have no problem switching this to a bind type of string which will not force double on the value and thus create all sorts of issues as you mentioned above.

I think that we can get away with doing this provided that it is crystal clear in the documentation that if one uses decimal they should expect a string.

[niden]

Work on this here: https://github.com/niden/cphalcon/tree/T13543-add-more-pdo-types

[niden]

Addressed in https://github.com/phalcon/cphalcon/pull/13562