diff --git a/.changepacks/changepack_log_mTA6OO1hfY3fTj98u37V0.json b/.changepacks/changepack_log_mTA6OO1hfY3fTj98u37V0.json new file mode 100644 index 0000000..279fe2c --- /dev/null +++ b/.changepacks/changepack_log_mTA6OO1hfY3fTj98u37V0.json @@ -0,0 +1 @@ +{"changes":{"crates/vespertide-loader/Cargo.toml":"Patch","crates/vespertide-macro/Cargo.toml":"Patch","crates/vespertide-query/Cargo.toml":"Patch","crates/vespertide-core/Cargo.toml":"Patch","crates/vespertide-planner/Cargo.toml":"Patch","crates/vespertide/Cargo.toml":"Patch","crates/vespertide-cli/Cargo.toml":"Patch","crates/vespertide-exporter/Cargo.toml":"Patch","crates/vespertide-config/Cargo.toml":"Patch"},"note":"Fix ordering on diff, sql commands, Support enum on sqlite","date":"2025-12-18T06:48:29.591923100Z"} \ No newline at end of file diff --git a/crates/vespertide-planner/src/diff.rs b/crates/vespertide-planner/src/diff.rs index 94e238b..4bdc85d 100644 --- a/crates/vespertide-planner/src/diff.rs +++ b/crates/vespertide-planner/src/diff.rs @@ -1,4 +1,4 @@ -use std::collections::{HashMap, HashSet, VecDeque}; +use std::collections::{BTreeMap, BTreeSet, HashSet, VecDeque}; use vespertide_core::{MigrationAction, MigrationPlan, TableConstraint, TableDef}; @@ -16,7 +16,8 @@ fn topological_sort_tables<'a>(tables: &[&'a TableDef]) -> Result = tables.iter().map(|t| t.name.as_str()).collect(); // Build adjacency list: for each table, list the tables it depends on (via FK) - let mut dependencies: HashMap<&str, Vec<&str>> = HashMap::new(); + // Use BTreeMap for consistent ordering + let mut dependencies: BTreeMap<&str, Vec<&str>> = BTreeMap::new(); for table in tables { let mut deps = Vec::new(); for constraint in &table.constraints { @@ -32,7 +33,8 @@ fn topological_sort_tables<'a>(tables: &[&'a TableDef]) -> Result = HashMap::new(); + // Use BTreeMap for consistent ordering + let mut in_degree: BTreeMap<&str, usize> = BTreeMap::new(); for table in tables { in_degree.entry(table.name.as_str()).or_insert(0); } @@ -49,15 +51,15 @@ fn topological_sort_tables<'a>(tables: &[&'a TableDef]) -> Result = VecDeque::new(); - for table in tables { - if in_degree.get(table.name.as_str()) == Some(&0) { - queue.push_back(table.name.as_str()); - } - } + // BTreeMap iteration is already sorted by key + let mut queue: VecDeque<&str> = in_degree + .iter() + .filter(|(_, deg)| **deg == 0) + .map(|(name, _)| *name) + .collect(); let mut result: Vec<&TableDef> = Vec::new(); - let table_map: HashMap<&str, &TableDef> = + let table_map: BTreeMap<&str, &TableDef> = tables.iter().map(|t| (t.name.as_str(), *t)).collect(); while let Some(table_name) = queue.pop_front() { @@ -65,17 +67,22 @@ fn topological_sort_tables<'a>(tables: &[&'a TableDef]) -> Result = BTreeSet::new(); for (dependent, deps) in &dependencies { if deps.contains(&table_name) && let Some(degree) = in_degree.get_mut(dependent) { *degree -= 1; if *degree == 0 { - queue.push_back(dependent); + ready_tables.insert(dependent); } } } + for t in ready_tables { + queue.push_back(t); + } } // Check for cycles @@ -105,7 +112,7 @@ fn extract_delete_table_name(action: &MigrationAction) -> &str { } } -fn sort_delete_tables(actions: &mut [MigrationAction], all_tables: &HashMap<&str, &TableDef>) { +fn sort_delete_tables(actions: &mut [MigrationAction], all_tables: &BTreeMap<&str, &TableDef>) { // Collect DeleteTable actions and their indices let delete_indices: Vec = actions .iter() @@ -124,14 +131,16 @@ fn sort_delete_tables(actions: &mut [MigrationAction], all_tables: &HashMap<&str } // Extract table names being deleted - let delete_table_names: HashSet<&str> = delete_indices + // Use BTreeSet for consistent ordering + let delete_table_names: BTreeSet<&str> = delete_indices .iter() .map(|&i| extract_delete_table_name(&actions[i])) .collect(); // Build dependency graph for tables being deleted // dependencies[A] = [B] means A has FK referencing B - let mut dependencies: HashMap<&str, Vec<&str>> = HashMap::new(); + // Use BTreeMap for consistent ordering + let mut dependencies: BTreeMap<&str, Vec<&str>> = BTreeMap::new(); for &table_name in &delete_table_names { let mut deps = Vec::new(); if let Some(table_def) = all_tables.get(table_name) { @@ -149,7 +158,8 @@ fn sort_delete_tables(actions: &mut [MigrationAction], all_tables: &HashMap<&str // Use Kahn's algorithm for topological sort // in_degree[A] = number of tables A depends on - let mut in_degree: HashMap<&str, usize> = HashMap::new(); + // Use BTreeMap for consistent ordering + let mut in_degree: BTreeMap<&str, usize> = BTreeMap::new(); for &table_name in &delete_table_names { in_degree.insert( table_name, @@ -158,28 +168,33 @@ fn sort_delete_tables(actions: &mut [MigrationAction], all_tables: &HashMap<&str } // Start with tables that have no dependencies (can be deleted last in creation order) - let mut queue: VecDeque<&str> = VecDeque::new(); - for &table_name in &delete_table_names { - if in_degree.get(table_name) == Some(&0) { - queue.push_back(table_name); - } - } + // BTreeMap iteration is already sorted + let mut queue: VecDeque<&str> = in_degree + .iter() + .filter(|(_, deg)| **deg == 0) + .map(|(name, _)| *name) + .collect(); let mut sorted_tables: Vec<&str> = Vec::new(); while let Some(table_name) = queue.pop_front() { sorted_tables.push(table_name); // For each table that has this one as a dependency, decrement its in-degree + // Use BTreeSet for consistent ordering of newly ready tables + let mut ready_tables: BTreeSet<&str> = BTreeSet::new(); for (&dependent, deps) in &dependencies { if deps.contains(&table_name) && let Some(degree) = in_degree.get_mut(dependent) { *degree -= 1; if *degree == 0 { - queue.push_back(dependent); + ready_tables.insert(dependent); } } } + for t in ready_tables { + queue.push_back(t); + } } // Reverse to get deletion order (tables with dependencies should be deleted first) @@ -234,11 +249,12 @@ pub fn diff_schemas(from: &[TableDef], to: &[TableDef]) -> Result, _>>()?; - let from_map: HashMap<_, _> = from_normalized + // Use BTreeMap for consistent ordering + let from_map: BTreeMap<_, _> = from_normalized .iter() .map(|t| (t.name.as_str(), t)) .collect(); - let to_map: HashMap<_, _> = to_normalized.iter().map(|t| (t.name.as_str(), t)).collect(); + let to_map: BTreeMap<_, _> = to_normalized.iter().map(|t| (t.name.as_str(), t)).collect(); // Drop tables that disappeared. for name in from_map.keys() { @@ -252,13 +268,13 @@ pub fn diff_schemas(from: &[TableDef], to: &[TableDef]) -> Result = from_tbl + // Columns - use BTreeMap for consistent ordering + let from_cols: BTreeMap<_, _> = from_tbl .columns .iter() .map(|c| (c.name.as_str(), c)) .collect(); - let to_cols: HashMap<_, _> = to_tbl + let to_cols: BTreeMap<_, _> = to_tbl .columns .iter() .map(|c| (c.name.as_str(), c)) @@ -300,13 +316,13 @@ pub fn diff_schemas(from: &[TableDef], to: &[TableDef]) -> Result = from_tbl + // Indexes - use BTreeMap for consistent ordering + let from_indexes: BTreeMap<_, _> = from_tbl .indexes .iter() .map(|i| (i.name.as_str(), i)) .collect(); - let to_indexes: HashMap<_, _> = to_tbl + let to_indexes: BTreeMap<_, _> = to_tbl .indexes .iter() .map(|i| (i.name.as_str(), i)) diff --git a/crates/vespertide-query/src/sql/add_column.rs b/crates/vespertide-query/src/sql/add_column.rs index fdeb7bb..3571bd4 100644 --- a/crates/vespertide-query/src/sql/add_column.rs +++ b/crates/vespertide-query/src/sql/add_column.rs @@ -3,9 +3,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_sea_column_def}; +use super::helpers::{ + build_create_enum_type_sql, build_schema_statement, build_sea_column_def, + collect_sqlite_enum_check_clauses, +}; use super::rename_table::build_rename_table; -use super::types::{BuiltQuery, DatabaseBackend}; +use super::types::{BuiltQuery, DatabaseBackend, RawSql}; use crate::error::QueryError; fn build_add_column_alter_for_backend( @@ -20,6 +23,14 @@ fn build_add_column_alter_for_backend( .to_owned() } +/// Check if the column type is an enum +fn is_enum_column(column: &ColumnDef) -> bool { + matches!( + column.r#type, + vespertide_core::ColumnType::Complex(vespertide_core::ComplexColumnType::Enum { .. }) + ) +} + pub fn build_add_column( backend: &DatabaseBackend, table: &str, @@ -27,8 +38,12 @@ pub fn build_add_column( fill_with: Option<&str>, current_schema: &[TableDef], ) -> Result, QueryError> { - // SQLite: only NOT NULL additions require table recreation - if *backend == DatabaseBackend::Sqlite && !column.nullable { + // SQLite: NOT NULL additions or enum columns require table recreation + // (enum columns need CHECK constraint which requires table recreation in SQLite) + let sqlite_needs_recreation = + *backend == DatabaseBackend::Sqlite && (!column.nullable || is_enum_column(column)); + + if sqlite_needs_recreation { let table_def = current_schema .iter() .find(|t| t.name == table) @@ -47,7 +62,25 @@ pub fn build_add_column( &new_columns, &table_def.constraints, ); - let create_query = BuiltQuery::CreateTable(Box::new(create_temp)); + + // 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(); diff --git a/crates/vespertide-query/src/sql/create_table.rs b/crates/vespertide-query/src/sql/create_table.rs index feaaf21..7d4be9e 100644 --- a/crates/vespertide-query/src/sql/create_table.rs +++ b/crates/vespertide-query/src/sql/create_table.rs @@ -2,8 +2,11 @@ use sea_query::{Alias, ForeignKey, Index, Table, TableCreateStatement}; use vespertide_core::{ColumnDef, ColumnType, ComplexColumnType, TableConstraint}; -use super::helpers::{build_create_enum_type_sql, build_sea_column_def, to_sea_fk_action}; -use super::types::{BuiltQuery, DatabaseBackend}; +use super::helpers::{ + build_create_enum_type_sql, build_schema_statement, build_sea_column_def, + collect_sqlite_enum_check_clauses, to_sea_fk_action, +}; +use super::types::{BuiltQuery, DatabaseBackend, RawSql}; use crate::error::QueryError; pub(crate) fn build_create_table_for_backend( @@ -145,7 +148,29 @@ pub fn build_create_table( table_constraints.iter().cloned().cloned().collect(); build_create_table_for_backend(backend, table, columns, &table_constraints_owned) }; - queries.push(BuiltQuery::CreateTable(Box::new(create_table_stmt))); + + // For SQLite, add CHECK constraints for enum columns + if matches!(backend, DatabaseBackend::Sqlite) { + let enum_check_clauses = collect_sqlite_enum_check_clauses(table, columns); + if !enum_check_clauses.is_empty() { + // Embed CHECK constraints into CREATE TABLE statement + let base_sql = build_schema_statement(&create_table_stmt, *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)); + } + queries.push(BuiltQuery::Raw(RawSql::per_backend( + modified_sql.clone(), + modified_sql.clone(), + modified_sql, + ))); + } else { + queries.push(BuiltQuery::CreateTable(Box::new(create_table_stmt))); + } + } else { + queries.push(BuiltQuery::CreateTable(Box::new(create_table_stmt))); + } // For Postgres and SQLite, add unique constraints as separate CREATE UNIQUE INDEX statements if matches!(backend, DatabaseBackend::Postgres | DatabaseBackend::Sqlite) { diff --git a/crates/vespertide-query/src/sql/helpers.rs b/crates/vespertide-query/src/sql/helpers.rs index d230345..76d476b 100644 --- a/crates/vespertide-query/src/sql/helpers.rs +++ b/crates/vespertide-query/src/sql/helpers.rs @@ -228,6 +228,43 @@ pub fn is_enum_type(column_type: &ColumnType) -> bool { ) } +/// Generate CHECK constraint name for SQLite enum column +/// Format: chk_{table}_{column} +pub fn build_sqlite_enum_check_name(table: &str, column: &str) -> String { + format!("chk_{}_{}", table, column) +} + +/// Generate CHECK constraint expression for SQLite enum column +/// Returns the constraint clause like: CONSTRAINT "chk_table_col" CHECK (col IN ('val1', 'val2')) +pub fn build_sqlite_enum_check_clause( + table: &str, + column: &str, + column_type: &ColumnType, +) -> Option { + if let ColumnType::Complex(ComplexColumnType::Enum { values, .. }) = column_type { + let name = build_sqlite_enum_check_name(table, column); + let values_sql = values + .iter() + .map(|v| format!("'{}'", v.replace('\'', "''"))) + .collect::>() + .join(", "); + Some(format!( + "CONSTRAINT \"{}\" CHECK (\"{}\" IN ({}))", + name, column, values_sql + )) + } else { + None + } +} + +/// Collect all CHECK constraints for enum columns in a table (for SQLite) +pub fn collect_sqlite_enum_check_clauses(table: &str, columns: &[ColumnDef]) -> Vec { + columns + .iter() + .filter_map(|col| build_sqlite_enum_check_clause(table, &col.name, &col.r#type)) + .collect() +} + /// Extract enum name from column type if it's an enum pub fn get_enum_name(column_type: &ColumnType) -> Option<&str> { if let ColumnType::Complex(ComplexColumnType::Enum { name, .. }) = column_type { diff --git a/crates/vespertide-query/src/sql/snapshots/vespertide_query__sql__add_column__tests__add_column_with_enum_type@add_column_with_enum_type_Sqlite.snap b/crates/vespertide-query/src/sql/snapshots/vespertide_query__sql__add_column__tests__add_column_with_enum_type@add_column_with_enum_type_Sqlite.snap index 3636fee..04ab18c 100644 --- a/crates/vespertide-query/src/sql/snapshots/vespertide_query__sql__add_column__tests__add_column_with_enum_type@add_column_with_enum_type_Sqlite.snap +++ b/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,5 +2,7 @@ source: crates/vespertide-query/src/sql/add_column.rs expression: sql --- -; -ALTER TABLE "users" ADD COLUMN "status" enum_text +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" diff --git a/crates/vespertide-query/src/sql/snapshots/vespertide_query__sql__create_table__tests__create_table_with_enum_column@create_table_with_enum_column_Sqlite.snap b/crates/vespertide-query/src/sql/snapshots/vespertide_query__sql__create_table__tests__create_table_with_enum_column@create_table_with_enum_column_Sqlite.snap index c930924..dbd4d7c 100644 --- a/crates/vespertide-query/src/sql/snapshots/vespertide_query__sql__create_table__tests__create_table_with_enum_column@create_table_with_enum_column_Sqlite.snap +++ b/crates/vespertide-query/src/sql/snapshots/vespertide_query__sql__create_table__tests__create_table_with_enum_column@create_table_with_enum_column_Sqlite.snap @@ -3,4 +3,4 @@ source: crates/vespertide-query/src/sql/create_table.rs expression: sql --- ; -CREATE TABLE "users" ( "id" integer NOT NULL, "status" enum_text NOT NULL DEFAULT 'active', PRIMARY KEY ("id") ) +CREATE TABLE "users" ( "id" integer NOT NULL, "status" enum_text NOT NULL DEFAULT 'active', PRIMARY KEY ("id") , CONSTRAINT "chk_users_status" CHECK ("status" IN ('active', 'inactive', 'pending')))