diff --git a/.changepacks/changepack_log_zSH28AldbFcrF8dMg5f-i.json b/.changepacks/changepack_log_zSH28AldbFcrF8dMg5f-i.json new file mode 100644 index 0000000..06f814c --- /dev/null +++ b/.changepacks/changepack_log_zSH28AldbFcrF8dMg5f-i.json @@ -0,0 +1 @@ +{"changes":{"crates/vespertide-core/Cargo.toml":"Patch","crates/vespertide-loader/Cargo.toml":"Patch","crates/vespertide-query/Cargo.toml":"Patch","crates/vespertide-naming/Cargo.toml":"Patch","crates/vespertide-exporter/Cargo.toml":"Patch","crates/vespertide-planner/Cargo.toml":"Patch","crates/vespertide/Cargo.toml":"Patch","crates/vespertide-macro/Cargo.toml":"Patch","crates/vespertide-config/Cargo.toml":"Patch","crates/vespertide-cli/Cargo.toml":"Patch"},"note":"Add rm fk required column option","date":"2026-03-30T05:55:13.437604900Z"} \ No newline at end of file diff --git a/CLAUDE.md b/CLAUDE.md index 57909ca..43c994c 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -1,172 +1 @@ -# CLAUDE.md - -This file provides guidance to Claude Code (claude.ai/code) when working with code in this repository. - -## Overview - -Vespertide is a Rust workspace for defining database schemas in JSON/YAML and generating migration plans and SQL from model diffs. It enables declarative schema management by comparing the current model state against a baseline reconstructed from applied migrations. - -The workspace uses Rust **edition 2024**. - -## Build and Test Commands - -```bash -# Build the entire workspace -cargo build - -# Run all tests -cargo test - -# Run tests for a specific crate -cargo test -p vespertide-core -cargo test -p vespertide-planner - -# Run a single test -cargo test -p vespertide-planner test_name - -# Run tests with output shown -cargo test -- --nocapture - -# Format code -cargo fmt - -# Lint (important: use all targets and features) -cargo clippy --all-targets --all-features - -# Regenerate JSON schemas -cargo run -p vespertide-schema-gen -- --out schemas - -# Snapshot testing (exporter crate) -cargo insta test -p vespertide-exporter -cargo insta accept - -# Run CLI commands (use -p vespertide-cli) -cargo run -p vespertide-cli -- init -cargo run -p vespertide-cli -- new user -cargo run -p vespertide-cli -- diff -cargo run -p vespertide-cli -- sql -cargo run -p vespertide-cli -- revision -m "message" -cargo run -p vespertide-cli -- status -cargo run -p vespertide-cli -- log -cargo run -p vespertide-cli -- export --orm seaorm -``` - -## Architecture - -### Core Data Flow - -1. **Schema Definition**: Users define tables in JSON files (`TableDef`) in the `models/` directory -2. **Baseline Reconstruction**: Applied migrations are replayed to rebuild the baseline schema -3. **Diffing**: Current models are compared against the baseline to compute changes -4. **Planning**: Changes are converted into a `MigrationPlan` with versioned actions -5. **SQL Generation**: Migration actions are translated into SQL statements - -### Crate Responsibilities - -- **vespertide-core**: Data structures (`TableDef`, `ColumnDef`, `ColumnType`, `MigrationAction`, `MigrationPlan`, constraints, indexes) - - `ColumnType` enum with `Simple(SimpleColumnType)` and `Complex(ComplexColumnType)` variants - - `ColumnType::to_sql()` and `ColumnType::to_rust_type()` methods for type conversion -- **vespertide-planner**: - - `schema_from_plans()`: Replays applied migrations to reconstruct baseline schema - - `diff_schemas()`: Compares two schemas and generates migration actions - - `plan_next_migration()`: Combines baseline reconstruction + diffing to create the next migration - - `apply_action()`: Applies a single migration action to a schema (used during replay) - - `validate_*()`: Validates schemas and migration plans -- **vespertide-query**: Converts `MigrationAction` → SQL with bind parameters - - Uses `ColumnType::to_sql()` method for SQL type conversion -- **vespertide-config**: Manages `vespertide.json` (models/migrations directories, naming case preferences) -- **vespertide-cli**: Command-line interface implementation -- **vespertide-exporter**: Exports schemas to other formats (e.g., SeaORM entities) - - Uses `ColumnType::to_rust_type(nullable)` method for Rust type conversion -- **vespertide-schema-gen**: Generates JSON Schema files for validation -- **vespertide-loader**: Loads migrations and models from filesystem (JSON/YAML) -- **vespertide-naming**: Naming conventions and case conversion helpers -- **vespertide-macro**: Compile-time migration macro (`vespertide_migration!`) for SeaORM -- **vespertide**: Main crate that re-exports vespertide-core and vespertide-macro - -### Key Architectural Patterns - -**Migration Replay Pattern**: The planner doesn't store a "current database state" - it reconstructs it by replaying all applied migrations in order. This ensures the baseline is always derivable from the migration history. - -**Declarative Diffing**: Users declare the desired end state in model files. The diff engine compares this against the reconstructed baseline to compute necessary changes. - -**Action-Based Migrations**: All changes are expressed as typed `MigrationAction` enums (CreateTable, AddColumn, ModifyColumnType, etc.) rather than raw SQL. SQL generation happens in a separate layer. - -## Important Implementation Details - -### ColumnDef Structure -When creating `ColumnDef` instances in tests or code, you must initialize ALL fields including the newer inline constraint fields: - -```rust -ColumnDef { - name: "id".into(), - r#type: ColumnType::Simple(SimpleColumnType::Integer), - nullable: false, - default: None, - comment: None, - primary_key: None, // Inline PK declaration - unique: None, // Inline unique constraint - index: None, // Inline index creation - foreign_key: None, // Inline FK definition -} -``` - -These inline fields (added recently) allow constraints to be defined directly on columns in addition to table-level `TableConstraint` definitions. - -### ColumnType Structure -`ColumnType` is an enum with two variants: -- `Simple(SimpleColumnType)`: Built-in types like `Integer`, `Text`, `Boolean`, etc. -- `Complex(ComplexColumnType)`: Types with parameters like `Varchar { length }` or `Custom { custom_type }` - -**Important**: In Rust code, always use `ColumnType::Simple(SimpleColumnType::Integer)` instead of the old `ColumnType::Integer` syntax. The `From` trait is implemented for convenience: -```rust -// These are equivalent: -ColumnType::Simple(SimpleColumnType::Integer) -SimpleColumnType::Integer.into() -``` - -### ColumnType Methods -`ColumnType` provides two utility methods: -- `to_sql()`: Returns the SQL type string (e.g., `"INTEGER"`, `"VARCHAR(255)"`) -- `to_rust_type(nullable: bool)`: Returns the Rust type string for SeaORM entity generation (e.g., `"i32"` or `"Option"`) - -These methods replace the old standalone functions `column_type_sql()` and `rust_type()`. - -### Foreign Key Definition -Foreign keys can be defined inline on columns via the `foreign_key` field: - -```rust -pub struct ForeignKeyDef { - pub ref_table: TableName, - pub ref_columns: Vec, - pub on_delete: Option, - pub on_update: Option, -} -``` - -### Migration Plan Validation -- Non-nullable columns added to existing tables require either a `default` value or a `fill_with` backfill expression -- Schemas are validated for constraint consistency before diffing -- The planner validates that column/table names follow the configured naming case - -### SQL Generation Target -SQL generation currently uses PostgreSQL-compatible syntax. The query builder can be extended to support other database systems. - -### JSON Schema Generation -The `vespertide-schema-gen` crate uses `schemars` to generate JSON Schemas from the Rust types. After modifying core data structures, regenerate schemas with: -```bash -cargo run -p vespertide-schema-gen -- --out schemas -``` - -Schema base URL can be overridden via `VESP_SCHEMA_BASE_URL` environment variable. - -## Testing Patterns - -- Tests use helper functions like `col()` and `table()` to reduce boilerplate -- Use `rstest` for parameterized tests (common in planner/query crates) -- Use `serial_test::serial` for tests that modify the filesystem or working directory -- Snapshot testing with `insta` is used in the exporter crate - -## Limitations - -- YAML loading is not implemented (templates can be generated but not parsed) +@AGENTS.md diff --git a/crates/vespertide-cli/CLAUDE.md b/crates/vespertide-cli/CLAUDE.md new file mode 100644 index 0000000..43c994c --- /dev/null +++ b/crates/vespertide-cli/CLAUDE.md @@ -0,0 +1 @@ +@AGENTS.md diff --git a/crates/vespertide-cli/src/commands/diff.rs b/crates/vespertide-cli/src/commands/diff.rs index c9e934b..8477b2d 100644 --- a/crates/vespertide-cli/src/commands/diff.rs +++ b/crates/vespertide-cli/src/commands/diff.rs @@ -462,6 +462,7 @@ mod tests { column: "email".into(), nullable: false, fill_with: None, + delete_null_rows: None, }, format!( "{} {}.{} {} {}", @@ -478,6 +479,7 @@ mod tests { column: "email".into(), nullable: true, fill_with: None, + delete_null_rows: None, }, format!( "{} {}.{} {} {}", diff --git a/crates/vespertide-cli/src/commands/revision.rs b/crates/vespertide-cli/src/commands/revision.rs index 33be2d4..cc37dcd 100644 --- a/crates/vespertide-cli/src/commands/revision.rs +++ b/crates/vespertide-cli/src/commands/revision.rs @@ -1,4 +1,4 @@ -use std::collections::{BTreeMap, HashMap}; +use std::collections::{BTreeMap, HashMap, HashSet}; use std::path::Path; use anyhow::{Context, Result}; @@ -10,8 +10,8 @@ use tokio::fs; use vespertide_config::FileFormat; use vespertide_core::{MigrationAction, MigrationPlan, TableConstraint, TableDef}; use vespertide_planner::{ - EnumFillWithRequired, find_missing_enum_fill_with, find_missing_fill_with, plan_next_migration, - schema_from_plans, + EnumFillWithRequired, FillWithRequired, find_missing_enum_fill_with, find_missing_fill_with, + plan_next_migration, schema_from_plans, }; use crate::utils::{ @@ -32,6 +32,18 @@ fn parse_fill_with_args(args: &[String]) -> HashMap<(String, String), String> { map } +/// Parse delete_null_rows arguments from CLI. +/// Format: table.column +fn parse_delete_null_rows_args(args: &[String]) -> HashSet<(String, String)> { + let mut set = HashSet::new(); + for arg in args { + if let Some((table, column)) = arg.split_once('.') { + set.insert((table.to_string(), column.to_string())); + } + } + set +} + /// Format the type info string for display. /// Includes column type and default value hint if available. fn format_type_info(column_type: &str, default_value: &str) -> String { @@ -221,8 +233,31 @@ fn apply_fill_with_to_plan( } } +/// Apply delete_null_rows flags to matching ModifyColumnNullable actions. +fn apply_delete_null_rows_to_plan( + plan: &mut MigrationPlan, + delete_set: &HashSet<(String, String)>, +) { + for action in &mut plan.actions { + if let MigrationAction::ModifyColumnNullable { + table, + column, + nullable, + delete_null_rows, + .. + } = action + && !*nullable + && delete_null_rows.is_none() + && delete_set.contains(&(table.clone(), column.clone())) + { + *delete_null_rows = Some(true); + } + } +} + /// Handle interactive fill_with collection if there are missing values. /// Returns the updated fill_values map after collecting from user. +#[cfg(test)] fn handle_missing_fill_with( plan: &mut MigrationPlan, fill_values: &mut HashMap<(String, String), String>, @@ -246,6 +281,71 @@ where Ok(()) } +#[cfg(not(tarpaulin_include))] +fn prompt_delete_null_rows(table: &str, column: &str) -> Result { + let confirmed = Confirm::new() + .with_prompt(format!(" Delete rows where {}.{} IS NULL?", table, column)) + .default(false) + .interact() + .context("failed to read confirmation")?; + Ok(confirmed) +} + +fn handle_delete_null_rows( + plan: &mut MigrationPlan, + missing: &mut Vec, + delete_set: &HashSet<(String, String)>, + prompt_fn: F, +) -> Result<()> +where + F: Fn(&str, &str) -> Result, +{ + let mut to_delete = Vec::new(); + let mut remaining = Vec::new(); + + for item in missing.drain(..) { + if item.has_foreign_key && !delete_set.contains(&(item.table.clone(), item.column.clone())) + { + // FK column without CLI arg — prompt user + println!( + " {} {}.{} has a foreign key constraint — fill_with may not work.", + "\u{2022}".bright_cyan(), + item.table.bright_white(), + item.column.bright_green() + ); + if prompt_fn(&item.table, &item.column)? { + to_delete.push((item.table.clone(), item.column.clone())); + } else { + remaining.push(item); + } + } else if delete_set.contains(&(item.table.clone(), item.column.clone())) { + to_delete.push((item.table.clone(), item.column.clone())); + } else { + remaining.push(item); + } + } + + // Apply delete_null_rows to plan + for (table, column) in &to_delete { + for action in &mut plan.actions { + if let MigrationAction::ModifyColumnNullable { + table: t, + column: c, + delete_null_rows, + .. + } = action + && t == table + && c == column + { + *delete_null_rows = Some(true); + } + } + } + + *missing = remaining; + Ok(()) +} + /// Collect enum fill_with values interactively for removed enum values. /// The `enum_prompt_fn` parameter handles enum type columns with selection UI. fn collect_enum_fill_with_values( @@ -549,7 +649,55 @@ where Ok(()) } -pub async fn cmd_revision(message: String, fill_with_args: Vec) -> Result<()> { +pub async fn cmd_revision( + message: String, + fill_with_args: Vec, + delete_null_rows_args: Vec, +) -> Result<()> { + cmd_revision_core( + message, + fill_with_args, + delete_null_rows_args, + RevisionPromptFns { + recreate_prompt_fn: prompt_recreate_tables, + delete_null_rows_prompt_fn: prompt_delete_null_rows, + fill_with_prompt_fn: prompt_fill_with_value, + enum_prompt_fn: prompt_enum_value, + enum_bare_prompt_fn: prompt_enum_value_bare, + }, + ) + .await +} + +struct RevisionPromptFns { + recreate_prompt_fn: R, + delete_null_rows_prompt_fn: D, + fill_with_prompt_fn: F, + enum_prompt_fn: E, + enum_bare_prompt_fn: EB, +} + +async fn cmd_revision_core( + message: String, + fill_with_args: Vec, + delete_null_rows_args: Vec, + prompt_fns: RevisionPromptFns, +) -> Result<()> +where + R: Fn(&[RecreateTableRequired]) -> Result, + D: Fn(&str, &str) -> Result, + F: Fn(&str, &str) -> Result, + E: Fn(&str, &[String]) -> Result, + EB: Fn(&str, &[String]) -> Result, +{ + let RevisionPromptFns { + recreate_prompt_fn, + delete_null_rows_prompt_fn, + fill_with_prompt_fn, + enum_prompt_fn, + enum_bare_prompt_fn, + } = prompt_fns; + let config = load_config()?; let current_models = load_models(&config)?; let applied_plans = load_migrations(&config)?; @@ -558,7 +706,7 @@ pub async fn cmd_revision(message: String, fill_with_args: Vec) -> Resul .map_err(|e| anyhow::anyhow!("planning error: {}", e))?; // Check for non-nullable FK changes that require table recreation. - handle_recreate_requirements(&mut plan, ¤t_models, prompt_recreate_tables)?; + handle_recreate_requirements(&mut plan, ¤t_models, recreate_prompt_fn)?; if plan.actions.is_empty() { println!( @@ -575,21 +723,38 @@ pub async fn cmd_revision(message: String, fill_with_args: Vec) -> Resul // Parse CLI fill_with arguments let mut fill_values = parse_fill_with_args(&fill_with_args); + let delete_set = parse_delete_null_rows_args(&delete_null_rows_args); // Apply any CLI-provided fill_with values first apply_fill_with_to_plan(&mut plan, &fill_values); + apply_delete_null_rows_to_plan(&mut plan, &delete_set); + + // Find all missing fill_with values + let mut missing = find_missing_fill_with(&plan, &baseline_schema); + + // Handle FK columns with delete_null_rows option first + if !missing.is_empty() { + handle_delete_null_rows( + &mut plan, + &mut missing, + &delete_set, + delete_null_rows_prompt_fn, + )?; + } - // Handle any missing fill_with values interactively - handle_missing_fill_with( - &mut plan, - &mut fill_values, - &baseline_schema, - prompt_fill_with_value, - prompt_enum_value, - )?; + // Handle remaining missing fill_with values interactively + if !missing.is_empty() { + collect_fill_with_values( + &missing, + &mut fill_values, + fill_with_prompt_fn, + enum_prompt_fn, + )?; + apply_fill_with_to_plan(&mut plan, &fill_values); + } // Handle any missing enum fill_with values (for removed enum values) interactively - handle_missing_enum_fill_with(&mut plan, &baseline_schema, prompt_enum_value_bare)?; + handle_missing_enum_fill_with(&mut plan, &baseline_schema, enum_bare_prompt_fn)?; plan.id = uuid::Uuid::new_v4().to_string(); plan.comment = Some(message); @@ -759,7 +924,7 @@ mod tests { write_model("users"); std_fs::create_dir_all(cfg.migrations_dir()).unwrap(); - cmd_revision("init".into(), vec![]).await.unwrap(); + cmd_revision("init".into(), vec![], vec![]).await.unwrap(); let entries: Vec<_> = std_fs::read_dir(cfg.migrations_dir()).unwrap().collect(); assert!(!entries.is_empty()); @@ -773,7 +938,7 @@ mod tests { let cfg = write_config(); // no models, no migrations -> plan with no actions -> early return - assert!(cmd_revision("noop".into(), vec![]).await.is_ok()); + assert!(cmd_revision("noop".into(), vec![], vec![]).await.is_ok()); // migrations dir should not be created assert!(!cfg.migrations_dir().exists()); } @@ -791,7 +956,7 @@ mod tests { std_fs::remove_dir_all(cfg.migrations_dir()).unwrap(); } - cmd_revision("yaml".into(), vec![]).await.unwrap(); + cmd_revision("yaml".into(), vec![], vec![]).await.unwrap(); let entries: Vec<_> = std_fs::read_dir(cfg.migrations_dir()).unwrap().collect(); assert!(!entries.is_empty()); @@ -1567,6 +1732,7 @@ mod tests { column: "status".into(), nullable: false, fill_with: None, + delete_null_rows: None, }], }; @@ -1702,6 +1868,7 @@ mod tests { column: "status".into(), nullable: false, fill_with: None, + delete_null_rows: None, }, ], }; @@ -1868,6 +2035,7 @@ mod tests { column_type: "text".to_string(), default_value: "''".to_string(), enum_values: None, + has_foreign_key: false, }]; let mut fill_values = HashMap::new(); @@ -1900,6 +2068,7 @@ mod tests { column_type: "text".to_string(), default_value: "''".to_string(), enum_values: None, + has_foreign_key: false, }, FillWithRequired { action_index: 1, @@ -1909,6 +2078,7 @@ mod tests { column_type: "text".to_string(), default_value: "''".to_string(), enum_values: None, + has_foreign_key: false, }, ]; @@ -1972,6 +2142,7 @@ mod tests { column_type: "text".to_string(), default_value: "''".to_string(), enum_values: None, + has_foreign_key: false, }]; let mut fill_values = HashMap::new(); @@ -2178,6 +2349,7 @@ mod tests { column: "status".into(), nullable: false, fill_with: None, + delete_null_rows: None, }, ], }; @@ -2237,6 +2409,7 @@ mod tests { "confirmed".to_string(), "shipped".to_string(), ]), + has_foreign_key: false, }]; let mut fill_values = HashMap::new(); @@ -2524,4 +2697,720 @@ mod tests { fn test_strip_enum_quotes_only_trailing() { assert_eq!(strip_enum_quotes("active'".to_string()), "active"); } + + #[test] + fn test_parse_delete_null_rows_args() { + let args = vec!["users.email".to_string(), "orders.user_id".to_string()]; + let result = parse_delete_null_rows_args(&args); + assert_eq!(result.len(), 2); + assert!(result.contains(&("users".to_string(), "email".to_string()))); + assert!(result.contains(&("orders".to_string(), "user_id".to_string()))); + } + + #[test] + fn test_parse_delete_null_rows_args_invalid_format() { + let args = vec!["invalid_no_dot".to_string(), "valid.column".to_string()]; + let result = parse_delete_null_rows_args(&args); + assert_eq!(result.len(), 1); + assert!(result.contains(&("valid".to_string(), "column".to_string()))); + } + + #[test] + fn test_parse_delete_null_rows_args_empty() { + let result = parse_delete_null_rows_args(&[]); + assert!(result.is_empty()); + } + + #[test] + fn test_apply_delete_null_rows_to_plan() { + use vespertide_core::MigrationPlan; + + let mut plan = MigrationPlan { + id: String::new(), + comment: None, + created_at: None, + version: 1, + actions: vec![MigrationAction::ModifyColumnNullable { + table: "orders".into(), + column: "user_id".into(), + nullable: false, + fill_with: None, + delete_null_rows: None, + }], + }; + + let mut delete_set = HashSet::new(); + delete_set.insert(("orders".to_string(), "user_id".to_string())); + apply_delete_null_rows_to_plan(&mut plan, &delete_set); + + match &plan.actions[0] { + MigrationAction::ModifyColumnNullable { + delete_null_rows, .. + } => { + assert_eq!(delete_null_rows, &Some(true)); + } + _ => panic!("Expected ModifyColumnNullable action"), + } + } + + #[test] + fn test_apply_delete_null_rows_to_plan_skips_nullable_true() { + use vespertide_core::MigrationPlan; + + let mut plan = MigrationPlan { + id: String::new(), + comment: None, + created_at: None, + version: 1, + actions: vec![MigrationAction::ModifyColumnNullable { + table: "orders".into(), + column: "user_id".into(), + nullable: true, + fill_with: None, + delete_null_rows: None, + }], + }; + + let mut delete_set = HashSet::new(); + delete_set.insert(("orders".to_string(), "user_id".to_string())); + apply_delete_null_rows_to_plan(&mut plan, &delete_set); + + match &plan.actions[0] { + MigrationAction::ModifyColumnNullable { + delete_null_rows, .. + } => { + assert_eq!(delete_null_rows, &None); + } + _ => panic!("Expected ModifyColumnNullable action"), + } + } + + #[test] + fn test_apply_delete_null_rows_to_plan_skips_already_set() { + use vespertide_core::MigrationPlan; + + let mut plan = MigrationPlan { + id: String::new(), + comment: None, + created_at: None, + version: 1, + actions: vec![MigrationAction::ModifyColumnNullable { + table: "orders".into(), + column: "user_id".into(), + nullable: false, + fill_with: None, + delete_null_rows: Some(false), + }], + }; + + let mut delete_set = HashSet::new(); + delete_set.insert(("orders".to_string(), "user_id".to_string())); + apply_delete_null_rows_to_plan(&mut plan, &delete_set); + + match &plan.actions[0] { + MigrationAction::ModifyColumnNullable { + delete_null_rows, .. + } => { + assert_eq!(delete_null_rows, &Some(false)); + } + _ => panic!("Expected ModifyColumnNullable action"), + } + } + + #[test] + fn test_apply_delete_null_rows_to_plan_no_match() { + use vespertide_core::MigrationPlan; + + let mut plan = MigrationPlan { + id: String::new(), + comment: None, + created_at: None, + version: 1, + actions: vec![MigrationAction::ModifyColumnNullable { + table: "orders".into(), + column: "user_id".into(), + nullable: false, + fill_with: None, + delete_null_rows: None, + }], + }; + + let mut delete_set = HashSet::new(); + delete_set.insert(("other_table".to_string(), "other_col".to_string())); + apply_delete_null_rows_to_plan(&mut plan, &delete_set); + + match &plan.actions[0] { + MigrationAction::ModifyColumnNullable { + delete_null_rows, .. + } => { + assert_eq!(delete_null_rows, &None); + } + _ => panic!("Expected ModifyColumnNullable action"), + } + } + + #[test] + fn test_handle_delete_null_rows_fk_accepted() { + use vespertide_core::MigrationPlan; + use vespertide_planner::FillWithRequired; + + let mut plan = MigrationPlan { + id: String::new(), + comment: None, + created_at: None, + version: 1, + actions: vec![MigrationAction::ModifyColumnNullable { + table: "orders".into(), + column: "user_id".into(), + nullable: false, + fill_with: None, + delete_null_rows: None, + }], + }; + + let mut missing = vec![FillWithRequired { + action_index: 0, + table: "orders".to_string(), + column: "user_id".to_string(), + action_type: "ModifyColumnNullable", + column_type: "integer".to_string(), + default_value: "0".to_string(), + enum_values: None, + has_foreign_key: true, + }]; + + let delete_set = HashSet::new(); + + let mock_prompt = |_table: &str, _column: &str| -> Result { Ok(true) }; + + let result = handle_delete_null_rows(&mut plan, &mut missing, &delete_set, mock_prompt); + assert!(result.is_ok()); + + assert!(missing.is_empty()); + + match &plan.actions[0] { + MigrationAction::ModifyColumnNullable { + delete_null_rows, .. + } => { + assert_eq!(delete_null_rows, &Some(true)); + } + _ => panic!("Expected ModifyColumnNullable"), + } + } + + #[test] + fn test_handle_delete_null_rows_fk_declined() { + use vespertide_core::MigrationPlan; + use vespertide_planner::FillWithRequired; + + let mut plan = MigrationPlan { + id: String::new(), + comment: None, + created_at: None, + version: 1, + actions: vec![MigrationAction::ModifyColumnNullable { + table: "orders".into(), + column: "user_id".into(), + nullable: false, + fill_with: None, + delete_null_rows: None, + }], + }; + + let mut missing = vec![FillWithRequired { + action_index: 0, + table: "orders".to_string(), + column: "user_id".to_string(), + action_type: "ModifyColumnNullable", + column_type: "integer".to_string(), + default_value: "0".to_string(), + enum_values: None, + has_foreign_key: true, + }]; + + let delete_set = HashSet::new(); + + let mock_prompt = |_table: &str, _column: &str| -> Result { Ok(false) }; + + let result = handle_delete_null_rows(&mut plan, &mut missing, &delete_set, mock_prompt); + assert!(result.is_ok()); + + assert_eq!(missing.len(), 1); + assert_eq!(missing[0].table, "orders"); + + match &plan.actions[0] { + MigrationAction::ModifyColumnNullable { + delete_null_rows, .. + } => { + assert_eq!(delete_null_rows, &None); + } + _ => panic!("Expected ModifyColumnNullable"), + } + } + + #[test] + fn test_handle_delete_null_rows_cli_provided() { + use vespertide_core::MigrationPlan; + use vespertide_planner::FillWithRequired; + + let mut plan = MigrationPlan { + id: String::new(), + comment: None, + created_at: None, + version: 1, + actions: vec![MigrationAction::ModifyColumnNullable { + table: "orders".into(), + column: "user_id".into(), + nullable: false, + fill_with: None, + delete_null_rows: None, + }], + }; + + let mut missing = vec![FillWithRequired { + action_index: 0, + table: "orders".to_string(), + column: "user_id".to_string(), + action_type: "ModifyColumnNullable", + column_type: "integer".to_string(), + default_value: "0".to_string(), + enum_values: None, + has_foreign_key: false, + }]; + + let mut delete_set = HashSet::new(); + delete_set.insert(("orders".to_string(), "user_id".to_string())); + + let mock_prompt = |_table: &str, _column: &str| -> Result { + panic!("Should not be called for CLI-provided items"); + }; + + let result = handle_delete_null_rows(&mut plan, &mut missing, &delete_set, mock_prompt); + assert!(result.is_ok()); + + assert!(missing.is_empty()); + + match &plan.actions[0] { + MigrationAction::ModifyColumnNullable { + delete_null_rows, .. + } => { + assert_eq!(delete_null_rows, &Some(true)); + } + _ => panic!("Expected ModifyColumnNullable"), + } + } + + #[test] + fn test_handle_delete_null_rows_non_fk_passthrough() { + use vespertide_core::MigrationPlan; + use vespertide_planner::FillWithRequired; + + let mut plan = MigrationPlan { + id: String::new(), + comment: None, + created_at: None, + version: 1, + actions: vec![MigrationAction::ModifyColumnNullable { + table: "users".into(), + column: "email".into(), + nullable: false, + fill_with: None, + delete_null_rows: None, + }], + }; + + let mut missing = vec![FillWithRequired { + action_index: 0, + table: "users".to_string(), + column: "email".to_string(), + action_type: "ModifyColumnNullable", + column_type: "text".to_string(), + default_value: "''".to_string(), + enum_values: None, + has_foreign_key: false, + }]; + + let delete_set = HashSet::new(); + + let mock_prompt = |_table: &str, _column: &str| -> Result { + panic!("Should not be called for non-FK items"); + }; + + let result = handle_delete_null_rows(&mut plan, &mut missing, &delete_set, mock_prompt); + assert!(result.is_ok()); + + assert_eq!(missing.len(), 1); + assert_eq!(missing[0].column, "email"); + + match &plan.actions[0] { + MigrationAction::ModifyColumnNullable { + delete_null_rows, .. + } => { + assert_eq!(delete_null_rows, &None); + } + _ => panic!("Expected ModifyColumnNullable"), + } + } + + #[test] + fn test_handle_delete_null_rows_prompt_error() { + use vespertide_core::MigrationPlan; + use vespertide_planner::FillWithRequired; + + let mut plan = MigrationPlan { + id: String::new(), + comment: None, + created_at: None, + version: 1, + actions: vec![MigrationAction::ModifyColumnNullable { + table: "orders".into(), + column: "user_id".into(), + nullable: false, + fill_with: None, + delete_null_rows: None, + }], + }; + + let mut missing = vec![FillWithRequired { + action_index: 0, + table: "orders".to_string(), + column: "user_id".to_string(), + action_type: "ModifyColumnNullable", + column_type: "integer".to_string(), + default_value: "0".to_string(), + enum_values: None, + has_foreign_key: true, + }]; + + let delete_set = HashSet::new(); + + let mock_prompt = |_table: &str, _column: &str| -> Result { + Err(anyhow::anyhow!("user cancelled")) + }; + + let result = handle_delete_null_rows(&mut plan, &mut missing, &delete_set, mock_prompt); + assert!(result.is_err()); + } + + /// Integration test: FK column nullable→not-null triggers handle_delete_null_rows (line 489) + #[tokio::test] + #[serial_test::serial] + async fn cmd_revision_core_handles_delete_null_rows_for_fk_column() { + use vespertide_core::MigrationPlan; + use vespertide_core::schema::foreign_key::{ForeignKeyDef, ForeignKeySyntax}; + + let tmp = tempdir().unwrap(); + let _guard = CwdGuard::new(&tmp.path().to_path_buf()); + + let cfg = write_config(); + std_fs::create_dir_all(cfg.migrations_dir()).unwrap(); + + // Write v1 migration: create "orders" table with nullable user_id + let v1 = MigrationPlan { + id: "v1-id".to_string(), + comment: Some("init".to_string()), + created_at: None, + version: 1, + actions: vec![MigrationAction::CreateTable { + table: "orders".into(), + columns: vec![ + ColumnDef { + name: "id".into(), + r#type: ColumnType::Simple(SimpleColumnType::Integer), + nullable: false, + default: None, + comment: None, + primary_key: None, + unique: None, + index: None, + foreign_key: None, + }, + ColumnDef { + name: "user_id".into(), + r#type: ColumnType::Simple(SimpleColumnType::Integer), + nullable: true, // nullable in v1 + default: None, + comment: None, + primary_key: None, + unique: None, + index: None, + foreign_key: None, + }, + ], + constraints: vec![ + TableConstraint::PrimaryKey { + auto_increment: false, + columns: vec!["id".into()], + }, + TableConstraint::ForeignKey { + name: Some("fk_orders__user_id".into()), + columns: vec!["user_id".into()], + ref_table: "users".into(), + ref_columns: vec!["id".into()], + on_delete: None, + on_update: None, + }, + ], + }], + }; + let v1_path = cfg.migrations_dir().join("0001_init.vespertide.json"); + std_fs::write(&v1_path, serde_json::to_string_pretty(&v1).unwrap()).unwrap(); + + // Write updated model: user_id is now NOT NULL + let models_dir = PathBuf::from("models"); + std_fs::create_dir_all(&models_dir).unwrap(); + let users_model = TableDef { + name: "users".to_string(), + description: None, + columns: vec![ColumnDef { + name: "id".into(), + r#type: ColumnType::Simple(SimpleColumnType::Integer), + nullable: false, + default: None, + comment: None, + primary_key: None, + unique: None, + index: None, + foreign_key: None, + }], + constraints: vec![TableConstraint::PrimaryKey { + auto_increment: false, + columns: vec!["id".into()], + }], + }; + std_fs::write( + models_dir.join("users.json"), + serde_json::to_string_pretty(&users_model).unwrap(), + ) + .unwrap(); + + let model = TableDef { + name: "orders".to_string(), + description: None, + columns: vec![ + ColumnDef { + name: "id".into(), + r#type: ColumnType::Simple(SimpleColumnType::Integer), + nullable: false, + default: None, + comment: None, + primary_key: None, + unique: None, + index: None, + foreign_key: None, + }, + ColumnDef { + name: "user_id".into(), + r#type: ColumnType::Simple(SimpleColumnType::Integer), + nullable: false, // NOT NULL now + default: None, + comment: None, + primary_key: None, + unique: None, + index: None, + foreign_key: Some(ForeignKeySyntax::Object(ForeignKeyDef { + ref_table: "users".into(), + ref_columns: vec!["id".into()], + on_delete: None, + on_update: None, + })), + }, + ], + constraints: vec![TableConstraint::PrimaryKey { + auto_increment: false, + columns: vec!["id".into()], + }], + }; + std_fs::write( + models_dir.join("orders.json"), + serde_json::to_string_pretty(&model).unwrap(), + ) + .unwrap(); + + // Mock prompts + let recreate_prompt = |_: &[RecreateTableRequired]| -> Result { Ok(true) }; + let delete_prompt = |_table: &str, _col: &str| -> Result { Ok(true) }; + let fill_prompt = |_p: &str, _d: &str| -> Result { + panic!("fill prompt should not be called — FK handled by delete_null_rows"); + }; + let enum_prompt = |_p: &str, _v: &[String]| -> Result { + panic!("enum prompt should not be called"); + }; + let enum_bare_prompt = |_p: &str, _v: &[String]| -> Result { + panic!("enum bare prompt should not be called"); + }; + + let result = cmd_revision_core( + "make user_id required".into(), + vec![], + vec![], + RevisionPromptFns { + recreate_prompt_fn: recreate_prompt, + delete_null_rows_prompt_fn: delete_prompt, + fill_with_prompt_fn: fill_prompt, + enum_prompt_fn: enum_prompt, + enum_bare_prompt_fn: enum_bare_prompt, + }, + ) + .await; + + assert!( + result.is_ok(), + "cmd_revision_core failed: {:?}", + result.err() + ); + + // Verify migration was created + let entries: Vec<_> = std_fs::read_dir(cfg.migrations_dir()) + .unwrap() + .filter_map(|e| e.ok()) + .collect(); + // Should have 2 files: v1 + new v2 + assert_eq!(entries.len(), 2); + } + + /// Integration test: non-FK column nullable→not-null triggers collect_fill_with_values (lines 494-495) + #[tokio::test] + #[serial_test::serial] + async fn cmd_revision_core_handles_fill_with_for_non_fk_column() { + use vespertide_core::MigrationPlan; + + let tmp = tempdir().unwrap(); + let _guard = CwdGuard::new(&tmp.path().to_path_buf()); + + let cfg = write_config(); + std_fs::create_dir_all(cfg.migrations_dir()).unwrap(); + + // Write v1 migration: create "users" table with nullable email + let v1 = MigrationPlan { + id: "v1-id".to_string(), + comment: Some("init".to_string()), + created_at: None, + version: 1, + actions: vec![MigrationAction::CreateTable { + table: "users".into(), + columns: vec![ + ColumnDef { + name: "id".into(), + r#type: ColumnType::Simple(SimpleColumnType::Integer), + nullable: false, + default: None, + comment: None, + primary_key: None, + unique: None, + index: None, + foreign_key: None, + }, + ColumnDef { + name: "email".into(), + r#type: ColumnType::Simple(SimpleColumnType::Text), + nullable: true, // nullable in v1 + default: None, + comment: None, + primary_key: None, + unique: None, + index: None, + foreign_key: None, + }, + ], + constraints: vec![TableConstraint::PrimaryKey { + auto_increment: false, + columns: vec!["id".into()], + }], + }], + }; + let v1_path = cfg.migrations_dir().join("0001_init.vespertide.json"); + std_fs::write(&v1_path, serde_json::to_string_pretty(&v1).unwrap()).unwrap(); + + // Write updated model: email is now NOT NULL (no default) + let models_dir = PathBuf::from("models"); + std_fs::create_dir_all(&models_dir).unwrap(); + let model = TableDef { + name: "users".to_string(), + description: None, + columns: vec![ + ColumnDef { + name: "id".into(), + r#type: ColumnType::Simple(SimpleColumnType::Integer), + nullable: false, + default: None, + comment: None, + primary_key: None, + unique: None, + index: None, + foreign_key: None, + }, + ColumnDef { + name: "email".into(), + r#type: ColumnType::Simple(SimpleColumnType::Text), + nullable: false, // NOT NULL now + default: None, + comment: None, + primary_key: None, + unique: None, + index: None, + foreign_key: None, + }, + ], + constraints: vec![TableConstraint::PrimaryKey { + auto_increment: false, + columns: vec!["id".into()], + }], + }; + std_fs::write( + models_dir.join("users.json"), + serde_json::to_string_pretty(&model).unwrap(), + ) + .unwrap(); + + // Mock prompts + let recreate_prompt = |_: &[RecreateTableRequired]| -> Result { Ok(true) }; + let delete_prompt = |_table: &str, _col: &str| -> Result { Ok(false) }; + let fill_prompt = |_p: &str, _d: &str| -> Result { Ok("'unknown'".to_string()) }; + let enum_prompt = |_p: &str, _v: &[String]| -> Result { + panic!("enum prompt should not be called"); + }; + let enum_bare_prompt = |_p: &str, _v: &[String]| -> Result { + panic!("enum bare prompt should not be called"); + }; + + let result = cmd_revision_core( + "make email required".into(), + vec![], + vec![], + RevisionPromptFns { + recreate_prompt_fn: recreate_prompt, + delete_null_rows_prompt_fn: delete_prompt, + fill_with_prompt_fn: fill_prompt, + enum_prompt_fn: enum_prompt, + enum_bare_prompt_fn: enum_bare_prompt, + }, + ) + .await; + + assert!( + result.is_ok(), + "cmd_revision_core failed: {:?}", + result.err() + ); + + // Verify migration was written with fill_with + let entries: Vec<_> = std_fs::read_dir(cfg.migrations_dir()) + .unwrap() + .filter_map(|e| e.ok()) + .collect(); + assert_eq!(entries.len(), 2); + + // Read the v2 migration and verify fill_with was applied + let v2_path = entries + .iter() + .find(|e| e.file_name().to_string_lossy().contains("0002")) + .expect("v2 migration not found"); + let v2_content = std_fs::read_to_string(v2_path.path()).unwrap(); + assert!( + v2_content.contains("fill_with"), + "Expected fill_with in migration, got: {}", + v2_content + ); + } } diff --git a/crates/vespertide-cli/src/main.rs b/crates/vespertide-cli/src/main.rs index 5a61f42..18b47a0 100644 --- a/crates/vespertide-cli/src/main.rs +++ b/crates/vespertide-cli/src/main.rs @@ -69,6 +69,10 @@ enum Commands { /// Format: table.column=value (can be specified multiple times) #[arg(long = "fill-with")] fill_with: Vec, + /// Delete rows with NULL values instead of filling. + /// Format: table.column (can be specified multiple times) + #[arg(long = "delete-null-rows")] + delete_null_rows: Vec, }, /// Initialize vespertide.json with defaults. Init, @@ -93,7 +97,11 @@ async fn main() -> Result<()> { Some(Commands::Log { backend }) => cmd_log(backend.into()).await, Some(Commands::New { name, format }) => cmd_new(name, format).await, Some(Commands::Status) => cmd_status().await, - Some(Commands::Revision { message, fill_with }) => cmd_revision(message, fill_with).await, + Some(Commands::Revision { + message, + fill_with, + delete_null_rows, + }) => cmd_revision(message, fill_with, delete_null_rows).await, Some(Commands::Init) => cmd_init().await, Some(Commands::Export { orm, export_dir }) => cmd_export(orm, export_dir).await, None => { diff --git a/crates/vespertide-core/CLAUDE.md b/crates/vespertide-core/CLAUDE.md new file mode 100644 index 0000000..43c994c --- /dev/null +++ b/crates/vespertide-core/CLAUDE.md @@ -0,0 +1 @@ +@AGENTS.md diff --git a/crates/vespertide-core/src/action.rs b/crates/vespertide-core/src/action.rs index 50d3938..5bdddfc 100644 --- a/crates/vespertide-core/src/action.rs +++ b/crates/vespertide-core/src/action.rs @@ -60,6 +60,11 @@ pub enum MigrationAction { nullable: bool, /// Required when changing from nullable to non-nullable to backfill existing NULL values. fill_with: Option, + /// When true, rows with NULL values in the column are deleted instead of backfilled. + /// Mutually exclusive with `fill_with`. Useful for FK columns where a valid fill value + /// may not exist. + #[serde(default, skip_serializing_if = "Option::is_none")] + delete_null_rows: Option, }, ModifyColumnDefault { table: TableName, @@ -164,11 +169,13 @@ impl MigrationAction { column, nullable, fill_with, + delete_null_rows, } => MigrationAction::ModifyColumnNullable { table: format!("{}{}", prefix, table), column, nullable, fill_with, + delete_null_rows, }, MigrationAction::ModifyColumnDefault { table, @@ -643,6 +650,7 @@ mod tests { column: "email".into(), nullable: false, fill_with: None, + delete_null_rows: None, }, "ModifyColumnNullable: users.email -> NOT NULL" )] @@ -652,6 +660,7 @@ mod tests { column: "email".into(), nullable: true, fill_with: None, + delete_null_rows: None, }, "ModifyColumnNullable: users.email -> NULL" )] @@ -931,6 +940,7 @@ mod tests { column: "email".into(), nullable: false, fill_with: Some("default@example.com".into()), + delete_null_rows: None, }; let prefixed = action.with_prefix("myapp_"); if let MigrationAction::ModifyColumnNullable { @@ -938,12 +948,14 @@ mod tests { column, nullable, fill_with, + delete_null_rows, } = prefixed { assert_eq!(table.as_str(), "myapp_users"); assert_eq!(column.as_str(), "email"); assert!(!nullable); assert_eq!(fill_with, Some("default@example.com".into())); + assert_eq!(delete_null_rows, None); } else { panic!("Expected ModifyColumnNullable"); } diff --git a/crates/vespertide-exporter/CLAUDE.md b/crates/vespertide-exporter/CLAUDE.md new file mode 100644 index 0000000..43c994c --- /dev/null +++ b/crates/vespertide-exporter/CLAUDE.md @@ -0,0 +1 @@ +@AGENTS.md diff --git a/crates/vespertide-planner/CLAUDE.md b/crates/vespertide-planner/CLAUDE.md new file mode 100644 index 0000000..43c994c --- /dev/null +++ b/crates/vespertide-planner/CLAUDE.md @@ -0,0 +1 @@ +@AGENTS.md diff --git a/crates/vespertide-planner/src/apply.rs b/crates/vespertide-planner/src/apply.rs index 4635420..6c29ebe 100644 --- a/crates/vespertide-planner/src/apply.rs +++ b/crates/vespertide-planner/src/apply.rs @@ -122,6 +122,7 @@ pub fn apply_action( column, nullable, fill_with: _, + delete_null_rows: _, } => { let tbl = schema .iter_mut() @@ -1326,6 +1327,7 @@ mod tests { column: "email".into(), nullable: false, fill_with: None, + delete_null_rows: None, }, ) .unwrap(); @@ -1344,6 +1346,7 @@ mod tests { column: "email".into(), nullable: false, fill_with: None, + delete_null_rows: None, }, ) .unwrap_err(); @@ -1366,6 +1369,7 @@ mod tests { column: "email".into(), nullable: false, fill_with: None, + delete_null_rows: None, }, ) .unwrap_err(); diff --git a/crates/vespertide-planner/src/diff.rs b/crates/vespertide-planner/src/diff.rs index 5b44287..201623a 100644 --- a/crates/vespertide-planner/src/diff.rs +++ b/crates/vespertide-planner/src/diff.rs @@ -527,6 +527,7 @@ pub fn diff_schemas(from: &[TableDef], to: &[TableDef]) -> Result Result<(), PlannerError> column, nullable, fill_with, + delete_null_rows, } => { // If changing from nullable to non-nullable, fill_with is required - if !nullable && fill_with.is_none() { + if !nullable && fill_with.is_none() && !delete_null_rows.unwrap_or(false) { return Err(PlannerError::MissingFillWith(table.clone(), column.clone())); } } @@ -462,6 +463,8 @@ pub struct FillWithRequired { pub default_value: String, /// Enum values if the column is an enum type (for selection UI). pub enum_values: Option>, + /// Whether the current column has a foreign key constraint. + pub has_foreign_key: bool, } /// Find all actions in a migration plan that require fill_with values. @@ -492,6 +495,7 @@ pub fn find_missing_fill_with( column_type: column.r#type.to_display_string(), default_value: column.r#type.default_fill_value().to_string(), enum_values: column.r#type.enum_variant_names(), + has_foreign_key: false, }); } } @@ -500,15 +504,18 @@ pub fn find_missing_fill_with( column, nullable, fill_with, + delete_null_rows, } => { // If changing from nullable to non-nullable, fill_with is required // UNLESS the column already has a default value (which will be used) - if !nullable && fill_with.is_none() { + if !nullable && fill_with.is_none() && !delete_null_rows.unwrap_or(false) { // Look up column from the current schema - let col_def = current_schema - .iter() - .find(|t| t.name == *table) - .and_then(|t| t.columns.iter().find(|c| c.name == *column)); + let table_def = current_schema.iter().find(|t| t.name == *table); + + let col_def = + table_def.and_then(|t| t.columns.iter().find(|c| c.name == *column)); + + let has_foreign_key = table_def.is_some_and(|t| t.constraints.iter().any(|constraint| matches!(constraint, TableConstraint::ForeignKey { columns, .. } if columns.iter().any(|col_name| col_name == column)))); // If column has a default value, fill_with is not needed if col_def.is_some_and(|c| c.default.is_some()) { @@ -532,6 +539,7 @@ pub fn find_missing_fill_with( column_type: col_type_str, default_value: default_val, enum_values: enum_vals, + has_foreign_key, }); } } @@ -1272,6 +1280,7 @@ mod tests { column: "email".into(), nullable: false, fill_with: None, + delete_null_rows: None, }], }; @@ -1298,6 +1307,7 @@ mod tests { column: "email".into(), nullable: false, fill_with: Some("'unknown'".into()), + delete_null_rows: None, }], }; @@ -1318,6 +1328,7 @@ mod tests { column: "email".into(), nullable: true, fill_with: None, + delete_null_rows: None, }], }; @@ -1900,6 +1911,7 @@ mod tests { column: "email".into(), nullable: false, fill_with: None, + delete_null_rows: None, }], }; @@ -1924,6 +1936,7 @@ mod tests { column: "email".into(), nullable: true, fill_with: None, + delete_null_rows: None, }], }; @@ -1943,6 +1956,7 @@ mod tests { column: "email".into(), nullable: false, fill_with: Some("'default'".into()), + delete_null_rows: None, }], }; @@ -1980,6 +1994,7 @@ mod tests { column: "status".into(), nullable: false, fill_with: None, + delete_null_rows: None, }], }; @@ -2020,6 +2035,7 @@ mod tests { column: "email".into(), nullable: false, fill_with: None, + delete_null_rows: None, }], }; @@ -2056,6 +2072,7 @@ mod tests { column: "status".into(), nullable: false, fill_with: None, + delete_null_rows: None, }, MigrationAction::AddColumn { table: "users".into(), diff --git a/crates/vespertide-query/CLAUDE.md b/crates/vespertide-query/CLAUDE.md new file mode 100644 index 0000000..43c994c --- /dev/null +++ b/crates/vespertide-query/CLAUDE.md @@ -0,0 +1 @@ +@AGENTS.md diff --git a/crates/vespertide-query/src/sql/mod.rs b/crates/vespertide-query/src/sql/mod.rs index 90cce86..ee2dbcf 100644 --- a/crates/vespertide-query/src/sql/mod.rs +++ b/crates/vespertide-query/src/sql/mod.rs @@ -104,12 +104,14 @@ pub fn build_action_queries_with_pending( column, nullable, fill_with, + delete_null_rows, } => build_modify_column_nullable( backend, table, column, *nullable, fill_with.as_deref(), + delete_null_rows.unwrap_or(false), current_schema, ), @@ -1268,6 +1270,7 @@ mod tests { column: "email".into(), nullable: false, fill_with: Some("'unknown'".into()), + delete_null_rows: None, }; let current_schema = vec![TableDef { name: "users".into(), diff --git a/crates/vespertide-query/src/sql/modify_column_nullable.rs b/crates/vespertide-query/src/sql/modify_column_nullable.rs index b8ce98f..6e5463c 100644 --- a/crates/vespertide-query/src/sql/modify_column_nullable.rs +++ b/crates/vespertide-query/src/sql/modify_column_nullable.rs @@ -18,12 +18,25 @@ pub fn build_modify_column_nullable( column: &str, nullable: bool, fill_with: Option<&str>, + delete_null_rows: bool, current_schema: &[TableDef], ) -> Result, QueryError> { let mut queries = Vec::new(); + // If delete_null_rows is set, delete rows with NULL values instead of updating + if !nullable && delete_null_rows { + let delete_sql = match backend { + DatabaseBackend::Postgres | DatabaseBackend::Sqlite => { + format!("DELETE FROM \"{}\" WHERE \"{}\" IS NULL", table, column) + } + DatabaseBackend::MySql => { + format!("DELETE FROM `{}` WHERE `{}` IS NULL", table, column) + } + }; + queries.push(BuiltQuery::Raw(RawSql::uniform(delete_sql))); + } // If changing to NOT NULL, first update existing NULL values if fill_with is provided - if !nullable && let Some(fill_value) = normalize_fill_with(fill_with) { + else if !nullable && let Some(fill_value) = normalize_fill_with(fill_with) { let fill_value = convert_default_for_backend(&fill_value, backend); let update_sql = match backend { DatabaseBackend::Postgres | DatabaseBackend::Sqlite => format!( @@ -201,8 +214,9 @@ mod tests { vec![], )]; - let result = - build_modify_column_nullable(&backend, "users", "email", nullable, fill_with, &schema); + let result = build_modify_column_nullable( + &backend, "users", "email", nullable, fill_with, false, &schema, + ); assert!(result.is_ok()); let queries = result.unwrap(); let sql = queries @@ -242,7 +256,8 @@ mod tests { return; } - let result = build_modify_column_nullable(&backend, "users", "email", false, None, &[]); + let result = + build_modify_column_nullable(&backend, "users", "email", false, None, false, &[]); assert!(result.is_err()); let err_msg = result.unwrap_err().to_string(); assert!(err_msg.contains("Table 'users' not found")); @@ -270,7 +285,8 @@ mod tests { vec![], )]; - let result = build_modify_column_nullable(&backend, "users", "email", false, None, &schema); + let result = + build_modify_column_nullable(&backend, "users", "email", false, None, false, &schema); assert!(result.is_err()); let err_msg = result.unwrap_err().to_string(); assert!(err_msg.contains("Column 'email' not found")); @@ -294,7 +310,8 @@ mod tests { }], )]; - let result = build_modify_column_nullable(&backend, "users", "email", false, None, &schema); + let result = + build_modify_column_nullable(&backend, "users", "email", false, None, false, &schema); assert!(result.is_ok()); let queries = result.unwrap(); let sql = queries @@ -348,6 +365,7 @@ mod tests { "paid_at", false, Some("NOW()"), + false, &schema, ); assert!(result.is_ok()); @@ -402,7 +420,8 @@ mod tests { vec![], )]; - let result = build_modify_column_nullable(&backend, "users", "email", false, None, &schema); + let result = + build_modify_column_nullable(&backend, "users", "email", false, None, false, &schema); assert!(result.is_ok()); let queries = result.unwrap(); let sql = queries @@ -429,4 +448,97 @@ mod tests { assert_snapshot!(sql); }); } + + /// Test delete_null_rows generates DELETE instead of UPDATE + #[rstest] + #[case::postgres_delete_null_rows(DatabaseBackend::Postgres)] + #[case::mysql_delete_null_rows(DatabaseBackend::MySql)] + #[case::sqlite_delete_null_rows(DatabaseBackend::Sqlite)] + fn test_delete_null_rows(#[case] backend: DatabaseBackend) { + let schema = vec![table_def( + "orders", + vec![ + col("id", ColumnType::Simple(SimpleColumnType::Integer), false), + col( + "user_id", + ColumnType::Simple(SimpleColumnType::Integer), + true, + ), + ], + vec![], + )]; + + let result = + build_modify_column_nullable(&backend, "orders", "user_id", false, None, true, &schema); + assert!(result.is_ok()); + let queries = result.unwrap(); + let sql = queries + .iter() + .map(|q| q.build(backend)) + .collect::>() + .join("\n"); + + assert!( + sql.contains("DELETE FROM"), + "Expected DELETE FROM in SQL, got: {}", + sql + ); + assert!( + sql.contains("IS NULL"), + "Expected IS NULL in SQL, got: {}", + sql + ); + assert!( + !sql.contains("UPDATE"), + "Should NOT contain UPDATE, got: {}", + sql + ); + + let suffix = format!( + "{}_delete_null_rows", + match backend { + DatabaseBackend::Postgres => "postgres", + DatabaseBackend::MySql => "mysql", + DatabaseBackend::Sqlite => "sqlite", + } + ); + + with_settings!({ snapshot_suffix => suffix }, { + assert_snapshot!(sql); + }); + } + + /// Test delete_null_rows=true with nullable=true does nothing special + #[rstest] + #[case::postgres_delete_null_rows_nullable(DatabaseBackend::Postgres)] + fn test_delete_null_rows_with_nullable_true(#[case] backend: DatabaseBackend) { + let schema = vec![table_def( + "orders", + vec![ + col("id", ColumnType::Simple(SimpleColumnType::Integer), false), + col( + "user_id", + ColumnType::Simple(SimpleColumnType::Integer), + false, + ), + ], + vec![], + )]; + + let result = + build_modify_column_nullable(&backend, "orders", "user_id", true, None, true, &schema); + assert!(result.is_ok()); + let queries = result.unwrap(); + let sql = queries + .iter() + .map(|q| q.build(backend)) + .collect::>() + .join("\n"); + + assert!( + !sql.contains("DELETE FROM"), + "Should NOT contain DELETE when nullable=true, got: {}", + sql + ); + } } diff --git a/crates/vespertide-query/src/sql/snapshots/vespertide_query__sql__modify_column_nullable__tests__delete_null_rows@mysql_delete_null_rows.snap b/crates/vespertide-query/src/sql/snapshots/vespertide_query__sql__modify_column_nullable__tests__delete_null_rows@mysql_delete_null_rows.snap new file mode 100644 index 0000000..1182518 --- /dev/null +++ b/crates/vespertide-query/src/sql/snapshots/vespertide_query__sql__modify_column_nullable__tests__delete_null_rows@mysql_delete_null_rows.snap @@ -0,0 +1,6 @@ +--- +source: crates/vespertide-query/src/sql/modify_column_nullable.rs +expression: sql +--- +DELETE FROM `orders` WHERE `user_id` IS NULL +ALTER TABLE `orders` MODIFY COLUMN `user_id` int NOT NULL diff --git a/crates/vespertide-query/src/sql/snapshots/vespertide_query__sql__modify_column_nullable__tests__delete_null_rows@postgres_delete_null_rows.snap b/crates/vespertide-query/src/sql/snapshots/vespertide_query__sql__modify_column_nullable__tests__delete_null_rows@postgres_delete_null_rows.snap new file mode 100644 index 0000000..d0b5347 --- /dev/null +++ b/crates/vespertide-query/src/sql/snapshots/vespertide_query__sql__modify_column_nullable__tests__delete_null_rows@postgres_delete_null_rows.snap @@ -0,0 +1,6 @@ +--- +source: crates/vespertide-query/src/sql/modify_column_nullable.rs +expression: sql +--- +DELETE FROM "orders" WHERE "user_id" IS NULL +ALTER TABLE "orders" ALTER COLUMN "user_id" SET NOT NULL diff --git a/crates/vespertide-query/src/sql/snapshots/vespertide_query__sql__modify_column_nullable__tests__delete_null_rows@sqlite_delete_null_rows.snap b/crates/vespertide-query/src/sql/snapshots/vespertide_query__sql__modify_column_nullable__tests__delete_null_rows@sqlite_delete_null_rows.snap new file mode 100644 index 0000000..3515a10 --- /dev/null +++ b/crates/vespertide-query/src/sql/snapshots/vespertide_query__sql__modify_column_nullable__tests__delete_null_rows@sqlite_delete_null_rows.snap @@ -0,0 +1,9 @@ +--- +source: crates/vespertide-query/src/sql/modify_column_nullable.rs +expression: sql +--- +DELETE FROM "orders" WHERE "user_id" IS NULL +CREATE TABLE "orders_temp" ( "id" integer NOT NULL, "user_id" integer NOT NULL ) +INSERT INTO "orders_temp" ("id", "user_id") SELECT "id", "user_id" FROM "orders" +DROP TABLE "orders" +ALTER TABLE "orders_temp" RENAME TO "orders"