PHP 8.1, SQLite: Decimal fields are hydrated as float instead of string #6812

Closed
opened 2026-01-22 15:39:16 +01:00 by admin · 19 comments
Owner

Originally created by @derrabus on GitHub (Aug 28, 2021).

Bug Report

Q A
BC Break no
Version 2.9.x

Summary

Currently, Doctrine hydrates fields with the decimal type to a string. After upgrading to PHP 8.1, I receive a float instead. This change of behavior might break applications, for instance if a string return type declaration is used for a getter method of that field.

How to reproduce

Execute the test case Doctrine\Tests\ORM\Functional\TypeTest::testDecimal on PHP 8.1.

Originally created by @derrabus on GitHub (Aug 28, 2021). ### Bug Report | Q | A |------------ | ------ | BC Break | no | Version | 2.9.x #### Summary Currently, Doctrine hydrates fields with the `decimal` type to a string. After upgrading to PHP 8.1, I receive a float instead. This change of behavior might break applications, for instance if a `string` return type declaration is used for a getter method of that field. #### How to reproduce Execute the test case `Doctrine\Tests\ORM\Functional\TypeTest::testDecimal` on PHP 8.1.
admin closed this issue 2026-01-22 15:39:16 +01:00
Author
Owner

@derrabus commented on GitHub (Aug 28, 2021):

btw: In order to run the tests with PHP 8.1, I had to switch some dependencies to dev versions:

{
    "doctrine/dbal": "~2.13.3@dev",
    "symfony/console": "~5.3.7@dev"
}
@derrabus commented on GitHub (Aug 28, 2021): btw: In order to run the tests with PHP 8.1, I had to switch some dependencies to dev versions: ```JSON { "doctrine/dbal": "~2.13.3@dev", "symfony/console": "~5.3.7@dev" } ```
Author
Owner

@derrabus commented on GitHub (Aug 28, 2021):

I've debugged the failing test and it appears that PDO has changed its behavior here.

Bildschirmfoto 2021-08-28 um 21 15 14 Bildschirmfoto 2021-08-28 um 21 14 46
@derrabus commented on GitHub (Aug 28, 2021): I've debugged the failing test and it appears that PDO has changed its behavior here. <img width="1072" alt="Bildschirmfoto 2021-08-28 um 21 15 14" src="https://user-images.githubusercontent.com/1506493/131228516-2dd15066-c2f0-4146-8284-9cab3dfd24e9.png"> <img width="1072" alt="Bildschirmfoto 2021-08-28 um 21 14 46" src="https://user-images.githubusercontent.com/1506493/131228519-d0c0ce68-8021-4a0f-932f-a958339a603c.png">
Author
Owner

@derrabus commented on GitHub (Aug 28, 2021):

Possibly realyted: php/php-src@438b025a28

@derrabus commented on GitHub (Aug 28, 2021): Possibly realyted: php/php-src@438b025a28cda2935613af412fc13702883dd3a2
Author
Owner

@greg0ire commented on GitHub (Aug 28, 2021):

🤔 should we transfer this issue to the DBAL? That's where $this->_stmt lives, right?

@greg0ire commented on GitHub (Aug 28, 2021): :thinking: should we transfer this issue to the DBAL? That's where `$this->_stmt` lives, right?
Author
Owner

@derrabus commented on GitHub (Aug 28, 2021):

Maybe. But honestly, I'm not sure PDO should behave that way. Casting a DECIMAL to a float might cause a loss of precision and rounding errors. Shall we report this to PHP instead? Maybe @morozov can advise here?

@derrabus commented on GitHub (Aug 28, 2021): Maybe. But honestly, I'm not sure PDO should behave that way. Casting a `DECIMAL` to a float might cause a loss of precision and rounding errors. Shall we report this to PHP instead? Maybe @morozov can advise here?
Author
Owner

@morozov commented on GitHub (Aug 28, 2021):

Casting a DECIMAL to a float might cause a loss of precision and rounding errors. Shall we report this to PHP instead?

Reporting upstream sounds right. I only noticed the change in behavior regarding integers when working on https://github.com/doctrine/dbal/pull/4682. But even then, using a large enough unsigned integer (if SQLite supports them) could lead to a loss of precision.

Since SQLite is an embedded engine, in theory, a precision loss may already happen during the write to the "database", so using proper types while fetching the data may not cause any new issues. A test scenario where precision is actually lost might help facilitate the discussion.

@morozov commented on GitHub (Aug 28, 2021): > Casting a `DECIMAL` to a float might cause a loss of precision and rounding errors. Shall we report this to PHP instead? Reporting upstream sounds right. I only noticed the change in behavior regarding integers when working on https://github.com/doctrine/dbal/pull/4682. But even then, using a large enough unsigned integer (if SQLite supports them) could lead to a loss of precision. Since SQLite is an embedded engine, in theory, a precision loss may already happen during the write to the "database", so using proper types while fetching the data may not cause any new issues. A test scenario where precision is actually lost might help facilitate the discussion.
Author
Owner

@derrabus commented on GitHub (Aug 29, 2021):

Reported as https://bugs.php.net/81397

@derrabus commented on GitHub (Aug 29, 2021): Reported as https://bugs.php.net/81397
Author
Owner

@Pictor13 commented on GitHub (Aug 30, 2021):

From the bug report:

"PDO could not tell apart DECIMAL from REAL in result sets because SQLite does not provide that information"

"But even in older PHP versions, the returned string was not necessarily exact, because SQLite3 internally stores the value as IEEE 754 binary64 (aka. double)."

"So there is no way of having precise decimals anyway.
You'd need to do the scaling yourself, and declare the column as INT (or anything that forces INTEGER type affinity)."
See also https://www.sqlite.org/datatype3.html.

@Pictor13 commented on GitHub (Aug 30, 2021): _From the bug report:_ > "PDO could not tell apart DECIMAL from REAL in result sets because SQLite does not provide that information" > "But even in older PHP versions, the returned string was not necessarily exact, because SQLite3 internally stores the value as IEEE 754 binary64 (aka. double)." > "So there is no way of having precise decimals anyway. > You'd need to do the scaling yourself, and declare the column as INT (or anything that forces INTEGER type affinity)." > See also <https://www.sqlite.org/datatype3.html>.
Author
Owner

@derrabus commented on GitHub (Aug 30, 2021):

Yeah. Sounds like we need to deal with it. What would be the proper place to address this?

  • If this is the first database engine that emits float values for DECIMAL values, we could add a is_float() check to Doctrine\DBAL\Types\DecimalType::convertToPHPValue() and run number_format() on the value.
  • If we decide that this is not a DBAL concern, ORM would need to cast the value when hydrating DECIMAL fields.
@derrabus commented on GitHub (Aug 30, 2021): Yeah. Sounds like we need to deal with it. What would be the proper place to address this? * If this is the first database engine that emits `float` values for `DECIMAL` values, we could add a `is_float()` check to `Doctrine\DBAL\Types\DecimalType::convertToPHPValue()` and run `number_format()` on the value. * If we decide that this is not a DBAL concern, ORM would need to cast the value when hydrating `DECIMAL` fields.
Author
Owner

@morozov commented on GitHub (Aug 30, 2021):

Before addressing the issue, let's reproduce it first. If the fact that SQLite stores decimal numbers internally as double causes precision issues, converting them from float to string via number_format() won't solve the problem.

FWIW, there's no consistency in handling numbers between database drivers for PHP, so unless there's a real problem, I'd just let it go.

@morozov commented on GitHub (Aug 30, 2021): Before addressing the issue, let's reproduce it first. If the fact that SQLite stores decimal numbers internally as double causes precision issues, converting them from float to string via `number_format()` won't solve the problem. FWIW, there's no consistency in handling numbers between database drivers for PHP, so unless there's a real problem, I'd just let it go.
Author
Owner

@derrabus commented on GitHub (Aug 30, 2021):

If the fact that SQLite stores decimal numbers internally as double causes precision issues, converting them from float to string via number_format() won't solve the problem.

No, but it solves the problem that users of the ORM expect decimals to be hydrated as strings as it has always been.

@derrabus commented on GitHub (Aug 30, 2021): > If the fact that SQLite stores decimal numbers internally as double causes precision issues, converting them from float to string via `number_format()` won't solve the problem. No, but it solves the problem that users of the ORM expect decimals to be hydrated as strings as it has always been.
Author
Owner

@morozov commented on GitHub (Aug 30, 2021):

I'd rather unset this expectation. The ORM cannot be in charge of type conversions like this. For instance, mysqli will return integers as int or string depending on the value (whether it fits into PHP_INT_MAX or not). It was our deliberate decision in the DBAL to not perform any conversion on top of that because behavior is caused by a limitation of PHP as a language.

The ORM users will not start receiving floats instead of decimals all of a sudden, it will only happen after an upgrade to PHP 8.1 which is a deliberate step they will be making, and handling the change should be up to them.

Note, the same will apply to the INT columns. Previously, SQLite users would expect to receive them as string but now it's an int. Will the ORM convert them back to string as well?

@morozov commented on GitHub (Aug 30, 2021): I'd rather unset this expectation. The ORM cannot be in charge of type conversions like this. For instance, mysqli will return integers as `int` or `string` depending on the value (whether it fits into `PHP_INT_MAX` or not). It was our deliberate decision in the DBAL to not perform any conversion on top of that because behavior is caused by a limitation of PHP as a language. The ORM users will not start receiving floats instead of decimals all of a sudden, it will only happen after an upgrade to PHP 8.1 which is a deliberate step they will be making, and handling the change should be up to them. Note, the same will apply to the `INT` columns. Previously, SQLite users would expect to receive them as `string` but now it's an `int`. Will the ORM convert them back to string as well?
Author
Owner

@derrabus commented on GitHub (Aug 30, 2021):

Note, the same will apply to the INT columns. Previously, SQLite users would expect to receive them as string but now it's an int. Will the ORM convert them back to string as well?

No, DBAL converted those strings to integers for us:

96990667d5/src/Types/IntegerType.php (L32-L35)

@derrabus commented on GitHub (Aug 30, 2021): > Note, the same will apply to the `INT` columns. Previously, SQLite users would expect to receive them as `string` but now it's an `int`. Will the ORM convert them back to string as well? No, DBAL converted those strings to integers for us: https://github.com/doctrine/dbal/blob/96990667d5373ad651e05adb03116f1009cf1ef4/src/Types/IntegerType.php#L32-L35
Author
Owner

@morozov commented on GitHub (Aug 30, 2021):

That's a really questionable "feature". See how it will behave handling an INT UNSIGNED (0…4294967295) on 32-bit PHP:

C:\php>php -v
PHP 8.0.10 (cli) (built: Aug 25 2021 08:42:00) ( NTS Visual C++ 2019 x86 )
Copyright (c) The PHP Group
Zend Engine v4.0.10, Copyright (c) Zend Technologies

C:\php>php -r "echo PHP_INT_MAX;"
2147483647

C:\php>php -r "echo (int) 2147483648;"
-2147483648

I'd consider this behavior a design flaw on the DBAL side. It also (indirectly) leads to more practical issues than this: https://github.com/doctrine/dbal/issues/4488.

@morozov commented on GitHub (Aug 30, 2021): That's a really questionable "feature". See how it will behave handling an `INT UNSIGNED` (0…4294967295) on 32-bit PHP: ``` C:\php>php -v PHP 8.0.10 (cli) (built: Aug 25 2021 08:42:00) ( NTS Visual C++ 2019 x86 ) Copyright (c) The PHP Group Zend Engine v4.0.10, Copyright (c) Zend Technologies C:\php>php -r "echo PHP_INT_MAX;" 2147483647 C:\php>php -r "echo (int) 2147483648;" -2147483648 ``` I'd consider this behavior a design flaw on the DBAL side. It also (indirectly) leads to more practical issues than this: https://github.com/doctrine/dbal/issues/4488.
Author
Owner

@derrabus commented on GitHub (Aug 31, 2021):

I understand your concern and I know the edge cases such a cast has. When using the ORM however, I would not optimize for the edge case but for usability and type safety. My average ORM app runs in a 64bit environment and does not use BIGINT UNSIGNED. In that case, an integer cast is what I want. I explicitly use an ORM because I don't want to cast database results myself.

@derrabus commented on GitHub (Aug 31, 2021): I understand your concern and I know the edge cases such a cast has. When using the ORM however, I would not optimize for the edge case but for usability and type safety. My average ORM app runs in a 64bit environment and does not use `BIGINT UNSIGNED`. In that case, an integer cast is what I want. I explicitly use an ORM because I don't want to cast database results myself.
Author
Owner

@beberlei commented on GitHub (Aug 31, 2021):

Yes type safety over precision is important here since the types values will most likely go into typed properties. However adding check for type is_float to the dbal type is also problematic, these type conversions easily run thousands of times even with just a handful of entities. Decimal isnt a wide spread type, so perf could be less relevant

@beberlei commented on GitHub (Aug 31, 2021): Yes type safety over precision is important here since the types values will most likely go into typed properties. However adding check for type is_float to the dbal type is also problematic, these type conversions easily run thousands of times even with just a handful of entities. Decimal isnt a wide spread type, so perf could be less relevant
Author
Owner

@morozov commented on GitHub (Aug 31, 2021):

I'd agree for the DBAL/ORM to provide type safety if they rejected knowingly unsupported values.

The APIs of the database extensions are designed with the limitations of PHP as a language in mind. The DBAL/ORM, try to improve this design by just ignoring the limitations at the cost of data integrity. For a database layer, corruption of the data it deals with is unacceptable even in the edge cases.

@morozov commented on GitHub (Aug 31, 2021): I'd agree for the DBAL/ORM to provide type safety if they rejected knowingly unsupported values. The APIs of the database extensions are designed with the limitations of PHP as a language in mind. The DBAL/ORM, try to improve this design by just ignoring the limitations at the cost of data integrity. For a database layer, corruption of the data it deals with is unacceptable even in the edge cases.
Author
Owner

@derrabus commented on GitHub (Sep 27, 2021):

I've made an attempt to fix the issue on DBAL's side: doctrine/dbal#4818

Performing a string cast is the best solution I could come up with. Alternatively, we could set the attribute PDO::ATTR_STRINGIFY_FETCHES to true on the PDO connection. This would restore the pre-8.1 behavior on SQLite and our tests would pass without doctrine/dbal#4818 agin. However, I'm a bit reluctant to fiddle with global settings on a connection that the app might use for other purposes as well.

@derrabus commented on GitHub (Sep 27, 2021): I've made an attempt to fix the issue on DBAL's side: doctrine/dbal#4818 Performing a string cast is the best solution I could come up with. Alternatively, we could set the attribute `PDO::ATTR_STRINGIFY_FETCHES` to `true` on the PDO connection. This would restore the pre-8.1 behavior on SQLite and our tests would pass without doctrine/dbal#4818 agin. However, I'm a bit reluctant to fiddle with global settings on a connection that the app might use for other purposes as well.
Author
Owner

@derrabus commented on GitHub (Sep 28, 2021):

Fixed via doctrine/dbal#4818

@derrabus commented on GitHub (Sep 28, 2021): Fixed via doctrine/dbal#4818
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: doctrine/archived-orm#6812