[Proposal] Add --skip-drop option#44
Conversation
cmd/mysqldef/mysqldef.go
Outdated
| File string `long:"file" description:"Read schema SQL from the file, rather than stdin" value-name:"sql_file" default:"-"` | ||
| DryRun bool `long:"dry-run" description:"Don't run DDLs but just show them"` | ||
| Export bool `long:"export" description:"Just dump the current schema to stdout"` | ||
| Safety bool `long:"safety" description:"Not make destructive changes such as Drop"` |
There was a problem hiding this comment.
I feel --skip-drop might be more descriptive about what it does than --safety. WDYT?
There was a problem hiding this comment.
I also think --skip-drop is better than --safety !
adapter/database.go
Outdated
| fmt.Println("-- Apply --") | ||
| for _, ddl := range ddls { | ||
| if isSafety && strings.Contains(ddl, "DROP") { | ||
| fmt.Printf("Not executed: %s;\n", ddl) |
There was a problem hiding this comment.
sqldef's output is designed to always be a valid input for mysql/psql. Could you prefix -- to make it a SQL comment?
And if you follow https://github.com/k0kubun/sqldef/pull/44/files#r339372757, it'd be -- Skipped: %s;.
There was a problem hiding this comment.
I understand.
I think --Skipped: %s; is good.
|
Could you add the same option to psqldef for consistency, and update the README as well? Also, it'd be really nice if we can have tests in mysqldef_test.go and psqldef_test.go. |
|
@k0kubun |
k0kubun
left a comment
k0kubun
left a comment
There was a problem hiding this comment.
Great job 👍
I'm a little concerned about just checking "DROP" inclusion in DDL (we could have DROP in a part of table/column/index names) and I'd prefer having a metadata like Drop: true for each statement in the future. However, I believe this implementation would work for most of usual situations, so I'm merging this.
2 participants
issue: https://github.com/k0kubun/sqldef/issues/43
I implemented the
--skip-dropoption as one way to avoid destructive changes.Please consider it.