sequelize: MSSQL 2016 DATETIME field updates broken due to DATETIME2 use
I’ve got an MSSQL db, with DATETIME fields (not DATETIMEOFFSET) and no matter what I do with sequelize/models, I can’t get updates to work without a casting error. If I update my DB to DATETIMEOFFSET (which I tested thanks to other similar issues I found), all is well. I think the presumed DATETIMEOFFSET use (from DATE) is too aggressive and I need to undo it rather than get pushed into a schema change. There an option/flag that will accomplish that, or is this a bug?
What are you doing?
Model definition for Batch
module.exports = function(sequelize, DataTypes) {
return sequelize.define('Batch', {
ID: {
type: DataTypes.INTEGER,
allowNull: false,
primaryKey: true,
autoIncrement: true
},
BatchDate: {
type: DataTypes.DATE,
allowNull: true,
defaultValue: '(getdate())'
},
{
tableName: 'Batch',
timestamps: false,
paranoid: true,
freezeTableName: true,
hasTrigger: true
});
};
#Try to update Batch.BatchDate with a perfectly valid date
Batch.find( { where: { ID: 31243 } } )
.then( batch => {
batch.BatchDate = new Date("2028-01-01T00:00:00.000Z");
batch.save();
//// The Following throws the exact same error as the use of save() above
//
// batch.update(
// { BatchDate: "2038-01-01T00:00:00.000Z" },
// { where: { ID: 31243 } } )
// .then( res => {
// app.bm.mods.sequelize.models.Batch.findAll( { where: { id: 31243 } } )
// .then( res => {
// console.dir( res );
// });
// })
// .catch( err => {
// console.dir ( err );
// });
});
#NOTE the ‘DATETIMEOFFSET’ use rather than DATETIME
exec sp_executesql @statement=N'declare @tmp table ([ID] INTEGER,[BatchDate] DATETIMEOFFSET);UPDATE [Batch] SET [BatchDate]=N''2028-01-01 00:00:00.000 +00:00'' OUTPUT INSERTED.[ID],INSERTED.[BatchDate] into @tmp WHERE [ID] = 31243;select * from @tmp'
What do you expect to happen?
I expected Sequelize to run an UPDATE transaction, setting the value of BatchDate as a standard JS Date and SQL DATETIME (not DATETIMEOFFSET).
What is actually happening?
SequelizeDatabaseError: Conversion failed when converting date and/or time from character string.
Sequelize is using DATETIMEOFFSET instead of the defined DATETIME.
Dialect: mssql Dialect version: 2016 Database version: 2016 Sequelize version: 4.2.x and 4.37.6 Tested with latest release: Yes, 4.37.6
About this issue
- Original URL
- State: open
- Created 6 years ago
- Reactions: 2
- Comments: 18
We’d love to see this get fully resolved!
This seems to be still an issue.
I temporarily fixed my app using this hack. https://github.com/sequelize/sequelize/issues/7930#issuecomment-410326038
It would be nice to see it fixed
Ultimately it comes down the DATE type provided by Sequelize has a format that isn’t compatible with MSSQL DATETIME, but is with DATETIMEDIFF.
I’ve used method overriding to provide my own format (definitely needs to be exposed as an option) and I think we need to extend logic for MSSQL to check if LIKE ‘DATE%’ and ‘DATETIME2’ and ‘DATETIMEDIFF’ to get FULL compatibility.
Have my solution, though it’s definitely not a complete one (and likely why I’m at this point) and I will be happy to advance this, but your contribution guidelines are a bit much for me right now so I’m going to provide you the change here and try to get a proper PR in place once I’ve got the required test environment dependencies working properly (namely, get my butt up to speed with docker).
Meet this issue too, hope that
Sequelize.Date
will be compatible withMSSQL DATETIME
or maybe just create a new DataTypeAND full circle…
https://github.com/sequelize/sequelize/blob/master/lib/dialects/mssql/data-types.js#L138 causes the UPDATE query to date my mssql:DATE values and create parameters as DATETIMEOFFSET and then something else is formatting the date (haven’t found it yet) as OFFSET (with “+00:00” added). SQL Server promptly throws an error because the actual type is DATETIME and if I had to guess, because it would result in a loss of precision. I think the choice to go from DATETIME to DATETIMEOFFSET was well intentioned, but as someone that already cared about time-zones, I use UTC for everything and so I don’t need it. I think someone that cared that much to use a newer datatype, would be happy to say so in the definition and we don’t have to assume for “DATE”, only for the internal fields that actually require it.
This making sense? @janmeier