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

auto set timestamp column when update & insert #854

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

liberwang1013
Copy link
Contributor

@liberwang1013 liberwang1013 commented Jul 9, 2022

PR Info

Adds

  • add created_at & updated_at attributes support when define model

More Details

In this implement, the timestamp column type should be defined as TIMESTAMP or TIMESTAMPTZ, so the Model must be defined line below

#[derive(Clone, Debug, PartialEq, DeriveEntityModel)]
#[sea_orm(table_name = "cake")]
pub struct Model {
    #[sea_orm(primary_key)]
    pub id: i64,
    #[sea_orm(default_value = "Sam")]
    pub name: String,
    #[sea_orm(created_at, column_type = "TimestampWithTimeZone")]
    pub created_at: DateTimeWithTimeZone,
    #[sea_orm(updated_at, column_type = "Timestamp")]
    pub updated_at: DateTime
}

Copy link
Member

@billy1624 billy1624 left a comment

Choose a reason for hiding this comment

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

Hey @liberwang1013, sorry for the delay. Thanks for the contributions!!

I have added some test cases, refactored a few places as well. Please review and then cherrypick :)

@liberwang1013
Copy link
Contributor Author

Hey @liberwang1013, sorry for the delay. Thanks for the contributions!!

I have added some test cases, refactored a few places as well. Please review and then cherrypick :)

* [liberwang1013/sea-orm@auto-update-timestamp-column...billy1624:pr/854](https://github.com/liberwang1013/sea-orm/compare/auto-update-timestamp-column...billy1624:pr/854)

Hi @billy1624 , thank you for your commit. I have already cherrypicked your commit, your commit is great. But, now we have to face a new problem that we didn't pass all checks. I have searched some documents try to figure out what's the problem . In this case, it seems that default value the system given is conflicted with msyql default sql_mode setttings.

CREATE TABLE `check` (
  `id` int NOT NULL AUTO_INCREMENT PRIMARY KEY,
  `pay` varchar(255) NOT NULL,
  `amount` double NOT NULL,
  `updated_at` timestamp NOT NULL,
  `created_at` timestamp NOT NULL
)

I think there are three ways to fix it

  • remove the NOT NULL constrain for updated_at & created_at columns
  • give the default value CURRENT_TIMESTAMP for updated_at & created_at columns
  • disable NO_ZERO_DATE sql mode in this case

what's your opinion?

@tyt2y3
Copy link
Member

tyt2y3 commented Aug 7, 2022

Yes, actually if you want a robust timestamps, the best way is to rely on DB's native DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP. And I think we need better SeaQuery support for this

@liberwang1013 liberwang1013 force-pushed the auto-update-timestamp-column branch from 2869439 to d9d77dc Compare August 8, 2022 16:00
@billy1624
Copy link
Member

Yes, actually if you want a robust timestamps, the best way is to rely on DB's native DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP. And I think we need better SeaQuery support for this

Note that not all database support such ON UPDATE CURRENT_TIMESTAMP syntax. For example, PostgreSQL doesn't have it. Instead, a trigger is needed to perform the operation. See https://stackoverflow.com/a/1036010/7059723

@billy1624
Copy link
Member

billy1624 commented Aug 10, 2022

Okay, we have two problems here


  1. Define timestamp column with default value of CURRENT_TIMESTAMP
#[derive(Clone, Debug, PartialEq, DeriveEntityModel)]
#[sea_orm(table_name = "check")]
pub struct Model {
    ...
    #[sea_orm(updated_at, default_expr = "Func::current_timestamp()")]
    pub updated_at: DateTimeWithTimeZone,
    #[sea_orm(created_at, default_expr = "Func::current_timestamp()")]
    pub created_at: DateTimeWithTimeZone,
}

Also, we could provide a shorthand for it

#[derive(Clone, Debug, PartialEq, DeriveEntityModel)]
#[sea_orm(table_name = "check")]
pub struct Model {
    ...
    #[sea_orm(updated_at, default_current_timestamp)]
    pub updated_at: DateTimeWithTimeZone,
    #[sea_orm(created_at, default_current_timestamp)]
    pub created_at: DateTimeWithTimeZone,
}

Related issues:


  1. Provide db specific way to set ON UPDATE CURRENT_TIMESTAMP

For example,

Related issues:

@liberwang1013
Copy link
Contributor Author

#[derive(Clone, Debug, PartialEq, DeriveEntityModel)]
#[sea_orm(table_name = "check")]
pub struct Model {
    ...
    #[sea_orm(updated_at, default_expr = "Func::current_timestamp()")]
    pub updated_at: DateTimeWithTimeZone,
    #[sea_orm(created_at, default_expr = "Func::current_timestamp()")]
    pub created_at: DateTimeWithTimeZone,
}

In this way, I prefer using CURRENT_TIMESTAMP directly. In some cases, the developer can use some sea-orm still unsupported expresssion such as Generated Columns features.

@liberwang1013
Copy link
Contributor Author

Okay, we have two problems here

1. Define timestamp column with default value of `CURRENT_TIMESTAMP`
#[derive(Clone, Debug, PartialEq, DeriveEntityModel)]
#[sea_orm(table_name = "check")]
pub struct Model {
    ...
    #[sea_orm(updated_at, default_expr = "Func::current_timestamp()")]
    pub updated_at: DateTimeWithTimeZone,
    #[sea_orm(created_at, default_expr = "Func::current_timestamp()")]
    pub created_at: DateTimeWithTimeZone,
}

Also, we could provide a shorthand for it

#[derive(Clone, Debug, PartialEq, DeriveEntityModel)]
#[sea_orm(table_name = "check")]
pub struct Model {
    ...
    #[sea_orm(updated_at, default_current_timestamp)]
    pub updated_at: DateTimeWithTimeZone,
    #[sea_orm(created_at, default_current_timestamp)]
    pub created_at: DateTimeWithTimeZone,
}

Related issues:

* [Table Column with Default Expression sea-query#347](https://github.com/SeaQL/sea-query/issues/347)


2. Provide db specific way to set `ON UPDATE CURRENT_TIMESTAMP`

For example,

* MySQL: https://dev.mysql.com/doc/refman/8.0/en/timestamp-initialization.html

* PostgreSQL: https://stackoverflow.com/a/1036010/7059723

* SQLite: No easy way to achieve it except https://stackoverflow.com/q/6578439/7059723

Related issues:

* [Add support for Postgresql's function and trigger sea-query#403](https://github.com/SeaQL/sea-query/issues/403)

I think I have found a better way to do this: support extra macro attribute.

@liberwang1013 liberwang1013 force-pushed the auto-update-timestamp-column branch 3 times, most recently from 7df929b to 37de813 Compare August 20, 2022 08:28
@billy1624
Copy link
Member

I think I have found a better way to do this: support extra macro attribute.

I'd say the extra attribute is good to have and it serve as a temporary workaround before default expression land. :)

Copy link
Member

@billy1624 billy1624 left a comment

Choose a reason for hiding this comment

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

Thanks!! @liberwang1013

@tyt2y3
Copy link
Member

tyt2y3 commented Jan 19, 2023

https://docs.rs/sea-query/latest/sea_query/table/struct.ColumnDef.html#method.default in SeaQuery now also accpets an Expr so we can now write:

let table = Table::create()
    .table(Char::Table)
    .col(ColumnDef::new(Char::FontId).integer().default(12i32))
    .col(
        ColumnDef::new(Char::CreatedAt)
            .timestamp()
            .default(Expr::current_timestamp())
            .not_null(),
    )
    .to_owned();

assert_eq!(
    table.to_string(MysqlQueryBuilder),
    [
        "CREATE TABLE `character` (",
        "`font_id` int DEFAULT 12,",
        "`created_at` timestamp DEFAULT CURRENT_TIMESTAMP NOT NULL",
        ")",
    ]
    .join(" ")
);

@liberwang1013 liberwang1013 force-pushed the auto-update-timestamp-column branch from 37de813 to fa4b65d Compare January 30, 2023 10:43
Comment on lines 214 to 216
if let Some(value) = orm_column_def.extra {
column_def.default(value);
}
Copy link
Member

Choose a reason for hiding this comment

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

Found a typo

Suggested change
if let Some(value) = orm_column_def.extra {
column_def.default(value);
}
if let Some(value) = orm_column_def.extra {
column_def.extra(value);
}

Copy link
Contributor Author

@liberwang1013 liberwang1013 Jan 30, 2023

Choose a reason for hiding this comment

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

Hi @billy1624 thanks for your comment,
what about replaced with column_def.default(SimpleExpr::CustomWithExpr(value, vec![]));, I found the generated default expr in schema is going to quoted with ' in this(column_def.default(value);) way.

@billy1624
Copy link
Member

Hey @liberwang1013, please check liberwang1013#6

@liberwang1013
Copy link
Contributor Author

https://docs.rs/sea-query/latest/sea_query/table/struct.ColumnDef.html#method.default in SeaQuery now also accpets an Expr so we can now write:

let table = Table::create()
    .table(Char::Table)
    .col(ColumnDef::new(Char::FontId).integer().default(12i32))
    .col(
        ColumnDef::new(Char::CreatedAt)
            .timestamp()
            .default(Expr::current_timestamp())
            .not_null(),
    )
    .to_owned();

assert_eq!(
    table.to_string(MysqlQueryBuilder),
    [
        "CREATE TABLE `character` (",
        "`font_id` int DEFAULT 12,",
        "`created_at` timestamp DEFAULT CURRENT_TIMESTAMP NOT NULL",
        ")",
    ]
    .join(" ")
);

hi @tyt2y3 , the code has been updated, please review it.

@tyt2y3
Copy link
Member

tyt2y3 commented Jan 31, 2023

Sorry but this will not be released on 0.11

@sutt0n
Copy link

sutt0n commented Jan 31, 2024

Any updates on this functionality getting in, or has something else similar made its way into the crate?

@carlocorradini
Copy link

Any update? 🤯
This is super useful ☺️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Triage
Development

Successfully merging this pull request may close these issues.

Automatically update created_at & updated_at timestamp columns on update
6 participants