Skip to content

Commit ffa14e5

Browse files
committed
Fix sqlite mig issue
1 parent ef25712 commit ffa14e5

10 files changed

Lines changed: 307 additions & 14 deletions

Cargo.lock

Lines changed: 10 additions & 10 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

crates/vespertide-planner/src/apply.rs

Lines changed: 161 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,12 +16,22 @@ pub fn apply_action(
1616
if schema.iter().any(|t| t.name == *table) {
1717
return Err(PlannerError::TableExists(table.clone()));
1818
}
19-
schema.push(TableDef {
19+
let table_def = TableDef {
2020
name: table.clone(),
2121
description: None,
2222
columns: columns.clone(),
2323
constraints: constraints.clone(),
24-
});
24+
};
25+
// Normalize to promote inline constraints (unique, index, foreign_key, primary_key)
26+
// to table-level TableConstraint entries. This is critical for SQLite which needs
27+
// to know about constraints when dropping columns.
28+
let normalized = table_def.normalize().map_err(|e| {
29+
PlannerError::TableValidation(format!(
30+
"Failed to normalize table '{}': {}",
31+
table, e
32+
))
33+
})?;
34+
schema.push(normalized);
2535
Ok(())
2636
}
2737
MigrationAction::DeleteTable { table } => {
@@ -49,6 +59,15 @@ pub fn apply_action(
4959
))
5060
} else {
5161
tbl.columns.push((**column).clone());
62+
// Re-normalize to promote any inline constraints on the new column
63+
// to table-level TableConstraint entries.
64+
let normalized = tbl.clone().normalize().map_err(|e| {
65+
PlannerError::TableValidation(format!(
66+
"Failed to normalize table '{}' after adding column '{}': {}",
67+
table, column.name, e
68+
))
69+
})?;
70+
*tbl = normalized;
5271
Ok(())
5372
}
5473
}
@@ -1142,6 +1161,146 @@ mod tests {
11421161
assert!(schema[0].columns[0].index.is_none());
11431162
}
11441163

1164+
// Tests for CreateTable normalizing inline constraints
1165+
#[test]
1166+
fn create_table_normalizes_inline_unique() {
1167+
let mut col_with_unique = col("email", ColumnType::Simple(SimpleColumnType::Text));
1168+
col_with_unique.unique = Some(vespertide_core::StrOrBoolOrArray::Bool(true));
1169+
1170+
let mut schema = vec![];
1171+
apply_action(
1172+
&mut schema,
1173+
&MigrationAction::CreateTable {
1174+
table: "users".into(),
1175+
columns: vec![col_with_unique],
1176+
constraints: vec![],
1177+
},
1178+
)
1179+
.unwrap();
1180+
1181+
// Inline unique: true should be normalized to a TableConstraint::Unique
1182+
assert!(
1183+
schema[0].constraints.iter().any(
1184+
|c| matches!(c, TableConstraint::Unique { columns, .. } if columns == &["email"])
1185+
),
1186+
"Expected a Unique constraint on 'email', got: {:?}",
1187+
schema[0].constraints
1188+
);
1189+
}
1190+
1191+
#[test]
1192+
fn create_table_normalizes_inline_index() {
1193+
let mut col_with_index = col("email", ColumnType::Simple(SimpleColumnType::Text));
1194+
col_with_index.index = Some(vespertide_core::StrOrBoolOrArray::Bool(true));
1195+
1196+
let mut schema = vec![];
1197+
apply_action(
1198+
&mut schema,
1199+
&MigrationAction::CreateTable {
1200+
table: "users".into(),
1201+
columns: vec![col_with_index],
1202+
constraints: vec![],
1203+
},
1204+
)
1205+
.unwrap();
1206+
1207+
// Inline index: true should be normalized to a TableConstraint::Index
1208+
assert!(
1209+
schema[0].constraints.iter().any(
1210+
|c| matches!(c, TableConstraint::Index { columns, .. } if columns == &["email"])
1211+
),
1212+
"Expected an Index constraint on 'email', got: {:?}",
1213+
schema[0].constraints
1214+
);
1215+
}
1216+
1217+
#[test]
1218+
fn create_table_normalizes_inline_primary_key() {
1219+
let mut col_with_pk = col("id", ColumnType::Simple(SimpleColumnType::Integer));
1220+
col_with_pk.primary_key =
1221+
Some(vespertide_core::schema::primary_key::PrimaryKeySyntax::Bool(true));
1222+
1223+
let mut schema = vec![];
1224+
apply_action(
1225+
&mut schema,
1226+
&MigrationAction::CreateTable {
1227+
table: "users".into(),
1228+
columns: vec![col_with_pk],
1229+
constraints: vec![],
1230+
},
1231+
)
1232+
.unwrap();
1233+
1234+
assert!(
1235+
schema[0].constraints.iter().any(
1236+
|c| matches!(c, TableConstraint::PrimaryKey { columns, .. } if columns == &["id"])
1237+
),
1238+
"Expected a PrimaryKey constraint on 'id', got: {:?}",
1239+
schema[0].constraints
1240+
);
1241+
}
1242+
1243+
// Tests for AddColumn normalizing inline constraints
1244+
#[test]
1245+
fn add_column_normalizes_inline_unique() {
1246+
let mut schema = vec![table(
1247+
"users",
1248+
vec![col("id", ColumnType::Simple(SimpleColumnType::Integer))],
1249+
vec![],
1250+
)];
1251+
1252+
let mut col_with_unique = col("email", ColumnType::Simple(SimpleColumnType::Text));
1253+
col_with_unique.unique = Some(vespertide_core::StrOrBoolOrArray::Bool(true));
1254+
1255+
apply_action(
1256+
&mut schema,
1257+
&MigrationAction::AddColumn {
1258+
table: "users".into(),
1259+
column: Box::new(col_with_unique),
1260+
fill_with: None,
1261+
},
1262+
)
1263+
.unwrap();
1264+
1265+
assert!(
1266+
schema[0].constraints.iter().any(
1267+
|c| matches!(c, TableConstraint::Unique { columns, .. } if columns == &["email"])
1268+
),
1269+
"Expected a Unique constraint on 'email' after AddColumn, got: {:?}",
1270+
schema[0].constraints
1271+
);
1272+
}
1273+
1274+
#[test]
1275+
fn add_column_normalizes_inline_index() {
1276+
let mut schema = vec![table(
1277+
"users",
1278+
vec![col("id", ColumnType::Simple(SimpleColumnType::Integer))],
1279+
vec![],
1280+
)];
1281+
1282+
let mut col_with_index = col("email", ColumnType::Simple(SimpleColumnType::Text));
1283+
col_with_index.index = Some(vespertide_core::StrOrBoolOrArray::Bool(true));
1284+
1285+
apply_action(
1286+
&mut schema,
1287+
&MigrationAction::AddColumn {
1288+
table: "users".into(),
1289+
column: Box::new(col_with_index),
1290+
fill_with: None,
1291+
},
1292+
)
1293+
.unwrap();
1294+
1295+
assert!(
1296+
schema[0].constraints.iter().any(
1297+
|c| matches!(c, TableConstraint::Index { columns, .. } if columns == &["email"])
1298+
),
1299+
"Expected an Index constraint on 'email' after AddColumn, got: {:?}",
1300+
schema[0].constraints
1301+
);
1302+
}
1303+
11451304
// Tests for ModifyColumnNullable
11461305
#[test]
11471306
fn apply_modify_column_nullable_success() {

crates/vespertide-query/src/builder.rs

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ pub fn build_plan_queries(
4848
mod tests {
4949
use super::*;
5050
use crate::sql::DatabaseBackend;
51+
use insta::{assert_snapshot, with_settings};
5152
use rstest::rstest;
5253
use vespertide_core::{
5354
ColumnDef, ColumnType, MigrationAction, MigrationPlan, SimpleColumnType,
@@ -117,6 +118,107 @@ mod tests {
117118
);
118119
}
119120

121+
fn build_sql_snapshot(result: &[BuiltQuery], backend: DatabaseBackend) -> String {
122+
result
123+
.iter()
124+
.map(|q| q.build(backend))
125+
.collect::<Vec<_>>()
126+
.join(";\n")
127+
}
128+
129+
/// Regression test: SQLite must emit DROP INDEX before DROP COLUMN when
130+
/// the column was created with inline `unique: true` (no explicit table constraint).
131+
/// Previously, apply_action didn't normalize inline constraints, so the evolving
132+
/// schema had empty constraints and SQLite's DROP COLUMN failed.
133+
#[rstest]
134+
#[case::postgres("postgres", DatabaseBackend::Postgres)]
135+
#[case::mysql("mysql", DatabaseBackend::MySql)]
136+
#[case::sqlite("sqlite", DatabaseBackend::Sqlite)]
137+
fn test_delete_column_after_create_table_with_inline_unique(
138+
#[case] title: &str,
139+
#[case] backend: DatabaseBackend,
140+
) {
141+
let mut col_with_unique = col("gift_code", ColumnType::Simple(SimpleColumnType::Text));
142+
col_with_unique.unique = Some(vespertide_core::StrOrBoolOrArray::Bool(true));
143+
144+
let plan = MigrationPlan {
145+
comment: None,
146+
created_at: None,
147+
version: 1,
148+
actions: vec![
149+
MigrationAction::CreateTable {
150+
table: "gift".into(),
151+
columns: vec![
152+
col("id", ColumnType::Simple(SimpleColumnType::Integer)),
153+
col_with_unique,
154+
],
155+
constraints: vec![], // No explicit constraints - only inline unique: true
156+
},
157+
MigrationAction::DeleteColumn {
158+
table: "gift".into(),
159+
column: "gift_code".into(),
160+
},
161+
],
162+
};
163+
164+
let result = build_plan_queries(&plan, &[]).unwrap();
165+
let queries = match backend {
166+
DatabaseBackend::Postgres => &result[1].postgres,
167+
DatabaseBackend::MySql => &result[1].mysql,
168+
DatabaseBackend::Sqlite => &result[1].sqlite,
169+
};
170+
let sql = build_sql_snapshot(queries, backend);
171+
172+
with_settings!({ snapshot_suffix => format!("inline_unique_{}", title) }, {
173+
assert_snapshot!(sql);
174+
});
175+
}
176+
177+
/// Same regression test for inline `index: true`.
178+
#[rstest]
179+
#[case::postgres("postgres", DatabaseBackend::Postgres)]
180+
#[case::mysql("mysql", DatabaseBackend::MySql)]
181+
#[case::sqlite("sqlite", DatabaseBackend::Sqlite)]
182+
fn test_delete_column_after_create_table_with_inline_index(
183+
#[case] title: &str,
184+
#[case] backend: DatabaseBackend,
185+
) {
186+
let mut col_with_index = col("email", ColumnType::Simple(SimpleColumnType::Text));
187+
col_with_index.index = Some(vespertide_core::StrOrBoolOrArray::Bool(true));
188+
189+
let plan = MigrationPlan {
190+
comment: None,
191+
created_at: None,
192+
version: 1,
193+
actions: vec![
194+
MigrationAction::CreateTable {
195+
table: "users".into(),
196+
columns: vec![
197+
col("id", ColumnType::Simple(SimpleColumnType::Integer)),
198+
col_with_index,
199+
],
200+
constraints: vec![],
201+
},
202+
MigrationAction::DeleteColumn {
203+
table: "users".into(),
204+
column: "email".into(),
205+
},
206+
],
207+
};
208+
209+
let result = build_plan_queries(&plan, &[]).unwrap();
210+
let queries = match backend {
211+
DatabaseBackend::Postgres => &result[1].postgres,
212+
DatabaseBackend::MySql => &result[1].mysql,
213+
DatabaseBackend::Sqlite => &result[1].sqlite,
214+
};
215+
let sql = build_sql_snapshot(queries, backend);
216+
217+
with_settings!({ snapshot_suffix => format!("inline_index_{}", title) }, {
218+
assert_snapshot!(sql);
219+
});
220+
}
221+
120222
#[test]
121223
fn test_build_plan_queries_sql_content() {
122224
let plan = MigrationPlan {
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
source: crates/vespertide-query/src/builder.rs
3+
expression: sql
4+
---
5+
ALTER TABLE `users` DROP COLUMN `email`

0 commit comments

Comments
 (0)