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

Large number is truncated by NumberToLocalizedStringTransformer #26795

Closed
BenoitDuffez opened this issue Apr 4, 2018 · 6 comments
Closed

Large number is truncated by NumberToLocalizedStringTransformer #26795

BenoitDuffez opened this issue Apr 4, 2018 · 6 comments

Comments

@BenoitDuffez
Copy link

BenoitDuffez commented Apr 4, 2018

Q A
Bug report? yes
Feature request? no
BC Break report? no
RFC? no
Symfony version 3.3.9

Context

I have a form based off of an entity that has a big int field:

/**
 * @var integer
 *
 * @ORM\Column(name="ticket_id", type="bigint")
 */
private $ticketId;

MySQL is the database engine and when entering manual queries it works perfectly.

The form is built with this type:

/**
 * {@inheritdoc}
 */
public function buildForm(FormBuilderInterface $builder, array $options) {
    $builder
        ->add('ticketId', IntegerType::class, ['scale' => 16]);
}

Note that I also tried the one argument add method with exactly the same result.

The issue

Now when I submit the form with an input of 201803221011791 it is saved as 201803221011790.

Analysis

I have debugged the code to this portion of NumberToLocalizedStringTransformer:184 where the data is cast as a float:

if (is_int($result) && $result === (int) $float = (float) $result) {
    $result = $float;
}

This looks like a float but it seems that PHP handles that as a double internally:

$ php -r '$result = 201803221011791; var_dump((float) $result);'
Command line code:1:
double(2.0180322101179E+14)

The precision is not lost yet:

$ php -r 'printf("%d\n", (float) 201803221011791);'
201803221011791

So later on in the round method line 244, it is parsed as a string (scale is 0 so the roundingCoef is 1):

// shift number to maintain the correct scale during rounding
$roundingCoef = pow(10, $this->scale);
// string representation to avoid rounding errors, similar to bcmul()
$number = (string) ($number * $roundingCoef);

So from now on number is 2:

$ php -r 'printf("%d\n", (string)((float) 201803221011791 * 1));'
2

Later on line 259 (roundingMode is ROUND_DOWN):

$number = $number > 0 ? floor($number) : ceil($number);

So basically we are doing this:

$ php -r 'printf("%d\n", floor((string)(float) 201803221011791));'
201803221011790

Which as you can see causes the last digit to be wrong.

Additional remark

The IntegerType uses IntegerToLocalizedStringTransformer, but the constructor erases the scale option, and calls the parent constructor with scale=0 hard-coded:

/**
 * Constructs a transformer.
 *
 * @param int  $scale        Unused
 * @param bool $grouping     Whether thousands should be grouped
 * @param int  $roundingMode One of the ROUND_ constants in this class
 */
public function __construct($scale = 0, $grouping = false, $roundingMode = self::ROUND_DOWN)
{
    if (null === $roundingMode) {
        $roundingMode = self::ROUND_DOWN;
    }

    parent::__construct(0, $grouping, $roundingMode);
}

The PHPDoc mentions the scale parameter is ignored, but this is not documented here: https://symfony.com/doc/3.4/reference/forms/types/number.html#scale

@stof
Copy link
Member

stof commented Apr 4, 2018

for the doc issue, please report it to https://github.com/symfony/symfony-docs (or even better, open a PR to fix it)

@BenoitDuffez
Copy link
Author

Done.

@ostrolucky
Copy link
Contributor

Second issue was already reported in #26734, incl. doc PR

@BenoitDuffez
Copy link
Author

Thanks @ostrolucky, so the doc stuff is now irrelevant in this issue. The integer "rounding" still is though.

@connorjclark
Copy link

connorjclark commented Jul 6, 2018

Looks like this was introduced in 2.8.5

@xabbuh
Copy link
Member

xabbuh commented Feb 2, 2019

Thank you for reporting. This should be fixed by #30063.

nicolas-grekas added a commit that referenced this issue Feb 8, 2019
… (xabbuh)

This PR was merged into the 3.4 branch.

Discussion
----------

[Form] don't lose int precision with not needed type casts

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #26795
| License       | MIT
| Doc PR        |

Commits
-------

72136f1 don't lose int precision with not needed type casts
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

8 participants