Skip to content

Add millisecond support for Date mapping type #1028

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

Closed
wants to merge 9 commits into from
Closed

Add millisecond support for Date mapping type #1028

wants to merge 9 commits into from

Conversation

gerryvdm
Copy link

Both MongoDB and the PHP DateTime class support milliseconds, but currently this information is lost in the date mapping type.

This patch will transfer the millisecond information between PHP and MongoDB.

@malarzm
Copy link
Member

malarzm commented Feb 12, 2015

@gerryvdm is Travis failure related to your changes? Also I think adding a test for this PR would be great :)

@gerryvdm
Copy link
Author

@malarzm Not sure, the error seems a bit exotic. Will add a test!

Gerry Vandermaesen added 2 commits February 12, 2015 13:20
@gerryvdm
Copy link
Author

This should also fix #901.

@malarzm
Copy link
Member

malarzm commented Feb 12, 2015

It's a bit blind guess but there's field mapped as date and as not saved in https://github.com/doctrine/mongodb-odm/blob/master/tests/Documents/File.php#L29 which is referenced by https://github.com/doctrine/mongodb-odm/blob/master/tests/Documents/Profile.php#L21 which should be the class with hydration problem - maybe that's it?

@gerryvdm
Copy link
Author

I think the problem is related to timezones. Will need to have a further look.

Gerry Vandermaesen added 4 commits February 12, 2015 14:25
While DateTime::setTimestamp() allows negative timestamps, DateTime::createFromFormat('U') does not.
@gerryvdm
Copy link
Author

Well that was harder than I imagined, but I think the problem is solved. Needed some workaround for allowing pre-unix era timestamps...

@malarzm
Copy link
Member

malarzm commented Feb 12, 2015

👍

@gerryvdm gerryvdm changed the title Add microsecond support for Date mapping type Add millisecond support for Date mapping type Feb 12, 2015
@gerryvdm
Copy link
Author

gerryvdm commented Mar 2, 2015

ping @jmikola. Any chance to review?

@malarzm malarzm added this to the 1.0.0-BETA13 milestone Mar 12, 2015
@lavoiesl
Copy link
Member

Fixes #1061

@lavoiesl
Copy link
Member

I hadn’t seen your PR and I created one as well: #1063.

The differences:

  • The closures are much smaller through the use of a public static function.
  • Microseconds specified with a float (either float or string) are also supported.
  • Microseconds specified with a DateTime string (i.e.: '2000-01-01 00:00:00.123') are also supported.
  • More tests for corner cases.

In the end, we could merge those two PRs, but I feel mine is more complete.

@jwage
Copy link
Member

jwage commented Mar 28, 2015

Holding on merge in favor of #1063

@jwage jwage closed this Mar 28, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants