AutoMigrate: Rename/Create/Drop tables #926
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
I had to update a lot of to the existing unit tests (~5-7 files) because I noticed that many of them weren't properly cleaning up the database, causing side-effects in the auto-migrate tests. UPD: opened #927 for this specific change. Should make it easier to review the current one once the other changes are merged. |
bevzzz
changed the title
- RawQuery - CreateTableQuery
Additionally: - allow custom FK names (limited applicability rn) - implement GetReverse for AddFK and DropFK - detect renamed foreign keys - EqualSignature handles empty models (no columns)
bevzzz
force-pushed
the
feature/automigrate-rename-table
branch
4 times, most recently
from
8708552
to
3ef6553
Compare
This is a WIP commit.
- Do not implement AppendQuery on Operation level. This is a leaky abstraction as queries are dialect-specific and migrate package should not be concerned with how they are constructed. - AlterTableQuery also is an unnecessary abstraction. Now pgdialect will just build a simple string-query for each Operation. - Moved operations.go to migrate/ package and deleted alt/ package. - Minor clean-ups and documentation. testChangeColumnType is commented out because the implementation is missing.
bevzzz
force-pushed
the
feature/automigrate-rename-table
branch
from
3ef6553
to
ce75416
Compare
Bufixes and improvements: - pgdialect.Inspector canonicalizes all default expressions (lowercase) to make sure they are always comparable with the model definition. - sqlschema.SchemaInspector canonicalizes all default expressions (lowercase) - pgdialect and sqlschema now support type-equivalence, which prevents unnecessary migrations like CHAR -> CHARACTER from being created. Changing PRIMARY KEY and UNIQUE-ness are outside of this commit's scope, because those constraints can span multiple columns.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR is my second attempt at implementing #456 :)
This time around I decided to start with the functionality we want to bring (
AutoMigrator), design a good interface and work from there.I took into account our discussions of the ALTER TABLE functionality from #726 and let each dialect export a simple
RenameTable(context.Context, string, string)function to be used byAutoMigrator's internals. My thinking was that, if auto migration is done well, the users wouldn't need to work with ALTER TABLE all that much.❕
AutoMigrator.Migrate()is just a sketch -- running it will do the migration and record it in the db, but the users won't be able to revert it because no migration file (.sql or .go) is generated at the moment.AutoMigrator.Run()should be used to run the migrations "in-place".There might be other improvements to make here (e.g. add more unit test cases) but I thought I'd put it out here sooner to get the feedback.
What is still to do:
type:)Other:
test.shto pass additional arguments togo test(for example, now possible to run 1 test with
./test.sh -run=TestAutoMigrator_Run)