tediousjs / node-mssql

Microsoft SQL Server client for Node.js
https://tediousjs.github.io/node-mssql
MIT License
2.23k stars 466 forks source link

Use Datetime2 as default type for javascript Date objects #1578

Closed johandalabacka closed 11 months ago

johandalabacka commented 11 months ago

By default is javascript date converted into Datetime and if you store this in a Datetime2 field will you maybe loose precision and also you can't use the whole range in years of Javascript Date

I know I can use mssql.map to remap Date to Datetime2 BUT it is hard to do this while using knex

Expected behaviour:

Storing the Date 2023-09-01T09:23:17.762Z in a Datetime2 and reading it back would return 2023-09-01T09:23:17.762Z

Actual behaviour:

I send in a Date 2023-09-01T09:23:17.762Z and if I read it will I instead get 2023-09-01T09:23:17.763Z

Configuration:

// paste relevant config here

Software versions

dhensby commented 11 months ago

I'm a bit confused by the knex reference as knex doesn't use mssql as a driver, it uses tedious, so I'm not sure why that's problematic. As for defaulting to Datetime2, as you point out this can be done as is.

But the issue as being reported is that dates seem to be stored with a rounding error which only happens with Datetime but not Datetime2?

johandalabacka commented 11 months ago

Hi. I thought knex used mssql because i intitalize it like this:

constant db = knex({
    client: 'mssql',
    connection: {
      type: 'ntlm',
      host: process.env.DB_HOST,
      user: process.env.DB_USER, ... 

But I'm pretty sure that Date:s are first converted to Datetime and then stored in the field (which is Datetime2)

If I do this by hand

select convert(datetime, convert(datetime2,'2023-09-01T09:23:17.762Z'))

i get the same answer as my code 2023-09-01 09:23:17.763

Am I just confused? Should I report this to another repo?

dhensby commented 11 months ago

Knex uses tedious (as can be seen in the package.json). Though mssql was used for some time, it was replaced with tedious in https://github.com/knex/knex/pull/2857 and that was released in v0.95.

The default typing/casting I'd have expected is dealt with by knex itself.

johandalabacka commented 11 months ago

Ah I see. Thanks for sorting this out. Have a nice day :)