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

Remove PostgreSQLPlatform::isNumericType() #5394

Merged
merged 1 commit into from
May 15, 2022

Conversation

morozov
Copy link
Member

@morozov morozov commented May 14, 2022

Remove the logic that tests the column type against specific types. The default value should be altered only if it has changed. If the new type is incompatible with the old default value, the new default value should be specified explicitly.

Although this is a bug fix, it may affect some use cases that rely on the existing behavior, so let's include it in the minor, not a patch release. A safer hotfix is implemented in #5395.

@@ -891,7 +891,6 @@ public function testAltersTableColumnCommentIfRequiredByType(): void
self::assertSame(
[
'ALTER TABLE "foo" ALTER "bar" TYPE TIMESTAMP(0) WITHOUT TIME ZONE',
'ALTER TABLE "foo" ALTER "bar" DROP DEFAULT',
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's no reason to expect this statement. It's probably expected because at the time of writing the test the schema comparison already worked like this.

@morozov morozov changed the base branch from 3.3.x to 3.4.x May 14, 2022 22:19

private function isNumericType(Type $type): bool
{
return $type instanceof IntegerType || $type instanceof BigIntType;
Copy link
Member Author

@morozov morozov May 14, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is likely the source of other bugs. E.g. if there is a custom type that uses an integer type instance to define its schema but doesn't extend it, it won't be detected as such.

@morozov morozov marked this pull request as ready for review May 14, 2022 22:21
@morozov morozov added this to the 3.4.0 milestone May 14, 2022
@morozov morozov requested review from derrabus and greg0ire May 14, 2022 22:33
@morozov morozov merged commit e9acb0a into doctrine:3.4.x May 15, 2022
@morozov morozov deleted the issues/5390 branch May 15, 2022 14:09
@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 16, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants