86b649d · reshape · git.iko.soy

git.iko.soy/reshape

on main help sign in

Copy indices to temporary columns for alter_column

86b649dfabianlindfors5 years ago

src/migrations/alter_column.rs+87−1

compare with main

unifiedsplit

@@ -150,6 +150,33 @@ impl Action for AlterColumn {

150 common::batch_touch_rows(db, &table.real_name, &column.real_name)

151 .context("failed to batch update existing rows")?;

152

153+ // Duplicate any indices to the temporary column

154+ let indices = common::get_indices_for_column(db, &table.real_name, &column.real_name)?;

155+ for (index_name, index_oid) in indices {

156+ let index_columns: Vec<String> = common::get_index_columns(db, &index_name)?

157+ .into_iter()

158+ .map(|idx_column| {

159+ // Replace column with temporary column for new index

160+ if idx_column == column.real_name {

161+ temporary_column_name.to_string()

162+ } else {

163+ idx_column

164+ }

165+ })

166+ .collect();

167+ let temp_index_name = self.temp_index_name(ctx, index_oid);

168+

169+ db.query(&format!(

170+ r#"

171+ CREATE INDEX CONCURRENTLY IF NOT EXISTS "{new_index_name}" ON "{table}" ({columns})

172+ "#,

173+ new_index_name = temp_index_name,

174+ table = table.real_name,

175+ columns = index_columns.join(", "),

176+ ))

177+ .context("failed to create temporary index")?;

178+ }

179+

153 // Add a temporary NOT NULL constraint if the column shouldn't be nullable.

180 // Add a temporary NOT NULL constraint if the column shouldn't be nullable.

154 // This constraint is set as NOT VALID so it doesn't apply to existing rows and

181 // This constraint is set as NOT VALID so it doesn't apply to existing rows and

155 // the existing rows don't need to be scanned under an exclusive lock.

182 // the existing rows don't need to be scanned under an exclusive lock.

@@ -246,10 +273,52 @@ impl Action for AlterColumn {

246 .context("failed to drop NOT NULL constraint")?;

273 .context("failed to drop NOT NULL constraint")?;

247 }

274 }

248

275

276+ // Replace old indices with the new temporary ones created for the temporary column

277+ let indices = common::get_indices_for_column(db, &self.table, &self.column)?;

278+ for (current_index_name, index_oid) in indices {

279+ // To keep the index handling idempotent, we need to do the following:

280+ // 1. Add a prefix to the existing index

281+ // 2. Rename temporary index to its final name

282+ // 3. Drop existing index concurrently

283+

284+ // Add prefix (if not already added) to existing index

285+ let prefix = "__reshape_old";

286+ let target_index_name = current_index_name.trim_start_matches(prefix);

287+ let old_index_name = format!("{}_{}", prefix, target_index_name);

288+ db.query(&format!(

289+ r#"

290+ ALTER INDEX IF EXISTS "{current_name}" RENAME TO "{new_name}"

291+ "#,

292+ current_name = target_index_name,

293+ new_name = old_index_name,

294+ ))

295+ .context("failed to rename old index")?;

296+

297+ // Rename temporary index to real name

298+ let temp_index_name = self.temp_index_name(ctx, index_oid);

299+ db.query(&format!(

300+ r#"

301+ ALTER INDEX IF EXISTS "{temp_index_name}" RENAME TO "{target_index_name}"

302+ "#,

303+ temp_index_name = temp_index_name,

304+ target_index_name = target_index_name,

305+ ))

306+ .context("failed to rename temporary index")?;

307+

308+ // Drop old index concurrently

309+ db.query(&format!(

310+ r#"

311+ DROP INDEX CONCURRENTLY IF EXISTS "{old_index_name}"

312+ "#,

313+ old_index_name = old_index_name,

314+ ))

315+ .context("failed to drop old index")?;

316+ }

317+

249 // Remove old column

318 // Remove old column

250 let query = format!(

319 let query = format!(

251 r#"

320 r#"

252− ALTER TABLE "{table}" DROP COLUMN "{column}" CASCADE

321+ ALTER TABLE "{table}" DROP COLUMN IF EXISTS "{column}" CASCADE

253 "#,

322 "#,

254 table = self.table,

323 table = self.table,

255 column = self.column,

324 column = self.column,

@@ -311,6 +380,19 @@ impl Action for AlterColumn {

311 }

380 }

312

381

313 fn abort(&self, ctx: &MigrationContext, db: &mut dyn Conn) -> anyhow::Result<()> {

382 fn abort(&self, ctx: &MigrationContext, db: &mut dyn Conn) -> anyhow::Result<()> {

383+ // Safely remove any indices created for the temporary column

384+ let temp_column_name = self.temporary_column_name(ctx);

385+ let indices = common::get_indices_for_column(db, &self.table, &temp_column_name)?;

386+ for (_, index_oid) in indices {

387+ let temp_index_name = self.temp_index_name(ctx, index_oid);

388+ db.query(&format!(

389+ r#"

390+ DROP INDEX CONCURRENTLY IF EXISTS "{index_name}"

391+ "#,

392+ index_name = temp_index_name,

393+ ))?;

394+ }

395+

314 // Drop temporary column

396 // Drop temporary column

315 let query = format!(

397 let query = format!(

316 r#"

398 r#"

@@ -359,6 +441,10 @@ impl AlterColumn {

359 format!("{}_alter_column_temporary", ctx.prefix())

441 format!("{}_alter_column_temporary", ctx.prefix())

360 }

442 }

361

443

444+ fn temp_index_name(&self, ctx: &MigrationContext, index_oid: u32) -> String {

445+ format!("{}_alter_column_temp_index_{}", ctx.prefix(), index_oid)

446+ }

447+

362 fn can_short_circuit(&self) -> bool {

448 fn can_short_circuit(&self) -> bool {

363 self.changes.name.is_some()

449 self.changes.name.is_some()

364 && self.changes.data_type.is_none()

450 && self.changes.data_type.is_none()

src/migrations/common.rs+77

@@ -1,3 +1,4 @@

1+use anyhow::anyhow;

1use derive_builder::Builder;

2use derive_builder::Builder;

2use postgres::types::{FromSql, ToSql};

3use postgres::types::{FromSql, ToSql};

3use serde::{Deserialize, Serialize};

4use serde::{Deserialize, Serialize};

@@ -180,3 +181,79 @@ fn get_primary_key_columns_for_table(

180

181

181 Ok(primary_key_columns)

182 Ok(primary_key_columns)

182}

183}

184+

185+pub fn get_indices_for_column(

186+ db: &mut dyn Conn,

187+ table: &str,

188+ column: &str,

189+) -> anyhow::Result<Vec<(String, u32)>> {

190+ let indices = db

191+ .query(&format!(

192+ "

193+ SELECT i.relname AS index_name, i.oid AS index_oid

194+ FROM pg_index ix

195+ JOIN pg_class t ON t.oid = ix.indrelid

196+ JOIN pg_class i ON i.oid = ix.indexrelid

197+ JOIN pg_attribute a ON

198+ a.attrelid = t.oid AND

199+ a.attnum = ANY(ix.indkey)

200+ WHERE

201+ t.relname = '{table}' AND

202+ a.attname = '{column}'

203+ ",

204+ table = table,

205+ column = column,

206+ ))?

207+ .iter()

208+ .map(|row| (row.get("index_name"), row.get("index_oid")))

209+ .collect();

210+

211+ Ok(indices)

212+}

213+

214+pub fn get_index_columns(db: &mut dyn Conn, index_name: &str) -> anyhow::Result<Vec<String>> {

215+ // Get all columns which are part of the index in order

216+ let (table_oid, column_nums) = db

217+ .query(&format!(

218+ "

219+ SELECT t.oid AS table_oid, ix.indkey::INTEGER[] AS columns

220+ FROM pg_index ix

221+ JOIN pg_class t ON t.oid = ix.indrelid

222+ JOIN pg_class i ON i.oid = ix.indexrelid

223+ WHERE

224+ i.relname = '{index_name}'

225+ ",

226+ index_name = index_name,

227+ ))?

228+ .first()

229+ .map(|row| {

230+ (

231+ row.get::<'_, _, u32>("table_oid"),

232+ row.get::<'_, _, Vec<i32>>("columns"),

233+ )

234+ })

235+ .ok_or_else(|| anyhow!("failed to get columns for index"))?;

236+

237+ // Get the name of each of the columns, still in order

238+ column_nums

239+ .iter()

240+ .map(|column_num| -> anyhow::Result<String> {

241+ let name: String = db

242+ .query(&format!(

243+ "

244+ SELECT attname AS name

245+ FROM pg_attribute

246+ WHERE attrelid = {table_oid}

247+ AND attnum = {column_num};

248+ ",

249+ table_oid = table_oid,

250+ column_num = column_num,

251+ ))?

252+ .first()

253+ .map(|row| row.get("name"))

254+ .ok_or_else(|| anyhow!("expected to find column"))?;

255+

256+ Ok(name)

257+ })

258+ .collect::<anyhow::Result<Vec<String>>>()

259+}

tests/alter_column.rs+90−1

@@ -1,5 +1,5 @@

1use reshape::migrations::{

2− AlterColumn, ColumnBuilder, ColumnChanges, CreateTableBuilder, Migration,

2+ AddIndex, AlterColumn, ColumnBuilder, ColumnChanges, CreateTableBuilder, Migration,

3};

4use reshape::Status;

@@ -531,3 +531,92 @@ fn alter_column_default() {

531 reshape.complete_migration().unwrap();

532 common::assert_cleaned_up(&mut new_db);

533}

534+

535+#[test]

536+fn alter_column_with_index() {

537+ let (mut reshape, mut db, _) = common::setup();

538+

539+ let create_users_table = Migration::new("create_user_table", None)

540+ .with_action(

541+ CreateTableBuilder::default()

542+ .name("users")

543+ .primary_key(vec!["id".to_string()])

544+ .columns(vec![

545+ ColumnBuilder::default()

546+ .name("id")

547+ .data_type("INTEGER")

548+ .build()

549+ .unwrap(),

550+ ColumnBuilder::default()

551+ .name("first_name")

552+ .data_type("TEXT")

553+ .build()

554+ .unwrap(),

555+ ColumnBuilder::default()

556+ .name("last_name")

557+ .data_type("TEXT")

558+ .build()

559+ .unwrap(),

560+ ])

561+ .build()

562+ .unwrap(),

563+ )

564+ .with_action(AddIndex {

565+ table: "users".to_string(),

566+ name: "users_name_idx".to_string(),

567+ columns: vec!["first_name".to_string(), "last_name".to_string()],

568+ });

569+ let uppercase_name = Migration::new("uppercase_name", None).with_action(AlterColumn {

570+ table: "users".to_string(),

571+ column: "last_name".to_string(),

572+ up: Some("UPPER(last_name)".to_string()),

573+ down: Some("LOWER(last_name)".to_string()),

574+ changes: ColumnChanges {

575+ data_type: None,

576+ nullable: None,

577+ name: None,

578+ default: None,

579+ },

580+ });

581+

582+ let first_migrations = vec![create_users_table.clone()];

583+ let second_migrations = vec![create_users_table.clone(), uppercase_name.clone()];

584+

585+ // Run first migration, should automatically finish

586+ reshape.migrate(first_migrations.clone()).unwrap();

587+ assert!(matches!(reshape.state.status, Status::Idle));

588+ assert_eq!(

589+ Some(&create_users_table.name),

590+ reshape.state.current_migration.as_ref()

591+ );

592+

593+ // Run second migration

594+ reshape.migrate(second_migrations.clone()).unwrap();

595+ db.simple_query(&reshape::schema_query_for_migration(

596+ &second_migrations.last().unwrap().name,

597+ ))

598+ .unwrap();

599+

600+ // Complete the second migration which should replace the existing column

601+ // with the temporary one

602+ reshape.complete_migration().unwrap();

603+

604+ // Make sure index still exists

605+ let result: i64 = db

606+ .query(

607+ "

608+ SELECT COUNT(*)

609+ FROM pg_catalog.pg_index

610+ JOIN pg_catalog.pg_class ON pg_index.indexrelid = pg_class.oid

611+ WHERE pg_class.relname = 'users_name_idx'

612+ ",

613+ &[],

614+ )

615+ .unwrap()

616+ .first()

617+ .map(|row| row.get(0))

618+ .unwrap();

619+ assert_eq!(1, result, "expected index to still exist");

620+

621+ common::assert_cleaned_up(&mut db);

622+}

tests/create_table.rs+14−14

@@ -176,20 +176,20 @@ fn create_table_with_foreign_keys() {

176 let foreign_key_columns: Vec<(String, String, String)> = db

177 .query(

178 "

179− SELECT

179+ SELECT

180− kcu.column_name,

180+ kcu.column_name,

181− ccu.table_name AS foreign_table_name,

181+ ccu.table_name AS foreign_table_name,

182− ccu.column_name AS foreign_column_name

182+ ccu.column_name AS foreign_column_name

183− FROM

183+ FROM

184− information_schema.table_constraints AS tc

184+ information_schema.table_constraints AS tc

185− JOIN information_schema.key_column_usage AS kcu

185+ JOIN information_schema.key_column_usage AS kcu

186− ON tc.constraint_name = kcu.constraint_name

186+ ON tc.constraint_name = kcu.constraint_name

187− AND tc.table_schema = kcu.table_schema

187+ AND tc.table_schema = kcu.table_schema

188− JOIN information_schema.constraint_column_usage AS ccu

188+ JOIN information_schema.constraint_column_usage AS ccu

189− ON ccu.constraint_name = tc.constraint_name

189+ ON ccu.constraint_name = tc.constraint_name

190− AND ccu.table_schema = tc.table_schema

190+ AND ccu.table_schema = tc.table_schema

191− WHERE tc.constraint_type = 'FOREIGN KEY' AND tc.table_name='items';

191+ WHERE tc.constraint_type = 'FOREIGN KEY' AND tc.table_name='items';

192− ",

192+ ",

193 &[],

194 )

195 .unwrap()