-
Notifications
You must be signed in to change notification settings - Fork 0
Feat/migration version #152
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
jjchen01
commented
Sep 19, 2025
- Explicit sql files for migration
- Upgrade sequentially
- Select for update to ensure only one instance is updating the schema.
func NewSchemaVersion() SchemaVersion { | ||
sv := SchemaVersion{ | ||
Migrations: []int64{ | ||
2, |
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.
We have to update the sequence file manually.
} | ||
|
||
if err := tx.WithContext(ctx). | ||
Raw("SELECT id, version FROM database_migration WHERE id = ? FOR UPDATE", m.ID). |
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.
Select for update to acquire a lock among instances.
|
||
if reset { | ||
// Still experiencing a race condition here, need to consult with DevOps regarding deployment strategy. | ||
if err := db.Exec("DROP SCHEMA IF EXISTS public CASCADE;").Error; err != nil { |
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.
race condition
continue | ||
} | ||
// get version sql file | ||
sqlFile := filepath.Join(migrationSqlFolder, fmt.Sprintf("%d.sql", migrationVersion)) |
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.
If migrationVersion
is 1, retrieve 1.sql
.
} | ||
|
||
if reset { | ||
// Still experiencing a race condition here, need to consult with DevOps regarding deployment strategy. |
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.
can we remove the logic there -))
...
we can run it manually on first steps
Don't risk our prod data
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.
reset
will be removed in the next release.
fileContentAsString := string(content) | ||
sqlCommands := strings.Split(fileContentAsString, ";") | ||
for _, command := range sqlCommands { | ||
db.Exec(command) |
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.
if one command failure, chains of errors happen.
can we log and throw error immediately.
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.
Actually, there is no error returned in the exec function, and we should test it before releasing the new schema.
0, | ||
}, | ||
} | ||
slices.Sort(sv.Migrations) |
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.
can we just add the name like
"0001_v01.0.sql"
"0002.sql"
and use it without sorting
men -)) we hard code but we sort it again make me feel unsafe
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.
Sorting ensures the correct sequence, in case code conflicts prevent us from adding elements back in order. If you're uncomfortable with it, we can remove it and hope we add them correctly each time.
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.
:) few comments, feel free to merge after address that