fix(query): treat modify_column_type.fill_with replacements as bare enum labels

.changepacks/changepack_log_A6HBSMdx7cre8I-RyNhPx.json

@@ -0,0 +1 @@
+{"changes":{"crates/vespertide-core/Cargo.toml":"Minor","crates/vespertide-query/Cargo.toml":"Minor","crates/vespertide-cli/Cargo.toml":"Minor"},"note":"modify_column_type.fill_with 를 bare enum 라벨로 확정 (스키마 문서와 구현 불일치로 enum 축소 마이그레이션이 invalid input value for enum 으로 실패하던 문제). 따옴표가 이미 붙은 기존 값은 한 겹 제거 + 1회 경고로 하위호환 유지","date":"2026-08-20T11:02:39.1090232Z"}

crates/vespertide-cli/src/commands/revision/prompts/fill_with.rs

@@ -136,6 +136,8 @@ pub(in crate::commands::revision) fn prompt_enum_value_bare(
 
 /// Strip SQL single-quotes from an enum value string.
 /// `BTreeMap` stores bare enum names; the SQL layer handles quoting via `Expr::val()`.
+/// A quoted value gets escaped twice and lands in the column as `'active'`, which
+/// `PostgreSQL` rejects with `invalid input value for enum`.
 pub(in crate::commands::revision) fn strip_enum_quotes(value: &str) -> String {
     value
         .trim_start_matches('\'')
@@ -290,6 +292,9 @@ where
 /// The original ordering of `remaining_values` is preserved for every entry
 /// other than the suggestion (which is hoisted to the top), so non-suggested
 /// options remain in a predictable order.
+///
+/// Every value passes through [`strip_enum_quotes`], so the returned mappings
+/// hold bare labels no matter what `enum_prompt_fn` returns.
 pub(in crate::commands::revision) fn collect_enum_fill_with_values<E>(
     missing: &[EnumFillWithRequired],
     enum_prompt_fn: E,
@@ -333,7 +338,7 @@ where
             }
             let ordered = reorder_with_suggestion(&item.remaining_values, suggestion.as_deref());
             let value = enum_prompt_fn(&prompt, &ordered)?;
-            mappings.insert(removed.clone(), value);
+            mappings.insert(removed.clone(), strip_enum_quotes(&value));
         }
         results.push((item.action_index, mappings));
     }

crates/vespertide-cli/src/commands/revision/tests/prompts.rs

@@ -556,6 +556,25 @@ fn test_collect_enum_fill_with_values_single_removal() {
     );
 }
 
+#[test]
+fn test_collect_enum_fill_with_values_strips_quotes_from_prompt_result() {
+    use vespertide_planner::EnumFillWithRequired;
+
+    let missing = vec![EnumFillWithRequired {
+        action_index: 0,
+        table: "plan".to_string(),
+        column: "sheet_policy".to_string(),
+        removed_values: vec!["OVER_500".to_string()],
+        remaining_values: vec!["FIXED".to_string(), "NEGOTIATION".to_string()],
+    }];
+
+    let quoting_enum =
+        |_prompt: &str, values: &[String]| -> Result<String> { Ok(format!("'{}'", values[0])) };
+
+    let collected = collect_enum_fill_with_values(&missing, quoting_enum).unwrap();
+    assert_eq!(collected[0].1.get("OVER_500"), Some(&"FIXED".to_string()));
+}
+
 #[test]
 fn test_collect_enum_fill_with_values_multiple_removals() {
     use vespertide_planner::EnumFillWithRequired;

crates/vespertide-core/src/action/mod.rs

@@ -84,7 +84,12 @@ pub enum MigrationAction {
         column: ColumnName,
         new_type: ColumnType,
         /// Mapping of removed enum values to replacement values for safe enum value removal.
-        /// e.g., `{"cancelled": "'pending'"}` generates an `UPDATE` before the type change.
+        /// Both sides are **bare** enum labels — write them exactly as they appear in the enum
+        /// `values` list, with no surrounding SQL quotes. The SQL generator binds them as data
+        /// values and adds the quoting itself.
+        /// e.g., `{"cancelled": "pending"}` generates an `UPDATE` before the type change.
+        /// A legacy pre-quoted replacement (`"'pending'"`) still works: one outer quote layer is
+        /// stripped with a warning.
         #[serde(default, skip_serializing_if = "Option::is_none")]
         fill_with: Option<BTreeMap<String, String>>,
         /// Strategy for transforming existing rows that would violate a *narrowed* new type

crates/vespertide-query/src/sql/modify_column_type/fill_with.rs

@@ -0,0 +1,212 @@
+//! `fill_with` UPDATE emission for `ModifyColumnType`.
+//!
+//! `fill_with` maps a removed enum label to the surviving label that replaces
+//! it. Both sides of the mapping are **bare** labels ??no SQL quoting ??because
+//! [`Expr::val`] binds them as data values and the query builder adds exactly
+//! one layer of quoting itself.
+
+use std::collections::BTreeMap;
+use std::sync::Once;
+
+use sea_query::{Alias, Expr, ExprTrait, Query};
+
+use crate::sql::types::BuiltQuery;
+
+/// Emitted at most once per process by [`strip_legacy_outer_quotes`].
+static LEGACY_QUOTE_WARNING: Once = Once::new();
+
+#[expect(
+    clippy::print_stderr,
+    reason = "one-time deprecation notice for legacy pre-quoted fill_with values; stderr keeps the emitted SQL on stdout intact"
+)]
+fn warn_legacy_quoted_replacement(column: &str, removed: &str, replacement: &str, bare: &str) {
+    LEGACY_QUOTE_WARNING.call_once(|| {
+        eprintln!(
+            "vespertide: warning: modify_column_type.fill_with replacement \
+             {replacement} for {column}.{removed} is wrapped in SQL single \
+             quotes. fill_with values are bare enum labels; the quotes were \
+             stripped for compatibility. Rewrite the migration to use {bare}."
+        );
+    });
+}
+
+/// Backward compatibility for migration files written against the older schema
+/// documentation, which showed the replacement already wrapped in SQL single
+/// quotes (`{"cancelled": "'pending'"}`).
+///
+/// Since [`Expr::val`] binds the replacement as a *data value*, a pre-quoted
+/// label is escaped into `'''pending'''`, whose content is the 9-character
+/// token `'pending'` rather than the 7-character label `pending`. `PostgreSQL`
+/// then rejects the UPDATE with `invalid input value for enum`.
+///
+/// When the value both starts and ends with a single quote, exactly one outer
+/// layer is stripped and a one-time warning is emitted. Everything else is
+/// passed through untouched.
+fn strip_legacy_outer_quotes<'a>(column: &str, removed: &str, replacement: &'a str) -> &'a str {
+    let Some(bare) = replacement
+        .strip_prefix('\'')
+        .and_then(|inner| inner.strip_suffix('\''))
+    else {
+        return replacement;
+    };
+
+    warn_legacy_quoted_replacement(column, removed, replacement, bare);
+    bare
+}
+
+/// Build UPDATE statements for `fill_with` mappings (removed enum values ??replacement values).
+/// Each entry generates: UPDATE "table" SET "column" = 'replacement' WHERE "column" = '`removed_value`'
+///
+/// Iteration follows the `BTreeMap` key order, so the emitted statements are
+/// deterministic across runs and platforms.
+fn build_fill_with_updates(
+    table: &str,
+    column: &str,
+    fill_with: &BTreeMap<String, String>,
+) -> Vec<BuiltQuery> {
+    fill_with
+        .iter()
+        .map(|(removed_value, replacement)| {
+            let replacement = strip_legacy_outer_quotes(column, removed_value, replacement);
+            let update_stmt = Query::update()
+                .table(Alias::new(table))
+                .value(Alias::new(column), Expr::val(replacement))
+                .and_where(Expr::col(Alias::new(column)).eq(removed_value.as_str()))
+                .to_owned();
+            BuiltQuery::Update(Box::new(update_stmt))
+        })
+        .collect()
+}
+
+/// Conditionally prepend `fill_with` UPDATEs to `queries`.
+///
+/// Centralises the byte-identical
+/// `if let Some(fw) = fill_with { queries.extend(build_fill_with_updates(...)); }`
+/// dance that the three `modify_column_type` paths each previously
+/// open-coded (`direct::build_postgres_enum_migration`,
+/// `direct::build_standard_type_modification`, and
+/// `sqlite_rebuild::build_modify_column_type_sqlite_temp_table`). Each
+/// callsite now collapses to a single line whose name reads
+/// "if a fill_with map exists, prepend its UPDATEs".
+pub(super) fn extend_fill_with_updates(
+    queries: &mut Vec<BuiltQuery>,
+    table: &str,
+    column: &str,
+    fill_with: Option<&BTreeMap<String, String>>,
+) {
+    if let Some(fw) = fill_with {
+        queries.extend(build_fill_with_updates(table, column, fw));
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use crate::sql::DatabaseBackend;
+    use crate::test_support::{backend_tag, joined_sql_semicolon};
+    use insta::{assert_snapshot, with_settings};
+    use rstest::rstest;
+
+    fn updates(fill_with: &BTreeMap<String, String>, backend: DatabaseBackend) -> String {
+        let mut queries = Vec::new();
+        extend_fill_with_updates(&mut queries, "plan", "sheet_policy", Some(fill_with));
+        joined_sql_semicolon(backend, &queries)
+    }
+
+    fn map(pairs: &[(&str, &str)]) -> BTreeMap<String, String> {
+        pairs
+            .iter()
+            .map(|(k, v)| ((*k).to_string(), (*v).to_string()))
+            .collect()
+    }
+
+    /// A bare enum label gets exactly one layer of SQL quoting from the query
+    /// builder ??the enum label reaches the column intact.
+    #[rstest]
+    #[case::postgres(DatabaseBackend::Postgres)]
+    #[case::mysql(DatabaseBackend::MySql)]
+    #[case::sqlite(DatabaseBackend::Sqlite)]
+    fn bare_replacement_gets_exactly_one_quote_layer(#[case] backend: DatabaseBackend) {
+        let sql = updates(&map(&[("OVER_500", "FIXED")]), backend);
+
+        assert!(
+            sql.contains("= 'FIXED'"),
+            "expected a single quote layer around FIXED, got: {sql}"
+        );
+        assert!(
+            !sql.contains("'''FIXED'''"),
+            "replacement must not be double-quoted, got: {sql}"
+        );
+
+        with_settings!({ snapshot_path => "../snapshots", snapshot_suffix => format!("fill_with_bare_replacement_{}", backend_tag(backend)) }, {
+            assert_snapshot!(sql);
+        });
+    }
+
+    /// Compatibility: a legacy pre-quoted replacement produces the SAME SQL as
+    /// the bare form, so migrations written against the old documentation keep
+    /// working.
+    #[rstest]
+    #[case::postgres(DatabaseBackend::Postgres)]
+    #[case::mysql(DatabaseBackend::MySql)]
+    #[case::sqlite(DatabaseBackend::Sqlite)]
+    fn quoted_replacement_matches_bare_replacement(#[case] backend: DatabaseBackend) {
+        let bare = updates(&map(&[("OVER_500", "FIXED")]), backend);
+        let quoted = updates(&map(&[("OVER_500", "'FIXED'")]), backend);
+
+        assert_eq!(quoted, bare);
+    }
+
+    /// Only the outer layer is stripped: a doubly-wrapped value keeps its inner
+    /// quotes, and a value with a stray quote on one side is left alone.
+    #[rstest]
+    #[case::double_wrapped("''FIXED''", "'FIXED'")]
+    #[case::leading_quote_only("'FIXED", "'FIXED")]
+    #[case::trailing_quote_only("FIXED'", "FIXED'")]
+    #[case::lone_quote("'", "'")]
+    #[case::empty_quotes("''", "")]
+    #[case::bare("FIXED", "FIXED")]
+    fn strip_legacy_outer_quotes_removes_at_most_one_layer(
+        #[case] input: &str,
+        #[case] expected: &str,
+    ) {
+        assert_eq!(
+            strip_legacy_outer_quotes("sheet_policy", "OVER_500", input),
+            expected
+        );
+    }
+
+    /// `fill_with` is a `BTreeMap`, so multiple mappings emit in sorted key
+    /// order regardless of insertion order.
+    #[rstest]
+    #[case::postgres(DatabaseBackend::Postgres)]
+    #[case::mysql(DatabaseBackend::MySql)]
+    #[case::sqlite(DatabaseBackend::Sqlite)]
+    fn multiple_mappings_are_deterministically_ordered(#[case] backend: DatabaseBackend) {
+        let ascending = map(&[
+            ("OVER_500", "FIXED"),
+            ("PER_SHEET", "NEGOTIATION"),
+            ("UNDER_100", "FIXED"),
+        ]);
+        let descending = map(&[
+            ("UNDER_100", "FIXED"),
+            ("PER_SHEET", "NEGOTIATION"),
+            ("OVER_500", "FIXED"),
+        ]);
+        let sql = updates(&ascending, backend);
+
+        assert_eq!(updates(&descending, backend), sql);
+
+        with_settings!({ snapshot_path => "../snapshots", snapshot_suffix => format!("fill_with_multiple_mappings_{}", backend_tag(backend)) }, {
+            assert_snapshot!(sql);
+        });
+    }
+
+    /// `None` contributes no statements.
+    #[test]
+    fn absent_fill_with_emits_nothing() {
+        let mut queries = Vec::new();
+        extend_fill_with_updates(&mut queries, "plan", "sheet_policy", None);
+        assert!(queries.is_empty());
+    }
+}

crates/vespertide-query/src/sql/modify_column_type/mod.rs

@@ -1,9 +1,12 @@
 mod direct;
+mod fill_with;
 mod narrowing_preprocess;
 mod sqlite_rebuild;
 
 pub use narrowing_preprocess::build_narrowing_preprocess;
 
+use fill_with::extend_fill_with_updates;
+
 use vespertide_core::NarrowingStrategy;
 
 /// Combine narrowing pre-processing (when `narrowing_strategy` is set) with
@@ -138,56 +141,13 @@ fn build_pg_alter_with_timezone(
 
 use std::collections::BTreeMap;
 
-use sea_query::{Alias, Expr, ExprTrait, Query};
-
 use vespertide_core::{ColumnType, TableDef};
 
 use self::direct::build_modify_column_type_direct;
 use self::sqlite_rebuild::build_modify_column_type_sqlite_temp_table;
 use super::types::{BuiltQuery, DatabaseBackend};
 use crate::error::QueryError;
 
-/// Build UPDATE statements for `fill_with` mappings (removed enum values → replacement values).
-/// Each entry generates: UPDATE "table" SET "column" = 'replacement' WHERE "column" = '`removed_value`'
-fn build_fill_with_updates(
-    table: &str,
-    column: &str,
-    fill_with: &BTreeMap<String, String>,
-) -> Vec<BuiltQuery> {
-    fill_with
-        .iter()
-        .map(|(removed_value, replacement)| {
-            let update_stmt = Query::update()
-                .table(Alias::new(table))
-                .value(Alias::new(column), Expr::val(replacement.as_str()))
-                .and_where(Expr::col(Alias::new(column)).eq(removed_value.as_str()))
-                .to_owned();
-            BuiltQuery::Update(Box::new(update_stmt))
-        })
-        .collect()
-}
-
-/// Conditionally prepend `fill_with` UPDATEs to `queries`.
-///
-/// Centralises the byte-identical
-/// `if let Some(fw) = fill_with { queries.extend(build_fill_with_updates(...)); }`
-/// dance that the three `modify_column_type` paths each previously
-/// open-coded (`direct::build_postgres_enum_migration`,
-/// `direct::build_standard_type_modification`, and
-/// `sqlite_rebuild::build_modify_column_type_sqlite_temp_table`). Each
-/// callsite now collapses to a single line whose name reads
-/// "if a fill_with map exists, prepend its UPDATEs".
-pub(super) fn extend_fill_with_updates(
-    queries: &mut Vec<BuiltQuery>,
-    table: &str,
-    column: &str,
-    fill_with: Option<&BTreeMap<String, String>>,
-) {
-    if let Some(fw) = fill_with {
-        queries.extend(build_fill_with_updates(table, column, fw));
-    }
-}
-
 pub fn build_modify_column_type(
     backend: DatabaseBackend,
     table: &str,

crates/vespertide-query/src/sql/snapshots/vespertide_query__sql__modify_column_type__fill_with__tests__bare_replacement_gets_exactly_one_quote_layer@fill_with_bare_replacement_mysql.snap

@@ -0,0 +1,5 @@
+---
+source: crates/vespertide-query/src/sql/modify_column_type/fill_with.rs
+expression: sql
+---
+UPDATE `plan` SET `sheet_policy` = 'FIXED' WHERE `sheet_policy` = 'OVER_500'

crates/vespertide-query/src/sql/snapshots/vespertide_query__sql__modify_column_type__fill_with__tests__bare_replacement_gets_exactly_one_quote_layer@fill_with_bare_replacement_postgres.snap

@@ -0,0 +1,5 @@
+---
+source: crates/vespertide-query/src/sql/modify_column_type/fill_with.rs
+expression: sql
+---
+UPDATE "plan" SET "sheet_policy" = 'FIXED' WHERE "sheet_policy" = 'OVER_500'

crates/vespertide-query/src/sql/snapshots/vespertide_query__sql__modify_column_type__fill_with__tests__bare_replacement_gets_exactly_one_quote_layer@fill_with_bare_replacement_sqlite.snap

@@ -0,0 +1,5 @@
+---
+source: crates/vespertide-query/src/sql/modify_column_type/fill_with.rs
+expression: sql
+---
+UPDATE "plan" SET "sheet_policy" = 'FIXED' WHERE "sheet_policy" = 'OVER_500'

crates/vespertide-query/src/sql/snapshots/vespertide_query__sql__modify_column_type__fill_with__tests__multiple_mappings_are_deterministically_ordered@fill_with_multiple_mappings_mysql.snap

@@ -0,0 +1,7 @@
+---
+source: crates/vespertide-query/src/sql/modify_column_type/fill_with.rs
+expression: sql
+---
+UPDATE `plan` SET `sheet_policy` = 'FIXED' WHERE `sheet_policy` = 'OVER_500';
+UPDATE `plan` SET `sheet_policy` = 'NEGOTIATION' WHERE `sheet_policy` = 'PER_SHEET';
+UPDATE `plan` SET `sheet_policy` = 'FIXED' WHERE `sheet_policy` = 'UNDER_100'

crates/vespertide-query/src/sql/snapshots/vespertide_query__sql__modify_column_type__fill_with__tests__multiple_mappings_are_deterministically_ordered@fill_with_multiple_mappings_postgres.snap

@@ -0,0 +1,7 @@
+---
+source: crates/vespertide-query/src/sql/modify_column_type/fill_with.rs
+expression: sql
+---
+UPDATE "plan" SET "sheet_policy" = 'FIXED' WHERE "sheet_policy" = 'OVER_500';
+UPDATE "plan" SET "sheet_policy" = 'NEGOTIATION' WHERE "sheet_policy" = 'PER_SHEET';
+UPDATE "plan" SET "sheet_policy" = 'FIXED' WHERE "sheet_policy" = 'UNDER_100'

crates/vespertide-query/src/sql/snapshots/vespertide_query__sql__modify_column_type__fill_with__tests__multiple_mappings_are_deterministically_ordered@fill_with_multiple_mappings_sqlite.snap

@@ -0,0 +1,7 @@
+---
+source: crates/vespertide-query/src/sql/modify_column_type/fill_with.rs
+expression: sql
+---
+UPDATE "plan" SET "sheet_policy" = 'FIXED' WHERE "sheet_policy" = 'OVER_500';
+UPDATE "plan" SET "sheet_policy" = 'NEGOTIATION' WHERE "sheet_policy" = 'PER_SHEET';
+UPDATE "plan" SET "sheet_policy" = 'FIXED' WHERE "sheet_policy" = 'UNDER_100'