Skip to content

Commit e7a69b1

Browse files
owjs3901owjs3901
authored
Merge pull request #133 from dev-five-git/fix-index-column-issue
Fix index column issue
2 parents 374be6b + 26ca7e9 commit e7a69b1

14 files changed

+476
-97
lines changed

‎.changepacks/changepack_log_kZhbf9xXT865BdVdu7Dl6.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
{"changes":{"crates/vespertide-exporter/Cargo.toml":"Patch","crates/vespertide/Cargo.toml":"Patch","crates/vespertide-naming/Cargo.toml":"Patch","crates/vespertide-config/Cargo.toml":"Patch","crates/vespertide-macro/Cargo.toml":"Patch","crates/vespertide-planner/Cargo.toml":"Patch","crates/vespertide-query/Cargo.toml":"Patch","crates/vespertide-cli/Cargo.toml":"Patch","crates/vespertide-core/Cargo.toml":"Patch","crates/vespertide-loader/Cargo.toml":"Patch"},"note":"Fix index column issue","date":"2026-04-13T08:37:23.992645500Z"}

‎Cargo.lock‎

Lines changed: 10 additions & 10 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎crates/vespertide-planner/src/diff.rs‎

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -561,13 +561,22 @@ pub fn diff_schemas(from: &[TableDef], to: &[TableDef]) -> Result<MigrationPlan,
561561
}
562562

563563
// Added columns
564-
// Note: Inline foreign keys are already converted to TableConstraint::ForeignKey
565-
// by normalize(), so they will be handled in the constraint diff below.
564+
// Inline constraints (index, unique, foreign_key, primary_key) are already
565+
// promoted to table-level TableConstraint entries by normalize(), so they
566+
// will be emitted as separate AddConstraint actions in the constraint diff
567+
// below. Strip them from the column def to prevent apply_action's normalize()
568+
// from adding phantom constraints to the evolving schema, which would cause
569+
// premature index recreation during SQLite temp table rebuilds.
566570
for (col, def) in &to_cols {
567571
if !from_cols.contains_key(col) {
572+
let mut col_def = (*def).clone();
573+
col_def.primary_key = None;
574+
col_def.unique = None;
575+
col_def.index = None;
576+
col_def.foreign_key = None;
568577
actions.push(MigrationAction::AddColumn {
569578
table: name.to_string(),
570-
column: Box::new((*def).clone()),
579+
column: Box::new(col_def),
571580
fill_with: None,
572581
});
573582
}

‎crates/vespertide-query/src/builder.rs‎

Lines changed: 123 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,25 @@ pub struct PlanQueries {
1313
pub sqlite: Vec<BuiltQuery>,
1414
}
1515

16+
/// Extract the target table name from any migration action.
17+
/// Returns `None` for `RawSql` (no table) and `RenameTable` (ambiguous).
18+
fn action_target_table(action: &MigrationAction) -> Option<&str> {
19+
match action {
20+
MigrationAction::CreateTable { table, .. }
21+
| MigrationAction::DeleteTable { table }
22+
| MigrationAction::AddColumn { table, .. }
23+
| MigrationAction::RenameColumn { table, .. }
24+
| MigrationAction::DeleteColumn { table, .. }
25+
| MigrationAction::ModifyColumnType { table, .. }
26+
| MigrationAction::ModifyColumnNullable { table, .. }
27+
| MigrationAction::ModifyColumnDefault { table, .. }
28+
| MigrationAction::ModifyColumnComment { table, .. }
29+
| MigrationAction::AddConstraint { table, .. }
30+
| MigrationAction::RemoveConstraint { table, .. } => Some(table),
31+
MigrationAction::RenameTable { .. } | MigrationAction::RawSql { .. } => None,
32+
}
33+
}
34+
1635
pub fn build_plan_queries(
1736
plan: &MigrationPlan,
1837
current_schema: &[TableDef],
@@ -27,8 +46,13 @@ pub fn build_plan_queries(
2746
// but haven't been physically created as DB indexes yet.
2847
// Without this, a temp table rebuild would recreate these indexes prematurely,
2948
// causing "index already exists" errors when their AddConstraint actions run later.
49+
//
50+
// This applies to ANY action that may trigger a SQLite temp table rebuild
51+
// (AddColumn with NOT NULL, ModifyColumn*, DeleteColumn, AddConstraint FK/PK/Check,
52+
// RemoveConstraint), not just AddConstraint.
53+
let action_table = action_target_table(action);
3054
let pending_constraints: Vec<vespertide_core::TableConstraint> =
31-
if let MigrationAction::AddConstraint { table, .. } = action {
55+
if let Some(table) = action_table {
3256
plan.actions[i + 1..]
3357
.iter()
3458
.filter_map(|a| {
@@ -765,4 +789,102 @@ mod tests {
765789
assert_snapshot!(sql);
766790
});
767791
}
792+
793+
// ── Two NOT NULL AddColumns with inline index + AddConstraint ────────
794+
795+
/// Regression test: when two NOT NULL columns with inline `index: true`
796+
/// are added sequentially, the second AddColumn triggers a SQLite temp
797+
/// table rebuild. At that point the evolving schema already contains the
798+
/// first column's index (from normalization). Without pending constraint
799+
/// awareness, the rebuild recreates that index, and the later
800+
/// AddConstraint for the same index fails with "index already exists".
801+
#[rstest]
802+
#[case::postgres("postgres", DatabaseBackend::Postgres)]
803+
#[case::mysql("mysql", DatabaseBackend::MySql)]
804+
#[case::sqlite("sqlite", DatabaseBackend::Sqlite)]
805+
fn test_two_not_null_add_columns_with_inline_index_no_duplicate(
806+
#[case] label: &str,
807+
#[case] backend: DatabaseBackend,
808+
) {
809+
use vespertide_core::DefaultValue;
810+
use vespertide_core::schema::str_or_bool::StrOrBoolOrArray;
811+
812+
let schema = vec![TableDef {
813+
name: "article".into(),
814+
description: None,
815+
columns: vec![
816+
col("id", ColumnType::Simple(SimpleColumnType::Integer)),
817+
col("title", ColumnType::Simple(SimpleColumnType::Text)),
818+
],
819+
constraints: vec![],
820+
}];
821+
822+
let plan = MigrationPlan {
823+
id: String::new(),
824+
comment: None,
825+
created_at: None,
826+
version: 1,
827+
actions: vec![
828+
// 1. Add NOT NULL column with inline index
829+
MigrationAction::AddColumn {
830+
table: "article".into(),
831+
column: Box::new(ColumnDef {
832+
name: "category_pinned".into(),
833+
r#type: ColumnType::Simple(SimpleColumnType::Boolean),
834+
nullable: false,
835+
default: Some(DefaultValue::Bool(false)),
836+
comment: None,
837+
primary_key: None,
838+
unique: None,
839+
index: Some(StrOrBoolOrArray::Bool(true)),
840+
foreign_key: None,
841+
}),
842+
fill_with: None,
843+
},
844+
// 2. Add another NOT NULL column with inline index
845+
MigrationAction::AddColumn {
846+
table: "article".into(),
847+
column: Box::new(ColumnDef {
848+
name: "main_pinned".into(),
849+
r#type: ColumnType::Simple(SimpleColumnType::Boolean),
850+
nullable: false,
851+
default: Some(DefaultValue::Bool(false)),
852+
comment: None,
853+
primary_key: None,
854+
unique: None,
855+
index: Some(StrOrBoolOrArray::Bool(true)),
856+
foreign_key: None,
857+
}),
858+
fill_with: None,
859+
},
860+
// 3. AddConstraint for main_pinned index
861+
MigrationAction::AddConstraint {
862+
table: "article".into(),
863+
constraint: TableConstraint::Index {
864+
name: None,
865+
columns: vec!["main_pinned".into()],
866+
},
867+
},
868+
// 4. AddConstraint for category_pinned index
869+
MigrationAction::AddConstraint {
870+
table: "article".into(),
871+
constraint: TableConstraint::Index {
872+
name: None,
873+
columns: vec!["category_pinned".into()],
874+
},
875+
},
876+
],
877+
};
878+
879+
let result = build_plan_queries(&plan, &schema).unwrap();
880+
881+
// Core invariant: no duplicate indexes across actions
882+
assert_no_duplicate_indexes_per_action(&result);
883+
assert_no_orphan_duplicate_indexes(&result);
884+
885+
let sql = collect_all_sql(&result, backend);
886+
with_settings!({ snapshot_suffix => format!("two_not_null_inline_index_{}", label) }, {
887+
assert_snapshot!(sql);
888+
});
889+
}
768890
}

‎crates/vespertide-query/src/snapshots/vespertide_query__builder__tests__two_not_null_add_columns_with_inline_index_no_duplicate@two_not_null_inline_index_mysql.snap‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
---
2+
source: crates/vespertide-query/src/builder.rs
3+
expression: sql
4+
---
5+
-- Action 0: AddColumn { table: "article", column: ColumnDef { name: "category_pinned", type: Simple(Boolean), nullable: false, default: Some(Bool(false)), comment: None, primary_key: None, unique: None, index: Some(Bool(true)), foreign_key: None }, fill_with: None }
6+
ALTER TABLE `article` ADD COLUMN `category_pinned` bool NOT NULL DEFAULT false
7+
8+
-- Action 1: AddColumn { table: "article", column: ColumnDef { name: "main_pinned", type: Simple(Boolean), nullable: false, default: Some(Bool(false)), comment: None, primary_key: None, unique: None, index: Some(Bool(true)), foreign_key: None }, fill_with: None }
9+
ALTER TABLE `article` ADD COLUMN `main_pinned` bool NOT NULL DEFAULT false
10+
11+
-- Action 2: AddConstraint { table: "article", constraint: Index { name: None, columns: ["main_pinned"] } }
12+
CREATE INDEX `ix_article__main_pinned` ON `article` (`main_pinned`)
13+
14+
-- Action 3: AddConstraint { table: "article", constraint: Index { name: None, columns: ["category_pinned"] } }
15+
CREATE INDEX `ix_article__category_pinned` ON `article` (`category_pinned`)

‎crates/vespertide-query/src/snapshots/vespertide_query__builder__tests__two_not_null_add_columns_with_inline_index_no_duplicate@two_not_null_inline_index_postgres.snap‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
---
2+
source: crates/vespertide-query/src/builder.rs
3+
expression: sql
4+
---
5+
-- Action 0: AddColumn { table: "article", column: ColumnDef { name: "category_pinned", type: Simple(Boolean), nullable: false, default: Some(Bool(false)), comment: None, primary_key: None, unique: None, index: Some(Bool(true)), foreign_key: None }, fill_with: None }
6+
ALTER TABLE "article" ADD COLUMN "category_pinned" bool NOT NULL DEFAULT false
7+
8+
-- Action 1: AddColumn { table: "article", column: ColumnDef { name: "main_pinned", type: Simple(Boolean), nullable: false, default: Some(Bool(false)), comment: None, primary_key: None, unique: None, index: Some(Bool(true)), foreign_key: None }, fill_with: None }
9+
ALTER TABLE "article" ADD COLUMN "main_pinned" bool NOT NULL DEFAULT false
10+
11+
-- Action 2: AddConstraint { table: "article", constraint: Index { name: None, columns: ["main_pinned"] } }
12+
CREATE INDEX "ix_article__main_pinned" ON "article" ("main_pinned")
13+
14+
-- Action 3: AddConstraint { table: "article", constraint: Index { name: None, columns: ["category_pinned"] } }
15+
CREATE INDEX "ix_article__category_pinned" ON "article" ("category_pinned")

‎crates/vespertide-query/src/snapshots/vespertide_query__builder__tests__two_not_null_add_columns_with_inline_index_no_duplicate@two_not_null_inline_index_sqlite.snap‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
---
2+
source: crates/vespertide-query/src/builder.rs
3+
expression: sql
4+
---
5+
-- Action 0: AddColumn { table: "article", column: ColumnDef { name: "category_pinned", type: Simple(Boolean), nullable: false, default: Some(Bool(false)), comment: None, primary_key: None, unique: None, index: Some(Bool(true)), foreign_key: None }, fill_with: None }
6+
CREATE TABLE "article_temp" ( "id" integer, "title" text, "category_pinned" boolean NOT NULL DEFAULT false );
7+
INSERT INTO "article_temp" ("id", "title", "category_pinned") SELECT "id", "title", false AS "category_pinned" FROM "article";
8+
DROP TABLE "article";
9+
ALTER TABLE "article_temp" RENAME TO "article"
10+
11+
-- Action 1: AddColumn { table: "article", column: ColumnDef { name: "main_pinned", type: Simple(Boolean), nullable: false, default: Some(Bool(false)), comment: None, primary_key: None, unique: None, index: Some(Bool(true)), foreign_key: None }, fill_with: None }
12+
CREATE TABLE "article_temp" ( "id" integer, "title" text, "category_pinned" boolean NOT NULL DEFAULT false, "main_pinned" boolean NOT NULL DEFAULT false );
13+
INSERT INTO "article_temp" ("id", "title", "category_pinned", "main_pinned") SELECT "id", "title", "category_pinned", false AS "main_pinned" FROM "article";
14+
DROP TABLE "article";
15+
ALTER TABLE "article_temp" RENAME TO "article"
16+
17+
-- Action 2: AddConstraint { table: "article", constraint: Index { name: None, columns: ["main_pinned"] } }
18+
CREATE INDEX "ix_article__main_pinned" ON "article" ("main_pinned")
19+
20+
-- Action 3: AddConstraint { table: "article", constraint: Index { name: None, columns: ["category_pinned"] } }
21+
CREATE INDEX "ix_article__category_pinned" ON "article" ("category_pinned")

0 commit comments

Comments
 (0)