diff --git a/.changepacks/changepack_log_aJ7xBfAqrv5H3mB2LGZpN.json b/.changepacks/changepack_log_aJ7xBfAqrv5H3mB2LGZpN.json new file mode 100644 index 0000000..2f34e21 --- /dev/null +++ b/.changepacks/changepack_log_aJ7xBfAqrv5H3mB2LGZpN.json @@ -0,0 +1 @@ +{"changes":{"crates/vespertide-config/Cargo.toml":"Patch","crates/vespertide-naming/Cargo.toml":"Patch","crates/vespertide-macro/Cargo.toml":"Patch","crates/vespertide/Cargo.toml":"Patch","crates/vespertide-query/Cargo.toml":"Patch","crates/vespertide-planner/Cargo.toml":"Patch","crates/vespertide-cli/Cargo.toml":"Patch","crates/vespertide-loader/Cargo.toml":"Patch","crates/vespertide-exporter/Cargo.toml":"Patch","crates/vespertide-core/Cargo.toml":"Patch"},"note":"Fix migration script Rm special column at sqlite","date":"2026-01-23T08:18:40.279346500Z"} \ No newline at end of file diff --git a/Cargo.lock b/Cargo.lock index a2ec8ac..40b0bf2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2995,7 +2995,7 @@ checksum = "0b928f33d975fc6ad9f86c8f283853ad26bdd5b10b7f1542aa2fa15e2289105a" [[package]] name = "vespertide" -version = "0.1.33" +version = "0.1.34" dependencies = [ "vespertide-core", "vespertide-macro", @@ -3003,7 +3003,7 @@ dependencies = [ [[package]] name = "vespertide-cli" -version = "0.1.33" +version = "0.1.34" dependencies = [ "anyhow", "assert_cmd", @@ -3028,7 +3028,7 @@ dependencies = [ [[package]] name = "vespertide-config" -version = "0.1.33" +version = "0.1.34" dependencies = [ "clap", "schemars", @@ -3038,7 +3038,7 @@ dependencies = [ [[package]] name = "vespertide-core" -version = "0.1.33" +version = "0.1.34" dependencies = [ "rstest", "schemars", @@ -3050,7 +3050,7 @@ dependencies = [ [[package]] name = "vespertide-exporter" -version = "0.1.33" +version = "0.1.34" dependencies = [ "insta", "rstest", @@ -3061,7 +3061,7 @@ dependencies = [ [[package]] name = "vespertide-loader" -version = "0.1.33" +version = "0.1.34" dependencies = [ "anyhow", "rstest", @@ -3076,7 +3076,7 @@ dependencies = [ [[package]] name = "vespertide-macro" -version = "0.1.33" +version = "0.1.34" dependencies = [ "proc-macro2", "quote", @@ -3093,11 +3093,11 @@ dependencies = [ [[package]] name = "vespertide-naming" -version = "0.1.33" +version = "0.1.34" [[package]] name = "vespertide-planner" -version = "0.1.33" +version = "0.1.34" dependencies = [ "insta", "rstest", @@ -3108,7 +3108,7 @@ dependencies = [ [[package]] name = "vespertide-query" -version = "0.1.33" +version = "0.1.34" dependencies = [ "insta", "rstest", diff --git a/crates/vespertide-query/src/sql/delete_column.rs b/crates/vespertide-query/src/sql/delete_column.rs index 230da0a..7d03ee4 100644 --- a/crates/vespertide-query/src/sql/delete_column.rs +++ b/crates/vespertide-query/src/sql/delete_column.rs @@ -1,19 +1,80 @@ -use sea_query::{Alias, Table}; +use sea_query::{Alias, Index, Query, Table}; -use vespertide_core::ColumnType; +use vespertide_core::{ColumnType, TableConstraint, TableDef}; +use super::create_table::build_create_table_for_backend; use super::helpers::build_drop_enum_type_sql; -use super::types::BuiltQuery; +use super::rename_table::build_rename_table; +use super::types::{BuiltQuery, DatabaseBackend}; /// Build SQL to delete a column, optionally with DROP TYPE for enum columns (PostgreSQL) +/// +/// For SQLite: Handles constraint removal before dropping the column: +/// - Unique/Index constraints: Dropped via DROP INDEX +/// - ForeignKey/PrimaryKey constraints: Uses temp table approach (recreate table without column) +/// +/// SQLite doesn't cascade constraint drops when a column is dropped. pub fn build_delete_column( + backend: &DatabaseBackend, table: &str, column: &str, column_type: Option<&ColumnType>, + current_schema: &[TableDef], ) -> Vec { let mut stmts = Vec::new(); - // Drop the column first + // SQLite: Check if we need special handling for constraints + if *backend == DatabaseBackend::Sqlite + && let Some(table_def) = current_schema.iter().find(|t| t.name == table) + { + // Check if the column has FK or PK constraints (requires temp table approach) + let has_fk_or_pk = table_def.constraints.iter().any(|c| { + c.columns().iter().any(|col| col == column) + && matches!( + c, + TableConstraint::ForeignKey { .. } | TableConstraint::PrimaryKey { .. } + ) + }); + + if has_fk_or_pk { + // Use temp table approach for FK/PK constraints + return build_delete_column_sqlite_temp_table(table, column, table_def, column_type); + } + + // For Unique/Index constraints, just drop the index first + for constraint in &table_def.constraints { + if constraint.columns().iter().any(|c| c == column) { + match constraint { + TableConstraint::Unique { name, columns } => { + let index_name = vespertide_naming::build_unique_constraint_name( + table, + columns, + name.as_deref(), + ); + let drop_idx = Index::drop() + .name(&index_name) + .table(Alias::new(table)) + .to_owned(); + stmts.push(BuiltQuery::DropIndex(Box::new(drop_idx))); + } + TableConstraint::Index { name, columns } => { + let index_name = + vespertide_naming::build_index_name(table, columns, name.as_deref()); + let drop_idx = Index::drop() + .name(&index_name) + .table(Alias::new(table)) + .to_owned(); + stmts.push(BuiltQuery::DropIndex(Box::new(drop_idx))); + } + // PK/FK constraints trigger temp table approach earlier; Check returns empty columns. + // This arm is defensive for future constraint types. + _ => {} + } + } + } + } + + // Drop the column let stmt = Table::alter() .table(Alias::new(table)) .drop_column(Alias::new(column)) @@ -31,13 +92,118 @@ pub fn build_delete_column( stmts } +/// SQLite temp table approach for deleting a column that has FK or PK constraints. +/// +/// Steps: +/// 1. Create temp table without the column (and without constraints referencing it) +/// 2. Copy data (excluding the deleted column) +/// 3. Drop original table +/// 4. Rename temp table to original name +/// 5. Recreate indexes that don't reference the deleted column +fn build_delete_column_sqlite_temp_table( + table: &str, + column: &str, + table_def: &TableDef, + column_type: Option<&ColumnType>, +) -> Vec { + let mut stmts = Vec::new(); + let temp_table = format!("{}_temp", table); + + // Build new columns list without the deleted column + let new_columns: Vec<_> = table_def + .columns + .iter() + .filter(|c| c.name != column) + .cloned() + .collect(); + + // Build new constraints list without constraints referencing the deleted column + let new_constraints: Vec<_> = table_def + .constraints + .iter() + .filter(|c| !c.columns().iter().any(|col| col == column)) + .cloned() + .collect(); + + // 1. Create temp table without the column + let create_temp_table = build_create_table_for_backend( + &DatabaseBackend::Sqlite, + &temp_table, + &new_columns, + &new_constraints, + ); + stmts.push(BuiltQuery::CreateTable(Box::new(create_temp_table))); + + // 2. Copy data (excluding the deleted column) + let column_aliases: Vec = new_columns.iter().map(|c| Alias::new(&c.name)).collect(); + let mut select_query = Query::select(); + for col_alias in &column_aliases { + select_query = select_query.column(col_alias.clone()).to_owned(); + } + select_query = select_query.from(Alias::new(table)).to_owned(); + + let insert_stmt = Query::insert() + .into_table(Alias::new(&temp_table)) + .columns(column_aliases.clone()) + .select_from(select_query) + .unwrap() + .to_owned(); + stmts.push(BuiltQuery::Insert(Box::new(insert_stmt))); + + // 3. Drop original table + let drop_table = Table::drop().table(Alias::new(table)).to_owned(); + stmts.push(BuiltQuery::DropTable(Box::new(drop_table))); + + // 4. Rename temp table to original name + stmts.push(build_rename_table(&temp_table, table)); + + // 5. Recreate indexes that don't reference the deleted column + for constraint in &new_constraints { + if let TableConstraint::Index { name, columns } = constraint { + let index_name = vespertide_naming::build_index_name(table, columns, name.as_deref()); + let mut idx_stmt = Index::create(); + idx_stmt = idx_stmt + .name(&index_name) + .table(Alias::new(table)) + .to_owned(); + for col_name in columns { + idx_stmt = idx_stmt.col(Alias::new(col_name)).to_owned(); + } + stmts.push(BuiltQuery::CreateIndex(Box::new(idx_stmt))); + } + } + + // If column type is an enum, drop the type after (PostgreSQL only, but include for completeness) + if let Some(col_type) = column_type + && let Some(drop_type_sql) = build_drop_enum_type_sql(table, col_type) + { + stmts.push(BuiltQuery::Raw(drop_type_sql)); + } + + stmts +} + #[cfg(test)] mod tests { use super::*; use crate::sql::types::DatabaseBackend; use insta::{assert_snapshot, with_settings}; use rstest::rstest; - use vespertide_core::{ComplexColumnType, SimpleColumnType}; + use vespertide_core::{ColumnDef, ComplexColumnType, SimpleColumnType}; + + fn col(name: &str, ty: ColumnType) -> ColumnDef { + ColumnDef { + name: name.to_string(), + r#type: ty, + nullable: true, + default: None, + comment: None, + primary_key: None, + unique: None, + index: None, + foreign_key: None, + } + } #[rstest] #[case::delete_column_postgres( @@ -60,7 +226,7 @@ mod tests { #[case] backend: DatabaseBackend, #[case] expected: &[&str], ) { - let result = build_delete_column("users", "email", None); + let result = build_delete_column(&backend, "users", "email", None, &[]); let sql = result[0].build(backend); for exp in expected { assert!( @@ -84,7 +250,13 @@ mod tests { name: "status".into(), values: EnumValues::String(vec!["active".into(), "inactive".into()]), }); - let result = build_delete_column("users", "status", Some(&enum_type)); + let result = build_delete_column( + &DatabaseBackend::Postgres, + "users", + "status", + Some(&enum_type), + &[], + ); // Should have 2 statements: ALTER TABLE and DROP TYPE assert_eq!(result.len(), 2); @@ -103,9 +275,761 @@ mod tests { #[test] fn test_delete_non_enum_column_no_drop_type() { let text_type = ColumnType::Simple(SimpleColumnType::Text); - let result = build_delete_column("users", "name", Some(&text_type)); + let result = build_delete_column( + &DatabaseBackend::Postgres, + "users", + "name", + Some(&text_type), + &[], + ); // Should only have 1 statement: ALTER TABLE assert_eq!(result.len(), 1); } + + #[test] + fn test_delete_column_sqlite_drops_unique_constraint_first() { + // SQLite should drop unique constraint index before dropping the column + let schema = vec![TableDef { + name: "gift".into(), + description: None, + columns: vec![ + col("id", ColumnType::Simple(SimpleColumnType::Integer)), + col("gift_code", ColumnType::Simple(SimpleColumnType::Text)), + ], + constraints: vec![TableConstraint::Unique { + name: None, + columns: vec!["gift_code".into()], + }], + }]; + + let result = + build_delete_column(&DatabaseBackend::Sqlite, "gift", "gift_code", None, &schema); + + // Should have 2 statements: DROP INDEX then ALTER TABLE DROP COLUMN + assert_eq!(result.len(), 2); + + let drop_index_sql = result[0].build(DatabaseBackend::Sqlite); + assert!( + drop_index_sql.contains("DROP INDEX"), + "Expected DROP INDEX, got: {}", + drop_index_sql + ); + assert!( + drop_index_sql.contains("uq_gift__gift_code"), + "Expected index name uq_gift__gift_code, got: {}", + drop_index_sql + ); + + let drop_column_sql = result[1].build(DatabaseBackend::Sqlite); + assert!( + drop_column_sql.contains("DROP COLUMN"), + "Expected DROP COLUMN, got: {}", + drop_column_sql + ); + } + + #[test] + fn test_delete_column_sqlite_drops_index_constraint_first() { + // SQLite should drop index before dropping the column + let schema = vec![TableDef { + name: "users".into(), + description: None, + columns: vec![ + col("id", ColumnType::Simple(SimpleColumnType::Integer)), + col("email", ColumnType::Simple(SimpleColumnType::Text)), + ], + constraints: vec![TableConstraint::Index { + name: None, + columns: vec!["email".into()], + }], + }]; + + let result = build_delete_column(&DatabaseBackend::Sqlite, "users", "email", None, &schema); + + // Should have 2 statements: DROP INDEX then ALTER TABLE DROP COLUMN + assert_eq!(result.len(), 2); + + let drop_index_sql = result[0].build(DatabaseBackend::Sqlite); + assert!(drop_index_sql.contains("DROP INDEX")); + assert!(drop_index_sql.contains("ix_users__email")); + + let drop_column_sql = result[1].build(DatabaseBackend::Sqlite); + assert!(drop_column_sql.contains("DROP COLUMN")); + } + + #[test] + fn test_delete_column_postgres_does_not_drop_constraints() { + // PostgreSQL cascades constraint drops, so we shouldn't emit extra DROP INDEX + let schema = vec![TableDef { + name: "gift".into(), + description: None, + columns: vec![ + col("id", ColumnType::Simple(SimpleColumnType::Integer)), + col("gift_code", ColumnType::Simple(SimpleColumnType::Text)), + ], + constraints: vec![TableConstraint::Unique { + name: None, + columns: vec!["gift_code".into()], + }], + }]; + + let result = build_delete_column( + &DatabaseBackend::Postgres, + "gift", + "gift_code", + None, + &schema, + ); + + // Should have only 1 statement: ALTER TABLE DROP COLUMN + assert_eq!(result.len(), 1); + + let drop_column_sql = result[0].build(DatabaseBackend::Postgres); + assert!(drop_column_sql.contains("DROP COLUMN")); + } + + #[test] + fn test_delete_column_sqlite_with_named_unique_constraint() { + // Test with a named unique constraint + let schema = vec![TableDef { + name: "gift".into(), + description: None, + columns: vec![ + col("id", ColumnType::Simple(SimpleColumnType::Integer)), + col("gift_code", ColumnType::Simple(SimpleColumnType::Text)), + ], + constraints: vec![TableConstraint::Unique { + name: Some("gift_code".into()), + columns: vec!["gift_code".into()], + }], + }]; + + let result = + build_delete_column(&DatabaseBackend::Sqlite, "gift", "gift_code", None, &schema); + + assert_eq!(result.len(), 2); + + let drop_index_sql = result[0].build(DatabaseBackend::Sqlite); + // Named constraint: uq_gift__gift_code (name is "gift_code") + assert!( + drop_index_sql.contains("uq_gift__gift_code"), + "Expected uq_gift__gift_code, got: {}", + drop_index_sql + ); + } + + #[test] + fn test_delete_column_sqlite_with_fk_uses_temp_table() { + // SQLite should use temp table approach when deleting a column with FK constraint + let schema = vec![TableDef { + name: "gift".into(), + description: None, + columns: vec![ + col("id", ColumnType::Simple(SimpleColumnType::Integer)), + col("sender_id", ColumnType::Simple(SimpleColumnType::BigInt)), + col("message", ColumnType::Simple(SimpleColumnType::Text)), + ], + constraints: vec![TableConstraint::ForeignKey { + name: None, + columns: vec!["sender_id".into()], + ref_table: "user".into(), + ref_columns: vec!["id".into()], + on_delete: None, + on_update: None, + }], + }]; + + let result = + build_delete_column(&DatabaseBackend::Sqlite, "gift", "sender_id", None, &schema); + + // Should use temp table approach: + // 1. CREATE TABLE gift_temp (without sender_id column) + // 2. INSERT INTO gift_temp SELECT ... FROM gift + // 3. DROP TABLE gift + // 4. ALTER TABLE gift_temp RENAME TO gift + assert!( + result.len() >= 4, + "Expected at least 4 statements for temp table approach, got: {}", + result.len() + ); + + let all_sql: Vec = result + .iter() + .map(|q| q.build(DatabaseBackend::Sqlite)) + .collect(); + let combined_sql = all_sql.join("\n"); + + // Verify temp table creation + assert!( + combined_sql.contains("CREATE TABLE") && combined_sql.contains("gift_temp"), + "Expected CREATE TABLE gift_temp, got: {}", + combined_sql + ); + + // Verify the new table doesn't have sender_id column + assert!( + !combined_sql.contains("\"sender_id\"") || combined_sql.contains("DROP TABLE"), + "New table should not contain sender_id column" + ); + + // Verify data copy (INSERT ... SELECT) + assert!( + combined_sql.contains("INSERT INTO"), + "Expected INSERT INTO for data copy, got: {}", + combined_sql + ); + + // Verify original table drop + assert!( + combined_sql.contains("DROP TABLE") && combined_sql.contains("\"gift\""), + "Expected DROP TABLE gift, got: {}", + combined_sql + ); + + // Verify rename + assert!( + combined_sql.contains("RENAME"), + "Expected RENAME for temp table, got: {}", + combined_sql + ); + } + + #[test] + fn test_delete_column_sqlite_with_fk_preserves_other_columns() { + // When using temp table approach, other columns should be preserved + let schema = vec![TableDef { + name: "gift".into(), + description: None, + columns: vec![ + col("id", ColumnType::Simple(SimpleColumnType::Integer)), + col("sender_id", ColumnType::Simple(SimpleColumnType::BigInt)), + col("receiver_id", ColumnType::Simple(SimpleColumnType::BigInt)), + col("message", ColumnType::Simple(SimpleColumnType::Text)), + ], + constraints: vec![ + TableConstraint::ForeignKey { + name: None, + columns: vec!["sender_id".into()], + ref_table: "user".into(), + ref_columns: vec!["id".into()], + on_delete: None, + on_update: None, + }, + TableConstraint::Index { + name: None, + columns: vec!["receiver_id".into()], + }, + ], + }]; + + let result = + build_delete_column(&DatabaseBackend::Sqlite, "gift", "sender_id", None, &schema); + + let all_sql: Vec = result + .iter() + .map(|q| q.build(DatabaseBackend::Sqlite)) + .collect(); + let combined_sql = all_sql.join("\n"); + + // Should preserve other columns + assert!(combined_sql.contains("\"id\""), "Should preserve id column"); + assert!( + combined_sql.contains("\"receiver_id\""), + "Should preserve receiver_id column" + ); + assert!( + combined_sql.contains("\"message\""), + "Should preserve message column" + ); + + // Should recreate index on receiver_id (not on sender_id) + assert!( + combined_sql.contains("CREATE INDEX") && combined_sql.contains("ix_gift__receiver_id"), + "Should recreate index on receiver_id, got: {}", + combined_sql + ); + } + + #[test] + fn test_delete_column_postgres_with_fk_does_not_use_temp_table() { + // PostgreSQL should NOT use temp table - just drop column directly + let schema = vec![TableDef { + name: "gift".into(), + description: None, + columns: vec![ + col("id", ColumnType::Simple(SimpleColumnType::Integer)), + col("sender_id", ColumnType::Simple(SimpleColumnType::BigInt)), + ], + constraints: vec![TableConstraint::ForeignKey { + name: None, + columns: vec!["sender_id".into()], + ref_table: "user".into(), + ref_columns: vec!["id".into()], + on_delete: None, + on_update: None, + }], + }]; + + let result = build_delete_column( + &DatabaseBackend::Postgres, + "gift", + "sender_id", + None, + &schema, + ); + + // Should have only 1 statement: ALTER TABLE DROP COLUMN + assert_eq!( + result.len(), + 1, + "PostgreSQL should only have 1 statement, got: {}", + result.len() + ); + + let sql = result[0].build(DatabaseBackend::Postgres); + assert!( + sql.contains("DROP COLUMN"), + "Expected DROP COLUMN, got: {}", + sql + ); + assert!( + !sql.contains("gift_temp"), + "PostgreSQL should not use temp table" + ); + } + + #[test] + fn test_delete_column_sqlite_with_pk_uses_temp_table() { + // SQLite should use temp table approach when deleting a column that's part of PK + let schema = vec![TableDef { + name: "order_items".into(), + description: None, + columns: vec![ + col("order_id", ColumnType::Simple(SimpleColumnType::Integer)), + col("product_id", ColumnType::Simple(SimpleColumnType::Integer)), + col("quantity", ColumnType::Simple(SimpleColumnType::Integer)), + ], + constraints: vec![TableConstraint::PrimaryKey { + auto_increment: false, + columns: vec!["order_id".into(), "product_id".into()], + }], + }]; + + let result = build_delete_column( + &DatabaseBackend::Sqlite, + "order_items", + "product_id", + None, + &schema, + ); + + // Should use temp table approach + assert!( + result.len() >= 4, + "Expected at least 4 statements for temp table approach, got: {}", + result.len() + ); + + let all_sql: Vec = result + .iter() + .map(|q| q.build(DatabaseBackend::Sqlite)) + .collect(); + let combined_sql = all_sql.join("\n"); + + assert!( + combined_sql.contains("order_items_temp"), + "Should use temp table approach for PK column deletion" + ); + } + + #[test] + fn test_delete_column_sqlite_unique_on_different_column_not_dropped() { + // When deleting a column in SQLite, UNIQUE constraints on OTHER columns should NOT be dropped + // This tests line 46's condition: only drop constraints that reference the deleted column + let schema = vec![TableDef { + name: "users".into(), + description: None, + columns: vec![ + col("id", ColumnType::Simple(SimpleColumnType::Integer)), + col("email", ColumnType::Simple(SimpleColumnType::Text)), + col("nickname", ColumnType::Simple(SimpleColumnType::Text)), + ], + constraints: vec![ + // UNIQUE on email (the column we're NOT deleting) + TableConstraint::Unique { + name: None, + columns: vec!["email".into()], + }, + ], + }]; + + // Delete nickname, which does NOT have the unique constraint + let result = + build_delete_column(&DatabaseBackend::Sqlite, "users", "nickname", None, &schema); + + // Should only have 1 statement: ALTER TABLE DROP COLUMN (no DROP INDEX needed) + assert_eq!( + result.len(), + 1, + "Should not drop UNIQUE on email when deleting nickname, got: {} statements", + result.len() + ); + + let sql = result[0].build(DatabaseBackend::Sqlite); + assert!( + sql.contains("DROP COLUMN"), + "Expected DROP COLUMN, got: {}", + sql + ); + assert!( + !sql.contains("DROP INDEX"), + "Should NOT drop the email UNIQUE constraint when deleting nickname" + ); + } + + #[test] + fn test_delete_column_sqlite_temp_table_filters_constraints_correctly() { + // When using temp table approach, constraints referencing the deleted column should be excluded, + // but constraints on OTHER columns should be preserved + // This tests lines 122-124: filter constraints by column reference + let schema = vec![TableDef { + name: "orders".into(), + description: None, + columns: vec![ + col("id", ColumnType::Simple(SimpleColumnType::Integer)), + col("user_id", ColumnType::Simple(SimpleColumnType::BigInt)), + col("status", ColumnType::Simple(SimpleColumnType::Text)), + col( + "created_at", + ColumnType::Simple(SimpleColumnType::Timestamp), + ), + ], + constraints: vec![ + // FK on user_id (column we're deleting) - should be excluded + TableConstraint::ForeignKey { + name: None, + columns: vec!["user_id".into()], + ref_table: "users".into(), + ref_columns: vec!["id".into()], + on_delete: None, + on_update: None, + }, + // Index on created_at (different column) - should be preserved and recreated + TableConstraint::Index { + name: None, + columns: vec!["created_at".into()], + }, + // Another FK on a different column - should be preserved + TableConstraint::ForeignKey { + name: None, + columns: vec!["status".into()], + ref_table: "statuses".into(), + ref_columns: vec!["code".into()], + on_delete: None, + on_update: None, + }, + ], + }]; + + let result = + build_delete_column(&DatabaseBackend::Sqlite, "orders", "user_id", None, &schema); + + let all_sql: Vec = result + .iter() + .map(|q| q.build(DatabaseBackend::Sqlite)) + .collect(); + let combined_sql = all_sql.join("\n"); + + // Should use temp table approach (FK triggers it) + assert!( + combined_sql.contains("orders_temp"), + "Should use temp table approach for FK column deletion" + ); + + // Index on created_at should be recreated after rename + assert!( + combined_sql.contains("ix_orders__created_at"), + "Index on created_at should be recreated, got: {}", + combined_sql + ); + + // The FK on user_id should NOT appear (deleted column) + // But the FK on status should be preserved + assert!( + combined_sql.contains("REFERENCES \"statuses\""), + "FK on status should be preserved, got: {}", + combined_sql + ); + + // Count FK references - should only be 1 (status FK, not user_id FK) + let fk_patterns = combined_sql.matches("REFERENCES").count(); + assert_eq!( + fk_patterns, 1, + "Only the FK on status should exist (not the one on user_id), got: {}", + combined_sql + ); + } + + // ==================== Snapshot Tests ==================== + + fn build_sql_snapshot(result: &[BuiltQuery], backend: DatabaseBackend) -> String { + result + .iter() + .map(|q| q.build(backend)) + .collect::>() + .join(";\n") + } + + #[rstest] + #[case::postgres("postgres", DatabaseBackend::Postgres)] + #[case::mysql("mysql", DatabaseBackend::MySql)] + #[case::sqlite("sqlite", DatabaseBackend::Sqlite)] + fn test_delete_column_with_unique_constraint( + #[case] title: &str, + #[case] backend: DatabaseBackend, + ) { + let schema = vec![TableDef { + name: "users".into(), + description: None, + columns: vec![ + col("id", ColumnType::Simple(SimpleColumnType::Integer)), + col("email", ColumnType::Simple(SimpleColumnType::Text)), + col("name", ColumnType::Simple(SimpleColumnType::Text)), + ], + constraints: vec![TableConstraint::Unique { + name: None, + columns: vec!["email".into()], + }], + }]; + + let result = build_delete_column(&backend, "users", "email", None, &schema); + let sql = build_sql_snapshot(&result, backend); + + with_settings!({ snapshot_suffix => format!("delete_column_with_unique_{}", title) }, { + assert_snapshot!(sql); + }); + } + + #[rstest] + #[case::postgres("postgres", DatabaseBackend::Postgres)] + #[case::mysql("mysql", DatabaseBackend::MySql)] + #[case::sqlite("sqlite", DatabaseBackend::Sqlite)] + fn test_delete_column_with_index_constraint( + #[case] title: &str, + #[case] backend: DatabaseBackend, + ) { + let schema = vec![TableDef { + name: "posts".into(), + description: None, + columns: vec![ + col("id", ColumnType::Simple(SimpleColumnType::Integer)), + col( + "created_at", + ColumnType::Simple(SimpleColumnType::Timestamp), + ), + col("title", ColumnType::Simple(SimpleColumnType::Text)), + ], + constraints: vec![TableConstraint::Index { + name: None, + columns: vec!["created_at".into()], + }], + }]; + + let result = build_delete_column(&backend, "posts", "created_at", None, &schema); + let sql = build_sql_snapshot(&result, backend); + + with_settings!({ snapshot_suffix => format!("delete_column_with_index_{}", title) }, { + assert_snapshot!(sql); + }); + } + + #[rstest] + #[case::postgres("postgres", DatabaseBackend::Postgres)] + #[case::mysql("mysql", DatabaseBackend::MySql)] + #[case::sqlite("sqlite", DatabaseBackend::Sqlite)] + fn test_delete_column_with_fk_constraint( + #[case] title: &str, + #[case] backend: DatabaseBackend, + ) { + let schema = vec![TableDef { + name: "orders".into(), + description: None, + columns: vec![ + col("id", ColumnType::Simple(SimpleColumnType::Integer)), + col("user_id", ColumnType::Simple(SimpleColumnType::BigInt)), + col("total", ColumnType::Simple(SimpleColumnType::Integer)), + ], + constraints: vec![TableConstraint::ForeignKey { + name: None, + columns: vec!["user_id".into()], + ref_table: "users".into(), + ref_columns: vec!["id".into()], + on_delete: None, + on_update: None, + }], + }]; + + let result = build_delete_column(&backend, "orders", "user_id", None, &schema); + let sql = build_sql_snapshot(&result, backend); + + with_settings!({ snapshot_suffix => format!("delete_column_with_fk_{}", title) }, { + assert_snapshot!(sql); + }); + } + + #[rstest] + #[case::postgres("postgres", DatabaseBackend::Postgres)] + #[case::mysql("mysql", DatabaseBackend::MySql)] + #[case::sqlite("sqlite", DatabaseBackend::Sqlite)] + fn test_delete_column_with_pk_constraint( + #[case] title: &str, + #[case] backend: DatabaseBackend, + ) { + let schema = vec![TableDef { + name: "order_items".into(), + description: None, + columns: vec![ + col("order_id", ColumnType::Simple(SimpleColumnType::Integer)), + col("product_id", ColumnType::Simple(SimpleColumnType::Integer)), + col("quantity", ColumnType::Simple(SimpleColumnType::Integer)), + ], + constraints: vec![TableConstraint::PrimaryKey { + auto_increment: false, + columns: vec!["order_id".into(), "product_id".into()], + }], + }]; + + let result = build_delete_column(&backend, "order_items", "product_id", None, &schema); + let sql = build_sql_snapshot(&result, backend); + + with_settings!({ snapshot_suffix => format!("delete_column_with_pk_{}", title) }, { + assert_snapshot!(sql); + }); + } + + #[rstest] + #[case::postgres("postgres", DatabaseBackend::Postgres)] + #[case::mysql("mysql", DatabaseBackend::MySql)] + #[case::sqlite("sqlite", DatabaseBackend::Sqlite)] + fn test_delete_column_with_fk_and_index_constraints( + #[case] title: &str, + #[case] backend: DatabaseBackend, + ) { + // Complex case: FK on the deleted column + Index on another column + let schema = vec![TableDef { + name: "orders".into(), + description: None, + columns: vec![ + col("id", ColumnType::Simple(SimpleColumnType::Integer)), + col("user_id", ColumnType::Simple(SimpleColumnType::BigInt)), + col( + "created_at", + ColumnType::Simple(SimpleColumnType::Timestamp), + ), + col("total", ColumnType::Simple(SimpleColumnType::Integer)), + ], + constraints: vec![ + TableConstraint::ForeignKey { + name: None, + columns: vec!["user_id".into()], + ref_table: "users".into(), + ref_columns: vec!["id".into()], + on_delete: None, + on_update: None, + }, + TableConstraint::Index { + name: None, + columns: vec!["created_at".into()], + }, + ], + }]; + + let result = build_delete_column(&backend, "orders", "user_id", None, &schema); + let sql = build_sql_snapshot(&result, backend); + + with_settings!({ snapshot_suffix => format!("delete_column_with_fk_and_index_{}", title) }, { + assert_snapshot!(sql); + }); + } + + #[test] + fn test_delete_column_sqlite_temp_table_with_enum_column() { + // SQLite temp table approach with enum column type + // This tests lines 122-124: enum type drop in temp table function + use vespertide_core::EnumValues; + + let enum_type = ColumnType::Complex(ComplexColumnType::Enum { + name: "order_status".into(), + values: EnumValues::String(vec![ + "pending".into(), + "shipped".into(), + "delivered".into(), + ]), + }); + + let schema = vec![TableDef { + name: "orders".into(), + description: None, + columns: vec![ + col("id", ColumnType::Simple(SimpleColumnType::Integer)), + col("user_id", ColumnType::Simple(SimpleColumnType::BigInt)), + col("status", enum_type.clone()), + ], + constraints: vec![TableConstraint::ForeignKey { + name: None, + columns: vec!["user_id".into()], + ref_table: "users".into(), + ref_columns: vec!["id".into()], + on_delete: None, + on_update: None, + }], + }]; + + // Delete the FK column (user_id) with an enum type - triggers temp table AND enum drop + let result = build_delete_column( + &DatabaseBackend::Sqlite, + "orders", + "user_id", + Some(&enum_type), + &schema, + ); + + // Should use temp table approach (FK triggers it) + DROP TYPE at end + assert!( + result.len() >= 4, + "Expected at least 4 statements for temp table approach, got: {}", + result.len() + ); + + let all_sql: Vec = result + .iter() + .map(|q| q.build(DatabaseBackend::Sqlite)) + .collect(); + let combined_sql = all_sql.join("\n"); + + // Verify temp table approach + assert!( + combined_sql.contains("orders_temp"), + "Should use temp table approach" + ); + + // The DROP TYPE statement should be empty for SQLite (only applies to PostgreSQL) + // but the code path should still be executed + let last_stmt = result.last().unwrap(); + let last_sql = last_stmt.build(DatabaseBackend::Sqlite); + // SQLite doesn't have DROP TYPE, so it should be empty string + assert!( + last_sql.is_empty() || !last_sql.contains("DROP TYPE"), + "SQLite should not emit DROP TYPE" + ); + + // Verify it DOES emit DROP TYPE for PostgreSQL + let pg_last_sql = last_stmt.build(DatabaseBackend::Postgres); + assert!( + pg_last_sql.contains("DROP TYPE"), + "PostgreSQL should emit DROP TYPE, got: {}", + pg_last_sql + ); + } } diff --git a/crates/vespertide-query/src/sql/mod.rs b/crates/vespertide-query/src/sql/mod.rs index 10477d6..cdac0e3 100644 --- a/crates/vespertide-query/src/sql/mod.rs +++ b/crates/vespertide-query/src/sql/mod.rs @@ -62,7 +62,13 @@ pub fn build_action_queries( .find(|t| t.name == *table) .and_then(|t| t.columns.iter().find(|c| c.name == *column)) .map(|c| &c.r#type); - Ok(build_delete_column(table, column, column_type)) + Ok(build_delete_column( + backend, + table, + column, + column_type, + current_schema, + )) } MigrationAction::ModifyColumnType { diff --git a/crates/vespertide-query/src/sql/snapshots/vespertide_query__sql__delete_column__tests__delete_column_with_fk_and_index_constraints@delete_column_with_fk_and_index_mysql.snap b/crates/vespertide-query/src/sql/snapshots/vespertide_query__sql__delete_column__tests__delete_column_with_fk_and_index_constraints@delete_column_with_fk_and_index_mysql.snap new file mode 100644 index 0000000..3a85be2 --- /dev/null +++ b/crates/vespertide-query/src/sql/snapshots/vespertide_query__sql__delete_column__tests__delete_column_with_fk_and_index_constraints@delete_column_with_fk_and_index_mysql.snap @@ -0,0 +1,5 @@ +--- +source: crates/vespertide-query/src/sql/delete_column.rs +expression: sql +--- +ALTER TABLE `orders` DROP COLUMN `user_id` diff --git a/crates/vespertide-query/src/sql/snapshots/vespertide_query__sql__delete_column__tests__delete_column_with_fk_and_index_constraints@delete_column_with_fk_and_index_postgres.snap b/crates/vespertide-query/src/sql/snapshots/vespertide_query__sql__delete_column__tests__delete_column_with_fk_and_index_constraints@delete_column_with_fk_and_index_postgres.snap new file mode 100644 index 0000000..624a984 --- /dev/null +++ b/crates/vespertide-query/src/sql/snapshots/vespertide_query__sql__delete_column__tests__delete_column_with_fk_and_index_constraints@delete_column_with_fk_and_index_postgres.snap @@ -0,0 +1,5 @@ +--- +source: crates/vespertide-query/src/sql/delete_column.rs +expression: sql +--- +ALTER TABLE "orders" DROP COLUMN "user_id" diff --git a/crates/vespertide-query/src/sql/snapshots/vespertide_query__sql__delete_column__tests__delete_column_with_fk_and_index_constraints@delete_column_with_fk_and_index_sqlite.snap b/crates/vespertide-query/src/sql/snapshots/vespertide_query__sql__delete_column__tests__delete_column_with_fk_and_index_constraints@delete_column_with_fk_and_index_sqlite.snap new file mode 100644 index 0000000..ebfb16a --- /dev/null +++ b/crates/vespertide-query/src/sql/snapshots/vespertide_query__sql__delete_column__tests__delete_column_with_fk_and_index_constraints@delete_column_with_fk_and_index_sqlite.snap @@ -0,0 +1,9 @@ +--- +source: crates/vespertide-query/src/sql/delete_column.rs +expression: sql +--- +CREATE TABLE "orders_temp" ( "id" integer, "created_at" timestamp_text, "total" integer ); +INSERT INTO "orders_temp" ("id", "created_at", "total") SELECT "id", "created_at", "total" FROM "orders"; +DROP TABLE "orders"; +ALTER TABLE "orders_temp" RENAME TO "orders"; +CREATE INDEX "ix_orders__created_at" ON "orders" ("created_at") diff --git a/crates/vespertide-query/src/sql/snapshots/vespertide_query__sql__delete_column__tests__delete_column_with_fk_constraint@delete_column_with_fk_mysql.snap b/crates/vespertide-query/src/sql/snapshots/vespertide_query__sql__delete_column__tests__delete_column_with_fk_constraint@delete_column_with_fk_mysql.snap new file mode 100644 index 0000000..3a85be2 --- /dev/null +++ b/crates/vespertide-query/src/sql/snapshots/vespertide_query__sql__delete_column__tests__delete_column_with_fk_constraint@delete_column_with_fk_mysql.snap @@ -0,0 +1,5 @@ +--- +source: crates/vespertide-query/src/sql/delete_column.rs +expression: sql +--- +ALTER TABLE `orders` DROP COLUMN `user_id` diff --git a/crates/vespertide-query/src/sql/snapshots/vespertide_query__sql__delete_column__tests__delete_column_with_fk_constraint@delete_column_with_fk_postgres.snap b/crates/vespertide-query/src/sql/snapshots/vespertide_query__sql__delete_column__tests__delete_column_with_fk_constraint@delete_column_with_fk_postgres.snap new file mode 100644 index 0000000..624a984 --- /dev/null +++ b/crates/vespertide-query/src/sql/snapshots/vespertide_query__sql__delete_column__tests__delete_column_with_fk_constraint@delete_column_with_fk_postgres.snap @@ -0,0 +1,5 @@ +--- +source: crates/vespertide-query/src/sql/delete_column.rs +expression: sql +--- +ALTER TABLE "orders" DROP COLUMN "user_id" diff --git a/crates/vespertide-query/src/sql/snapshots/vespertide_query__sql__delete_column__tests__delete_column_with_fk_constraint@delete_column_with_fk_sqlite.snap b/crates/vespertide-query/src/sql/snapshots/vespertide_query__sql__delete_column__tests__delete_column_with_fk_constraint@delete_column_with_fk_sqlite.snap new file mode 100644 index 0000000..ffb7439 --- /dev/null +++ b/crates/vespertide-query/src/sql/snapshots/vespertide_query__sql__delete_column__tests__delete_column_with_fk_constraint@delete_column_with_fk_sqlite.snap @@ -0,0 +1,8 @@ +--- +source: crates/vespertide-query/src/sql/delete_column.rs +expression: sql +--- +CREATE TABLE "orders_temp" ( "id" integer, "total" integer ); +INSERT INTO "orders_temp" ("id", "total") SELECT "id", "total" FROM "orders"; +DROP TABLE "orders"; +ALTER TABLE "orders_temp" RENAME TO "orders" diff --git a/crates/vespertide-query/src/sql/snapshots/vespertide_query__sql__delete_column__tests__delete_column_with_index_constraint@delete_column_with_index_mysql.snap b/crates/vespertide-query/src/sql/snapshots/vespertide_query__sql__delete_column__tests__delete_column_with_index_constraint@delete_column_with_index_mysql.snap new file mode 100644 index 0000000..ed96f6a --- /dev/null +++ b/crates/vespertide-query/src/sql/snapshots/vespertide_query__sql__delete_column__tests__delete_column_with_index_constraint@delete_column_with_index_mysql.snap @@ -0,0 +1,5 @@ +--- +source: crates/vespertide-query/src/sql/delete_column.rs +expression: sql +--- +ALTER TABLE `posts` DROP COLUMN `created_at` diff --git a/crates/vespertide-query/src/sql/snapshots/vespertide_query__sql__delete_column__tests__delete_column_with_index_constraint@delete_column_with_index_postgres.snap b/crates/vespertide-query/src/sql/snapshots/vespertide_query__sql__delete_column__tests__delete_column_with_index_constraint@delete_column_with_index_postgres.snap new file mode 100644 index 0000000..5f3a374 --- /dev/null +++ b/crates/vespertide-query/src/sql/snapshots/vespertide_query__sql__delete_column__tests__delete_column_with_index_constraint@delete_column_with_index_postgres.snap @@ -0,0 +1,5 @@ +--- +source: crates/vespertide-query/src/sql/delete_column.rs +expression: sql +--- +ALTER TABLE "posts" DROP COLUMN "created_at" diff --git a/crates/vespertide-query/src/sql/snapshots/vespertide_query__sql__delete_column__tests__delete_column_with_index_constraint@delete_column_with_index_sqlite.snap b/crates/vespertide-query/src/sql/snapshots/vespertide_query__sql__delete_column__tests__delete_column_with_index_constraint@delete_column_with_index_sqlite.snap new file mode 100644 index 0000000..dd1a7a7 --- /dev/null +++ b/crates/vespertide-query/src/sql/snapshots/vespertide_query__sql__delete_column__tests__delete_column_with_index_constraint@delete_column_with_index_sqlite.snap @@ -0,0 +1,6 @@ +--- +source: crates/vespertide-query/src/sql/delete_column.rs +expression: sql +--- +DROP INDEX "ix_posts__created_at"; +ALTER TABLE "posts" DROP COLUMN "created_at" diff --git a/crates/vespertide-query/src/sql/snapshots/vespertide_query__sql__delete_column__tests__delete_column_with_pk_constraint@delete_column_with_pk_mysql.snap b/crates/vespertide-query/src/sql/snapshots/vespertide_query__sql__delete_column__tests__delete_column_with_pk_constraint@delete_column_with_pk_mysql.snap new file mode 100644 index 0000000..3523f53 --- /dev/null +++ b/crates/vespertide-query/src/sql/snapshots/vespertide_query__sql__delete_column__tests__delete_column_with_pk_constraint@delete_column_with_pk_mysql.snap @@ -0,0 +1,5 @@ +--- +source: crates/vespertide-query/src/sql/delete_column.rs +expression: sql +--- +ALTER TABLE `order_items` DROP COLUMN `product_id` diff --git a/crates/vespertide-query/src/sql/snapshots/vespertide_query__sql__delete_column__tests__delete_column_with_pk_constraint@delete_column_with_pk_postgres.snap b/crates/vespertide-query/src/sql/snapshots/vespertide_query__sql__delete_column__tests__delete_column_with_pk_constraint@delete_column_with_pk_postgres.snap new file mode 100644 index 0000000..d9d7477 --- /dev/null +++ b/crates/vespertide-query/src/sql/snapshots/vespertide_query__sql__delete_column__tests__delete_column_with_pk_constraint@delete_column_with_pk_postgres.snap @@ -0,0 +1,5 @@ +--- +source: crates/vespertide-query/src/sql/delete_column.rs +expression: sql +--- +ALTER TABLE "order_items" DROP COLUMN "product_id" diff --git a/crates/vespertide-query/src/sql/snapshots/vespertide_query__sql__delete_column__tests__delete_column_with_pk_constraint@delete_column_with_pk_sqlite.snap b/crates/vespertide-query/src/sql/snapshots/vespertide_query__sql__delete_column__tests__delete_column_with_pk_constraint@delete_column_with_pk_sqlite.snap new file mode 100644 index 0000000..1e65dce --- /dev/null +++ b/crates/vespertide-query/src/sql/snapshots/vespertide_query__sql__delete_column__tests__delete_column_with_pk_constraint@delete_column_with_pk_sqlite.snap @@ -0,0 +1,8 @@ +--- +source: crates/vespertide-query/src/sql/delete_column.rs +expression: sql +--- +CREATE TABLE "order_items_temp" ( "order_id" integer, "quantity" integer ); +INSERT INTO "order_items_temp" ("order_id", "quantity") SELECT "order_id", "quantity" FROM "order_items"; +DROP TABLE "order_items"; +ALTER TABLE "order_items_temp" RENAME TO "order_items" diff --git a/crates/vespertide-query/src/sql/snapshots/vespertide_query__sql__delete_column__tests__delete_column_with_unique_constraint@delete_column_with_unique_mysql.snap b/crates/vespertide-query/src/sql/snapshots/vespertide_query__sql__delete_column__tests__delete_column_with_unique_constraint@delete_column_with_unique_mysql.snap new file mode 100644 index 0000000..ef4ee24 --- /dev/null +++ b/crates/vespertide-query/src/sql/snapshots/vespertide_query__sql__delete_column__tests__delete_column_with_unique_constraint@delete_column_with_unique_mysql.snap @@ -0,0 +1,5 @@ +--- +source: crates/vespertide-query/src/sql/delete_column.rs +expression: sql +--- +ALTER TABLE `users` DROP COLUMN `email` diff --git a/crates/vespertide-query/src/sql/snapshots/vespertide_query__sql__delete_column__tests__delete_column_with_unique_constraint@delete_column_with_unique_postgres.snap b/crates/vespertide-query/src/sql/snapshots/vespertide_query__sql__delete_column__tests__delete_column_with_unique_constraint@delete_column_with_unique_postgres.snap new file mode 100644 index 0000000..afe3bf0 --- /dev/null +++ b/crates/vespertide-query/src/sql/snapshots/vespertide_query__sql__delete_column__tests__delete_column_with_unique_constraint@delete_column_with_unique_postgres.snap @@ -0,0 +1,5 @@ +--- +source: crates/vespertide-query/src/sql/delete_column.rs +expression: sql +--- +ALTER TABLE "users" DROP COLUMN "email" diff --git a/crates/vespertide-query/src/sql/snapshots/vespertide_query__sql__delete_column__tests__delete_column_with_unique_constraint@delete_column_with_unique_sqlite.snap b/crates/vespertide-query/src/sql/snapshots/vespertide_query__sql__delete_column__tests__delete_column_with_unique_constraint@delete_column_with_unique_sqlite.snap new file mode 100644 index 0000000..0c78fab --- /dev/null +++ b/crates/vespertide-query/src/sql/snapshots/vespertide_query__sql__delete_column__tests__delete_column_with_unique_constraint@delete_column_with_unique_sqlite.snap @@ -0,0 +1,6 @@ +--- +source: crates/vespertide-query/src/sql/delete_column.rs +expression: sql +--- +DROP INDEX "uq_users__email"; +ALTER TABLE "users" DROP COLUMN "email"