From 21de5082df985beee19f7ef1052d87d622e58ef5 Mon Sep 17 00:00:00 2001 From: cizordj <32869222+cizordj@users.noreply.github.com> Date: Sun, 3 Sep 2023 10:37:03 -0300 Subject: [PATCH 1/2] Change BigIntType Casting - Cast to int within safe range, else to string. - Update the documentation regarding the BigIntType - Add unit tests using assertSame - Add integration test with both signed and unsigned integers - Add the breaking change to the UPGRADE.md file --- UPGRADE.md | 11 ++ docs/en/reference/types.rst | 20 ++-- src/Types/BigIntType.php | 24 +++- tests/Functional/BigIntConversionTest.php | 140 ++++++++++++++++++++++ tests/Types/BigIntTypeTest.php | 91 ++++++++++++++ 5 files changed, 271 insertions(+), 15 deletions(-) create mode 100644 tests/Functional/BigIntConversionTest.php create mode 100644 tests/Types/BigIntTypeTest.php diff --git a/UPGRADE.md b/UPGRADE.md index 2ea323e3891..f4edeaaa847 100644 --- a/UPGRADE.md +++ b/UPGRADE.md @@ -2389,3 +2389,14 @@ The constructor of `Doctrine\DBAL\Exception\DriverException` is now `@internal`. - all `Configuration` methods are now typed - `Configuration::setSchemaAssetsFilter()` now returns `void` - `Configuration::$_attributes` has been removed; use individual properties in subclasses instead + +## BC Break: BIGINT types are now converted to integers + +Before: + +BIGINT values retrieved from the database used to be converted to PHP string. + +After: + +BIGINT values are cast to a PHP integer if the value is within the PHP integer range. +Otherwise, it converts to a string. diff --git a/docs/en/reference/types.rst b/docs/en/reference/types.rst index 4fe6dc3c833..b1135448391 100644 --- a/docs/en/reference/types.rst +++ b/docs/en/reference/types.rst @@ -83,22 +83,22 @@ bigint ++++++ Maps and converts 8-byte integer values. -Unsigned integer values have a range of **0** to **18446744073709551615** while signed +Unsigned integer values have a range of **0** to **18446744073709551615**, while signed integer values have a range of **−9223372036854775808** to **9223372036854775807**. If you know the integer data you want to store always fits into one of these ranges you should consider using this type. -Values retrieved from the database are always converted to PHP's ``string`` type -or ``null`` if no data is present. +Values retrieved from the database are always converted to PHP's ``integer`` type +or ``string`` if the data exceeds PHP's integer safe limits. +Otherwise, returns ``null`` if no data is present. .. note:: - For compatibility reasons this type is not converted to an integer - as PHP can only represent big integer values as real integers on - systems with a 64-bit architecture and would fall back to approximated - float values otherwise which could lead to false assumptions in applications. - - Not all of the database vendors support unsigned integers, so such an assumption - might not be propagated to the database. + Due to architectural differences, 32-bit PHP systems have a smaller + integer range than their 64-bit counterparts. On 32-bit systems, + values exceeding this range will be represented as strings instead + of integers. Bear in mind that not all database vendors + support unsigned integers, so schema configuration cannot be + enforced. Decimal types ^^^^^^^^^^^^^ diff --git a/src/Types/BigIntType.php b/src/Types/BigIntType.php index 1d068a42c2d..613f3da0a7c 100644 --- a/src/Types/BigIntType.php +++ b/src/Types/BigIntType.php @@ -7,8 +7,11 @@ use Doctrine\DBAL\ParameterType; use Doctrine\DBAL\Platforms\AbstractPlatform; +use const PHP_INT_MAX; +use const PHP_INT_MIN; + /** - * Type that maps a database BIGINT to a PHP string. + * Type that maps a database BIGINT to a PHP int. */ class BigIntType extends Type implements PhpIntegerMappingType { @@ -22,18 +25,29 @@ public function getSQLDeclaration(array $column, AbstractPlatform $platform): st public function getBindingType(): ParameterType { - return ParameterType::STRING; + return ParameterType::INTEGER; } /** * @param T $value * - * @return (T is null ? null : string) + * @return (T is null ? null : int|string) * * @template T */ - public function convertToPHPValue(mixed $value, AbstractPlatform $platform): ?string + public function convertToPHPValue(mixed $value, AbstractPlatform $platform): int|string|null { - return $value === null ? null : (string) $value; + if ($value === null) { + return null; + } + + if ( + $value >= PHP_INT_MIN && + $value <= PHP_INT_MAX + ) { + return (int) $value; + } + + return (string) $value; } } diff --git a/tests/Functional/BigIntConversionTest.php b/tests/Functional/BigIntConversionTest.php new file mode 100644 index 00000000000..e77372f4e00 --- /dev/null +++ b/tests/Functional/BigIntConversionTest.php @@ -0,0 +1,140 @@ +typeInstance = new BigIntType(); + + $table = new Table('bigint_conversion_test'); + $table->addColumn('id', Types::SMALLINT, ['notnull' => true]); + $table->addColumn('signed_integer', Types::BIGINT, ['notnull' => false]); + $table->addColumn('unsigned_integer', Types::BIGINT, [ + 'notnull' => false, + 'unsigned' => true, + ]); + $table->setPrimaryKey(['id']); + $this->dropAndCreateTable($table); + } + + public function testShouldConvertToZeroInteger(): void + { + $this->connection->insert('bigint_conversion_test', [ + 'id' => 0, + 'signed_integer' => 0, + ]); + $this->assertPHPValue( + 0, + 'SELECT signed_integer from bigint_conversion_test WHERE id = 0', + ); + } + + public function testShouldConvertToPhpMinimumInteger(): void + { + $this->connection->insert('bigint_conversion_test', [ + 'id' => 1, + 'signed_integer' => PHP_INT_MIN, + ]); + $this->assertPHPValue( + PHP_INT_MIN, + 'SELECT signed_integer from bigint_conversion_test WHERE id = 1', + ); + } + + public function testShouldConvertToPhpMaximumInteger(): void + { + $this->connection->insert('bigint_conversion_test', [ + 'id' => 2, + 'signed_integer' => PHP_INT_MAX, + ]); + $this->assertPHPValue( + PHP_INT_MAX, + 'SELECT signed_integer from bigint_conversion_test WHERE id = 2', + ); + } + + public function testShouldConvertToPositiveIntegerNumber(): void + { + $this->connection->insert('bigint_conversion_test', [ + 'id' => 3, + 'signed_integer' => PHP_INT_MAX - 1, + ]); + $this->assertPHPValue( + PHP_INT_MAX - 1, + 'SELECT signed_integer from bigint_conversion_test WHERE id = 3', + ); + } + + public function testShouldConvertToNegativeIntegerNumber(): void + { + $this->connection->insert('bigint_conversion_test', [ + 'id' => 4, + 'signed_integer' => PHP_INT_MIN + 1, + ]); + $this->assertPHPValue( + PHP_INT_MIN + 1, + 'SELECT signed_integer from bigint_conversion_test WHERE id = 4', + ); + } + + public function testShouldConvertSlightlyOutOfPhpIntegerRangeUnsignedValueToString(): void + { + $this->connection->insert('bigint_conversion_test', [ + 'id' => 5, + 'unsigned_integer' => '9223372036854775808', + ]); + $this->assertPHPValue( + '9223372036854775808', + 'SELECT unsigned_integer from bigint_conversion_test WHERE id = 5', + ); + } + + public function testShouldConvertMaximumUnsignedIntegerValueToString(): void + { + $this->connection->insert('bigint_conversion_test', [ + 'id' => 6, + 'unsigned_integer' => '18446744073709551615', + ]); + $this->assertPHPValue( + '18446744073709551615', + 'SELECT unsigned_integer from bigint_conversion_test WHERE id = 6', + ); + } + + public function testShouldConvertNearlyMaximumUnsignedIntegerValueToString(): void + { + $this->connection->insert('bigint_conversion_test', [ + 'id' => 7, + 'unsigned_integer' => '18446744073709551610', + ]); + $this->assertPHPValue( + '18446744073709551610', + 'SELECT unsigned_integer from bigint_conversion_test WHERE id = 7', + ); + } + + private function assertPHPValue(mixed $expected, string $sql): void + { + self::assertSame( + $expected, + $this->typeInstance->convertToPHPValue( + $this->connection->fetchOne($sql), + $this->connection->getDatabasePlatform(), + ), + ); + } +} diff --git a/tests/Types/BigIntTypeTest.php b/tests/Types/BigIntTypeTest.php new file mode 100644 index 00000000000..ba988b1f955 --- /dev/null +++ b/tests/Types/BigIntTypeTest.php @@ -0,0 +1,91 @@ +platform = $this->createMock(AbstractPlatform::class); + $this->type = new BigIntType(); + } + + public function testShouldConvertPhpIntMinToInteger(): void + { + self::assertSame( + PHP_INT_MIN, + $this->type->convertToPHPValue(PHP_INT_MIN, $this->platform), + ); + } + + public function testShouldConvertPhpIntMaxToInteger(): void + { + self::assertSame( + PHP_INT_MAX, + $this->type->convertToPHPValue(PHP_INT_MAX, $this->platform), + ); + } + + public function testShouldConvertPhpIntMinAsStringToInteger(): void + { + self::assertSame( + PHP_INT_MIN, + $this->type->convertToPHPValue((string) PHP_INT_MIN, $this->platform), + ); + } + + public function testShouldConvertPhpIntMaxAsStringToInteger(): void + { + self::assertSame( + PHP_INT_MAX, + $this->type->convertToPHPValue((string) PHP_INT_MAX, $this->platform), + ); + } + + public function testShouldConvertZeroIntegerToInteger(): void + { + self::assertSame( + 0, + $this->type->convertToPHPValue(0, $this->platform), + ); + } + + public function testShouldConvertZeroStringToInteger(): void + { + self::assertSame( + 0, + $this->type->convertToPHPValue('0', $this->platform), + ); + } + + public function testShouldConvertSafeNegativeValueToInteger(): void + { + self::assertSame( + PHP_INT_MIN + 1, + $this->type->convertToPHPValue(PHP_INT_MIN + 1, $this->platform), + ); + } + + public function testShouldConvertSafePositiveValueToInteger(): void + { + self::assertSame( + PHP_INT_MAX - 1, + $this->type->convertToPHPValue(PHP_INT_MAX - 1, $this->platform), + ); + } +} From abcd9220029ca22bb5b05f9432b14e8011c20ff1 Mon Sep 17 00:00:00 2001 From: cizordj <32869222+cizordj@users.noreply.github.com> Date: Thu, 14 Sep 2023 17:18:46 -0300 Subject: [PATCH 2/2] Fix broken tests on MySql --- src/Types/BigIntType.php | 6 +++--- tests/Types/BigIntTypeTest.php | 26 ++++++++++++++++++++------ 2 files changed, 23 insertions(+), 9 deletions(-) diff --git a/src/Types/BigIntType.php b/src/Types/BigIntType.php index 613f3da0a7c..19c703a8945 100644 --- a/src/Types/BigIntType.php +++ b/src/Types/BigIntType.php @@ -25,7 +25,7 @@ public function getSQLDeclaration(array $column, AbstractPlatform $platform): st public function getBindingType(): ParameterType { - return ParameterType::INTEGER; + return ParameterType::STRING; } /** @@ -42,8 +42,8 @@ public function convertToPHPValue(mixed $value, AbstractPlatform $platform): int } if ( - $value >= PHP_INT_MIN && - $value <= PHP_INT_MAX + ($value >= PHP_INT_MIN && $value < PHP_INT_MAX) || + (string) $value === (string) PHP_INT_MAX ) { return (int) $value; } diff --git a/tests/Types/BigIntTypeTest.php b/tests/Types/BigIntTypeTest.php index ba988b1f955..21032826860 100644 --- a/tests/Types/BigIntTypeTest.php +++ b/tests/Types/BigIntTypeTest.php @@ -45,7 +45,10 @@ public function testShouldConvertPhpIntMinAsStringToInteger(): void { self::assertSame( PHP_INT_MIN, - $this->type->convertToPHPValue((string) PHP_INT_MIN, $this->platform), + $this->type->convertToPHPValue( + (string) PHP_INT_MIN, + $this->platform, + ), ); } @@ -53,16 +56,16 @@ public function testShouldConvertPhpIntMaxAsStringToInteger(): void { self::assertSame( PHP_INT_MAX, - $this->type->convertToPHPValue((string) PHP_INT_MAX, $this->platform), + $this->type->convertToPHPValue( + (string) PHP_INT_MAX, + $this->platform, + ), ); } public function testShouldConvertZeroIntegerToInteger(): void { - self::assertSame( - 0, - $this->type->convertToPHPValue(0, $this->platform), - ); + self::assertSame(0, $this->type->convertToPHPValue(0, $this->platform)); } public function testShouldConvertZeroStringToInteger(): void @@ -88,4 +91,15 @@ public function testShouldConvertSafePositiveValueToInteger(): void $this->type->convertToPHPValue(PHP_INT_MAX - 1, $this->platform), ); } + + public function testShouldConvertMaximumUnsignedIntegerValueToString(): void + { + self::assertSame( + '18446744073709551615', + $this->type->convertToPHPValue( + '18446744073709551615', + $this->platform, + ), + ); + } }