Fix temp_table issue

Author: owjs3901

.gitignore

@@ -3,3 +3,4 @@ local.db
 settings.local.json
 coverage
 lcov.info
+.sisyphus

crates/vespertide-planner/src/apply.rs

@@ -186,7 +186,11 @@ pub fn apply_action(
                 .iter_mut()
                 .find(|t| t.name == *table)
                 .ok_or_else(|| PlannerError::TableNotFound(table.clone()))?;
-            tbl.constraints.push(constraint.clone());
+            // Skip if an equivalent constraint already exists (e.g. inline index
+            // was already promoted to table-level by normalize() during AddColumn)
+            if !tbl.constraints.contains(constraint) {
+                tbl.constraints.push(constraint.clone());
+            }
             Ok(())
         }
         MigrationAction::RemoveConstraint { table, constraint } => {

crates/vespertide-planner/src/error.rs

@@ -36,6 +36,10 @@ pub enum PlannerError {
     DuplicateEnumValue(String, String, String, i32),
     #[error("{0}")]
     InvalidEnumDefault(#[from] Box<InvalidEnumDefaultError>),
+    #[error(
+        "auto_increment on non-integer column: {0}.{1} (type {2} does not support auto_increment)"
+    )]
+    InvalidAutoIncrement(String, String, String),
 }
 
 #[derive(Debug, Error)]

crates/vespertide-planner/src/validate.rs

@@ -61,6 +61,44 @@ fn validate_table(
         return Err(PlannerError::MissingPrimaryKey(table.name.clone()));
     }
 
+    // Validate auto_increment columns have integer types
+    for constraint in &table.constraints {
+        if let TableConstraint::PrimaryKey {
+            auto_increment: true,
+            columns,
+        } = constraint
+        {
+            for col_name in columns {
+                if let Some(column) = table.columns.iter().find(|c| c.name == *col_name)
+                    && !column.r#type.supports_auto_increment() {
+                        return Err(PlannerError::InvalidAutoIncrement(
+                            table.name.clone(),
+                            col_name.clone(),
+                            format!("{:?}", column.r#type),
+                        ));
+                    }
+            }
+        }
+    }
+
+    // Validate auto_increment on inline primary_key definitions
+    use vespertide_core::schema::primary_key::PrimaryKeySyntax;
+    for column in &table.columns {
+        if let Some(pk_syntax) = &column.primary_key {
+            let has_auto_increment = match pk_syntax {
+                PrimaryKeySyntax::Bool(_) => false,
+                PrimaryKeySyntax::Object(pk_def) => pk_def.auto_increment,
+            };
+            if has_auto_increment && !column.r#type.supports_auto_increment() {
+                return Err(PlannerError::InvalidAutoIncrement(
+                    table.name.clone(),
+                    column.name.clone(),
+                    format!("{:?}", column.r#type),
+                ));
+            }
+        }
+    }
+
     // Validate columns (enum types)
     for column in &table.columns {
         validate_column(column, &table.name)?;
@@ -463,6 +501,7 @@ pub fn find_missing_fill_with(plan: &MigrationPlan) -> Vec<FillWithRequired> {
 mod tests {
     use super::*;
     use rstest::rstest;
+    use vespertide_core::schema::primary_key::{PrimaryKeyDef, PrimaryKeySyntax};
     use vespertide_core::{
         ColumnDef, ColumnType, ComplexColumnType, EnumValues, NumValue, SimpleColumnType,
         TableConstraint,
@@ -1826,4 +1865,61 @@ mod tests {
         let missing = find_missing_fill_with(&plan);
         assert!(missing.is_empty());
     }
+
+    #[test]
+    fn validate_auto_increment_on_text_column_fails() {
+        let table_def = table(
+            "users",
+            vec![col("id", ColumnType::Simple(SimpleColumnType::Text))],
+            vec![TableConstraint::PrimaryKey {
+                auto_increment: true,
+                columns: vec!["id".into()],
+            }],
+        );
+
+        let result = validate_table(&table_def, &std::collections::HashMap::new());
+        assert!(result.is_err());
+        match result {
+            Err(PlannerError::InvalidAutoIncrement(table_name, col_name, _)) => {
+                assert_eq!(table_name, "users");
+                assert_eq!(col_name, "id");
+            }
+            _ => panic!("Expected InvalidAutoIncrement error"),
+        }
+    }
+
+    #[test]
+    fn validate_auto_increment_on_integer_column_succeeds() {
+        let table_def = table(
+            "users",
+            vec![col("id", ColumnType::Simple(SimpleColumnType::Integer))],
+            vec![TableConstraint::PrimaryKey {
+                auto_increment: true,
+                columns: vec!["id".into()],
+            }],
+        );
+
+        let result = validate_table(&table_def, &std::collections::HashMap::new());
+        assert!(result.is_ok());
+    }
+
+    #[test]
+    fn validate_inline_auto_increment_on_text_column_fails() {
+        let mut col_def = col("id", ColumnType::Simple(SimpleColumnType::Text));
+        col_def.primary_key = Some(PrimaryKeySyntax::Object(PrimaryKeyDef {
+            auto_increment: true,
+        }));
+
+        let table_def = table("users", vec![col_def], vec![]);
+
+        let result = validate_table(&table_def, &std::collections::HashMap::new());
+        assert!(result.is_err());
+        match result {
+            Err(PlannerError::InvalidAutoIncrement(table_name, col_name, _)) => {
+                assert_eq!(table_name, "users");
+                assert_eq!(col_name, "id");
+            }
+            _ => panic!("Expected InvalidAutoIncrement error"),
+        }
+    }
 }

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

@@ -2,13 +2,12 @@ use sea_query::{Alias, Expr, Query, Table, TableAlterStatement};
 
 use vespertide_core::{ColumnDef, TableDef};
 
-use super::create_table::build_create_table_for_backend;
 use super::helpers::{
-    build_create_enum_type_sql, build_schema_statement, build_sea_column_def_with_table,
-    collect_sqlite_enum_check_clauses, normalize_enum_default, normalize_fill_with,
+    build_create_enum_type_sql, build_sea_column_def_with_table, build_sqlite_temp_table_create,
+    normalize_enum_default, normalize_fill_with, recreate_indexes_after_rebuild,
 };
 use super::rename_table::build_rename_table;
-use super::types::{BuiltQuery, DatabaseBackend, RawSql};
+use super::types::{BuiltQuery, DatabaseBackend};
 use crate::error::QueryError;
 
 fn build_add_column_alter_for_backend(
@@ -50,32 +49,16 @@ pub fn build_add_column(
         new_columns.push(column.clone());
 
         let temp_table = format!("{}_temp", table);
-        let create_temp = build_create_table_for_backend(
+
+        // 1. Create temporary table with all CHECK constraints (enum + explicit)
+        let create_query = build_sqlite_temp_table_create(
             backend,
             &temp_table,
+            table,
             &new_columns,
             &table_def.constraints,
         );
 
-        // For SQLite, add CHECK constraints for enum columns
-        // Use original table name for constraint naming (table will be renamed back)
-        let enum_check_clauses = collect_sqlite_enum_check_clauses(table, &new_columns);
-        let create_query = if !enum_check_clauses.is_empty() {
-            let base_sql = build_schema_statement(&create_temp, *backend);
-            let mut modified_sql = base_sql;
-            if let Some(pos) = modified_sql.rfind(')') {
-                let check_sql = enum_check_clauses.join(", ");
-                modified_sql.insert_str(pos, &format!(", {}", check_sql));
-            }
-            BuiltQuery::Raw(RawSql::per_backend(
-                modified_sql.clone(),
-                modified_sql.clone(),
-                modified_sql,
-            ))
-        } else {
-            BuiltQuery::CreateTable(Box::new(create_temp))
-        };
-
         // Copy existing data, filling new column
         let mut select_query = Query::select();
         for col in &table_def.columns {
@@ -112,21 +95,8 @@ pub fn build_add_column(
             BuiltQuery::DropTable(Box::new(Table::drop().table(Alias::new(table)).to_owned()));
         let rename_query = build_rename_table(&temp_table, table);
 
-        // Recreate indexes from Index constraints
-        let mut index_queries = Vec::new();
-        for constraint in &table_def.constraints {
-            if let vespertide_core::TableConstraint::Index { name, columns } = constraint {
-                let index_name =
-                    vespertide_naming::build_index_name(table, columns, name.as_deref());
-                let mut idx_stmt = sea_query::Index::create();
-                idx_stmt = idx_stmt.name(&index_name).to_owned();
-                for col_name in columns {
-                    idx_stmt = idx_stmt.col(Alias::new(col_name)).to_owned();
-                }
-                idx_stmt = idx_stmt.table(Alias::new(table)).to_owned();
-                index_queries.push(BuiltQuery::CreateIndex(Box::new(idx_stmt)));
-            }
-        }
+        // Recreate indexes (both regular and UNIQUE)
+        let index_queries = recreate_indexes_after_rebuild(table, &table_def.constraints);
 
         let mut stmts = vec![create_query, insert_query, drop_query, rename_query];
         stmts.extend(index_queries);