From 17dad75dd5c56b1920cc92f70304ad0ea266f7b7 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 24 Jan 2026 17:12:45 +0000 Subject: [PATCH 1/2] Add tern migrate command with up/down subcommands FEATURE: Add tern migrate up/down commands with tern up/down as aliases Implements the migrate command with two subcommands: - migrate up: Run pending migrations (same as existing up) - migrate down: Revert the most recently applied migration The down command computes inverse operations for reversible migrations and removes the migration record from the database tracking tables. Irreversible operations (like DropTable) will error with a clear message. tern up and tern down are now aliases for tern migrate up/down for convenience. --- src/cli/migrate/down.rs | 613 ++++++++++++++++++++++++++++++++++++ src/cli/migrate/mod.rs | 38 +++ src/cli/migrate/up.rs | 387 +++++++++++++++++++++++ src/cli/mod.rs | 19 +- src/db/execution/error.rs | 24 ++ src/db/execution/tracker.rs | 62 ++++ 6 files changed, 1141 insertions(+), 2 deletions(-) create mode 100644 src/cli/migrate/down.rs create mode 100644 src/cli/migrate/mod.rs create mode 100644 src/cli/migrate/up.rs diff --git a/src/cli/migrate/down.rs b/src/cli/migrate/down.rs new file mode 100644 index 0000000..7d6d47c --- /dev/null +++ b/src/cli/migrate/down.rs @@ -0,0 +1,613 @@ +//! Migrate down command. +//! +//! This command reverts the most recently applied migration. + +use std::path::PathBuf; + +use anstream::println; +use clap::Args; +use miette::{Context, IntoDiagnostic}; +use serde::Serialize; + +use crate::cli::{OutputFormat, ensure_backend_initialized, load_backend, print_json}; +use crate::db::execution::MigrationTracker; +use crate::db::migrate::{MigrationPlan, Operation, PostgresRenderer, RenderConfig}; +use crate::db::state::{Migration, StateBackend}; +use crate::db::{self}; + +/// Revert the most recently applied migration. +/// +/// Connects to a database and reverts the last applied migration. +/// Only one migration is reverted at a time for safety. +#[derive(Debug, Clone, Args)] +pub struct Down { + /// PostgreSQL connection string + #[arg(env = "DATABASE_URL")] + pub url: Option, + + /// PostgreSQL connection string (alternative to positional) + #[arg(long, env = "DATABASE_URL")] + pub database_url: Option, + + /// The database schema to migrate + #[arg(long, default_value = "public")] + pub schema: String, + + /// Path to the state directory + #[arg(long)] + pub path: Option, + + /// Show what would be reverted without actually reverting + #[arg(long)] + pub dry_run: bool, + + /// Output format + #[arg(long, default_value = "text")] + pub format: OutputFormat, +} + +/// Output structure for down command JSON format. +#[derive(Debug, Serialize)] +pub struct DownOutput { + /// Whether the operation was successful. + pub success: bool, + /// Whether this was a dry run. + pub dry_run: bool, + /// The migration that was reverted (or would be reverted). + #[serde(skip_serializing_if = "Option::is_none")] + pub migration: Option, + /// Error message if failed. + #[serde(skip_serializing_if = "Option::is_none")] + pub error: Option, +} + +/// Information about a reverted migration. +#[derive(Debug, Serialize)] +pub struct RevertedMigrationInfo { + /// Migration ID (hex). + pub id: String, + /// Migration description. + pub description: String, + /// Number of operations that were reverted. + pub operation_count: usize, + /// Number of SQL statements executed to revert. + #[serde(skip_serializing_if = "Option::is_none")] + pub statement_count: Option, +} + +impl std::fmt::Display for DownOutput { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + if let Some(migration) = &self.migration { + if self.dry_run { + writeln!(f, "Would revert migration (dry run):")?; + writeln!(f)?; + writeln!( + f, + " [{}...] {}", + &migration.id[..12.min(migration.id.len())], + migration.description + )?; + writeln!(f, " Operations to revert: {}", migration.operation_count)?; + writeln!(f)?; + writeln!(f, "Run without --dry-run to revert this migration.")?; + } else if self.success { + writeln!(f, "Reverted migration:")?; + writeln!(f)?; + writeln!( + f, + " [{}...] {}", + &migration.id[..12.min(migration.id.len())], + migration.description + )?; + if let Some(stmt_count) = migration.statement_count { + writeln!(f, " SQL statements executed: {}", stmt_count)?; + } + writeln!(f)?; + writeln!(f, "Migration reverted successfully.")?; + } else { + writeln!(f, "Migration revert failed!")?; + writeln!(f)?; + if let Some(err) = &self.error { + writeln!(f, "Error: {}", err)?; + } + } + } else if let Some(err) = &self.error { + writeln!(f, "Error: {}", err)?; + } else { + writeln!(f, "No migrations to revert.")?; + } + + Ok(()) + } +} + +/// Computes the inverse operations needed to revert a migration. +/// +/// Returns the operations in reverse order (last applied first reverted). +fn compute_inverse_operations(operations: &[Operation]) -> Result, String> { + let mut inverse_ops = Vec::new(); + + // Process operations in reverse order + for op in operations.iter().rev() { + let inverse = compute_single_inverse(op)?; + inverse_ops.push(inverse); + } + + Ok(inverse_ops) +} + +/// Computes the inverse of a single operation. +fn compute_single_inverse(op: &Operation) -> Result { + match op { + // Enum operations + Operation::CreateEnum { schema, enum_type } => Ok(Operation::DropEnum { + schema: schema.clone(), + name: enum_type.name.clone(), + }), + Operation::DropEnum { .. } => { + Err("Cannot revert DropEnum: enum definition is not preserved".to_string()) + } + Operation::RenameEnum { schema, from, to } => Ok(Operation::RenameEnum { + schema: schema.clone(), + from: to.clone(), + to: from.clone(), + }), + Operation::AddEnumValue { .. } => Err( + "Cannot revert AddEnumValue: PostgreSQL does not support removing enum values" + .to_string(), + ), + + // Sequence operations + Operation::CreateSequence { schema, sequence } => Ok(Operation::DropSequence { + schema: schema.clone(), + name: sequence.name.clone(), + }), + Operation::DropSequence { .. } => { + Err("Cannot revert DropSequence: sequence definition is not preserved".to_string()) + } + Operation::RenameSequence { schema, from, to } => Ok(Operation::RenameSequence { + schema: schema.clone(), + from: to.clone(), + to: from.clone(), + }), + Operation::AlterSequence { .. } => { + Err("Cannot revert AlterSequence: previous values are not preserved".to_string()) + } + + // Table operations + Operation::CreateTable { schema, table } => Ok(Operation::DropTable { + schema: schema.clone(), + name: table.name.clone(), + }), + Operation::DropTable { .. } => { + Err("Cannot revert DropTable: table definition and data are not preserved".to_string()) + } + Operation::RenameTable { schema, from, to } => Ok(Operation::RenameTable { + schema: schema.clone(), + from: to.clone(), + to: from.clone(), + }), + + // Column operations + Operation::AddColumn { + schema, + table, + column, + } => Ok(Operation::DropColumn { + schema: schema.clone(), + table: table.clone(), + name: column.name.clone(), + }), + Operation::DropColumn { .. } => Err( + "Cannot revert DropColumn: column definition and data are not preserved".to_string(), + ), + Operation::RenameColumn { + schema, + table, + from, + to, + } => Ok(Operation::RenameColumn { + schema: schema.clone(), + table: table.clone(), + from: to.clone(), + to: from.clone(), + }), + Operation::AlterColumn { .. } => { + Err("Cannot revert AlterColumn: previous values are not preserved".to_string()) + } + + // Constraint operations + Operation::AddConstraint { + schema, + table, + constraint, + } => Ok(Operation::DropConstraint { + schema: schema.clone(), + table: table.clone(), + name: constraint.name.clone(), + }), + Operation::DropConstraint { .. } => { + Err("Cannot revert DropConstraint: constraint definition is not preserved".to_string()) + } + Operation::RenameConstraint { + schema, + table, + from, + to, + } => Ok(Operation::RenameConstraint { + schema: schema.clone(), + table: table.clone(), + from: to.clone(), + to: from.clone(), + }), + + // Index operations + Operation::CreateIndex { + schema, + index, + concurrently, + .. + } => Ok(Operation::DropIndex { + schema: schema.clone(), + name: index.name.clone(), + concurrently: *concurrently, + }), + Operation::DropIndex { .. } => { + Err("Cannot revert DropIndex: index definition is not preserved".to_string()) + } + Operation::RenameIndex { schema, from, to } => Ok(Operation::RenameIndex { + schema: schema.clone(), + from: to.clone(), + to: from.clone(), + }), + + // View operations + Operation::CreateView { schema, view } => Ok(Operation::DropView { + schema: schema.clone(), + name: view.name.clone(), + is_materialized: view.is_materialized, + }), + Operation::DropView { .. } => { + Err("Cannot revert DropView: view definition is not preserved".to_string()) + } + Operation::RenameView { + schema, + from, + to, + is_materialized, + } => Ok(Operation::RenameView { + schema: schema.clone(), + from: to.clone(), + to: from.clone(), + is_materialized: *is_materialized, + }), + Operation::ReplaceView { .. } => { + Err("Cannot revert ReplaceView: previous view definition is not preserved".to_string()) + } + Operation::RefreshMaterializedView { .. } => { + // Refreshing a materialized view is idempotent, so we can just skip it + // Return a no-op by creating a comment that won't change anything + // Actually, there's no true no-op, so we'll just error + Err( + "Cannot revert RefreshMaterializedView: this operation cannot be undone" + .to_string(), + ) + } + + // Comment operations + Operation::SetComment { .. } => { + Err("Cannot revert SetComment: previous comment is not preserved".to_string()) + } + } +} + +impl Down { + /// Dispatch the down command. + pub async fn dispatch(self) -> miette::Result<()> { + // Resolve database URL (positional > --database-url > env) + let db_url = self + .url + .or(self.database_url) + .ok_or_else(|| miette::miette!("Database URL required. Provide as argument, --database-url, or set DATABASE_URL environment variable."))?; + + let backend = load_backend(self.path.as_deref()); + ensure_backend_initialized(&backend).await?; + + if !matches!(self.format, OutputFormat::Json) { + println!("Connecting to database..."); + } + + // Connect to the database + let client = db::connect(&db_url) + .await + .into_diagnostic() + .wrap_err("Failed to connect to database")?; + + // Create tracker + let tracker = MigrationTracker::new(&client, &self.schema); + + // Ensure tracking infrastructure exists + tracker.ensure_schema().await.into_diagnostic()?; + + // Get the current migration ID from the database + let current_id = match tracker.get_current_migration_id().await.into_diagnostic()? { + Some(id) => id, + None => { + let output = DownOutput { + success: false, + dry_run: self.dry_run, + migration: None, + error: Some("No migrations have been applied to revert.".to_string()), + }; + match self.format { + OutputFormat::Text => println!("{}", output), + OutputFormat::Json => print_json(&output), + OutputFormat::Sql => println!("-- No migrations to revert"), + } + std::process::exit(1); + } + }; + + // Find the migration in the local backend + let migration = find_migration_by_hex_id(&backend, ¤t_id) + .await + .into_diagnostic() + .wrap_err("Failed to find migration in local state")?; + + let migration = match migration { + Some(m) => m, + None => { + return Err(miette::miette!( + "Migration {} is recorded in the database but not found in local state.\n\nThis may indicate state corruption or that migrations were applied from a different source.", + current_id + )); + } + }; + + // Check if this is a baseline migration + if migration.is_baseline() { + return Err(miette::miette!( + "Cannot revert the baseline migration.\n\nThe baseline migration represents the initial database state and cannot be reverted." + )); + } + + // Compute inverse operations + let inverse_ops = match compute_inverse_operations(&migration.operations) { + Ok(ops) => ops, + Err(msg) => { + let output = DownOutput { + success: false, + dry_run: self.dry_run, + migration: Some(RevertedMigrationInfo { + id: migration.id.to_hex(), + description: migration.description.clone(), + operation_count: migration.operations.len(), + statement_count: None, + }), + error: Some(msg), + }; + match self.format { + OutputFormat::Text => println!("{}", output), + OutputFormat::Json => print_json(&output), + OutputFormat::Sql => println!("-- Cannot compute inverse operations"), + } + std::process::exit(1); + } + }; + + if self.dry_run { + // Just show what would be reverted + let output = DownOutput { + success: true, + dry_run: true, + migration: Some(RevertedMigrationInfo { + id: migration.id.to_hex(), + description: migration.description.clone(), + operation_count: migration.operations.len(), + statement_count: None, + }), + error: None, + }; + + match self.format { + OutputFormat::Text => println!("{}", output), + OutputFormat::Json => print_json(&output), + OutputFormat::Sql => { + println!( + "-- Dry run: would revert migration {}", + migration.id.to_short_hex() + ); + // Render the inverse operations to show what SQL would be executed + let renderer = PostgresRenderer::new(RenderConfig::default()); + let plan = MigrationPlan::from_operations(inverse_ops); + let script = plan.render(&renderer); + println!("{}", script.to_sql()); + } + } + } else { + // Execute the revert + if !matches!(self.format, OutputFormat::Json) { + println!("Reverting migration {}...", migration.id.to_short_hex()); + } + + // Render migration to SQL + let renderer = PostgresRenderer::new(RenderConfig::default()); + let plan = MigrationPlan::from_operations(inverse_ops); + let script = plan.render(&renderer); + let sql = script.to_sql(); + let statement_count = script.all_statements().len(); + + // Begin transaction + client + .execute("BEGIN", &[]) + .await + .into_diagnostic() + .wrap_err("Failed to begin transaction")?; + + // Set search path to target schema + let set_search_path = format!("SET search_path TO {}", self.schema); + if let Err(e) = client.batch_execute(&set_search_path).await { + let _ = client.execute("ROLLBACK", &[]).await; + return Err(miette::miette!("Failed to set search_path: {}", e)); + } + + // Execute the inverse operations + if !sql.is_empty() + && let Err(e) = client.batch_execute(&sql).await + { + let _ = client.execute("ROLLBACK", &[]).await; + let output = DownOutput { + success: false, + dry_run: false, + migration: Some(RevertedMigrationInfo { + id: migration.id.to_hex(), + description: migration.description.clone(), + operation_count: migration.operations.len(), + statement_count: Some(statement_count), + }), + error: Some(e.to_string()), + }; + match self.format { + OutputFormat::Text => println!("{}", output), + OutputFormat::Json => print_json(&output), + OutputFormat::Sql => println!("-- Revert failed: {}", e), + } + std::process::exit(1); + } + + // Remove migration record from tracking tables + if let Err(e) = tracker.unrecord_migration().await { + let _ = client.execute("ROLLBACK", &[]).await; + return Err(miette::miette!("Failed to unrecord migration: {}", e)); + } + + // Commit transaction + client + .execute("COMMIT", &[]) + .await + .into_diagnostic() + .wrap_err("Failed to commit transaction")?; + + let output = DownOutput { + success: true, + dry_run: false, + migration: Some(RevertedMigrationInfo { + id: migration.id.to_hex(), + description: migration.description.clone(), + operation_count: migration.operations.len(), + statement_count: Some(statement_count), + }), + error: None, + }; + + match self.format { + OutputFormat::Text => println!("{}", output), + OutputFormat::Json => print_json(&output), + OutputFormat::Sql => { + println!("-- Migration reverted successfully"); + } + } + } + + Ok(()) + } +} + +/// Find a migration by its hex ID string. +async fn find_migration_by_hex_id( + backend: &crate::db::state::LocalFileBackend, + hex_id: &str, +) -> Result, crate::db::state::StateError> { + let all_migrations = backend.get_all_migrations().await?; + Ok(all_migrations.into_iter().find(|m| m.id.to_hex() == hex_id)) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn down_output_no_migrations_display() { + let output = DownOutput { + success: false, + dry_run: false, + migration: None, + error: Some("No migrations to revert".to_string()), + }; + let display = format!("{}", output); + assert!(display.contains("No migrations to revert")); + } + + #[test] + fn down_output_dry_run_display() { + let output = DownOutput { + success: true, + dry_run: true, + migration: Some(RevertedMigrationInfo { + id: "abc123def456".to_string(), + description: "Add users table".to_string(), + operation_count: 3, + statement_count: None, + }), + error: None, + }; + let display = format!("{}", output); + assert!(display.contains("Would revert migration (dry run)")); + assert!(display.contains("Add users table")); + assert!(display.contains("Run without --dry-run")); + } + + #[test] + fn down_output_success_display() { + let output = DownOutput { + success: true, + dry_run: false, + migration: Some(RevertedMigrationInfo { + id: "abc123def456".to_string(), + description: "Add users table".to_string(), + operation_count: 3, + statement_count: Some(5), + }), + error: None, + }; + let display = format!("{}", output); + assert!(display.contains("Reverted migration")); + assert!(display.contains("Migration reverted successfully")); + } + + #[test] + fn down_output_failure_display() { + let output = DownOutput { + success: false, + dry_run: false, + migration: Some(RevertedMigrationInfo { + id: "abc123def456".to_string(), + description: "Add users table".to_string(), + operation_count: 3, + statement_count: None, + }), + error: Some("relation 'users' does not exist".to_string()), + }; + let display = format!("{}", output); + assert!(display.contains("Migration revert failed")); + assert!(display.contains("relation 'users' does not exist")); + } + + #[test] + fn down_output_serializes() { + let output = DownOutput { + success: true, + dry_run: false, + migration: Some(RevertedMigrationInfo { + id: "abc".to_string(), + description: "Test".to_string(), + operation_count: 1, + statement_count: Some(2), + }), + error: None, + }; + let json = serde_json::to_string(&output).unwrap(); + assert!(json.contains("\"success\":true")); + assert!(json.contains("\"operation_count\":1")); + } +} diff --git a/src/cli/migrate/mod.rs b/src/cli/migrate/mod.rs new file mode 100644 index 0000000..18e86d0 --- /dev/null +++ b/src/cli/migrate/mod.rs @@ -0,0 +1,38 @@ +//! Migrate command with up/down subcommands. +//! +//! This module provides the `tern migrate` command which offers precise control +//! over database migrations. + +pub mod down; +pub mod up; + +pub use down::Down; +pub use up::Up; + +use clap::Subcommand; + +/// Migrate subcommands for precise migration control. +#[derive(Debug, Subcommand, Clone)] +pub enum MigrateAction { + /// Run pending migrations against a database + /// + /// Connects to a database and applies all migrations that haven't been + /// applied yet. Each migration runs in its own transaction. + Up(Up), + + /// Revert the most recently applied migration + /// + /// Connects to a database and reverts the last applied migration. + /// Only one migration is reverted at a time for safety. + Down(Down), +} + +impl MigrateAction { + /// Dispatch migrate subcommands. + pub async fn dispatch(self) -> miette::Result<()> { + match self { + MigrateAction::Up(args) => args.dispatch().await, + MigrateAction::Down(args) => args.dispatch().await, + } + } +} diff --git a/src/cli/migrate/up.rs b/src/cli/migrate/up.rs new file mode 100644 index 0000000..5e16b6c --- /dev/null +++ b/src/cli/migrate/up.rs @@ -0,0 +1,387 @@ +//! Migrate up command. +//! +//! This command runs pending migrations against a live database. + +use std::path::PathBuf; + +use anstream::println; +use clap::Args; +use miette::{Context, IntoDiagnostic}; +use serde::Serialize; + +use crate::cli::{OutputFormat, ensure_backend_initialized, load_backend, print_json}; +use crate::db::execution::MigrationExecutor; +use crate::db::state::StateBackend; +use crate::db::{self}; + +/// Run pending migrations against a live database. +/// +/// Connects to a database and applies all migrations that haven't been +/// applied yet. Each migration runs in its own transaction. +#[derive(Debug, Clone, Args)] +pub struct Up { + /// PostgreSQL connection string + #[arg(env = "DATABASE_URL")] + pub url: Option, + + /// PostgreSQL connection string (alternative to positional) + #[arg(long, env = "DATABASE_URL")] + pub database_url: Option, + + /// The database schema to migrate + #[arg(long, default_value = "public")] + pub schema: String, + + /// Path to the state directory + #[arg(long)] + pub path: Option, + + /// Show pending migrations without applying + #[arg(long)] + pub dry_run: bool, + + /// Output format + #[arg(long, default_value = "text")] + pub format: OutputFormat, +} + +/// Output structure for up command JSON format. +#[derive(Debug, Serialize)] +pub struct UpOutput { + /// Whether the operation was successful. + pub success: bool, + /// Whether this was a dry run. + pub dry_run: bool, + /// Number of migrations applied or pending. + pub migration_count: usize, + /// Details of each migration. + pub migrations: Vec, + /// Error message if failed. + #[serde(skip_serializing_if = "Option::is_none")] + pub error: Option, +} + +/// Information about a single migration. +#[derive(Debug, Serialize)] +pub struct MigrationInfo { + /// Migration ID (hex). + pub id: String, + /// Migration description. + pub description: String, + /// Number of SQL statements (if applied). + #[serde(skip_serializing_if = "Option::is_none")] + pub statement_count: Option, + /// Schema hash after applying (if applied). + #[serde(skip_serializing_if = "Option::is_none")] + pub schema_hash: Option, + /// Whether this migration was applied. + pub applied: bool, +} + +impl std::fmt::Display for UpOutput { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + if self.migration_count == 0 { + writeln!(f, "Database is up to date. No pending migrations.")?; + return Ok(()); + } + + if self.dry_run { + writeln!(f, "Pending migrations (dry run):")?; + writeln!(f)?; + for (i, m) in self.migrations.iter().enumerate() { + writeln!( + f, + " {}. [{}...] {}", + i + 1, + &m.id[..12.min(m.id.len())], + m.description + )?; + } + writeln!(f)?; + writeln!(f, "Run without --dry-run to apply these migrations.")?; + } else if self.success { + writeln!(f, "Applying {} pending migration(s):", self.migration_count)?; + writeln!(f)?; + for (i, m) in self.migrations.iter().enumerate() { + let status = if m.applied { "OK" } else { "SKIPPED" }; + // Pad description to align status + let desc = if m.description.len() > 40 { + format!("{}...", &m.description[..37]) + } else { + m.description.clone() + }; + writeln!( + f, + " [{}/{}] {:.<50} {}", + i + 1, + self.migration_count, + desc, + status + )?; + } + writeln!(f)?; + writeln!(f, "All migrations applied successfully.")?; + } else { + writeln!(f, "Migration failed!")?; + writeln!(f)?; + if let Some(err) = &self.error { + writeln!(f, "Error: {}", err)?; + } + writeln!(f)?; + writeln!( + f, + "Applied {} of {} migration(s) before failure.", + self.migrations.iter().filter(|m| m.applied).count(), + self.migration_count + )?; + } + + Ok(()) + } +} + +impl Up { + /// Dispatch the up command. + pub async fn dispatch(self) -> miette::Result<()> { + // Resolve database URL (positional > --database-url > env) + let db_url = self + .url + .or(self.database_url) + .ok_or_else(|| miette::miette!("Database URL required. Provide as argument, --database-url, or set DATABASE_URL environment variable."))?; + + let backend = load_backend(self.path.as_deref()); + ensure_backend_initialized(&backend).await?; + + // Check that we have migrations to apply + let index = backend.get_migration_index().await.into_diagnostic()?; + if index.is_empty() { + return Err(miette::miette!( + "No migrations found in state backend.\n\nRun 'tern generate' to create a migration first." + )); + } + + if !matches!(self.format, OutputFormat::Json) { + println!("Connecting to database..."); + } + + // Connect to the database + let client = db::connect(&db_url) + .await + .into_diagnostic() + .wrap_err("Failed to connect to database")?; + + // Create executor + let executor = MigrationExecutor::new(&client, &backend, &self.schema); + + if self.dry_run { + // Just show pending migrations + if !matches!(self.format, OutputFormat::Json) { + println!("Checking migration status..."); + } + + let pending = executor + .get_pending() + .await + .into_diagnostic() + .wrap_err("Failed to get pending migrations")?; + + let migrations: Vec = pending + .iter() + .map(|m| MigrationInfo { + id: m.id.to_hex(), + description: m.description.clone(), + statement_count: None, + schema_hash: None, + applied: false, + }) + .collect(); + + let output = UpOutput { + success: true, + dry_run: true, + migration_count: migrations.len(), + migrations, + error: None, + }; + + match self.format { + OutputFormat::Text => println!("{}", output), + OutputFormat::Json => print_json(&output), + OutputFormat::Sql => { + println!("-- Dry run: showing pending migrations"); + for m in pending { + println!("-- Migration: {} - {}", m.id.to_short_hex(), m.description); + } + } + } + } else { + // Execute pending migrations + if !matches!(self.format, OutputFormat::Json) { + println!("Checking migration status..."); + } + + let result = executor + .execute_pending() + .await + .into_diagnostic() + .wrap_err("Failed to execute migrations")?; + + let migrations: Vec = result + .applied + .iter() + .map(|r| MigrationInfo { + id: r.migration_id.to_hex(), + description: r.description.clone(), + statement_count: Some(r.statement_count), + schema_hash: Some(r.schema_hash.clone()), + applied: true, + }) + .collect(); + + let output = UpOutput { + success: result.success, + dry_run: false, + migration_count: migrations.len(), + migrations, + error: result.error.clone(), + }; + + match self.format { + OutputFormat::Text => println!("{}", output), + OutputFormat::Json => print_json(&output), + OutputFormat::Sql => { + if result.success { + println!("-- All migrations applied successfully"); + } else if let Some(err) = &result.error { + println!("-- Migration failed: {}", err); + } + } + } + + // Exit with error if migrations failed + if !result.success { + std::process::exit(1); + } + } + + Ok(()) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn up_output_no_migrations_display() { + let output = UpOutput { + success: true, + dry_run: false, + migration_count: 0, + migrations: vec![], + error: None, + }; + let display = format!("{}", output); + assert!(display.contains("Database is up to date")); + } + + #[test] + fn up_output_dry_run_display() { + let output = UpOutput { + success: true, + dry_run: true, + migration_count: 2, + migrations: vec![ + MigrationInfo { + id: "abc123def456".to_string(), + description: "Add users table".to_string(), + statement_count: None, + schema_hash: None, + applied: false, + }, + MigrationInfo { + id: "789xyz123456".to_string(), + description: "Add orders table".to_string(), + statement_count: None, + schema_hash: None, + applied: false, + }, + ], + error: None, + }; + let display = format!("{}", output); + assert!(display.contains("Pending migrations (dry run)")); + assert!(display.contains("Add users table")); + assert!(display.contains("Add orders table")); + assert!(display.contains("Run without --dry-run")); + } + + #[test] + fn up_output_success_display() { + let output = UpOutput { + success: true, + dry_run: false, + migration_count: 2, + migrations: vec![ + MigrationInfo { + id: "abc123def456".to_string(), + description: "Add users table".to_string(), + statement_count: Some(3), + schema_hash: Some("hash1".to_string()), + applied: true, + }, + MigrationInfo { + id: "789xyz123456".to_string(), + description: "Add orders table".to_string(), + statement_count: Some(2), + schema_hash: Some("hash2".to_string()), + applied: true, + }, + ], + error: None, + }; + let display = format!("{}", output); + assert!(display.contains("Applying 2 pending migration(s)")); + assert!(display.contains("All migrations applied successfully")); + } + + #[test] + fn up_output_failure_display() { + let output = UpOutput { + success: false, + dry_run: false, + migration_count: 2, + migrations: vec![MigrationInfo { + id: "abc123def456".to_string(), + description: "Add users table".to_string(), + statement_count: Some(3), + schema_hash: Some("hash1".to_string()), + applied: true, + }], + error: Some("relation 'foo' does not exist".to_string()), + }; + let display = format!("{}", output); + assert!(display.contains("Migration failed")); + assert!(display.contains("relation 'foo' does not exist")); + } + + #[test] + fn up_output_serializes() { + let output = UpOutput { + success: true, + dry_run: false, + migration_count: 1, + migrations: vec![MigrationInfo { + id: "abc".to_string(), + description: "Test".to_string(), + statement_count: Some(1), + schema_hash: Some("hash".to_string()), + applied: true, + }], + error: None, + }; + let json = serde_json::to_string(&output).unwrap(); + assert!(json.contains("\"success\":true")); + assert!(json.contains("\"migration_count\":1")); + } +} diff --git a/src/cli/mod.rs b/src/cli/mod.rs index 39536bf..4b1fb66 100644 --- a/src/cli/mod.rs +++ b/src/cli/mod.rs @@ -12,6 +12,7 @@ pub mod history; pub mod import; pub mod init; pub mod inspect; +pub mod migrate; pub mod print_migrations; pub mod record; pub mod schema; @@ -162,11 +163,23 @@ pub enum CliCommand { /// and generates a migration that would transform the schema. Generate(generate::Generate), - /// Run pending migrations against a database + /// Migration commands (up/down) + /// + /// Commands for running and reverting database migrations. + #[command(subcommand)] + Migrate(migrate::MigrateAction), + + /// Run pending migrations against a database (alias for 'migrate up') /// /// Connects to a database and applies all migrations that haven't been /// applied yet. Each migration runs in its own transaction. - Up(up::Up), + Up(migrate::Up), + + /// Revert the most recently applied migration (alias for 'migrate down') + /// + /// Connects to a database and reverts the last applied migration. + /// Only one migration is reverted at a time for safety. + Down(migrate::Down), /// List migration history /// @@ -289,7 +302,9 @@ impl CliCommand { CliCommand::Check(action) => action.dispatch().await, CliCommand::Import(args) => args.dispatch().await, CliCommand::Generate(args) => args.dispatch().await, + CliCommand::Migrate(action) => action.dispatch().await, CliCommand::Up(args) => args.dispatch().await, + CliCommand::Down(args) => args.dispatch().await, CliCommand::History(args) => args.dispatch().await, CliCommand::Show(args) => args.dispatch().await, CliCommand::Verify(args) => args.dispatch().await, diff --git a/src/db/execution/error.rs b/src/db/execution/error.rs index b8ad7c6..61f3d07 100644 --- a/src/db/execution/error.rs +++ b/src/db/execution/error.rs @@ -74,6 +74,30 @@ pub enum ExecutionError { #[error("query error: {0}")] #[diagnostic(code(tern::execution::query))] Query(String), + + /// No migrations to revert. + #[error("no migrations have been applied to the database")] + #[diagnostic( + code(tern::execution::no_migrations_to_revert), + help("Run 'tern up' to apply migrations first before attempting to revert.") + )] + NoMigrationsToRevert, + + /// Revert operation failed. + #[error("failed to revert migration {migration_id}: {message}")] + #[diagnostic(code(tern::execution::revert_failed))] + RevertFailed { + migration_id: String, + message: String, + }, + + /// Cannot revert baseline migration. + #[error("cannot revert baseline migration {0}")] + #[diagnostic( + code(tern::execution::cannot_revert_baseline), + help("The baseline migration represents the initial state and cannot be reverted.") + )] + CannotRevertBaseline(String), } /// Represents the result of applying a single migration. diff --git a/src/db/execution/tracker.rs b/src/db/execution/tracker.rs index 09ff8ae..f16596e 100644 --- a/src/db/execution/tracker.rs +++ b/src/db/execution/tracker.rs @@ -260,6 +260,68 @@ impl<'a> MigrationTracker<'a> { Ok(row.get(0)) } + + /// Removes the most recent migration record from the tracking tables. + /// + /// This should be called within a transaction after successfully + /// reverting the migration operations. + /// + /// # Returns + /// + /// Returns the ID of the removed migration if successful. + pub async fn unrecord_migration(&self) -> Result { + // Get the current (most recent) migration + let current_id = self + .get_current_migration_id() + .await? + .ok_or(ExecutionError::NoMigrationsToRevert)?; + + // Get the current migration's sequence to find the previous one + let current_migration = self + .get_migration(¤t_id) + .await? + .ok_or(ExecutionError::MigrationNotFound(current_id.clone()))?; + + // Find the previous migration (if any) + let previous = if current_migration.sequence > 1 { + let row = self + .client + .query_opt( + "SELECT id FROM tern.migrations WHERE sequence = $1", + &[&(current_migration.sequence - 1)], + ) + .await + .map_err(|e| ExecutionError::Query(e.to_string()))?; + row.map(|r| r.get::<_, String>(0)) + } else { + None + }; + + // Delete the current migration record + self.client + .execute("DELETE FROM tern.migrations WHERE id = $1", &[¤t_id]) + .await + .map_err(|e| ExecutionError::Query(e.to_string()))?; + + // Update the current pointer + if let Some(prev_id) = previous { + self.client + .execute( + "UPDATE tern.current SET migration_id = $1, updated_at = now()", + &[&prev_id], + ) + .await + .map_err(|e| ExecutionError::Query(e.to_string()))?; + } else { + // No previous migration - delete the current pointer + self.client + .execute("DELETE FROM tern.current", &[]) + .await + .map_err(|e| ExecutionError::Query(e.to_string()))?; + } + + Ok(current_id) + } } #[cfg(test)] From 1c55bd594a9f55494a870b4e8aea89d48678e60c Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 24 Jan 2026 17:43:47 +0000 Subject: [PATCH 2/2] Compute and persist down migrations at generation time FEATURE: Store inverse operations in migration files for offline reversal This changes down migrations from being computed on-the-fly at runtime to being computed and persisted when migrations are generated. Key changes: - Rename Migration.operations to up_operations - Add Migration.down_operations field for pre-computed inverse operations - Add src/db/migrate/inverse.rs module for computing inverse operations - Update all migration generation paths (generate, import, init) to compute and store down_operations - Simplify down command to use pre-computed operations - Add is_reversible() method to Migration Breaking change: Migration JSON format now uses up_operations/down_operations instead of operations field. --- src/cli/generate/mod.rs | 29 ++- src/cli/history/mod.rs | 1 + src/cli/import/mod.rs | 11 +- src/cli/inspect/mod.rs | 6 +- src/cli/migrate/down.rs | 248 +++--------------- src/cli/record/mod.rs | 15 +- src/cli/schema/migrate/mod.rs | 11 +- src/cli/show/mod.rs | 2 +- src/db/compile/codegen.rs | 9 +- src/db/compile/mod.rs | 12 +- src/db/execution/executor.rs | 16 +- src/db/migrate/inverse.rs | 388 ++++++++++++++++++++++++++++ src/db/migrate/mod.rs | 2 + src/db/state/init.rs | 15 +- src/db/state/local.rs | 14 +- src/db/state/mod.rs | 3 +- src/db/state/types.rs | 77 ++++-- tests/compile_integration_tests.rs | 2 + tests/state_reconstruction_tests.rs | 1 + 19 files changed, 601 insertions(+), 261 deletions(-) create mode 100644 src/db/migrate/inverse.rs diff --git a/src/cli/generate/mod.rs b/src/cli/generate/mod.rs index 3b5beb1..f23c556 100644 --- a/src/cli/generate/mod.rs +++ b/src/cli/generate/mod.rs @@ -13,7 +13,9 @@ use serde::Serialize; use crate::cli::{OutputFormat, ensure_backend_initialized, load_backend, print_json}; use crate::db::diff::breaking::{BreakingChange, MitigationStrategy, analyze_breaking_changes}; use crate::db::diff::diff_namespaces; -use crate::db::migrate::{MigrationPlan, PostgresRenderer, RenderConfig}; +use crate::db::migrate::{ + MigrationPlan, PostgresRenderer, RenderConfig, compute_inverse_operations, +}; use crate::db::pglite::SchemaLoader; use crate::db::state::{Migration, StateBackend, StateHash}; @@ -70,6 +72,8 @@ pub struct GenerateOutput { /// Breaking changes with details. #[serde(skip_serializing_if = "Vec::is_empty")] pub breaking_changes: Vec, + /// Whether the migration can be reversed with `down`. + pub is_reversible: bool, } /// Breaking change output for JSON serialization. @@ -117,6 +121,11 @@ impl std::fmt::Display for GenerateOutput { " To state: {}", &self.target_state_hash[..16.min(self.target_state_hash.len())] )?; + writeln!( + f, + " Reversible: {}", + if self.is_reversible { "yes" } else { "no" } + )?; if self.has_breaking_changes { writeln!(f)?; @@ -127,6 +136,12 @@ impl std::fmt::Display for GenerateOutput { } } + if !self.is_reversible { + writeln!(f)?; + writeln!(f, "NOTE: This migration contains irreversible operations.")?; + writeln!(f, " Running 'tern down' will not be possible.")?; + } + if self.dry_run { writeln!(f)?; writeln!(f, "This was a dry run. No migration was recorded.")?; @@ -195,6 +210,7 @@ impl Generate { target_state_hash: target_hash.to_hex(), dry_run: self.dry_run, breaking_changes: vec![], + is_reversible: true, // Empty migration is trivially reversible }; print_json(&output); } @@ -232,10 +248,15 @@ impl Generate { // Get the breaking changes for the migration let breaking_changes = analysis.into_changes(); + // Compute inverse operations for the down migration + let inverse_result = compute_inverse_operations(&plan.operations); + let down_operations = inverse_result.operations; + // Create the migration let migration = Migration::new( &self.description, plan.operations.clone(), + down_operations, source_hash, target_hash, breaking_changes.clone(), @@ -250,13 +271,14 @@ impl Generate { let output = GenerateOutput { migration_id: migration.id.to_hex(), description: self.description.clone(), - operation_count: migration.operations.len(), + operation_count: migration.up_operations.len(), has_breaking_changes: !breaking_changes.is_empty(), has_destructive_changes: destructive_count > 0, source_state_hash: source_hash.to_hex(), target_state_hash: target_hash.to_hex(), dry_run: self.dry_run, breaking_changes: breaking_change_outputs, + is_reversible: migration.is_reversible(), }; // Record the migration (unless dry run) @@ -308,6 +330,7 @@ mod tests { operation_count: 3, has_breaking_changes: false, has_destructive_changes: false, + is_reversible: true, source_state_hash: "0".repeat(64), target_state_hash: "1".repeat(64), dry_run: false, @@ -328,6 +351,7 @@ mod tests { operation_count: 1, has_breaking_changes: false, has_destructive_changes: false, + is_reversible: true, source_state_hash: "0".repeat(64), target_state_hash: "1".repeat(64), dry_run: true, @@ -346,6 +370,7 @@ mod tests { operation_count: 1, has_breaking_changes: false, has_destructive_changes: false, + is_reversible: true, source_state_hash: "src".to_string(), target_state_hash: "tgt".to_string(), dry_run: false, diff --git a/src/cli/history/mod.rs b/src/cli/history/mod.rs index 515ac23..364f029 100644 --- a/src/cli/history/mod.rs +++ b/src/cli/history/mod.rs @@ -230,6 +230,7 @@ mod tests { let m = Migration::new( "Second migration", vec![], + vec![], current_hash, crate::db::state::StateHash::from_bytes([1u8; 32]), vec![], diff --git a/src/cli/import/mod.rs b/src/cli/import/mod.rs index e32d1b4..c67a831 100644 --- a/src/cli/import/mod.rs +++ b/src/cli/import/mod.rs @@ -12,7 +12,9 @@ use serde::Serialize; use crate::cli::{OutputFormat, ensure_backend_initialized, load_backend, print_json}; use crate::db::diff::breaking::analyze_breaking_changes; use crate::db::diff::diff_namespaces; -use crate::db::migrate::{MigrationPlan, PostgresRenderer, RenderConfig}; +use crate::db::migrate::{ + MigrationPlan, PostgresRenderer, RenderConfig, compute_inverse_operations, +}; use crate::db::query::{PostgresCatalog, load_namespace}; use crate::db::state::{Migration, StateBackend, StateHash}; use crate::db::{self}; @@ -256,9 +258,14 @@ impl Import { let source_hash = StateHash::from_namespace(&expected_schema); let target_hash = StateHash::from_namespace(&live_schema); + // Compute inverse operations for the down migration + let inverse_result = compute_inverse_operations(&plan.operations); + let down_operations = inverse_result.operations; + let migration = Migration::new( &self.description, plan.operations.clone(), + down_operations, source_hash, target_hash, breaking_changes, @@ -280,7 +287,7 @@ impl Import { has_changes: true, migration_id, description: self.description, - operation_count: migration.operations.len(), + operation_count: migration.up_operations.len(), dry_run: self.dry_run, summary, }; diff --git a/src/cli/inspect/mod.rs b/src/cli/inspect/mod.rs index d10f2b5..0648dc5 100644 --- a/src/cli/inspect/mod.rs +++ b/src/cli/inspect/mod.rs @@ -184,10 +184,10 @@ fn inspect_json_migration(path: &PathBuf) -> miette::Result { .into_diagnostic() .wrap_err("Failed to parse migration JSON")?; - // Try to generate SQL from operations - let sql_statements = if !migration.operations.is_empty() { + // Try to generate SQL from up_operations + let sql_statements = if !migration.up_operations.is_empty() { use crate::db::migrate::{MigrationPlan, PostgresRenderer, RenderConfig}; - let plan = MigrationPlan::from_operations(migration.operations.clone()); + let plan = MigrationPlan::from_operations(migration.up_operations.clone()); let renderer = PostgresRenderer::new(RenderConfig::default()); let script = plan.render(&renderer); Some( diff --git a/src/cli/migrate/down.rs b/src/cli/migrate/down.rs index 7d6d47c..f1dbbcf 100644 --- a/src/cli/migrate/down.rs +++ b/src/cli/migrate/down.rs @@ -1,6 +1,7 @@ //! Migrate down command. //! -//! This command reverts the most recently applied migration. +//! This command reverts the most recently applied migration using the +//! pre-computed down_operations stored in the migration file. use std::path::PathBuf; @@ -11,7 +12,7 @@ use serde::Serialize; use crate::cli::{OutputFormat, ensure_backend_initialized, load_backend, print_json}; use crate::db::execution::MigrationTracker; -use crate::db::migrate::{MigrationPlan, Operation, PostgresRenderer, RenderConfig}; +use crate::db::migrate::{MigrationPlan, PostgresRenderer, RenderConfig}; use crate::db::state::{Migration, StateBackend}; use crate::db::{self}; @@ -68,7 +69,7 @@ pub struct RevertedMigrationInfo { pub id: String, /// Migration description. pub description: String, - /// Number of operations that were reverted. + /// Number of up operations in the migration. pub operation_count: usize, /// Number of SQL statements executed to revert. #[serde(skip_serializing_if = "Option::is_none")] @@ -121,186 +122,6 @@ impl std::fmt::Display for DownOutput { } } -/// Computes the inverse operations needed to revert a migration. -/// -/// Returns the operations in reverse order (last applied first reverted). -fn compute_inverse_operations(operations: &[Operation]) -> Result, String> { - let mut inverse_ops = Vec::new(); - - // Process operations in reverse order - for op in operations.iter().rev() { - let inverse = compute_single_inverse(op)?; - inverse_ops.push(inverse); - } - - Ok(inverse_ops) -} - -/// Computes the inverse of a single operation. -fn compute_single_inverse(op: &Operation) -> Result { - match op { - // Enum operations - Operation::CreateEnum { schema, enum_type } => Ok(Operation::DropEnum { - schema: schema.clone(), - name: enum_type.name.clone(), - }), - Operation::DropEnum { .. } => { - Err("Cannot revert DropEnum: enum definition is not preserved".to_string()) - } - Operation::RenameEnum { schema, from, to } => Ok(Operation::RenameEnum { - schema: schema.clone(), - from: to.clone(), - to: from.clone(), - }), - Operation::AddEnumValue { .. } => Err( - "Cannot revert AddEnumValue: PostgreSQL does not support removing enum values" - .to_string(), - ), - - // Sequence operations - Operation::CreateSequence { schema, sequence } => Ok(Operation::DropSequence { - schema: schema.clone(), - name: sequence.name.clone(), - }), - Operation::DropSequence { .. } => { - Err("Cannot revert DropSequence: sequence definition is not preserved".to_string()) - } - Operation::RenameSequence { schema, from, to } => Ok(Operation::RenameSequence { - schema: schema.clone(), - from: to.clone(), - to: from.clone(), - }), - Operation::AlterSequence { .. } => { - Err("Cannot revert AlterSequence: previous values are not preserved".to_string()) - } - - // Table operations - Operation::CreateTable { schema, table } => Ok(Operation::DropTable { - schema: schema.clone(), - name: table.name.clone(), - }), - Operation::DropTable { .. } => { - Err("Cannot revert DropTable: table definition and data are not preserved".to_string()) - } - Operation::RenameTable { schema, from, to } => Ok(Operation::RenameTable { - schema: schema.clone(), - from: to.clone(), - to: from.clone(), - }), - - // Column operations - Operation::AddColumn { - schema, - table, - column, - } => Ok(Operation::DropColumn { - schema: schema.clone(), - table: table.clone(), - name: column.name.clone(), - }), - Operation::DropColumn { .. } => Err( - "Cannot revert DropColumn: column definition and data are not preserved".to_string(), - ), - Operation::RenameColumn { - schema, - table, - from, - to, - } => Ok(Operation::RenameColumn { - schema: schema.clone(), - table: table.clone(), - from: to.clone(), - to: from.clone(), - }), - Operation::AlterColumn { .. } => { - Err("Cannot revert AlterColumn: previous values are not preserved".to_string()) - } - - // Constraint operations - Operation::AddConstraint { - schema, - table, - constraint, - } => Ok(Operation::DropConstraint { - schema: schema.clone(), - table: table.clone(), - name: constraint.name.clone(), - }), - Operation::DropConstraint { .. } => { - Err("Cannot revert DropConstraint: constraint definition is not preserved".to_string()) - } - Operation::RenameConstraint { - schema, - table, - from, - to, - } => Ok(Operation::RenameConstraint { - schema: schema.clone(), - table: table.clone(), - from: to.clone(), - to: from.clone(), - }), - - // Index operations - Operation::CreateIndex { - schema, - index, - concurrently, - .. - } => Ok(Operation::DropIndex { - schema: schema.clone(), - name: index.name.clone(), - concurrently: *concurrently, - }), - Operation::DropIndex { .. } => { - Err("Cannot revert DropIndex: index definition is not preserved".to_string()) - } - Operation::RenameIndex { schema, from, to } => Ok(Operation::RenameIndex { - schema: schema.clone(), - from: to.clone(), - to: from.clone(), - }), - - // View operations - Operation::CreateView { schema, view } => Ok(Operation::DropView { - schema: schema.clone(), - name: view.name.clone(), - is_materialized: view.is_materialized, - }), - Operation::DropView { .. } => { - Err("Cannot revert DropView: view definition is not preserved".to_string()) - } - Operation::RenameView { - schema, - from, - to, - is_materialized, - } => Ok(Operation::RenameView { - schema: schema.clone(), - from: to.clone(), - to: from.clone(), - is_materialized: *is_materialized, - }), - Operation::ReplaceView { .. } => { - Err("Cannot revert ReplaceView: previous view definition is not preserved".to_string()) - } - Operation::RefreshMaterializedView { .. } => { - // Refreshing a materialized view is idempotent, so we can just skip it - // Return a no-op by creating a comment that won't change anything - // Actually, there's no true no-op, so we'll just error - Err( - "Cannot revert RefreshMaterializedView: this operation cannot be undone" - .to_string(), - ) - } - - // Comment operations - Operation::SetComment { .. } => { - Err("Cannot revert SetComment: previous comment is not preserved".to_string()) - } - } -} - impl Down { /// Dispatch the down command. pub async fn dispatch(self) -> miette::Result<()> { @@ -371,29 +192,32 @@ impl Down { )); } - // Compute inverse operations - let inverse_ops = match compute_inverse_operations(&migration.operations) { - Ok(ops) => ops, - Err(msg) => { - let output = DownOutput { - success: false, - dry_run: self.dry_run, - migration: Some(RevertedMigrationInfo { - id: migration.id.to_hex(), - description: migration.description.clone(), - operation_count: migration.operations.len(), - statement_count: None, - }), - error: Some(msg), - }; - match self.format { - OutputFormat::Text => println!("{}", output), - OutputFormat::Json => print_json(&output), - OutputFormat::Sql => println!("-- Cannot compute inverse operations"), - } - std::process::exit(1); + // Check if the migration is reversible (has pre-computed down_operations) + if !migration.is_reversible() { + let output = DownOutput { + success: false, + dry_run: self.dry_run, + migration: Some(RevertedMigrationInfo { + id: migration.id.to_hex(), + description: migration.description.clone(), + operation_count: migration.up_operations.len(), + statement_count: None, + }), + error: Some( + "This migration contains irreversible operations and cannot be reverted." + .to_string(), + ), + }; + match self.format { + OutputFormat::Text => println!("{}", output), + OutputFormat::Json => print_json(&output), + OutputFormat::Sql => println!("-- Migration is not reversible"), } - }; + std::process::exit(1); + } + + // Use the pre-computed down_operations + let down_ops = migration.down_operations.clone(); if self.dry_run { // Just show what would be reverted @@ -403,7 +227,7 @@ impl Down { migration: Some(RevertedMigrationInfo { id: migration.id.to_hex(), description: migration.description.clone(), - operation_count: migration.operations.len(), + operation_count: migration.up_operations.len(), statement_count: None, }), error: None, @@ -417,9 +241,9 @@ impl Down { "-- Dry run: would revert migration {}", migration.id.to_short_hex() ); - // Render the inverse operations to show what SQL would be executed + // Render the down operations to show what SQL would be executed let renderer = PostgresRenderer::new(RenderConfig::default()); - let plan = MigrationPlan::from_operations(inverse_ops); + let plan = MigrationPlan::from_operations(down_ops); let script = plan.render(&renderer); println!("{}", script.to_sql()); } @@ -432,7 +256,7 @@ impl Down { // Render migration to SQL let renderer = PostgresRenderer::new(RenderConfig::default()); - let plan = MigrationPlan::from_operations(inverse_ops); + let plan = MigrationPlan::from_operations(down_ops); let script = plan.render(&renderer); let sql = script.to_sql(); let statement_count = script.all_statements().len(); @@ -451,7 +275,7 @@ impl Down { return Err(miette::miette!("Failed to set search_path: {}", e)); } - // Execute the inverse operations + // Execute the down operations if !sql.is_empty() && let Err(e) = client.batch_execute(&sql).await { @@ -462,7 +286,7 @@ impl Down { migration: Some(RevertedMigrationInfo { id: migration.id.to_hex(), description: migration.description.clone(), - operation_count: migration.operations.len(), + operation_count: migration.up_operations.len(), statement_count: Some(statement_count), }), error: Some(e.to_string()), @@ -494,7 +318,7 @@ impl Down { migration: Some(RevertedMigrationInfo { id: migration.id.to_hex(), description: migration.description.clone(), - operation_count: migration.operations.len(), + operation_count: migration.up_operations.len(), statement_count: Some(statement_count), }), error: None, diff --git a/src/cli/record/mod.rs b/src/cli/record/mod.rs index 9f694b1..fbe4d0f 100644 --- a/src/cli/record/mod.rs +++ b/src/cli/record/mod.rs @@ -95,10 +95,10 @@ impl Record { let new_state = if let Some(ref state) = migration.checkpoint_state { state.clone() } else { - // We need to apply the operations to the current state + // We need to apply the up_operations to the current state let current_state = backend.get_current_state().await.into_diagnostic()?; current_state - .apply(&migration.operations) + .apply(&migration.up_operations) .into_diagnostic() .wrap_err("Failed to apply migration operations")? }; @@ -205,7 +205,15 @@ pub fn create_sync_migration( parent_hash: StateHash, resulting_hash: StateHash, ) -> Migration { - Migration::new(description, vec![], parent_hash, resulting_hash, vec![]) + // Sync migrations have no operations, so they're trivially reversible + Migration::new( + description, + vec![], + vec![], + parent_hash, + resulting_hash, + vec![], + ) } #[cfg(test)] @@ -261,6 +269,7 @@ mod tests { let migration = Migration::new( "Test migration", vec![], + vec![], current_hash, StateHash::from_bytes([1u8; 32]), vec![], diff --git a/src/cli/schema/migrate/mod.rs b/src/cli/schema/migrate/mod.rs index 1d3caed..ec6b207 100644 --- a/src/cli/schema/migrate/mod.rs +++ b/src/cli/schema/migrate/mod.rs @@ -14,7 +14,9 @@ use super::diff::BreakingChangeOutput; use crate::cli::{OutputFormat, ensure_backend_initialized, load_backend, print_json}; use crate::db::diff::breaking::{MitigationStrategy, analyze_breaking_changes}; use crate::db::diff::diff_namespaces; -use crate::db::migrate::{MigrationPlan, PostgresRenderer, RenderConfig}; +use crate::db::migrate::{ + MigrationPlan, PostgresRenderer, RenderConfig, compute_inverse_operations, +}; use crate::db::pglite::SchemaLoader; use crate::db::state::{Migration, StateBackend, StateHash}; @@ -146,10 +148,15 @@ impl Migrate { // Get the breaking changes for the migration let breaking_changes = analysis.into_changes(); + // Compute inverse operations for the down migration + let inverse_result = compute_inverse_operations(&plan.operations); + let down_operations = inverse_result.operations; + // Create the migration let migration = Migration::new( &self.description, plan.operations.clone(), + down_operations, source_hash, target_hash, breaking_changes.clone(), @@ -164,7 +171,7 @@ impl Migrate { let output = SchemaMigrateOutput { migration_id: migration.id.to_hex(), description: self.description.clone(), - operation_count: migration.operations.len(), + operation_count: migration.up_operations.len(), has_breaking_changes: !breaking_changes.is_empty(), has_destructive_changes: destructive_count > 0, source_state_hash: source_hash.to_hex(), diff --git a/src/cli/show/mod.rs b/src/cli/show/mod.rs index eeb1f9b..18060c8 100644 --- a/src/cli/show/mod.rs +++ b/src/cli/show/mod.rs @@ -132,7 +132,7 @@ impl Show { // Generate SQL if needed let sql_statements = if matches!(self.format, OutputFormat::Sql) { - let plan = MigrationPlan::from_operations(migration.operations.clone()); + let plan = MigrationPlan::from_operations(migration.up_operations.clone()); let renderer = PostgresRenderer::new(RenderConfig::default()); let script = plan.render(&renderer); Some( diff --git a/src/db/compile/codegen.rs b/src/db/compile/codegen.rs index a77b09a..e4418a0 100644 --- a/src/db/compile/codegen.rs +++ b/src/db/compile/codegen.rs @@ -637,6 +637,7 @@ mod tests { Migration::new( "Test migration", vec![], + vec![], StateHash::zero(), StateHash::from_bytes([1u8; 32]), vec![], @@ -653,6 +654,7 @@ mod tests { let migration = Migration::new( "Add users table", ops.clone(), + vec![], StateHash::zero(), StateHash::from_bytes([1u8; 32]), vec![], @@ -777,6 +779,7 @@ mod tests { let migration = Migration::new( "Drop users table", ops.clone(), + vec![], StateHash::zero(), StateHash::from_bytes([1u8; 32]), breaking_changes, @@ -810,6 +813,7 @@ mod tests { let migration = Migration::new( "Destructive changes", vec![], + vec![], StateHash::zero(), StateHash::from_bytes([1u8; 32]), breaking_changes, @@ -927,6 +931,7 @@ mod tests { let migration = Migration::new( "Test with 'quotes' and \"double quotes\"", vec![], + vec![], StateHash::zero(), StateHash::from_bytes([1u8; 32]), vec![], @@ -950,9 +955,10 @@ mod tests { let compiler = MigrationCompiler::with_config(CompilerConfig::macro_only()); // Create a migration with SQL containing special characters - let migration = Migration::new( + let _migration = Migration::new( "Test SQL", vec![], + vec![], StateHash::zero(), StateHash::from_bytes([1u8; 32]), vec![], @@ -978,6 +984,7 @@ mod tests { let migration = Migration::new( "Line 1\nLine 2\nLine 3", vec![], + vec![], StateHash::zero(), StateHash::from_bytes([1u8; 32]), vec![], diff --git a/src/db/compile/mod.rs b/src/db/compile/mod.rs index 9c937ad..d379b24 100644 --- a/src/db/compile/mod.rs +++ b/src/db/compile/mod.rs @@ -219,7 +219,7 @@ impl std::fmt::Display for PackageFormat { use crate::db::diff::breaking::analyze_breaking_changes; use crate::db::diff::diff_namespaces; -use crate::db::migrate::MigrationPlan; +use crate::db::migrate::{MigrationPlan, compute_inverse_operations}; use crate::db::model::Namespace; use crate::db::state::{Migration, StateHash}; @@ -399,10 +399,15 @@ pub fn compile_migration( let source_hash = StateHash::from_namespace(source); let target_hash = StateHash::from_namespace(target); + // Step 4.5: Compute inverse operations for down migration + let inverse_result = compute_inverse_operations(&plan.operations); + let down_operations = inverse_result.operations; + // Step 5: Create the migration record let migration = Migration::new( &options.description, plan.operations.clone(), + down_operations, source_hash, target_hash, breaking_changes.into_changes(), @@ -553,6 +558,7 @@ mod tests { let migration = Migration::new( "Add email column to users table", plan.operations.clone(), + vec![], source_hash, target_hash, vec![], // No breaking changes for adding nullable column @@ -617,6 +623,7 @@ mod tests { let migration = Migration::new( "Drop users table", plan.operations.clone(), + vec![], source_hash, target_hash, breaking_changes.into_changes(), @@ -682,6 +689,7 @@ mod tests { let migration = Migration::new( "Create users table with status enum", plan.operations.clone(), + vec![], source_hash, target_hash, vec![], @@ -714,6 +722,7 @@ mod tests { let migration = Migration::new( r#"Add "quoted" column with 'single quotes' and \backslashes\"#, plan.operations.clone(), + vec![], source_hash, target_hash, vec![], @@ -746,6 +755,7 @@ mod tests { let migration = Migration::new( "Empty migration", plan.operations.clone(), + vec![], source_hash, target_hash, vec![], diff --git a/src/db/execution/executor.rs b/src/db/execution/executor.rs index 01dabe0..976b42b 100644 --- a/src/db/execution/executor.rs +++ b/src/db/execution/executor.rs @@ -182,7 +182,7 @@ impl<'a> MigrationExecutor<'a> { ) -> Result { // Render migration to SQL let renderer = PostgresRenderer::new(RenderConfig::default()); - let plan = MigrationPlan::from_operations(migration.operations.clone()); + let plan = MigrationPlan::from_operations(migration.up_operations.clone()); let script = plan.render(&renderer); let sql = script.to_sql(); let statement_count = script.all_statements().len(); @@ -250,16 +250,16 @@ impl<'a> MigrationExecutor<'a> { } } -/// Computes the BLAKE3 hash of a migration's operations. +/// Computes the BLAKE3 hash of a migration's up_operations. /// /// This hash is used to detect if a migration has been modified since it was -/// applied. Only the operations array is hashed, not the description or +/// applied. Only the up_operations array is hashed, not the description or /// timestamps, allowing descriptions to be updated without triggering /// divergence errors. fn compute_migration_hash(migration: &Migration) -> String { let mut hasher = blake3::Hasher::new(); let ops_json = - serde_json::to_vec(&migration.operations).expect("operations should be serializable"); + serde_json::to_vec(&migration.up_operations).expect("operations should be serializable"); hasher.update(&ops_json); hasher.finalize().to_hex().to_string() } @@ -273,14 +273,16 @@ mod tests { fn compute_migration_hash_is_deterministic() { let migration1 = Migration::new( "Test migration", - vec![], + vec![], // up_operations + vec![], // down_operations StateHash::zero(), StateHash::zero(), vec![], ); let migration2 = Migration::new( "Different description", // Different description - vec![], // Same operations + vec![], // Same up_operations + vec![], // down_operations StateHash::zero(), StateHash::zero(), vec![], @@ -289,7 +291,7 @@ mod tests { let hash1 = compute_migration_hash(&migration1); let hash2 = compute_migration_hash(&migration2); - // Hashes should be the same since only operations matter + // Hashes should be the same since only up_operations matter assert_eq!(hash1, hash2); } } diff --git a/src/db/migrate/inverse.rs b/src/db/migrate/inverse.rs new file mode 100644 index 0000000..4896f96 --- /dev/null +++ b/src/db/migrate/inverse.rs @@ -0,0 +1,388 @@ +//! Inverse operation computation for migration reversal. +//! +//! This module provides functions to compute the inverse (reverse) operations +//! needed to revert a migration. Inverse operations are computed at migration +//! generation time and stored alongside the forward operations. + +use super::Operation; + +/// Error type for inverse operation computation. +#[derive(Debug, Clone, thiserror::Error)] +pub enum InverseError { + /// The operation cannot be reversed. + #[error("{0}")] + Irreversible(String), +} + +/// Result of computing inverse operations. +#[derive(Debug, Clone)] +pub struct InverseResult { + /// The computed inverse operations, in reverse order. + pub operations: Vec, + + /// Warnings about operations that could not be reversed. + /// If this is non-empty, the `operations` list will be empty. + pub errors: Vec, +} + +impl InverseResult { + /// Returns true if all operations were successfully inverted. + pub fn is_success(&self) -> bool { + self.errors.is_empty() + } + + /// Returns true if any operations could not be inverted. + pub fn has_errors(&self) -> bool { + !self.errors.is_empty() + } +} + +/// Computes the inverse operations needed to revert a migration. +/// +/// Returns the operations in reverse order (last applied first reverted). +/// If any operation is irreversible, returns an empty operations list with errors. +pub fn compute_inverse_operations(operations: &[Operation]) -> InverseResult { + let mut inverse_ops = Vec::new(); + let mut errors = Vec::new(); + + // Process operations in reverse order + for op in operations.iter().rev() { + match compute_single_inverse(op) { + Ok(inverse) => inverse_ops.push(inverse), + Err(e) => errors.push(e), + } + } + + // If there were any errors, return empty operations + if !errors.is_empty() { + InverseResult { + operations: vec![], + errors, + } + } else { + InverseResult { + operations: inverse_ops, + errors: vec![], + } + } +} + +/// Computes the inverse of a single operation. +fn compute_single_inverse(op: &Operation) -> Result { + match op { + // Enum operations + Operation::CreateEnum { schema, enum_type } => Ok(Operation::DropEnum { + schema: schema.clone(), + name: enum_type.name.clone(), + }), + Operation::DropEnum { .. } => Err(InverseError::Irreversible( + "Cannot revert DropEnum: enum definition is not preserved".to_string(), + )), + Operation::RenameEnum { schema, from, to } => Ok(Operation::RenameEnum { + schema: schema.clone(), + from: to.clone(), + to: from.clone(), + }), + Operation::AddEnumValue { .. } => Err(InverseError::Irreversible( + "Cannot revert AddEnumValue: PostgreSQL does not support removing enum values" + .to_string(), + )), + + // Sequence operations + Operation::CreateSequence { schema, sequence } => Ok(Operation::DropSequence { + schema: schema.clone(), + name: sequence.name.clone(), + }), + Operation::DropSequence { .. } => Err(InverseError::Irreversible( + "Cannot revert DropSequence: sequence definition is not preserved".to_string(), + )), + Operation::RenameSequence { schema, from, to } => Ok(Operation::RenameSequence { + schema: schema.clone(), + from: to.clone(), + to: from.clone(), + }), + Operation::AlterSequence { .. } => Err(InverseError::Irreversible( + "Cannot revert AlterSequence: previous values are not preserved".to_string(), + )), + + // Table operations + Operation::CreateTable { schema, table } => Ok(Operation::DropTable { + schema: schema.clone(), + name: table.name.clone(), + }), + Operation::DropTable { .. } => Err(InverseError::Irreversible( + "Cannot revert DropTable: table definition and data are not preserved".to_string(), + )), + Operation::RenameTable { schema, from, to } => Ok(Operation::RenameTable { + schema: schema.clone(), + from: to.clone(), + to: from.clone(), + }), + + // Column operations + Operation::AddColumn { + schema, + table, + column, + } => Ok(Operation::DropColumn { + schema: schema.clone(), + table: table.clone(), + name: column.name.clone(), + }), + Operation::DropColumn { .. } => Err(InverseError::Irreversible( + "Cannot revert DropColumn: column definition and data are not preserved".to_string(), + )), + Operation::RenameColumn { + schema, + table, + from, + to, + } => Ok(Operation::RenameColumn { + schema: schema.clone(), + table: table.clone(), + from: to.clone(), + to: from.clone(), + }), + Operation::AlterColumn { .. } => Err(InverseError::Irreversible( + "Cannot revert AlterColumn: previous values are not preserved".to_string(), + )), + + // Constraint operations + Operation::AddConstraint { + schema, + table, + constraint, + } => Ok(Operation::DropConstraint { + schema: schema.clone(), + table: table.clone(), + name: constraint.name.clone(), + }), + Operation::DropConstraint { .. } => Err(InverseError::Irreversible( + "Cannot revert DropConstraint: constraint definition is not preserved".to_string(), + )), + Operation::RenameConstraint { + schema, + table, + from, + to, + } => Ok(Operation::RenameConstraint { + schema: schema.clone(), + table: table.clone(), + from: to.clone(), + to: from.clone(), + }), + + // Index operations + Operation::CreateIndex { + schema, + index, + concurrently, + .. + } => Ok(Operation::DropIndex { + schema: schema.clone(), + name: index.name.clone(), + concurrently: *concurrently, + }), + Operation::DropIndex { .. } => Err(InverseError::Irreversible( + "Cannot revert DropIndex: index definition is not preserved".to_string(), + )), + Operation::RenameIndex { schema, from, to } => Ok(Operation::RenameIndex { + schema: schema.clone(), + from: to.clone(), + to: from.clone(), + }), + + // View operations + Operation::CreateView { schema, view } => Ok(Operation::DropView { + schema: schema.clone(), + name: view.name.clone(), + is_materialized: view.is_materialized, + }), + Operation::DropView { .. } => Err(InverseError::Irreversible( + "Cannot revert DropView: view definition is not preserved".to_string(), + )), + Operation::RenameView { + schema, + from, + to, + is_materialized, + } => Ok(Operation::RenameView { + schema: schema.clone(), + from: to.clone(), + to: from.clone(), + is_materialized: *is_materialized, + }), + Operation::ReplaceView { .. } => Err(InverseError::Irreversible( + "Cannot revert ReplaceView: previous view definition is not preserved".to_string(), + )), + Operation::RefreshMaterializedView { .. } => Err(InverseError::Irreversible( + "Cannot revert RefreshMaterializedView: this operation cannot be undone".to_string(), + )), + + // Comment operations + Operation::SetComment { .. } => Err(InverseError::Irreversible( + "Cannot revert SetComment: previous comment is not preserved".to_string(), + )), + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::db::model::{EnumType, Table, TableKind}; + use crate::db::schema::{Oid, SchemaName, TableName, TypeName}; + + fn test_schema() -> SchemaName { + SchemaName::try_new("public".to_string()).unwrap() + } + + fn test_table_name() -> TableName { + TableName::try_new("users".to_string()).unwrap() + } + + fn test_table() -> Table { + Table { + oid: Oid::new(1), + name: test_table_name(), + kind: TableKind::Regular, + columns: vec![], + constraints: vec![], + indexes: vec![], + comment: None, + } + } + + fn test_enum() -> EnumType { + EnumType { + oid: Oid::new(1), + name: TypeName::try_new("status".to_string()).unwrap(), + values: vec!["active".to_string(), "inactive".to_string()], + comment: None, + } + } + + #[test] + fn create_table_inverse_is_drop_table() { + let op = Operation::CreateTable { + schema: test_schema(), + table: test_table(), + }; + let result = compute_inverse_operations(&[op]); + + assert!(result.is_success()); + assert_eq!(result.operations.len(), 1); + assert!(matches!( + &result.operations[0], + Operation::DropTable { schema, name } + if schema.as_ref() == "public" && name.as_ref() == "users" + )); + } + + #[test] + fn drop_table_is_irreversible() { + let op = Operation::DropTable { + schema: test_schema(), + name: test_table_name(), + }; + let result = compute_inverse_operations(&[op]); + + assert!(result.has_errors()); + assert!(result.operations.is_empty()); + assert_eq!(result.errors.len(), 1); + } + + #[test] + fn rename_table_swaps_names() { + let op = Operation::RenameTable { + schema: test_schema(), + from: TableName::try_new("old_name".to_string()).unwrap(), + to: TableName::try_new("new_name".to_string()).unwrap(), + }; + let result = compute_inverse_operations(&[op]); + + assert!(result.is_success()); + assert!(matches!( + &result.operations[0], + Operation::RenameTable { from, to, .. } + if from.as_ref() == "new_name" && to.as_ref() == "old_name" + )); + } + + #[test] + fn create_enum_inverse_is_drop_enum() { + let op = Operation::CreateEnum { + schema: test_schema(), + enum_type: test_enum(), + }; + let result = compute_inverse_operations(&[op]); + + assert!(result.is_success()); + assert!(matches!( + &result.operations[0], + Operation::DropEnum { name, .. } + if name.as_ref() == "status" + )); + } + + #[test] + fn add_enum_value_is_irreversible() { + let op = Operation::AddEnumValue { + schema: test_schema(), + enum_name: TypeName::try_new("status".to_string()).unwrap(), + value: "pending".to_string(), + position: crate::db::migrate::EnumValuePosition::End, + }; + let result = compute_inverse_operations(&[op]); + + assert!(result.has_errors()); + assert!(result.operations.is_empty()); + } + + #[test] + fn multiple_operations_reversed_in_order() { + let ops = vec![ + Operation::CreateEnum { + schema: test_schema(), + enum_type: test_enum(), + }, + Operation::CreateTable { + schema: test_schema(), + table: test_table(), + }, + ]; + let result = compute_inverse_operations(&ops); + + assert!(result.is_success()); + assert_eq!(result.operations.len(), 2); + + // Order should be reversed: DropTable first, then DropEnum + assert!(matches!(&result.operations[0], Operation::DropTable { .. })); + assert!(matches!(&result.operations[1], Operation::DropEnum { .. })); + } + + #[test] + fn mixed_reversible_irreversible_fails() { + let ops = vec![ + Operation::CreateTable { + schema: test_schema(), + table: test_table(), + }, + Operation::DropTable { + schema: test_schema(), + name: TableName::try_new("other".to_string()).unwrap(), + }, + ]; + let result = compute_inverse_operations(&ops); + + // Should fail because DropTable is irreversible + assert!(result.has_errors()); + assert!(result.operations.is_empty()); + } + + #[test] + fn empty_operations_returns_empty() { + let result = compute_inverse_operations(&[]); + assert!(result.is_success()); + assert!(result.operations.is_empty()); + } +} diff --git a/src/db/migrate/mod.rs b/src/db/migrate/mod.rs index dedc1b0..7000131 100644 --- a/src/db/migrate/mod.rs +++ b/src/db/migrate/mod.rs @@ -33,6 +33,7 @@ mod collector; mod error; +mod inverse; mod operation; mod ordering; mod plan; @@ -41,6 +42,7 @@ mod script; pub use collector::{CollectorConfig, OperationCollector}; pub use error::MigrationError; +pub use inverse::{InverseError, InverseResult, compute_inverse_operations}; pub use operation::{ ColumnChanges, CommentTarget, DefaultChange, EnumValuePosition, GeneratedChange, IdentityChange, ObjectKind, Operation, OperationId, SequenceChanges, SetColumnType, diff --git a/src/db/state/init.rs b/src/db/state/init.rs index 08eebcb..e48d994 100644 --- a/src/db/state/init.rs +++ b/src/db/state/init.rs @@ -9,7 +9,7 @@ #![allow(unused_assignments)] use crate::db::diff::diff_namespaces; -use crate::db::migrate::MigrationPlan; +use crate::db::migrate::{MigrationPlan, compute_inverse_operations}; use crate::db::model::Namespace; use crate::db::query::{Catalog, QueryError, load_namespace}; @@ -118,8 +118,13 @@ where let diff = diff_namespaces(&empty, &namespace); let plan = MigrationPlan::from_diff(&diff); + // Compute inverse operations for the down migration + let inverse_result = compute_inverse_operations(&plan.operations); + let down_operations = inverse_result.operations; + // Create and save baseline migration with operations - let baseline = Migration::baseline_with_operations(namespace.clone(), plan.operations); + let baseline = + Migration::baseline_with_operations(namespace.clone(), plan.operations, down_operations); backend.save_migration(&baseline).await?; // Save current state @@ -252,7 +257,7 @@ mod tests { assert!(baseline.is_baseline()); assert!(baseline.is_checkpoint()); assert!(baseline.parent_state_hash.is_zero()); - assert_eq!(baseline.operations.len(), 0); + assert_eq!(baseline.up_operations.len(), 0); // Verify backend state assert!(backend.is_initialized().await.unwrap()); @@ -307,8 +312,8 @@ mod tests { assert!(baseline.parent_state_hash.is_zero()); assert!(baseline.is_checkpoint()); - // NEW: Baseline now has operations to create the schema from scratch - assert!(!baseline.operations.is_empty()); + // NEW: Baseline now has up_operations to create the schema from scratch + assert!(!baseline.up_operations.is_empty()); // Should have at least one operation to create the table assert!(baseline.operation_count() >= 1); diff --git a/src/db/state/local.rs b/src/db/state/local.rs index ddd8308..8d1a6fd 100644 --- a/src/db/state/local.rs +++ b/src/db/state/local.rs @@ -483,10 +483,10 @@ impl StateBackend for LocalFileBackend { // Find nearest checkpoint and apply operations let (mut state, start_idx) = self.find_nearest_checkpoint(&migrations, target_idx)?; - // Apply operations from checkpoint to target (exclusive of checkpoint migration itself + // Apply up_operations from checkpoint to target (exclusive of checkpoint migration itself // since its state is already included) for migration in &migrations[start_idx..=target_idx] { - state = state.apply(&migration.operations).map_err(|e| { + state = state.apply(&migration.up_operations).map_err(|e| { StateError::ReconstructionFailed { id: migration.id, message: e.to_string(), @@ -688,6 +688,7 @@ mod tests { let m2 = Migration::new( "Second migration", vec![], + vec![], m1.resulting_state_hash, test_state_hash(2), vec![], @@ -697,6 +698,7 @@ mod tests { let m3 = Migration::new( "Third migration", vec![], + vec![], m2.resulting_state_hash, test_state_hash(3), vec![], @@ -735,7 +737,7 @@ mod tests { // After second migration let result_hash = test_state_hash(99); - let m2 = Migration::new("Second", vec![], expected_hash, result_hash, vec![]); + let m2 = Migration::new("Second", vec![], vec![], expected_hash, result_hash, vec![]); backend.save_migration(&m2).await.unwrap(); assert_eq!(backend.get_current_state_hash().await.unwrap(), result_hash); @@ -757,6 +759,7 @@ mod tests { let m2 = Migration::new( "Second", vec![], + vec![], m1.resulting_state_hash, test_state_hash(22), vec![], @@ -790,6 +793,7 @@ mod tests { let m2 = Migration::new( "Second", vec![], + vec![], m1.resulting_state_hash, test_state_hash(22), vec![], @@ -816,6 +820,7 @@ mod tests { let m2 = Migration::new( "Second", vec![], + vec![], m1.resulting_state_hash, test_state_hash(22), vec![], @@ -825,6 +830,7 @@ mod tests { let m3 = Migration::new( "Third", vec![], + vec![], m2.resulting_state_hash, test_state_hash(33), vec![], @@ -923,6 +929,7 @@ mod tests { let m2 = Migration::new( "Second", vec![], + vec![], m1.resulting_state_hash, test_state_hash(2), vec![], @@ -932,6 +939,7 @@ mod tests { let m3 = Migration::new( "Third", vec![], + vec![], m2.resulting_state_hash, test_state_hash(3), vec![], diff --git a/src/db/state/mod.rs b/src/db/state/mod.rs index 09fb5f1..7df4e9d 100644 --- a/src/db/state/mod.rs +++ b/src/db/state/mod.rs @@ -380,7 +380,7 @@ mod tests { })?; for migration in &migrations[start_idx..=target_idx] { - state = state.apply(&migration.operations).map_err(|e| { + state = state.apply(&migration.up_operations).map_err(|e| { StateError::ReconstructionFailed { id: migration.id, message: e.to_string(), @@ -465,6 +465,7 @@ mod tests { let m2 = Migration::new( "Second", vec![], + vec![], m1.resulting_state_hash, StateHash::from_bytes([22u8; 32]), vec![], diff --git a/src/db/state/types.rs b/src/db/state/types.rs index 2ffbf23..9bb5119 100644 --- a/src/db/state/types.rs +++ b/src/db/state/types.rs @@ -300,8 +300,12 @@ pub struct Migration { /// When the migration was created (not applied). pub created_at: Timestamp, - /// The operations that make up this migration. - pub operations: Vec, + /// The forward operations that make up this migration (applied with `up`). + pub up_operations: Vec, + + /// The reverse operations to undo this migration (applied with `down`). + /// Empty if the migration contains irreversible operations. + pub down_operations: Vec, /// Hash of the schema state before this migration. pub parent_state_hash: StateHash, @@ -321,23 +325,25 @@ pub struct Migration { impl Migration { /// Creates a new migration with the given parameters. /// - /// The migration ID is computed automatically from the content. + /// The migration ID is computed automatically from the up_operations content. #[must_use] pub fn new( description: impl Into, - operations: Vec, + up_operations: Vec, + down_operations: Vec, parent_state_hash: StateHash, resulting_state_hash: StateHash, breaking_changes: Vec, ) -> Self { let description = description.into(); - let id = MigrationId::from_content(&operations, &parent_state_hash, &description); + let id = MigrationId::from_content(&up_operations, &parent_state_hash, &description); Self { id, description, created_at: Timestamp::now(), - operations, + up_operations, + down_operations, parent_state_hash, resulting_state_hash, breaking_changes, @@ -359,7 +365,8 @@ impl Migration { id, description, created_at: Timestamp::now(), - operations: vec![], + up_operations: vec![], + down_operations: vec![], parent_state_hash: StateHash::zero(), resulting_state_hash: resulting_hash, breaking_changes: vec![], @@ -376,18 +383,24 @@ impl Migration { /// # Arguments /// /// * `namespace` - The target schema state - /// * `operations` - Operations to create the schema from empty + /// * `up_operations` - Operations to create the schema from empty + /// * `down_operations` - Operations to revert to empty (typically drop all) #[must_use] - pub fn baseline_with_operations(namespace: Namespace, operations: Vec) -> Self { + pub fn baseline_with_operations( + namespace: Namespace, + up_operations: Vec, + down_operations: Vec, + ) -> Self { let description = "Baseline migration from existing database".to_string(); - let id = MigrationId::from_content(&operations, &StateHash::zero(), &description); + let id = MigrationId::from_content(&up_operations, &StateHash::zero(), &description); let resulting_hash = StateHash::from_namespace(&namespace); Self { id, description, created_at: Timestamp::now(), - operations, + up_operations, + down_operations, parent_state_hash: StateHash::zero(), resulting_state_hash: resulting_hash, breaking_changes: vec![], @@ -418,16 +431,24 @@ impl Migration { self.checkpoint_state.is_some() } - /// Returns true if this is a baseline migration (no operations). + /// Returns true if this is a baseline migration (no up operations). #[must_use] pub fn is_baseline(&self) -> bool { - self.operations.is_empty() && self.parent_state_hash.is_zero() + self.up_operations.is_empty() && self.parent_state_hash.is_zero() } - /// Returns the number of operations in this migration. + /// Returns the number of up operations in this migration. #[must_use] pub fn operation_count(&self) -> usize { - self.operations.len() + self.up_operations.len() + } + + /// Returns true if this migration can be reverted. + /// + /// A migration can be reverted if it has down operations defined. + #[must_use] + pub fn is_reversible(&self) -> bool { + !self.down_operations.is_empty() } } @@ -734,8 +755,8 @@ mod tests { fn new_migration_computes_id() { let parent = StateHash::zero(); let result = StateHash::from_bytes([1u8; 32]); - let m1 = Migration::new("test", vec![], parent, result, vec![]); - let m2 = Migration::new("test", vec![], parent, result, vec![]); + let m1 = Migration::new("test", vec![], vec![], parent, result, vec![]); + let m2 = Migration::new("test", vec![], vec![], parent, result, vec![]); // Same content should produce same ID assert_eq!(m1.id, m2.id); @@ -745,13 +766,33 @@ mod tests { fn with_checkpoint_sets_state() { let parent = StateHash::zero(); let result = StateHash::from_bytes([1u8; 32]); - let migration = Migration::new("test", vec![], parent, result, vec![]); + let migration = Migration::new("test", vec![], vec![], parent, result, vec![]); assert!(!migration.is_checkpoint()); let ns = Namespace::empty("public"); let migration = migration.with_checkpoint(ns); assert!(migration.is_checkpoint()); } + + #[test] + fn is_reversible_with_down_operations() { + use crate::db::schema::{SchemaName, TableName}; + + let parent = StateHash::zero(); + let result = StateHash::from_bytes([1u8; 32]); + + // Migration without down operations is not reversible + let migration = Migration::new("test", vec![], vec![], parent, result, vec![]); + assert!(!migration.is_reversible()); + + // Migration with down operations is reversible + let down_ops = vec![Operation::DropTable { + schema: SchemaName::try_new("public".to_string()).unwrap(), + name: TableName::try_new("users".to_string()).unwrap(), + }]; + let migration = Migration::new("test", vec![], down_ops, parent, result, vec![]); + assert!(migration.is_reversible()); + } } mod migration_index_tests { diff --git a/tests/compile_integration_tests.rs b/tests/compile_integration_tests.rs index 6ded796..ff65513 100644 --- a/tests/compile_integration_tests.rs +++ b/tests/compile_integration_tests.rs @@ -778,6 +778,7 @@ mod low_level_api { let migration = Migration::new( "Create users table", plan.operations.clone(), + vec![], source_hash, target_hash, breaking_changes.into_changes(), @@ -811,6 +812,7 @@ mod low_level_api { let migration = Migration::new( "Create users", plan.operations.clone(), + vec![], source_hash, target_hash, vec![], diff --git a/tests/state_reconstruction_tests.rs b/tests/state_reconstruction_tests.rs index 45c54f9..20bff9c 100644 --- a/tests/state_reconstruction_tests.rs +++ b/tests/state_reconstruction_tests.rs @@ -44,6 +44,7 @@ fn build_migration( let migration = Migration::new( description, plan.operations, + vec![], parent_hash, resulting_hash, vec![],