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

Allow all datetime types to be used as version #7554

Conversation

nico-incubiq
Copy link

I am working on an existing database with fields of type timestamp. In order to map that to a DateTime object in Doctrine, I've found the solution of adding the @Version annotation. But I'd really prefer to get a DateTimeImmutable, which is impossible.

At the moment a field of type datetime_immutable with annotation @Version will trigger this following error:

Locking type "datetime_immutable" (specified in "App\Entity\Bakery", field "createdAt") is not supported by Doctrine.

I cannot see any reason not to allow any type of datetime to be mapped to a timestamp in database, so this PR allows all of them. Please tell me if i'm missing something.

Copy link
Member

@SenseException SenseException left a comment

Choose a reason for hiding this comment

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

Thank you for your contribution. Can you please add tests for all the supported types?

I'm not sure about if the ORM version 2.6 should introduce these changes. I'll wait what others have to say about the version.

@lcobucci
Copy link
Member

lcobucci commented Jan 4, 2019

@lcobucci
Copy link
Member

lcobucci commented Jan 4, 2019

As @SenseException mentioned this would be ok for v3 (master)

@nico-incubiq
Copy link
Author

Yeah i'm currently working with "datetime", but i'd rather get a DatetimeImmutable rather than just a DateTime. When is v3 due? Since this isn't a breaking change I expected it to be fine in 2.6.

@lcobucci
Copy link
Member

lcobucci commented Jan 4, 2019

@nico-incubiq this is considered a new feature, which should go in a new minor release - not patch. You can propose this to v2.7 but we were planning to only have deprecation notices there, however there's no due date set for v3.

@nico-incubiq
Copy link
Author

I've opened a new PR targeting v3 and added tests to it, I'll close this one then.

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.

3 participants