-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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.
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
sqlite date macro support #2491
Conversation
…lite library. Presently this only functions for the `time` feature, because I don't use the `chrono` feature.
Add chrono date support.
@Arcayr please address CI failures. |
All checks passed now. Thanks! |
I've actually noticed that this breaks applications relying on serialising time, and probably in a couple of other ways, due to the compiler trying to turn all time types from OffsetDateTime into PrimitiveDateTime. Switching the entries around in the file modified in the original PR fixes it. I haven't poked deep enough into the sqlx macro generation to work out why. If anyone has a pointer for a fix I can get it incorporated. |
@Arcayr it's pretty dumb: it just tries the types in order and checks https://docs.rs/sqlx/latest/sqlx/trait.Type.html#method.compatible So if you place the new entry before another compatible type, it will take precedent. Since SQLite doesn't have dedicated data types for dates and times, we use the string of the declared column type: sqlx/sqlx-sqlite/src/type_info.rs Line 94 in 253d8c9
|
I thought that was the case but didn't want to presume, I haven't been around Rust code long enough yet. :)
In this case, I imagine it'd be fine just using OffsetDateTime instead of PrimitiveDateTime, as you get conversion issues the other way. I'll update the PR accordingly and rebase if you agree. (One alternative would perhaps be to use something like |
Switch order of time::OffsetDateTime and time::PrimitiveDateTime.
I ended up switching them, as OffsetDateTime is a superset of PrimitiveDateTime in all senses anyway. This should be ready to merge if you're happy with it. Thanks! |
Changing the order of existing declarations is technically a breaking change but in this case it makes sense. |
Adds DATE pseudo-type support for sqlite into the macros for both
chrono
andtime
.Just adding the few lines mentioned in #2026 (comment).
I'm aware sqlite has no actual support for the types and simply treats them as strings, however given we already kinda support it in sqlx I figured it may as well be integrated.