Skip to content

Adding support for fractional seconds #274

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
May 8, 2017
Merged

Adding support for fractional seconds #274

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 27 additions & 1 deletion lib/migration.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -723,7 +723,8 @@ function mixinMigration(MySQL, mysql) {
dt = numericOptionsByType(p, dt);
break;
case 'Date':
dt = columnType(p, 'DATETIME'); // Currently doesn't need options.
dt = columnType(p, 'DATETIME');
dt = dateOptionsByType(p, dt);
break;
case 'Boolean':
dt = 'TINYINT(1)';
Expand Down Expand Up @@ -893,6 +894,31 @@ function mixinMigration(MySQL, mysql) {
return columnType;
}

function dateOptionsByType(p, columnType) {
switch (columnType.toLowerCase()) {
default:
case 'date':
break;
case 'time':
case 'datetime':
case 'timestamp':
Copy link
Contributor

Choose a reason for hiding this comment

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

Also should we only add precision fractional second for timestamp values? According to MySQL documentation

A DATETIME or TIMESTAMP value can include a trailing fractional seconds part in up to microseconds (6 digits) precision

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think I don't understand your comment. As the MySQL documentation said: TIME, DATETIME and TIMESTAMP types can have a precision. The DATE-type (of course) not.

This function checks the columntype and for columns with a type of time, datetime and timestamp and a precision is defined, the columnType will modified (see line 915), for example to DATETIME(3). This will allow to save values like 2017-05-07 12:34:56.789. When the column is of the type DATE, the function returns the colomnType without any modification.

columnType = dateOptions(p, columnType);
break;
}
return columnType;
}

function dateOptions(p, columnType) {
var precision = 0;

if (p.precision) {
precision = Number(p.precision);
columnType += '(' + precision + ')';
}

return columnType;
}

function unsigned(p, columnType) {
if (p.unsigned) {
columnType += ' UNSIGNED';
Expand Down
2 changes: 1 addition & 1 deletion lib/mysql.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -412,7 +412,7 @@ MySQL.prototype.fromColumnValue = function(prop, val) {
// MySQL allows, unless NO_ZERO_DATE is set, dummy date/time entries
// new Date() will return Invalid Date for those, so we need to handle
// those separate.
if (!val || val == '0000-00-00 00:00:00' || val == '0000-00-00') {
if (!val || /^0{4}(-00){2}( (00:){2}0{2}(\.0{1,6}){0,1}){0,1}$/.test(val)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure what the end of the regex means

{0,1}$/.test(val)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This regex checks if val is one of the following values:

0000-00-00
0000-00-00 00:00:00
0000-00-00 00:00:00.0
0000-00-00 00:00:00.00
0000-00-00 00:00:00.000
0000-00-00 00:00:00.0000
0000-00-00 00:00:00.00000
0000-00-00 00:00:00.000000

You can see it working over here: https://regex101.com/r/IkM0wc/2

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, I get what the regex does. I am confused about the end part of the regex.

.test(val)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, I get it! The test() method checks if the string specified in the function argument matches the regex. It will return true or false. See also this article on MDN.

Copy link
Contributor

Choose a reason for hiding this comment

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

That makes much more sense now.

val = null;
}
break;
Expand Down
19 changes: 19 additions & 0 deletions test/datetime.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ describe('MySQL datetime handling', function() {
married: Boolean,
dob: {type: 'DateString'},
createdAt: {type: Date, default: Date},
lastLogon: {type: Date, precision: 3, default: Date},
Copy link
Contributor

Choose a reason for hiding this comment

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

hi @joostdebruijn from the login on line#900, a property with date type won't execute dateOptions() which appends precision config, can you take a look?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Are you sure? I ran the tests again and dateOptions() is hit twice for my specific test. The first time for property createdAt (skipping the if at migration.js:914), resulting in a DATETIME. The second time for the property lastLogon (hitting the if at migration.js:914) and resulting in a DATETIME(3).

Please note that the Loopback-type 'Date' maps on 'DATETIME' by default, see documentation and migration.js:725. The method dateOptions() handles the columnType and not the type as in Loopback-type.

Copy link
Contributor

Choose a reason for hiding this comment

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

ah I see, it's executed after dt = columnType(p, 'DATETIME');. Thanks.

};

// Modifying the connection timezones mid-flight is a pain,
Expand Down Expand Up @@ -92,4 +93,22 @@ describe('MySQL datetime handling', function() {
});
}
});

it('should allow use of fractional seconds', function(done) {
var d = new Date('1971-06-22T12:34:56.789Z');
return Person.create({
name: 'Mr. Pink',
gender: 'M',
lastLogon: d,
}).then(function(inst) {
return Person.findById(inst.id);
}).then(function(inst) {
inst.should.not.eql(null);
var lastLogon = new Date(inst.lastLogon);
lastLogon.toJSON().should.eql(d.toJSON());
return done();
}).catch(function(err) {
return done(err);
});
});
});