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

[8.x] Fix CURRENT_TIMESTAMP as default when changing column #39377

Merged
merged 1 commit into from
Oct 28, 2021

Conversation

J0sh0nat0r
Copy link
Contributor

This PR implements PhpDateTimeMappingType for TimestampType, which allows us to use CURRENT_TIMESTAMP as the default value when changing a column (DBAL)
Without this change CURRENT_TIMSTAMP is treated as a string

@taylorotwell
Copy link
Member

Is there any documentation within Doctrine on why we should do this or how it behaves?

@J0sh0nat0r
Copy link
Contributor Author

It appears that this is the only attached behaviour within Doctine. I couldn't find any documentation regarding it, maybe @TomHAnderson can provide some more insight here?

@@ -4,9 +4,10 @@

use Doctrine\DBAL\DBALException;
use Doctrine\DBAL\Platforms\AbstractPlatform;
use Doctrine\DBAL\Types\PhpDateTimeMappingType;
Copy link
Member

Choose a reason for hiding this comment

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

does this exist in the oldest version of doctrine that we support?

Copy link
Contributor

Choose a reason for hiding this comment

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

@TomHAnderson
Copy link
Contributor

It appears that this is the only attached behaviour within Doctine. I couldn't find any documentation regarding it, maybe @TomHAnderson can provide some more insight here?

The interface and default value desired in this PR is implemented originally here: doctrine/dbal@9e75bb0#diff-485a2415348fbc0e83be9feae953842b696202061ea7bf321568f2c0934a2fbdR2285

@taylorotwell taylorotwell merged commit ac399a4 into laravel:8.x Oct 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants