SeaQL / sea-orm

🐚 An async & dynamic ORM for Rust
https://www.sea-ql.org/SeaORM/
Apache License 2.0
6.91k stars 477 forks source link

SQLite: Format of DateTimeUtc is wrong #2290

Closed BeowulfR closed 3 days ago

BeowulfR commented 1 month ago

Description

When saving a value of type DateTimeUtc in the database, an incorrect format is used.

This prevents, e.g., the comparison between CURRENT_TIMESTAMP and the value. Additionally, it causes compatibility issues with other applications using the same database.

Steps to Reproduce

  1. Create a struct with a field of type DateTimeUtc
  2. Save model to database

Expected Behavior

The timestamp is saved like this: 2024-07-17 17:02:14, to remain compatible and follow the defaults. (https://www.sqlite.org/lang_datefunc.html)

Actual Behavior

The timestamp is saved like this: 2024-07-17T17:02:14.727131+00:00

Workarounds

I don't know if there is a workaround — can the format be specified? But it should be by default the format specified by sqlite.

Reproducible Example

Just create a Model with a field of type DateTimeUtc:

pub struct Model {
    #[sea_orm(primary_key)]
    pub id: i32,
    pub updated_at: Option<DateTimeUtc>
}

Versions

Sea-Orm version: 1.0.0-rc.7 Rust: 1.79.0 Platform e.g.: stable-x86_64-apple-darwin

BeowulfR commented 1 month ago

Any information regarding this — or about a good workaround? This is a blocker because, e.g., the following code always returns true:

Expr::col(Column::ExpiresAt).gte(Expr::current_timestamp())
Expurple commented 1 month ago

I'm not sure how to force SeaORM to save model fields in a different format. You can apply a workaround at the level of your query. Something like this (not tested):

Expr::cust_with_expr("unixepoch(?)", Expr::col(Column::ExpiresAt)).gte(Expr::cust("unixepoch('now')"))

According to the documentation you linked, 2024-07-17T17:02:14.727131+00:00 is a valid time value that should be parsed by unixepoch, even though the formatting doesn't match the default SQLite formatting.

BeowulfR commented 1 month ago

According to the documentation you linked, 2024-07-17T17:02:14.727131+00:00 is a valid time value that should be parsed by unixepoch, even though the formatting doesn't match the default SQLite formatting.

Where? I cannot find a format with milliseconds and timezones.

The datetime() function returns the date and time formatted as YYYY-MM-DD HH:MM:SS or as YYYY-MM-DD HH:MM:SS.SSS if the subsec modifier is used.

And the problem is, there are other applications which also depend on the same data structure. When now seaORM saves all timestamps in a different format this causes problems or at least confusion.

Where is the format defined? Is it on your side or in sqlx?

Expurple commented 1 month ago

And the problem is, there are other applications which also depend on the same data structure. When now seaORM saves all timestamps in a different format this causes problems or at least confusion.

Sure, I agree, it would be better if SeaORM used the standard format.

Where is the format defined? Is it on your side or in sqlx?

I'm not a maintainer and I'm not familiar with the lower-level part that interacts with SQLx, so I don't know. I just suggested a quick workaround that doesn't require digging into that. If you need to store the right format, then don't use that workaround. I guess, your options are:

I cannot find a format with milliseconds and timezones.

It's this format with T and milliseconds:

  1. YYYY-MM-DDTHH:MM:SS.SSS

The timezone is OK, as the doc says two paragraphs ahead:

Formats 2 through 10 may be optionally followed by a timezone indicator of the form "[+-]HH:MM" or just "Z".

It gives this example:

2013-10-07T08:23:19.120Z

BeowulfR commented 1 month ago

Executing additional statements to reformat the dates after SeaORM saves them. ActiveModelBehavior::after_save may be a good place to do that automatically, if N+1s won't be an issue for your use case.

This is not an option, as it only creates preventable load.

I'm not a maintainer and I'm not familiar with the lower-level part that interacts with SQLx, so I don't know.

Ok, thanks anyway and then I hope for an answer from a maintainer or similar.

BeowulfR commented 1 month ago

Is there any maintainer who wants to say something about this? :thinking:

Expurple commented 1 month ago

They seem barely active for the last month or two. I don't know why. You can also try reaching out on their Discord server.

BeowulfR commented 1 month ago

Thanks, but I do not have Discord...

Sad that they are only barely active - any reason known for this?

Unfortunately, there is no other asynchronous ORM for SQlite and I depend on the bug fix :smile:

Expurple commented 1 month ago

Well, it's FOSS. It's probably easier to fork and fix the issue than to switch your entire project to a different ORM.

I'm in a similar position right now, I use my own fork with some functionality that I really needed quickly (#2265). It's been over a month with no response about merging it.

BeowulfR commented 1 month ago

Well, it's FOSS. It's probably easier to fork and fix the issue than to switch your entire project to a different ORM.

If you have the time and power, yeah... Switching wouldn't be a big problem, the project is currently still in it's creation state but their is no alternative.

BeowulfR commented 1 month ago

In this test: https://github.com/SeaQL/sea-orm/blob/eafaeb4d4dcd2cffc0f8c58a1dd27063331c010f/tests/common/features/satellite.rs#L9 the format is even the expected format (in the default value), but I cannot find a place, where DateTimeUtc is converted...

BeowulfR commented 1 month ago

Nevermind, found it in sea-query , the strange thing is only, the format is other then I can observe...

https://github.com/SeaQL/sea-query/blob/master/src/value.rs#L1067

Probably the default format of to_string is the problem :thinking:

BeowulfR commented 1 month ago

Mmh nope: https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=42fa924b3e35d4570250912324505b45 results in a different format...

BeowulfR commented 1 month ago

Ok, I think I found the error source.

Instead of this function: https://github.com/launchbadge/sqlx/blob/a892ebc6e283f443145f92bbc7fce4ae44547331/sqlx-sqlite/src/types/chrono.rs#L73 , this function: https://github.com/launchbadge/sqlx/blob/a892ebc6e283f443145f92bbc7fce4ae44547331/sqlx-sqlite/src/types/chrono.rs#L64C1-L71C2 is called, which leads to an incorrect format.

BeowulfR commented 3 days ago

Created a PR against SQLx