Skip to content
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

Support the use of chrono::DateTime<Utc> in sea-orm #429

Closed
wants to merge 7 commits into from

Conversation

charleschege
Copy link
Contributor

Support sea-orm derive macros for using Sea-Query DateTimeUtc.

An update to sea-query dependency from depending on the fork will be made once sea-query PR supporting DateTimeUtc is approved

Add documentation for this

Temporarily use a fork to include new Sea-query code
@charleschege
Copy link
Contributor Author

@billy1624 @tyt2y3 Thoughts on this also welcome

@billy1624 billy1624 mentioned this pull request Jan 6, 2022
@billy1624
Copy link
Member

We need some testing against all three databases

@charleschege
Copy link
Contributor Author

@billy1624 Can you clarify this or give an example. Currently, the example I can reference is at tests/common/features/applog.rs

Comment on lines +72 to +73
#[cfg(feature = "with-chrono")]
pub type DateTimeUtc = chrono::DateTime<chrono::Utc>;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The example would be inserting and updating ActiveModel with DateTimeUtc attribute.

e.g.

#[derive(Clone, Debug, PartialEq, DeriveEntityModel)]
#[sea_orm(table_name = "applog")]
pub struct Model {
    #[sea_orm(primary_key)]
    pub id: i32,
    pub action: String,
    pub json: Json,
    pub created_at: DateTimeWithTimeZone,
    pub deleted_at: DateTimeUtc,
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@billy1624 Some help on understanding the results of github action failures. They seem to build fine and store the values in the databases, but retrieval is an issue.

For example, asserting on mysql

2022-01-07T14:12:10.675164Z  INFO sqlx::query: /* SQLx ping */; rows: 0, elapsed: 5.909ms  
Error: Query("error returned from database: column \"launch_date\" is of type timestamp with time zone but expression is of type text")
thread 'main' panicked at 'assertion failed: `(left == right)`
  left: `1`,
 right: `0`: the test returned a termination value with a non-zero status code (1) which indicates a failure', /rustc/f1edd0429582dd29cccacaf50fd134b05593bd9c/library/test/src/lib.rs:195:5
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem you encountered is rooted in SeaQL/sea-query#222 (comment)

@charleschege
Copy link
Contributor Author

@tyt2y3 @billy1624 Anyone who has an idea on how to fix the errors from the CI running mysql 5.7 ?
Running the program panics with

 "CREATE TABLE IF NOT EXISTS `satellites` ( `id` int NOT NULL AUTO_INCREMENT PRIMARY KEY, `satellite_name` varchar(255) NOT NULL, `launch_date` timestamp NOT NULL, `deployment_date` timestamp NOT NULL )"
thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: Database(MySqlDatabaseError { code: Some("42000"), number: 1067, message: "Invalid default value for 'deployment_date'" })', src/main.rs:57:18

This is an issue with older versions of mysql, in this case version 5.7. Solving the problem on my end involved changing myql 5.7 configuration as specified in this article https://ixnfo.com/en/the-solution-of-error-error-1067-42000-at-line-211-invalid-default-value-for-blablabla.html

So what I am asking, is this a configuration issue, if not how do I solve the issue from sea-orm ? I am out of ideas

@charleschege
Copy link
Contributor Author

In case you need more details, re-running the tasks can provide this or check my fork https://github.com/charleschege/sea-orm/runs/4748288791?check_suite_focus=true

Comment on lines +3 to +11
#[derive(Clone, Debug, PartialEq, DeriveEntityModel)]
#[sea_orm(table_name = "satellites")]
pub struct Model {
#[sea_orm(primary_key)]
pub id: i32,
pub satellite_name: String,
pub launch_date: DateTimeUtc,
pub deployment_date: DateTimeUtc,
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @charleschege, you could set one of the timestamp column to nullable. This will solve the problem, let say changing the type of deployment_date attribute to Option<DateTimeUtc>

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @billy1624

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I am a little bit confused about some results, does using the derive macro yield different results than the Table::create statement?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But using the manual method

sea_query::Table::create()
                .table(Entity)
                .col(
                    ColumnDef::new(Column::Id)
                        .integer()
                        .not_null()
                        .auto_increment()
                        .primary_key(),
                )
                .col(ColumnDef::new(Column::SatelliteName).string().not_null())
                .col(
                    ColumnDef::new(Column::LaunchDate)
                        .timestamp_with_time_zone()
                        .not_null(),
                )
                .col(
                    ColumnDef::new(Column::DeploymentDate)
                        .timestamp_with_time_zone()
                        .not_null(),
                )
                .if_not_exists()
                .to_owned();

Assertion doesn't fail, @billy1624 is there a way to ensure fields are not null using the derive macro ? The module docs in https://docs.rs/sea-orm/latest/sea_orm/ dont show how to do this if possible

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If launch_date is nullable then the table create statement will be like...

.col(
    ColumnDef::new(Column:: LaunchDate)
        .timestamp_with_time_zone(),
)

@tyt2y3
Copy link
Member

tyt2y3 commented Jan 16, 2022

What we are missing is a default expression like DEFAULT now().
We might need to add that in sea-query first though.

@billy1624 billy1624 marked this pull request as draft January 26, 2022 10:07
@tyt2y3 tyt2y3 closed this Jan 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Support DateTime<Utc> & DateTime<Local>
3 participants