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

Fix AutoMigrate, alterColumn The previous modifications were ignored #7380

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

Conversation

general252
Copy link

@general252 general252 commented Mar 5, 2025

The AutoMigrate function, when a field in struct has a default value, modifying the size value is invalid

type Object struct {
	ID   uint
	Name string `gorm:"column:name;default:'';size:16"`
}

->

type Object struct {
	ID   uint
	Name string `gorm:"column:name;default:'';size:16"` // update size 16 -> 100, the size in the database is still 16
}

Copy link
Member

@a631807682 a631807682 left a comment

Choose a reason for hiding this comment

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

It seems that the entire logic https://github.com/go-gorm/gorm/pull/7380/files#diff-ffbc6f3f562ac2aba57e62bedbac96edba9ad9708fe9349613bf4fa1765baf65R534-R557 does not need to be rechecked if alterColumn is true.

Please merge the commit ee3b549 of this master branch to ensure the test runs.

@general252
Copy link
Author

It seems that the entire logic https://github.com/go-gorm/gorm/pull/7380/files#diff-ffbc6f3f562ac2aba57e62bedbac96edba9ad9708fe9349613bf4fa1765baf65R534-R557 does not need to be rechecked if alterColumn is true.

Please merge the commit ee3b549 of this master branch to ensure the test runs.

Yes, when alterColumn is true, the subsequent check can be ignored.

When alterColumn is true, alterColumn = dv != field.DefaultValue may change alterColumn to false, making the field unable to be modified.

@a631807682
Copy link
Member

It seems that the entire logic #7380 (files) does not need to be rechecked if alterColumn is true.
Please merge the commit ee3b549 of this master branch to ensure the test runs.

Yes, when alterColumn is true, the subsequent check can be ignored.

When alterColumn is true, alterColumn = dv != field.DefaultValue may change alterColumn to false, making the field unable to be modified.

I means in the MigrateColumn function https://github.com/go-gorm/gorm/blob/master/migrator/migrator.go#L483-L565, when alterColumn is checked to be true, can all subsequent checks be omitted?
For example, if the types are different, the nullable check can be omitted, and if the nullables are different, the default value check can be omitted.

@general252
Copy link
Author

It seems that the entire logic #7380 (files) does not need to be rechecked if alterColumn is true.如果 alterColumn 为 true,则似乎不需要重新检查整个逻辑 #7380(文件)。
Please merge the commit ee3b549 of this master branch to ensure the test runs.请合并此 master 分支的提交 ee3b549 以确保测试运行。

Yes, when alterColumn is true, the subsequent check can be ignored.是的,当 alterColumntrue 时,可以忽略后续检查。
When alterColumn is true, alterColumn = dv != field.DefaultValue may change alterColumn to false, making the field unable to be modified.当 alterColumntrue 时, alterColumn = dv != field.DefaultValue 可能会将 alterColumn 更改为 false,从而导致字段无法修改。

I means in the MigrateColumn function https://github.com/go-gorm/gorm/blob/master/migrator/migrator.go#L483-L565, when alterColumn is checked to be true, can all subsequent checks be omitted?我的意思是,在 MigrateColumn 函数 https://github.com/go-gorm/gorm/blob/master/migrator/migrator.go#L483-L565 中,当 alterColumn 被检查为 true 时,是否可以省略所有后续检查? For example, if the types are different, the nullable check can be omitted, and if the nullables are different, the default value check can be omitted.例如,如果类型不同,则可以省略可为 null 的检查,如果可为 null 的检查不同,则可以省略默认值检查。

I understand what you mean, and I also found that the original code has repeated checks, all of which only confirm alterColumn==true https://github.com/go-gorm/gorm/blob/master/migrator/migrator.go#L567-L571

@a631807682
Copy link
Member

Yes, I think this part of the duplicate check can be ignored.

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

Successfully merging this pull request may close these issues.

2 participants