sqlx icon indicating copy to clipboard operation
sqlx copied to clipboard

Support mssql's DateTime2 type via chrono

Open milgner opened this issue 3 years ago • 4 comments

There was some previous conversation about it in #1251 but I didn't find any code in the current version of the repository to get it working. So I wrote my own version, based on the implementation from the go-mssqldb library.

Unfortunately, In order to add it to have a full integration test for type casting, I had to add chrono to the dev-dependencies in Cargo.toml. And since one cannot have features with the same name as a dependency, I had to rename the feature from chrono to with_chrono.

Any suggestions for improvement welcome.

milgner avatar Feb 06 '22 02:02 milgner

Simplified the code to not require lazy_static, added working tests and fixed a bug with scale>5 along the way. Let me know if you'd like to change anything.

milgner avatar Feb 06 '22 15:02 milgner

I further improved the implementation by decoding datetime2 as chrono::NaiveDateTime instead and adding support for datetimeoffset, too, which is treated as chrono::DateTime with proper offset information. Suggestions for additional improvement welcome.

milgner avatar Feb 13 '22 14:02 milgner

@milgner It appears that the only flaw here is a lack of a cargo fmt run before all checks would pass; Can we try to get this merge-ready before it drifts too far from master?

Additionally, whether this is done in this PR or otherwise, enabling the chrono types in /sqlx-core/src/any/types may now be viable, and would allow use of the any feature with chrono.

Dessix avatar Feb 26 '22 04:02 Dessix

@milgner It appears that the only flaw here is a lack of a cargo fmt run before all checks would pass; Can we try to get this merge-ready before it drifts too far from master?

Of course! I haven't had a chance this week to check on this but I'll gladly run it through cargo fmt and rebase it!

Additionally, whether this is done in this PR or otherwise, enabling the chrono types in /sqlx-core/src/any/types may now be viable, and would allow use of the any feature with chrono.

Not sure what the implications might be here. I think I'll try creating a separate branch and create a second PR for that.

milgner avatar Feb 26 '22 07:02 milgner

Any possibilities to get this merged?

zibebe avatar Aug 27 '22 09:08 zibebe

I see that there are merge conflicts again. I can quickly resolve those if desired. But a short :+1: or :-1: would be appreciated so I don't have to resolve them anew every second week :wink:

milgner avatar Aug 29 '22 16:08 milgner

I really hope you get some 👍 for this. With #2073 it's the only thing preventing me from using sqlx for mssql.

zibebe avatar Aug 29 '22 16:08 zibebe

Thank you for taking the time to open this PR. However, we're now in the process of splitting out the MSSQL driver for our SQLx Pro offering and it will be rewritten from scratch to fix some longstanding issues, so we will not be accepting your contribution at this time.

We'll make sure that the new MSSQL driver supports the datetime types via chrono and time

abonander avatar Sep 15 '22 00:09 abonander