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 BC for nullish DB values as strings. #6348

Merged
merged 1 commit into from
Apr 16, 2015
Merged

Conversation

dereuromark
Copy link
Member

When working with legacy apps/DBs, there often is the (valid in MySQL) string value 0000-00-00 ... used for NULL. That currently breaks terribly, as it is converted to a negative value in Carbon:

object(Cake\I18n\Time) {
    'time' => '-0001-11-30T00:00:00+0000',
    'timezone' => 'UTC',
    'fixedNowTime' => false
}

making "empty" comparisons difficult.

And then on top throws errors like

IntlDateFormatter::format(): datefmt_format: error calling ::getTimeStamp() on the object [CORE\src\I18n\Time.php, line 597]

This is an easy fix here for BC and doesn't hurt IMO.

PS: "MySQL permits you to store a “zero” value of '0000-00-00' as a “dummy date.” This is in some cases more convenient than using NULL values, and uses less data and index space" https://dev.mysql.com/doc/refman/5.0/en/date-and-time-types.html Even though people really should be using NULL here as default value :)

@dereuromark dereuromark added this to the 3.0.2 milestone Apr 16, 2015
@lorenzo
Copy link
Member

lorenzo commented Apr 16, 2015

I'm ok with this

@@ -103,7 +103,7 @@ public function toDatabase($value, Driver $driver)
*/
public function toPHP($value, Driver $driver)
{
if ($value === null) {
if ($value === null || (int)$value === 0) {
Copy link
Member

Choose a reason for hiding this comment

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

Why not check for allballs specifically? Int casting will fail on '0000-01-01 00:00:00' which is a 'valid' date.

Copy link
Member Author

Choose a reason for hiding this comment

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

Well, that date is not null (as in null worthy) per se, so I wouldn't want to catch/support that either way.

You are right, specific check would be better in that case. But only on the first part of this string to also support date (0000-00-00) then, as well.
So this was a compromise to handle it quickly.

So better

substr($string, 0, 10) === '0000-00-00'

then

Copy link
Member Author

Choose a reason for hiding this comment

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

According to some sources The concept of a year "zero" is a modern myth (but a very popular one). In our calendar, C.E. 1 follows immediately after 1 B.C.E.
So we dont have to support 0000-... :trollface:

Copy link
Member

Choose a reason for hiding this comment

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

ok.

markstory added a commit that referenced this pull request Apr 16, 2015
Allow BC for nullish DB values as strings.
@markstory markstory merged commit d58cfbb into master Apr 16, 2015
@markstory markstory deleted the master-fix-datetime branch April 16, 2015 21:49
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.

3 participants