Fix workspace migration failure (#36911)

This fixes a regression on nightly introduced in
https://github.com/zed-industries/zed/pull/36714

Release Notes:

- N/A
This commit is contained in:
Max Brunsfeld 2025-08-25 17:27:52 -07:00 committed by GitHub
parent f8667a8379
commit d43df9e841
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
16 changed files with 582 additions and 495 deletions

View file

@ -1,8 +1,12 @@
use crate::connection::Connection;
pub trait Domain: 'static {
fn name() -> &'static str;
fn migrations() -> &'static [&'static str];
const NAME: &str;
const MIGRATIONS: &[&str];
fn should_allow_migration_change(_index: usize, _old: &str, _new: &str) -> bool {
false
}
}
pub trait Migrator: 'static {
@ -17,7 +21,11 @@ impl Migrator for () {
impl<D: Domain> Migrator for D {
fn migrate(connection: &Connection) -> anyhow::Result<()> {
connection.migrate(Self::name(), Self::migrations())
connection.migrate(
Self::NAME,
Self::MIGRATIONS,
Self::should_allow_migration_change,
)
}
}

View file

@ -34,7 +34,12 @@ impl Connection {
/// Note: Unlike everything else in SQLez, migrations are run eagerly, without first
/// preparing the SQL statements. This makes it possible to do multi-statement schema
/// updates in a single string without running into prepare errors.
pub fn migrate(&self, domain: &'static str, migrations: &[&'static str]) -> Result<()> {
pub fn migrate(
&self,
domain: &'static str,
migrations: &[&'static str],
mut should_allow_migration_change: impl FnMut(usize, &str, &str) -> bool,
) -> Result<()> {
self.with_savepoint("migrating", || {
// Setup the migrations table unconditionally
self.exec(indoc! {"
@ -65,9 +70,14 @@ impl Connection {
&sqlformat::QueryParams::None,
Default::default(),
);
if completed_migration == migration {
if completed_migration == migration
|| migration.trim().starts_with("-- ALLOW_MIGRATION_CHANGE")
{
// Migration already run. Continue
continue;
} else if should_allow_migration_change(index, &completed_migration, &migration)
{
continue;
} else {
anyhow::bail!(formatdoc! {"
Migration changed for {domain} at step {index}
@ -108,6 +118,7 @@ mod test {
a TEXT,
b TEXT
)"}],
disallow_migration_change,
)
.unwrap();
@ -136,6 +147,7 @@ mod test {
d TEXT
)"},
],
disallow_migration_change,
)
.unwrap();
@ -214,7 +226,11 @@ mod test {
// Run the migration verifying that the row got dropped
connection
.migrate("test", &["DELETE FROM test_table"])
.migrate(
"test",
&["DELETE FROM test_table"],
disallow_migration_change,
)
.unwrap();
assert_eq!(
connection
@ -232,7 +248,11 @@ mod test {
// Run the same migration again and verify that the table was left unchanged
connection
.migrate("test", &["DELETE FROM test_table"])
.migrate(
"test",
&["DELETE FROM test_table"],
disallow_migration_change,
)
.unwrap();
assert_eq!(
connection
@ -252,27 +272,28 @@ mod test {
.migrate(
"test migration",
&[
indoc! {"
CREATE TABLE test (
col INTEGER
)"},
indoc! {"
INSERT INTO test (col) VALUES (1)"},
"CREATE TABLE test (col INTEGER)",
"INSERT INTO test (col) VALUES (1)",
],
disallow_migration_change,
)
.unwrap();
let mut migration_changed = false;
// Create another migration with the same domain but different steps
let second_migration_result = connection.migrate(
"test migration",
&[
indoc! {"
CREATE TABLE test (
color INTEGER
)"},
indoc! {"
INSERT INTO test (color) VALUES (1)"},
"CREATE TABLE test (color INTEGER )",
"INSERT INTO test (color) VALUES (1)",
],
|_, old, new| {
assert_eq!(old, "CREATE TABLE test (col INTEGER)");
assert_eq!(new, "CREATE TABLE test (color INTEGER)");
migration_changed = true;
false
},
);
// Verify new migration returns error when run
@ -284,7 +305,11 @@ mod test {
let connection = Connection::open_memory(Some("test_create_alter_drop"));
connection
.migrate("first_migration", &["CREATE TABLE table1(a TEXT) STRICT;"])
.migrate(
"first_migration",
&["CREATE TABLE table1(a TEXT) STRICT;"],
disallow_migration_change,
)
.unwrap();
connection
@ -305,6 +330,7 @@ mod test {
ALTER TABLE table2 RENAME TO table1;
"}],
disallow_migration_change,
)
.unwrap();
@ -312,4 +338,8 @@ mod test {
assert_eq!(res, "test text");
}
fn disallow_migration_change(_: usize, _: &str, _: &str) -> bool {
false
}
}

View file

@ -278,12 +278,8 @@ mod test {
enum TestDomain {}
impl Domain for TestDomain {
fn name() -> &'static str {
"test"
}
fn migrations() -> &'static [&'static str] {
&["CREATE TABLE test(col1 TEXT, col2 TEXT) STRICT;"]
}
const NAME: &str = "test";
const MIGRATIONS: &[&str] = &["CREATE TABLE test(col1 TEXT, col2 TEXT) STRICT;"];
}
for _ in 0..100 {
@ -312,12 +308,9 @@ mod test {
fn wild_zed_lost_failure() {
enum TestWorkspace {}
impl Domain for TestWorkspace {
fn name() -> &'static str {
"workspace"
}
const NAME: &str = "workspace";
fn migrations() -> &'static [&'static str] {
&["
const MIGRATIONS: &[&str] = &["
CREATE TABLE workspaces(
workspace_id INTEGER PRIMARY KEY,
dock_visible INTEGER, -- Boolean
@ -336,8 +329,7 @@ mod test {
ON DELETE CASCADE
ON UPDATE CASCADE
) STRICT;
"]
}
"];
}
let builder =