Fix idx issue

Author: owjs3901

Cargo.lock

@@ -179,6 +179,57 @@ pub fn apply_action(
                 .find(|t| t.name == *table)
                 .ok_or_else(|| PlannerError::TableNotFound(table.clone()))?;
             tbl.constraints.retain(|c| c != constraint);
+
+            // Also clear inline column fields that correspond to the removed constraint
+            // This ensures normalize() won't re-add the constraint from inline fields
+            match constraint {
+                TableConstraint::Unique { name, columns } => {
+                    // For unnamed single-column unique constraints, clear the column's inline unique
+                    if name.is_none()
+                        && columns.len() == 1
+                        && let Some(col) = tbl.columns.iter_mut().find(|c| c.name == columns[0])
+                    {
+                        col.unique = None;
+                    }
+                    // For named constraints, clear inline unique references to this constraint name
+                    if let Some(constraint_name) = name {
+                        for col in &mut tbl.columns {
+                            if let Some(vespertide_core::StrOrBoolOrArray::Array(names)) =
+                                &mut col.unique
+                            {
+                                names.retain(|n| n != constraint_name);
+                                if names.is_empty() {
+                                    col.unique = None;
+                                }
+                            } else if let Some(vespertide_core::StrOrBoolOrArray::Str(n)) =
+                                &col.unique
+                                && n == constraint_name
+                            {
+                                col.unique = None;
+                            }
+                        }
+                    }
+                }
+                TableConstraint::PrimaryKey { columns, .. } => {
+                    // Clear inline primary_key for columns in this constraint
+                    for col_name in columns {
+                        if let Some(col) = tbl.columns.iter_mut().find(|c| &c.name == col_name) {
+                            col.primary_key = None;
+                        }
+                    }
+                }
+                TableConstraint::ForeignKey { columns, .. } => {
+                    // Clear inline foreign_key for columns in this constraint
+                    for col_name in columns {
+                        if let Some(col) = tbl.columns.iter_mut().find(|c| &c.name == col_name) {
+                            col.foreign_key = None;
+                        }
+                    }
+                }
+                TableConstraint::Check { .. } => {
+                    // Check constraints don't have inline representation
+                }
+            }
             Ok(())
         }
     }

crates/vespertide-planner/src/apply.rs

@@ -160,4 +160,76 @@ mod tests {
         let users = schema.iter().find(|t| t.name == "users").unwrap();
         assert_eq!(users, &expected_users);
     }
+
+    /// Test that RemoveConstraint works when table was created with both
+    /// inline unique column AND table-level unique constraint for the same column
+    #[test]
+    fn remove_constraint_with_inline_and_table_level_unique() {
+        use vespertide_core::StrOrBoolOrArray;
+
+        // Simulate migration 0001: CreateTable with both inline unique and table-level constraint
+        let create_plan = MigrationPlan {
+            comment: None,
+            created_at: None,
+            version: 1,
+            actions: vec![MigrationAction::CreateTable {
+                table: "users".into(),
+                columns: vec![ColumnDef {
+                    name: "email".into(),
+                    r#type: ColumnType::Simple(SimpleColumnType::Text),
+                    nullable: false,
+                    default: None,
+                    comment: None,
+                    primary_key: None,
+                    unique: Some(StrOrBoolOrArray::Bool(true)), // inline unique
+                    index: None,
+                    foreign_key: None,
+                }],
+                constraints: vec![TableConstraint::Unique {
+                    name: None,
+                    columns: vec!["email".into()],
+                }], // table-level unique (duplicate!)
+            }],
+        };
+
+        // Migration 0002: RemoveConstraint
+        let remove_plan = MigrationPlan {
+            comment: None,
+            created_at: None,
+            version: 2,
+            actions: vec![MigrationAction::RemoveConstraint {
+                table: "users".into(),
+                constraint: TableConstraint::Unique {
+                    name: None,
+                    columns: vec!["email".into()],
+                },
+            }],
+        };
+
+        let schema = schema_from_plans(&[create_plan, remove_plan]).unwrap();
+        let users = schema.iter().find(|t| t.name == "users").unwrap();
+
+        println!("Constraints after apply: {:?}", users.constraints);
+        println!("Column unique field: {:?}", users.columns[0].unique);
+
+        // After apply_action:
+        // - constraints is empty (RemoveConstraint removed the table-level one)
+        // - but column still has unique: Some(Bool(true))!
+
+        // Now simulate what diff_schemas does - it normalizes the baseline
+        let normalized = users.clone().normalize().unwrap();
+        println!("Constraints after normalize: {:?}", normalized.constraints);
+
+        // After normalize:
+        // - inline unique (column.unique = true) is converted to table-level constraint
+        // - So we'd still have one unique constraint!
+
+        // This is the bug: diff_schemas normalizes both baseline and target,
+        // but the baseline still has inline unique that gets re-added.
+        assert!(
+            normalized.constraints.is_empty(),
+            "Expected no constraints after normalize, but got: {:?}",
+            normalized.constraints
+        );
+    }
 }

crates/vespertide-planner/src/schema.rs

@@ -153,10 +153,14 @@ pub fn build_add_constraint(
         }
         TableConstraint::Unique { name, columns } => {
             // SQLite does not support ALTER TABLE ... ADD CONSTRAINT UNIQUE
-            let mut idx = Index::create().table(Alias::new(table)).unique().to_owned();
-            if let Some(n) = name {
-                idx = idx.name(n).to_owned();
-            }
+            // Always generate a proper name: uq_{table}_{key} or uq_{table}_{columns}
+            let index_name =
+                super::helpers::build_unique_constraint_name(table, columns, name.as_deref());
+            let mut idx = Index::create()
+                .table(Alias::new(table))
+                .name(&index_name)
+                .unique()
+                .to_owned();
             for col in columns {
                 idx = idx.col(Alias::new(col)).to_owned();
             }
@@ -394,17 +398,17 @@ mod tests {
     #[case::add_constraint_unique_named_postgres(
         "add_constraint_unique_named_postgres",
         DatabaseBackend::Postgres,
-        &["CREATE UNIQUE INDEX \"uq_email\" ON \"users\" (\"email\")"]
+        &["CREATE UNIQUE INDEX \"uq_users__uq_email\" ON \"users\" (\"email\")"]
     )]
     #[case::add_constraint_unique_named_mysql(
         "add_constraint_unique_named_mysql",
         DatabaseBackend::MySql,
-        &["CREATE UNIQUE INDEX `uq_email` ON `users` (`email`)"]
+        &["CREATE UNIQUE INDEX `uq_users__uq_email` ON `users` (`email`)"]
     )]
     #[case::add_constraint_unique_named_sqlite(
         "add_constraint_unique_named_sqlite",
         DatabaseBackend::Sqlite,
-        &["CREATE UNIQUE INDEX \"uq_email\" ON \"users\" (\"email\")"]
+        &["CREATE UNIQUE INDEX \"uq_users__uq_email\" ON \"users\" (\"email\")"]
     )]
     #[case::add_constraint_foreign_key_postgres(
         "add_constraint_foreign_key_postgres",

crates/vespertide-query/src/sql/add_constraint.rs

@@ -59,10 +59,13 @@ pub(crate) fn build_create_table_for_backend(
                 // For MySQL, we can add unique index directly in CREATE TABLE
                 // For Postgres and SQLite, we'll handle it separately in build_create_table
                 if matches!(backend, DatabaseBackend::MySql) {
-                    let mut idx = Index::create().unique().to_owned();
-                    if let Some(n) = name {
-                        idx = idx.name(n).to_owned();
-                    }
+                    // Always generate a proper name: uq_{table}_{key} or uq_{table}_{columns}
+                    let index_name = super::helpers::build_unique_constraint_name(
+                        table,
+                        unique_cols,
+                        name.as_deref(),
+                    );
+                    let mut idx = Index::create().name(&index_name).unique().to_owned();
                     for col in unique_cols {
                         idx = idx.col(Alias::new(col)).to_owned();
                     }
@@ -79,10 +82,10 @@ pub(crate) fn build_create_table_for_backend(
                 on_delete,
                 on_update,
             } => {
-                let mut fk = ForeignKey::create();
-                if let Some(n) = name {
-                    fk = fk.name(n).to_owned();
-                }
+                // Always generate a proper name: fk_{table}_{key} or fk_{table}_{columns}
+                let fk_name =
+                    super::helpers::build_foreign_key_name(table, fk_cols, name.as_deref());
+                let mut fk = ForeignKey::create().name(&fk_name).to_owned();
                 fk = fk.from_tbl(Alias::new(table)).to_owned();
                 for col in fk_cols {
                     fk = fk.from_col(Alias::new(col)).to_owned();
@@ -180,10 +183,17 @@ pub fn build_create_table(
                 columns: unique_cols,
             } = constraint
             {
-                let mut idx = Index::create().table(Alias::new(table)).unique().to_owned();
-                if let Some(n) = name {
-                    idx = idx.name(n).to_owned();
-                }
+                // Always generate a proper name: uq_{table}_{key} or uq_{table}_{columns}
+                let index_name = super::helpers::build_unique_constraint_name(
+                    table,
+                    unique_cols,
+                    name.as_deref(),
+                );
+                let mut idx = Index::create()
+                    .table(Alias::new(table))
+                    .name(&index_name)
+                    .unique()
+                    .to_owned();
                 for col in unique_cols {
                     idx = idx.col(Alias::new(col)).to_owned();
                 }

crates/vespertide-query/src/sql/create_table.rs

@@ -238,10 +238,44 @@ pub fn is_enum_type(column_type: &ColumnType) -> bool {
     )
 }
 
+/// Generate index name from table name, columns, and optional user-provided key
+/// Always includes table name to avoid conflicts across tables.
+/// Uses double underscore to separate table name from the rest.
+/// Format: ix_{table}__{key} or ix_{table}__{col1}_{col2}...
+pub fn build_index_name(table: &str, columns: &[String], key: Option<&str>) -> String {
+    match key {
+        Some(k) => format!("ix_{}__{}", table, k),
+        None => format!("ix_{}__{}", table, columns.join("_")),
+    }
+}
+
+/// Generate unique constraint name from table name, columns, and optional user-provided key
+/// Always includes table name to avoid conflicts across tables.
+/// Uses double underscore to separate table name from the rest.
+/// Format: uq_{table}__{key} or uq_{table}__{col1}_{col2}...
+pub fn build_unique_constraint_name(table: &str, columns: &[String], key: Option<&str>) -> String {
+    match key {
+        Some(k) => format!("uq_{}__{}", table, k),
+        None => format!("uq_{}__{}", table, columns.join("_")),
+    }
+}
+
+/// Generate foreign key constraint name from table name, columns, and optional user-provided key
+/// Always includes table name to avoid conflicts across tables.
+/// Uses double underscore to separate table name from the rest.
+/// Format: fk_{table}__{key} or fk_{table}__{col1}_{col2}...
+pub fn build_foreign_key_name(table: &str, columns: &[String], key: Option<&str>) -> String {
+    match key {
+        Some(k) => format!("fk_{}__{}", table, k),
+        None => format!("fk_{}__{}", table, columns.join("_")),
+    }
+}
+
 /// Generate CHECK constraint name for SQLite enum column
-/// Format: chk_{table}_{column}
+/// Uses double underscore to separate table name from the rest.
+/// Format: chk_{table}__{column}
 pub fn build_sqlite_enum_check_name(table: &str, column: &str) -> String {
-    format!("chk_{}_{}", table, column)
+    format!("chk_{}__{}", table, column)
 }
 
 /// Generate CHECK constraint expression for SQLite enum column

crates/vespertide-query/src/sql/helpers.rs

@@ -2,7 +2,7 @@
 source: crates/vespertide-query/src/sql/add_column.rs
 expression: sql
 ---
-CREATE TABLE "users_temp" ( "id" integer NOT NULL, "status" enum_text , CONSTRAINT "chk_users_status" CHECK ("status" IN ('active', 'inactive')));
+CREATE TABLE "users_temp" ( "id" integer NOT NULL, "status" enum_text , CONSTRAINT "chk_users__status" CHECK ("status" IN ('active', 'inactive')));
 INSERT INTO "users_temp" ("id", "status") SELECT "id", NULL AS "status" FROM "users";
 DROP TABLE "users";
 ALTER TABLE "users_temp" RENAME TO "users"

crates/vespertide-query/src/sql/snapshots/vespertide_query__sql__add_column__tests__add_column_with_enum_type@add_column_with_enum_type_Sqlite.snap

@@ -2,4 +2,4 @@
 source: crates/vespertide-query/src/sql/add_constraint.rs
 expression: "result.iter().map(|q| q.build(backend)).collect::<Vec<String>>().join(\"\\n\")"
 ---
-CREATE UNIQUE INDEX `uq_email` ON `users` (`email`)
+CREATE UNIQUE INDEX `uq_users__uq_email` ON `users` (`email`)

crates/vespertide-query/src/sql/snapshots/vespertide_query__sql__add_constraint__tests__add_constraint@add_constraint_add_constraint_unique_named_mysql.snap

@@ -2,4 +2,4 @@
 source: crates/vespertide-query/src/sql/add_constraint.rs
 expression: "result.iter().map(|q| q.build(backend)).collect::<Vec<String>>().join(\"\\n\")"
 ---
-CREATE UNIQUE INDEX "uq_email" ON "users" ("email")
+CREATE UNIQUE INDEX "uq_users__uq_email" ON "users" ("email")

crates/vespertide-query/src/sql/snapshots/vespertide_query__sql__add_constraint__tests__add_constraint@add_constraint_add_constraint_unique_named_postgres.snap

@@ -2,4 +2,4 @@
 source: crates/vespertide-query/src/sql/add_constraint.rs
 expression: "result.iter().map(|q| q.build(backend)).collect::<Vec<String>>().join(\"\\n\")"
 ---
-CREATE UNIQUE INDEX "uq_email" ON "users" ("email")
+CREATE UNIQUE INDEX "uq_users__uq_email" ON "users" ("email")