Skip to content

Commit fa1a353

Browse files
authored
Fix unique column in schema sync (#2971)
* Fix unique column in schema sync * Move code * Fixup
1 parent af8061e commit fa1a353

3 files changed

Lines changed: 285 additions & 5 deletions

File tree

sea-orm-arrow/Cargo.toml

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,3 @@ with-bigdecimal = ["sea-query/with-bigdecimal"]
2525
with-chrono = ["sea-query/with-chrono"]
2626
with-rust_decimal = ["sea-query/with-rust_decimal"]
2727
with-time = ["sea-query/with-time"]
28-
29-
[patch.crates-io]
30-
# sea-query = { path = "../../sea-query" }

src/schema/builder.rs

Lines changed: 59 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
use super::{Schema, TopologicalSort};
22
use crate::{ConnectionTrait, DbBackend, DbErr, EntityTrait, Statement};
33
use sea_query::{
4-
ForeignKeyCreateStatement, IndexCreateStatement, TableAlterStatement, TableCreateStatement,
5-
TableName, TableRef, extension::postgres::TypeCreateStatement,
4+
ForeignKeyCreateStatement, Index, IndexCreateStatement, IntoIden, TableAlterStatement,
5+
TableCreateStatement, TableName, TableRef, extension::postgres::TypeCreateStatement,
66
};
77

88
/// A schema builder that can take a registry of Entities and synchronize it with database.
@@ -387,6 +387,46 @@ impl EntitySchemaInfo {
387387
db.execute(&stmt).await?;
388388
}
389389
}
390+
if let Some(existing_table) = existing_table {
391+
// For columns with a column-level UNIQUE constraint (#[sea_orm(unique)]) that
392+
// already exist in the table but do not yet have a unique index, create one.
393+
for column_def in self.table.get_columns() {
394+
if column_def.get_column_spec().unique {
395+
let col_name = column_def.get_column_name();
396+
let col_exists = existing_table
397+
.get_columns()
398+
.iter()
399+
.any(|c| c.get_column_name() == col_name);
400+
if !col_exists {
401+
// Column is being added in this sync pass; the ALTER TABLE ADD COLUMN
402+
// will include the UNIQUE inline, so no separate index needed.
403+
continue;
404+
}
405+
let already_unique = existing_table.get_indexes().iter().any(|idx| {
406+
if !idx.is_unique_key() {
407+
return false;
408+
}
409+
let cols = idx.get_index_spec().get_column_names();
410+
cols.len() == 1 && cols[0] == col_name
411+
});
412+
if !already_unique {
413+
let table_name =
414+
self.table.get_table_name().expect("table must have a name");
415+
let tbl_str = table_name.sea_orm_table().to_string();
416+
let table_ref = table_name.clone();
417+
db.execute(
418+
Index::create()
419+
.name(format!("idx-{tbl_str}-{col_name}"))
420+
.table(table_ref)
421+
.col(col_name.into_iden())
422+
.unique()
423+
.if_not_exists(),
424+
)
425+
.await?;
426+
}
427+
}
428+
}
429+
}
390430
if let Some(existing_table) = existing_table {
391431
// find all unique keys from existing table
392432
// if it no longer exist in new schema, drop it
@@ -401,6 +441,23 @@ impl EntitySchemaInfo {
401441
break;
402442
}
403443
}
444+
// Also check if the unique index corresponds to a column-level UNIQUE
445+
// constraint (from #[sea_orm(unique)]). These are embedded in the CREATE
446+
// TABLE column definition and not tracked in self.indexes, so we must not
447+
// try to drop them during sync.
448+
if !has_index {
449+
let index_cols = existing_index.get_index_spec().get_column_names();
450+
if index_cols.len() == 1 {
451+
for column_def in self.table.get_columns() {
452+
if column_def.get_column_name() == index_cols[0]
453+
&& column_def.get_column_spec().unique
454+
{
455+
has_index = true;
456+
break;
457+
}
458+
}
459+
}
460+
}
404461
if !has_index {
405462
if let Some(drop_existing) = existing_index.get_index_spec().get_name() {
406463
db.execute(sea_query::Index::drop().name(drop_existing))

tests/schema_sync_tests.rs

Lines changed: 226 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,226 @@
1+
#![allow(unused_imports, dead_code)]
2+
3+
pub mod common;
4+
5+
use crate::common::TestContext;
6+
use sea_orm::{
7+
DatabaseConnection, DbErr,
8+
entity::*,
9+
query::*,
10+
sea_query::{Condition, Expr, Query},
11+
};
12+
13+
// Scenario 1: table is first synced with a `#[sea_orm(unique)]` column already
14+
// present. Repeated syncs must not drop the column-level unique constraint.
15+
mod item_v1 {
16+
use sea_orm::entity::prelude::*;
17+
18+
#[sea_orm::model]
19+
#[derive(Clone, Debug, PartialEq, Eq, DeriveEntityModel)]
20+
#[sea_orm(table_name = "sync_item")]
21+
pub struct Model {
22+
#[sea_orm(primary_key)]
23+
pub id: i32,
24+
#[sea_orm(unique)]
25+
pub name: String,
26+
}
27+
28+
impl ActiveModelBehavior for ActiveModel {}
29+
}
30+
31+
// Scenario 2a: initial version of the table — no unique column yet.
32+
mod product_v1 {
33+
use sea_orm::entity::prelude::*;
34+
35+
#[sea_orm::model]
36+
#[derive(Clone, Debug, PartialEq, Eq, DeriveEntityModel)]
37+
#[sea_orm(table_name = "sync_product")]
38+
pub struct Model {
39+
#[sea_orm(primary_key)]
40+
pub id: i32,
41+
}
42+
43+
impl ActiveModelBehavior for ActiveModel {}
44+
}
45+
46+
// Scenario 2b: updated version — a unique column is added.
47+
mod product_v2 {
48+
use sea_orm::entity::prelude::*;
49+
50+
#[sea_orm::model]
51+
#[derive(Clone, Debug, PartialEq, Eq, DeriveEntityModel)]
52+
#[sea_orm(table_name = "sync_product")]
53+
pub struct Model {
54+
#[sea_orm(primary_key)]
55+
pub id: i32,
56+
#[sea_orm(unique)]
57+
pub sku: String,
58+
}
59+
60+
impl ActiveModelBehavior for ActiveModel {}
61+
}
62+
63+
// Scenario 3a: initial version — column exists without UNIQUE.
64+
mod user_v1 {
65+
use sea_orm::entity::prelude::*;
66+
67+
#[sea_orm::model]
68+
#[derive(Clone, Debug, PartialEq, Eq, DeriveEntityModel)]
69+
#[sea_orm(table_name = "sync_user")]
70+
pub struct Model {
71+
#[sea_orm(primary_key)]
72+
pub id: i32,
73+
pub email: String,
74+
}
75+
76+
impl ActiveModelBehavior for ActiveModel {}
77+
}
78+
79+
// Scenario 3b: updated version — the existing column is made unique.
80+
mod user_v2 {
81+
use sea_orm::entity::prelude::*;
82+
83+
#[sea_orm::model]
84+
#[derive(Clone, Debug, PartialEq, Eq, DeriveEntityModel)]
85+
#[sea_orm(table_name = "sync_user")]
86+
pub struct Model {
87+
#[sea_orm(primary_key)]
88+
pub id: i32,
89+
#[sea_orm(unique)]
90+
pub email: String,
91+
}
92+
93+
impl ActiveModelBehavior for ActiveModel {}
94+
}
95+
96+
/// Regression test for <https://github.com/SeaQL/sea-orm/issues/2970>.
97+
///
98+
/// A table with a `#[sea_orm(unique)]` column is created on the first sync.
99+
/// The subsequent sync must not attempt to drop the column-level unique index.
100+
#[sea_orm_macros::test]
101+
async fn test_sync_unique_column_no_drop() -> Result<(), DbErr> {
102+
let ctx = TestContext::new("test_sync_unique_column_no_drop").await;
103+
let db = &ctx.db;
104+
105+
#[cfg(feature = "schema-sync")]
106+
{
107+
// First sync: creates the table with the unique column
108+
db.get_schema_builder()
109+
.register(item_v1::Entity)
110+
.sync(db)
111+
.await?;
112+
113+
// Second sync: must not try to drop the column-level unique index
114+
db.get_schema_builder()
115+
.register(item_v1::Entity)
116+
.sync(db)
117+
.await?;
118+
119+
#[cfg(feature = "sqlx-postgres")]
120+
assert!(
121+
pg_index_exists(db, "sync_item", "sync_item_name_key").await?,
122+
"unique index on `sync_item.name` should still exist after repeated sync"
123+
);
124+
}
125+
126+
Ok(())
127+
}
128+
129+
/// Regression test for <https://github.com/SeaQL/sea-orm/issues/2970>.
130+
///
131+
/// A unique column is added to an existing table via sync (ALTER TABLE ADD
132+
/// COLUMN … UNIQUE), which creates a column-level unique index. A subsequent
133+
/// sync must not attempt to drop that index.
134+
#[sea_orm_macros::test]
135+
#[cfg(not(any(feature = "sqlx-sqlite", feature = "rusqlite")))]
136+
async fn test_sync_add_unique_column_no_drop() -> Result<(), DbErr> {
137+
let ctx = TestContext::new("test_sync_add_unique_column_no_drop").await;
138+
let db = &ctx.db;
139+
140+
#[cfg(feature = "schema-sync")]
141+
{
142+
// First sync: creates the table without the unique column
143+
db.get_schema_builder()
144+
.register(product_v1::Entity)
145+
.sync(db)
146+
.await?;
147+
148+
// Second sync: adds the unique column via ALTER TABLE ADD COLUMN … UNIQUE
149+
db.get_schema_builder()
150+
.register(product_v2::Entity)
151+
.sync(db)
152+
.await?;
153+
154+
// Third sync: must not try to drop the unique index created above
155+
db.get_schema_builder()
156+
.register(product_v2::Entity)
157+
.sync(db)
158+
.await?;
159+
160+
#[cfg(feature = "sqlx-postgres")]
161+
assert!(
162+
pg_index_exists(db, "sync_product", "sync_product_sku_key").await?,
163+
"unique index on `sync_product.sku` should still exist after repeated sync"
164+
);
165+
}
166+
167+
Ok(())
168+
}
169+
170+
/// Scenario 3: an existing column is made unique in a later sync.
171+
///
172+
/// When a column that already exists in the DB is annotated with
173+
/// `#[sea_orm(unique)]`, the sync must create a unique index for it.
174+
#[sea_orm_macros::test]
175+
async fn test_sync_make_existing_column_unique() -> Result<(), DbErr> {
176+
let ctx = TestContext::new("test_sync_make_existing_column_unique").await;
177+
let db = &ctx.db;
178+
179+
#[cfg(feature = "schema-sync")]
180+
{
181+
// First sync: creates the table with a plain (non-unique) email column
182+
db.get_schema_builder()
183+
.register(user_v1::Entity)
184+
.sync(db)
185+
.await?;
186+
187+
// Second sync: email is now marked unique — should create the unique index
188+
db.get_schema_builder()
189+
.register(user_v2::Entity)
190+
.sync(db)
191+
.await?;
192+
193+
// Third sync: must not try to drop or re-create the index
194+
db.get_schema_builder()
195+
.register(user_v2::Entity)
196+
.sync(db)
197+
.await?;
198+
199+
#[cfg(feature = "sqlx-postgres")]
200+
assert!(
201+
pg_index_exists(db, "sync_user", "idx-sync_user-email").await?,
202+
"unique index on `sync_user.email` should be created when column is made unique"
203+
);
204+
}
205+
206+
Ok(())
207+
}
208+
209+
#[cfg(feature = "sqlx-postgres")]
210+
async fn pg_index_exists(db: &DatabaseConnection, table: &str, index: &str) -> Result<bool, DbErr> {
211+
db.query_one(
212+
Query::select()
213+
.expr(Expr::cust("COUNT(*) > 0"))
214+
.from("pg_indexes")
215+
.cond_where(
216+
Condition::all()
217+
.add(Expr::cust("schemaname = CURRENT_SCHEMA()"))
218+
.add(Expr::col("tablename").eq(table))
219+
.add(Expr::col("indexname").eq(index)),
220+
),
221+
)
222+
.await?
223+
.unwrap()
224+
.try_get_by_index(0)
225+
.map_err(DbErr::from)
226+
}

0 commit comments

Comments
 (0)