Skip to content

legacyDateTypeProcessing flag for datetime processing #265

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

Closed
wants to merge 2 commits into from
Closed

legacyDateTypeProcessing flag for datetime processing #265

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f04d390
- add legacyDateTypeProcessing flag for old style datetime processing
darknos Apr 22, 2017
272ba3c
Merge remote-tracking branch 'upstream/master'
darknos Apr 26, 2017
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
37 changes: 21 additions & 16 deletions lib/mysql.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,9 +135,6 @@ function generateOptions(settings) {
charset: s.collation.toUpperCase(), // Correct by docs despite seeming odd.
supportBigNumbers: s.supportBigNumbers,
connectionLimit: s.connectionLimit,
//prevent mysqljs from converting DATE, DATETIME and TIMESTAMP types
//to javascript Date object
dateStrings: true,
};

// Don't configure the DB if the pool can be used for multiple DBs
Expand All @@ -155,6 +152,11 @@ function generateOptions(settings) {
options[p] = s[p];
}
}
//discussed in #257
//prevent mysqljs from converting DATE, DATETIME and TIMESTAMP types
//to javascript Date object
if (s.legacyDateTypeProcessing === undefined) s.legacyDateTypeProcessing = true;
if (!s.legacyDateTypeProcessing) options.dateStrings = true;
}
return options;
}
Expand Down Expand Up @@ -382,14 +384,15 @@ MySQL.prototype.toColumnValue = function(prop, val) {
if (!val.toUTCString) {
val = new Date(val);
}
if (this.settings.legacyDateTypeProcessing)
return dateToMysql(val, 'Z');


if (prop.dataType == 'date') {
return dateToMysql(val).substring(0, 10);
} else if (prop.dataType == 'timestamp' || (prop.mysql && prop.mysql.dataType == 'timestamp')) {
var tz = this.client.config.connectionConfig.timezone;
return dateToMysql(val, tz);
}
return dateToMysql(val);
var tz = this.client.config.connectionConfig.timezone;
return dateToMysql(val, tz);
}
if (prop.type === Boolean) {
return !!val;
Expand Down Expand Up @@ -451,16 +454,18 @@ MySQL.prototype.fromColumnValue = function(prop, val) {
if (!val || val == '0000-00-00 00:00:00' || val == '0000-00-00') {
val = null;
} else {
var dateString = val;
var tz = this.client.config.connectionConfig.timezone;
if (prop.dataType == 'date') {
dateString += ' 00:00:00';
}
//if datatype is timestamp and zimezone is not local - convert to proper timezone
if (tz !== 'local' && (prop.dataType == 'timestamp' || (prop.mysql && prop.mysql.dataType == 'timestamp'))) {
dateString += ' ' + tz;
if (this.settings.legacyDateTypeProcessing) {
val = new Date(val.toString().replace(/GMT.*$/, 'GMT'));
} else {
var dateString = val;
var tz = this.client.config.connectionConfig.timezone;
if (prop.dataType == 'date' || (prop.mysql && prop.mysql.dataType == 'date')) {
dateString += ' 00:00:00';
} else if (tz !== 'local') {
dateString += ' ' + tz;
}
val = new Date(dateString);
}
return new Date(dateString);
}
break;
case 'Boolean':
Expand Down
2 changes: 1 addition & 1 deletion test/date_types.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,7 @@ describe('MySQL DATE, DATETTIME, TIMESTAMP types on server with non local TZ (+0
});

var prepareModel = function(tz, done) {
db = getSchema({timezone: tz});
db = getSchema({timezone: tz, legacyDateTypeProcessing:false});
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are we defaulting to false? We should be defaulting to true to preserve old behaviour and forcing users to turn on the flag for the new major/breaking behaviour.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

default was 'undefined' or true. these tests were on fixed date types processing. but it is not mater now, because #257 was reverted. :(

DateModel = db.define('DateModel', {
id: {type: Number, id: 1, generated: true},
datetimeField: {type: Date, dataType: 'datetime', null: false},
Expand Down