Replace gorm.Migrator calls with sql #2846
Conversation
| AND sql LIKE ? | ||
| ` | ||
| column = "%`" + column + "`%" |
There was a problem hiding this comment.
I based this on the gorm migrator implementation we're using now: https://github.com/go-gorm/sqlite/blob/v1.3.6/migrator.go#L70-L71
The sqlite_master table stores the CREATE TABLE statement as sql, so we have to use substring matching to find the column.
The gorm implementation has a lot more sql LIKE ? parts of the query. My guess is that different sqlite implementations might use different quoting for column names. This works for our existing use cases, and since we won't support sqlite in production, this seems sufficient for our use case.
|
|
||
| // DefaultOptions used for tests | ||
| var DefaultOptions = Options{ | ||
| UseTransaction: false, |
There was a problem hiding this comment.
should probably start using transactions when we drop sqlite
| return nil | ||
| } | ||
|
|
||
| if err := tx.Migrator().AddColumn(&models.Destination{}, "last_seen_at"); err != nil { | ||
| stmt := `ALTER TABLE destinations ADD COLUMN last_seen_at timestamp with time zone` |
There was a problem hiding this comment.
This is good, I prefer the timezone in the db, but do you know if other tables in postgres have been getting timestamp with time zone? might cause subtle problems somewhere if they're different.
There was a problem hiding this comment.
Ya, this matches what gorm.AutoMigrate produces, you can see that here: https://github.com/infrahq/infra/pull/2843/files#r940287138
And move DefaultOptions to test code, since it is only used by tests. Also format the SQL statements with consistent casing.
Also replace our tableExists test helper. These are replaced with a new HasTable function that should be easy to change to stdlib.
This one doesn't really need a helper, we can write the DROP TABLE statement in place.
Replace with regular sql statements.
dnephin
force-pushed
the
dnephin/data-reaplce-gorm-migrator
branch
from
c8f16f8
to
f1b3126
Compare
Summary
gorm.Migrator(not to be confused withgormigratewhich we already replaced) provides helper functions for inspecting and changing the schema.This PR replaces calls to those helpers with regular SQL. In a couple cases (
HasTable,HasColumn) I added a helper function to thedata/migratorpackage, because these two require inspecting the informational schema and are a bit more involved.This PR keeps sqlite support by writing the equivalent sqlite queries (which are not much different), but it is important to note that the sqlite versions of a couple of these are not idempotent.
The only remaining use of
gorm.MigratorisAutoMigrate, which will be replaced next.Related Issues
Related to #2768