Skip to content
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

Migration steps with same name are erroneously skipped #11

Closed
asg017 opened this issue Oct 27, 2023 · 3 comments
Closed

Migration steps with same name are erroneously skipped #11

asg017 opened this issue Oct 27, 2023 · 3 comments
Labels
bug Something isn't working

Comments

@asg017
Copy link

asg017 commented Oct 27, 2023

Discovered when I had two separate migrations with different migration_set names but the same step name.

The first migration:

from sqlite_migrate import Migrations

m1 = Migrations("m1")

@m1()
def m001_init(db):
    pass

The second:

from sqlite_migrate import Migrations
m2 = Migrations("m2")

@m2()
def m001_init(db):
    pass

The m1.m001_init() step runs successfully, but m2.m001_init() is skipped. This is because they share the same step name m001_init, but they should have a different namespace anyway.

Caused by this, I think:

if migration.name not in already_applied

@simonw simonw added the bug Something isn't working label Oct 27, 2023
@simonw
Copy link
Owner

simonw commented Oct 27, 2023

Ouch, there's another problem here:

def ensure_migrations_table(self, db: "Database"):
"""
Create _sqlite_migrations table if it doesn't already exist
"""
table = _table(db, self.migrations_table)
if not table.exists():
table.create(
{
"migration_set": str,
"name": str,
"applied_at": str,
},
pk="name",
)

The primary key on _sqlite_migrations is on name - but clearly it should be. compound primary key on (migration_set, name) instead - otherwise you can't have two migration sets with duplicate names.

@simonw
Copy link
Owner

simonw commented Oct 27, 2023

Frustrating that we don't have migrations for the _sqlite_migrations table itself! I'm going to have to write code that knows how to upgrade that table by introspecting if it has the correct primary key.

@simonw
Copy link
Owner

simonw commented Oct 27, 2023

I manually tested it against a file that had previously been migrated.

Before it had the old single primary key:

$ sqlite-utils schema creatures.db 
CREATE TABLE [_sqlite_migrations] (
   [migration_set] TEXT,
   [name] TEXT PRIMARY KEY,
   [applied_at] TEXT
);
CREATE TABLE [creatures] (
   [id] INTEGER PRIMARY KEY,
   [name] TEXT,
   [species] TEXT
, [weight] FLOAT);
$ sqlite-utils migrate creatures.db

Now it has the new primary key scheme:

$ sqlite-utils schema creatures.db 
CREATE TABLE "_sqlite_migrations" (
   [migration_set] TEXT,
   [name] TEXT,
   [applied_at] TEXT,
   PRIMARY KEY ([migration_set], [name])
);
CREATE TABLE "creatures" (
   [id] INTEGER PRIMARY KEY,
   [name] TEXT,
   [species] TEXT,
   [weight] FLOAT,
   [age] INTEGER,
   [shoe_size] INTEGER
);

Running it a second time makes no changes:

$ sqlite-utils migrate creatures.db
$ sqlite-utils schema creatures.db 
CREATE TABLE "_sqlite_migrations" (
   [migration_set] TEXT,
   [name] TEXT,
   [applied_at] TEXT,
   PRIMARY KEY ([migration_set], [name])
);
CREATE TABLE "creatures" (
   [id] INTEGER PRIMARY KEY,
   [name] TEXT,
   [species] TEXT,
   [weight] FLOAT,
   [age] INTEGER,
   [shoe_size] INTEGER
);

simonw added a commit that referenced this issue Oct 27, 2023
@simonw simonw closed this as completed Oct 27, 2023
simonw added a commit that referenced this issue Oct 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants