Added support for migrations with comments (mysql only) #25
Conversation
| @@ -238,6 +239,18 @@ func (m *Migrator) ApplyMigration(migration *Migration, mType migrationType) err | |||
|
|
|||
| // Perform the migration. | |||
| for _, cmd := range commands { | |||
| cmd = strings.ReplaceAll(cmd, "\n", "") // strip off leading whitespace | |||
| if strings.HasPrefix(cmd, "SELECT") && strings.HasSuffix(cmd, "AS comment") { | |||
There was a problem hiding this comment.
Do we want to normalize the query here with strings.ToLower?
There was a problem hiding this comment.
I'd say yes for any SQL keywords, but that would mess up the capitalization of other strings that we may not want to mess with.
| @@ -3,11 +3,15 @@ CREATE TABLE test ( | |||
| PRIMARY KEY (id) | |||
| ); | |||
|
|
|||
| SELECT 'my comment' AS comment; | |||
There was a problem hiding this comment.
Perhaps any select should be printed out? I see no other reason to include selects in migrations.
There was a problem hiding this comment.
I could see that being helpful, but in the migrate func I specifically chose to use the transaction.QueryRow interface func since it would always return only one row. I guess you could allow someone to run a query that returned multiple rows, but then you'd open up more possibilities and you could inadvertently spam your gomigrate output with tons of rows.
| m.logger.Printf("Error executing query comment: %v", err) | ||
| return err | ||
| } | ||
| m.logger.Printf("Comment: %v", comment) |
There was a problem hiding this comment.
I'm not sure if I like the term "comment" to refer to this. Maybe "debug output" works best?
There was a problem hiding this comment.
I can see merit in that too, but would you prefer adding a new opt-in flag on the migrator to enable SELECT comments/debug statements? I could also be fine with no prefix label and just print out the raw comment/debug statement too. Thoughts?
|
@DavidHuie Any thoughts to my above comments? |
|
Ping? @DavidHuie ? |
I wanted a way for gomigrate to print out comments/status inside a migration. This only applies for mysql, since it's the only supported db type with multi-command migrations.