Doctrine DBAL MysqliStatement broken for Float to Double conversion in de_DE locale (or locales that use ',' for floats)

[mmucklo]

Bug Report

Q	A
BC Break	no
Version	2.9.2

Summary

When using the mysqli driver, the MysqliStatement does not correctly parse the type of a float during bind params, setting it as type 's' (string), when it should be type 'd' (double) - is what I think is going on.

This does not seem to happen with the PDO mysql driver.

An error occurs to the user such as the following:

  An exception occurred while executing 'INSERT INTO grumpy_puppy (broken) VALUES (?)' with params [0.12345678901]:

  Data truncated for column 'broken' at row 1

This was discovered here:
mmucklo/DtcQueueBundle#98

And for now the following work around seems to work: mmucklo/DtcQueueBundle@33358df

I think this is because https://www.php.net/manual/en/mysqli-stmt.bind-param.php specifies that 'd' should be used for type double, but MysqliStatement does not even have a 'd' in the list of possible types here: https://github.com/doctrine/dbal/blob/master/lib/Doctrine/DBAL/Driver/Mysqli/MysqliStatement.php#L30-L37

So consequently neither does the conversion code ever check for php floats and specify that they are of type 'd', however I have not tested this hypothesis yet. It took me about a week part time just to reproduce the problem consistently and package it up so that someone else could run it (see below):

Current behaviour

Floats can't be inserted into Mysql double columns using the Mysqli driver when in a locale that uses a ',' instead of a '.'

This happens because the float is turned into a string and the SQL standard necessitates floats instead to use '.' regardless of locale.

How to reproduce

First, install docker and docker-compose for your platform.

git clone https://github.com/mmucklo/docker-symfony.git
cd docker-symfony
git checkout doctrine-bug
docker-compose build
docker-compose up

Wait for mysql, then PHP to start up.
This will take up to 30-45 seconds (as it sleeps for mysql warmup).
You should then see a Migration that goes by in php:

php_1    |   ++ migrating 20190719053443
php_1    |
php_1    |      -> CREATE TABLE grumpy_puppy (id INT AUTO_INCREMENT NOT NULL, broken DOUBLE PRECISION DEFAULT NULL, PRIMARY KEY(id)) DEFAULT CHARACTER SET utf8mb4 COLLATE utf8mb4_unicode_ci ENGINE = InnoDB
php_1    |

As well as some additional startup messages.
When you see the following message, then you're ready to test from a different terminal window:

php_1    | [23-Jul-2019 05:32:58] NOTICE: ready to handle connections

NOW, log into another terminal session on the same machine, and cd to the docker-symfony directory from above. And run the following:

docker-compose exec php bash
php -d 'auto_prepend_file=/var/www/symfony/setlocale.php' bin/console app:create-grumpy-puppy -vvv

You should see the error reported above, along with a full stack trace.

NOTE: the "auto_prepend_file=" forces a script to run that forces php into the de_DE locale. I couldn't seem to get the locale switch to work from the command line otherwise.

If you want to edit files inside of the container and play around, you can install vim, for example by:

apt install vim

If you want to check mysql (in another terminal cd to the docker-symfony directory from above):

docker-compose exec mysql bash
mysql -u doctrine -p doctrine
# password is doctrine

Other than that everything should be self contained.

If you want to blow away the tester and start again:

From inside of docker-symfony:

docker-compose down
docker volume rm doctrine-bug_doctrine-symfony

If the second command doesn't work (to remove the volume, you may want to do a "docker volume ls" and search for the proper volume).

If you want to just look at the Entity, etc, before you run things, you can start here:
https://github.com/mmucklo/docker-symfony/tree/doctrine-bug/php

Expected behaviour

Command above should complete successfully.

[SenseException]

Thank you for the effort putting this example together and reporting this issue.

I'm not sure if I understand this correct about the number format. Is the same behaviour expected for 4.2 and 4,2 when given to DBAL? As far as I know there isn't an ICU feature implemented to make DBAL handle different international number formats. This is something that has to be handled by the user.

I would appreciate it if you can create a DBAL only example or a PR with a failing test that reproduces the mentioned behavior instead of a whole docker setup.

[mmucklo]

So the float is stored as a float internally, and passing it into DBAL, it's sent as a float, but when you pass it to MySQL it gets converted from a float to a string based on your MysqliStatement driver's implementation.

The problem is when a float is turned into a string in a locale that uses ',' as the decimal separator is that Mysql then sees a float specified with a ',' which is not according to the SQL standard (which uses '.').

Like I said in the issue, it's probably due to the way parameter binding is done in the MysqliStatement.php file. For Mysqli, there's a separate type 'd' that should be used when binding floats (doubles), otherwise when you bind as a string, this problem seems to manifest.

This does not happen using the PDO driver, only the mysqli driver.

See this hash table here:
https://github.com/doctrine/dbal/blob/master/lib/Doctrine/DBAL/Driver/Mysqli/MysqliStatement.php#L30-L37

PDO doesn't have a separate type for double, but Mysqli does (https://www.php.net/manual/en/mysqli-stmt.bind-param.php). I think the fix is to add one more row to that has map for type 'd', and then handle the binding of parameters of type float mapping to 'd' correctly.

[mmucklo]

@SenseException I created a PR which seems to address the problem.