-
-
Notifications
You must be signed in to change notification settings - Fork 4.1k
Fix AutoMigrate default value comparison for string fields (issue #7590) #7591
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
Conversation
…ms can access this field from outside the package.
|
Fix This pull request addresses issue #7590 by refining how Key Changes• Added a Affected Areas• This summary was automatically generated by @propel-code-bot |
| case schema.String: | ||
| if dv != field.DefaultValue || dv != strings.Trim(field.DefaultValue, "'") { | ||
| alterColumn = true | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[CriticalError]
Logic error in string default value comparison: The condition dv != field.DefaultValue || dv != strings.Trim(field.DefaultValue, "'") will always evaluate to true because of the OR operator. If dv == field.DefaultValue, then dv != strings.Trim(field.DefaultValue, "'") will likely be true (unless the field value has quotes). This means alterColumn will always be set to true for string fields, causing unnecessary column alterations.
The logic should use AND operator to check if the values are different in both forms:
| case schema.String: | |
| if dv != field.DefaultValue || dv != strings.Trim(field.DefaultValue, "'") { | |
| alterColumn = true | |
| } | |
| case schema.String: | |
| if dv != field.DefaultValue && dv != strings.Trim(field.DefaultValue, "'") { | |
| alterColumn = true | |
| } |
⚡ Committable suggestion
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
Context for Agents
[**CriticalError**]
Logic error in string default value comparison: The condition `dv != field.DefaultValue || dv != strings.Trim(field.DefaultValue, "'")` will always evaluate to `true` because of the OR operator. If `dv == field.DefaultValue`, then `dv != strings.Trim(field.DefaultValue, "'")` will likely be true (unless the field value has quotes). This means `alterColumn` will always be set to `true` for string fields, causing unnecessary column alterations.
The logic should use AND operator to check if the values are different in both forms:
```suggestion
case schema.String:
if dv != field.DefaultValue && dv != strings.Trim(field.DefaultValue, "'") {
alterColumn = true
}
```
⚡ **Committable suggestion**
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
File: migrator/migrator.go
Line: 566
migrator/migrator.go
Outdated
| v2, _ := strconv.ParseBool(field.DefaultValue) | ||
| alterColumn = v1 != v2 | ||
| case schema.String: | ||
| if dv != field.DefaultValue && dv != strings.Trim(field.DefaultValue, "'") { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
change to strings.Trim(field.DefaultValue, "'\"")?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got it, already updated.
|
@jinzhu Can this be merged into the main branch? |
bugfix:AutoMigrate #7590