1680906 · reshape · git.iko.soy

git.iko.soy/reshape

Add support for unique and non-B-tree indices in alter_column

commit1680906e160e5b468db3f05c88db0c6bd89c8c27 authorfabianlindfors <fabian@flapplabs.se> dateSun 16 Jan 2022, 00:36 (5 years ago) parent9cac252

4 files changed+197−18

compare with main

unifiedsplit

src/migrations/alter_column.rs+12−9 src/migrations/common.rs+20−3 tests/add_index.rs+6−6 tests/alter_column.rs+159

src/migrations/alter_column.rs+12−9

@@ -157,8 +157,8 @@ impl Action for AlterColumn {

157157

158158 // Duplicate any indices to the temporary column

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

160− for (index_name, index_oid) in indices {

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

160+ for index in indices {

161+ let index_columns: Vec<String> = common::get_index_columns(db, &index.name)?

162162 .into_iter()

163163 .map(|idx_column| {

164164 // Replace column with temporary column for new index

@@ -169,15 +169,18 @@ impl Action for AlterColumn {

169169 }

170170 })

171171 .collect();

172− let temp_index_name = self.temp_index_name(ctx, index_oid);

172+ let temp_index_name = self.temp_index_name(ctx, index.oid);

173+

174+ let unique_def = if index.unique { "UNIQUE" } else { "" };

173175

174176 db.query(&format!(

175177 r#"

176− CREATE INDEX CONCURRENTLY IF NOT EXISTS "{new_index_name}" ON "{table}" ({columns})

178+ CREATE {unique_def} INDEX CONCURRENTLY IF NOT EXISTS "{new_index_name}" ON "{table}" USING {index_type} ({columns})

177179 "#,

178180 new_index_name = temp_index_name,

179181 table = table.real_name,

180182 columns = index_columns.join(", "),

183+ index_type = index.index_type,

181184 ))

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

183186 }

@@ -280,7 +283,7 @@ impl Action for AlterColumn {

280283

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

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

283− for (current_index_name, index_oid) in indices {

286+ for current_index in indices {

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

285288 // 1. Add a prefix to the existing index

286289 // 2. Rename temporary index to its final name

@@ -288,7 +291,7 @@ impl Action for AlterColumn {

288291

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

290293 let prefix = "__reshape_old";

291− let target_index_name = current_index_name.trim_start_matches(prefix);

294+ let target_index_name = current_index.name.trim_start_matches(prefix);

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

293296 db.query(&format!(

294297 r#"

@@ -300,7 +303,7 @@ impl Action for AlterColumn {

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

301304

302305 // Rename temporary index to real name

303− let temp_index_name = self.temp_index_name(ctx, index_oid);

306+ let temp_index_name = self.temp_index_name(ctx, current_index.oid);

304307 db.query(&format!(

305308 r#"

306309 ALTER INDEX IF EXISTS "{temp_index_name}" RENAME TO "{target_index_name}"

@@ -388,8 +391,8 @@ impl Action for AlterColumn {

388391 // Safely remove any indices created for the temporary column

389392 let temp_column_name = self.temporary_column_name(ctx);

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

391− for (_, index_oid) in indices {

392− let temp_index_name = self.temp_index_name(ctx, index_oid);

394+ for index in indices {

395+ let temp_index_name = self.temp_index_name(ctx, index.oid);

393396 db.query(&format!(

394397 r#"

395398 DROP INDEX CONCURRENTLY IF EXISTS "{index_name}"

src/migrations/common.rs+20−3

@@ -173,18 +173,30 @@ fn get_primary_key_columns_for_table(

173173 Ok(primary_key_columns)

174174}

175175

176+pub struct Index {

177+ pub name: String,

178+ pub oid: u32,

179+ pub unique: bool,

180+ pub index_type: String,

181+}

182+

176183pub fn get_indices_for_column(

177184 db: &mut dyn Conn,

178185 table: &str,

179186 column: &str,

180−) -> anyhow::Result<Vec<(String, u32)>> {

187+) -> anyhow::Result<Vec<Index>> {

181188 let indices = db

182189 .query(&format!(

183190 "

184− SELECT i.relname AS index_name, i.oid AS index_oid

191+ SELECT

192+ i.relname AS name,

193+ i.oid AS oid,

194+ ix.indisunique AS unique,

195+ am.amname AS type

185196 FROM pg_index ix

186197 JOIN pg_class t ON t.oid = ix.indrelid

187198 JOIN pg_class i ON i.oid = ix.indexrelid

199+ JOIN pg_am am ON i.relam = am.oid

188200 JOIN pg_attribute a ON

189201 a.attrelid = t.oid AND

190202 a.attnum = ANY(ix.indkey)

@@ -196,7 +208,12 @@ pub fn get_indices_for_column(

196208 column = column,

197209 ))?

198210 .iter()

199− .map(|row| (row.get("index_name"), row.get("index_oid")))

211+ .map(|row| Index {

212+ name: row.get("name"),

213+ oid: row.get("oid"),

214+ unique: row.get("unique"),

215+ index_type: row.get("type"),

216+ })

200217 .collect();

201218

202219 Ok(indices)

tests/add_index.rs+6−6

@@ -126,11 +126,11 @@ fn add_index_unique() {

126126 let (is_ready, is_valid, is_unique): (bool, bool, bool) = db

127127 .query(

128128 "

129− SELECT pg_index.indisready, pg_index.indisvalid, pg_index.indisunique

130− FROM pg_catalog.pg_index

131− JOIN pg_catalog.pg_class ON pg_index.indexrelid = pg_class.oid

132− WHERE pg_class.relname = 'name_idx'

133− ",

129+ SELECT pg_index.indisready, pg_index.indisvalid, pg_index.indisunique

130+ FROM pg_catalog.pg_index

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

132+ WHERE pg_class.relname = 'name_idx'

133+ ",

134134 &[],

135135 )

136136 .unwrap()

@@ -191,7 +191,7 @@ fn add_index_with_type() {

191191 );

192192

193193 test.intermediate(|db, _| {

194− // Ensure index is valid, ready and unique

194+ // Ensure index is valid, ready and has the right type

195195 let (is_ready, is_valid, index_type): (bool, bool, String) = db

196196 .query(

197197 "

tests/alter_column.rs+159

@@ -474,3 +474,162 @@ fn alter_column_with_index() {

474474

475475 test.run();

476476}

477+

478+#[test]

479+fn alter_column_with_unique_index() {

480+ let mut test = Test::new("Alter column with unique index");

481+

482+ test.first_migration(

483+ r#"

484+ name = "create_user_table"

485+

486+ [[actions]]

487+ type = "create_table"

488+ name = "users"

489+ primary_key = ["id"]

490+

491+ [[actions.columns]]

492+ name = "id"

493+ type = "INTEGER"

494+

495+ [[actions.columns]]

496+ name = "name"

497+ type = "TEXT"

498+

499+ [[actions]]

500+ type = "add_index"

501+ table = "users"

502+

503+ [actions.index]

504+ name = "name_idx"

505+ columns = ["name"]

506+ unique = true

507+ "#,

508+ );

509+

510+ test.second_migration(

511+ r#"

512+ name = "uppercase_name"

513+

514+ [[actions]]

515+ type = "alter_column"

516+ table = "users"

517+ column = "name"

518+ up = "UPPER(name)"

519+ down = "LOWER(name)"

520+ "#,

521+ );

522+

523+ test.after_first(|db| {

524+ db.simple_query("INSERT INTO users (id, name) VALUES (1, 'Test')")

525+ .unwrap();

526+ });

527+

528+ test.intermediate(|old_db, new_db| {

529+ // Try inserting a value which duplicates the uppercase value of an existing row

530+ let result = new_db.simple_query("INSERT INTO users (id, name) VALUES (2, 'TEST')");

531+ assert!(

532+ result.is_err(),

533+ "expected duplicate insert to new schema to fail"

534+ );

535+

536+ // Try inserting a value which duplicates the lowercase value of an existing row

537+ new_db

538+ .simple_query("INSERT INTO users (id, name) VALUES (2, 'JOHN')")

539+ .unwrap();

540+ let result = old_db.simple_query("INSERT INTO users (id, name) VALUES (3, 'john')");

541+ assert!(

542+ result.is_err(),

543+ "expected duplicate insert to old schema to fail"

544+ );

545+ });

546+

547+ test.after_completion(|db| {

548+ // Make sure index still exists

549+ let is_unique: bool = db

550+ .query(

551+ "

552+ SELECT pg_index.indisunique

553+ FROM pg_catalog.pg_index

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

555+ WHERE pg_class.relname = 'name_idx'

556+ ",

557+ &[],

558+ )

559+ .unwrap()

560+ .first()

561+ .map(|row| row.get("indisunique"))

562+ .unwrap();

563+ assert!(is_unique, "expected index to still be unique");

564+ });

565+

566+ test.run();

567+}

568+

569+#[test]

570+fn alter_column_with_hash_index() {

571+ let mut test = Test::new("Alter column with custom index type");

572+

573+ test.first_migration(

574+ r#"

575+ name = "create_user_table"

576+

577+ [[actions]]

578+ type = "create_table"

579+ name = "users"

580+ primary_key = ["id"]

581+

582+ [[actions.columns]]

583+ name = "id"

584+ type = "INTEGER"

585+

586+ [[actions.columns]]

587+ name = "name"

588+ type = "TEXT"

589+

590+ [[actions]]

591+ type = "add_index"

592+ table = "users"

593+

594+ [actions.index]

595+ name = "name_idx"

596+ columns = ["name"]

597+ type = "hash"

598+ "#,

599+ );

600+

601+ test.second_migration(

602+ r#"

603+ name = "uppercase_name"

604+

605+ [[actions]]

606+ type = "alter_column"

607+ table = "users"

608+ column = "name"

609+ up = "UPPER(name)"

610+ down = "LOWER(name)"

611+ "#,

612+ );

613+

614+ test.after_completion(|db| {

615+ // Make sure index still has type GIN

616+ let index_type: String = db

617+ .query(

618+ "

619+ SELECT pg_am.amname

620+ FROM pg_catalog.pg_index

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

622+ JOIN pg_catalog.pg_am ON pg_class.relam = pg_am.oid

623+ WHERE pg_class.relname = 'name_idx'

624+ ",

625+ &[],

626+ )

627+ .unwrap()

628+ .first()

629+ .map(|row| row.get("amname"))

630+ .unwrap();

631+ assert_eq!("hash", index_type);

632+ });

633+

634+ test.run();

635+}