From d304a8fb6c41d674f220db77012b0575bfce0b5e Mon Sep 17 00:00:00 2001 From: James Agnew Date: Fri, 19 Jul 2019 12:13:35 -0400 Subject: [PATCH] Add a safety check to rename column task --- .../jpa/migrate/taskdef/RenameColumnTask.java | 13 +++++++++++ .../migrate/taskdef/RenameColumnTaskTest.java | 22 +++++++++++++++++++ 2 files changed, 35 insertions(+) diff --git a/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/RenameColumnTask.java b/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/RenameColumnTask.java index 7f4be59a17b..596273c0ef0 100644 --- a/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/RenameColumnTask.java +++ b/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/RenameColumnTask.java @@ -24,6 +24,8 @@ import ca.uhn.fhir.jpa.migrate.JdbcUtils; import org.apache.commons.lang3.Validate; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import org.springframework.jdbc.core.ColumnMapRowMapper; +import org.springframework.jdbc.core.JdbcTemplate; import java.sql.SQLException; import java.util.Set; @@ -57,6 +59,17 @@ public class RenameColumnTask extends BaseTableTask { boolean haveNewName = columnNames.contains(myNewName.toUpperCase()); if (haveOldName && haveNewName) { if (myDeleteTargetColumnFirstIfBothExist) { + + Integer rowsWithData = getConnectionProperties().getTxTemplate().execute(t -> { + String sql = "SELECT * FROM " + getTableName() + " WHERE " + myNewName + " IS NOT NULL"; + JdbcTemplate jdbcTemplate = getConnectionProperties().newJdbcTemplate(); + jdbcTemplate.setMaxRows(1); + return jdbcTemplate.query(sql, new ColumnMapRowMapper()).size(); + }); + if (rowsWithData > 0) { + throw new SQLException("Can not rename " + getTableName() + "." + myOldName + " to " + myNewName + " because both columns exist and data exists in " + myNewName); + } + ourLog.info("Table {} has columns {} and {} - Going to drop {} before renaming", getTableName(), myOldName, myNewName, myNewName); String sql = DropColumnTask.createSql(getTableName(), myNewName); executeSql(getTableName(), sql); diff --git a/hapi-fhir-jpaserver-migrate/src/test/java/ca/uhn/fhir/jpa/migrate/taskdef/RenameColumnTaskTest.java b/hapi-fhir-jpaserver-migrate/src/test/java/ca/uhn/fhir/jpa/migrate/taskdef/RenameColumnTaskTest.java index 14404326ee2..b9351563513 100644 --- a/hapi-fhir-jpaserver-migrate/src/test/java/ca/uhn/fhir/jpa/migrate/taskdef/RenameColumnTaskTest.java +++ b/hapi-fhir-jpaserver-migrate/src/test/java/ca/uhn/fhir/jpa/migrate/taskdef/RenameColumnTaskTest.java @@ -47,6 +47,28 @@ public class RenameColumnTaskTest extends BaseTest { assertThat(columnNames.toString(), columnNames, containsInAnyOrder("PID", "TEXTCOL")); } + @Test + public void testBothExistDeleteTargetFirstDataExistsInSourceAndTarget() throws SQLException { + executeSql("create table SOMETABLE (PID bigint not null, TEXTCOL varchar(255), myTextCol varchar(255))"); + executeSql("INSERT INTO SOMETABLE (PID, TEXTCOL, myTextCol) VALUES (123, 'AAA', 'BBB')"); + + RenameColumnTask task = new RenameColumnTask(); + task.setTableName("SOMETABLE"); + task.setDescription("Drop an index"); + task.setOldName("myTextCol"); + task.setNewName("TEXTCOL"); + task.setDeleteTargetColumnFirstIfBothExist(true); + getMigrator().addTask(task); + + try { + getMigrator().migrate(); + fail(); + } catch (InternalErrorException e) { + assertEquals("Failure executing task \"Drop an index\", aborting! Cause: java.sql.SQLException: Can not rename SOMETABLE.myTextCol to TEXTCOL because both columns exist and data exists in TEXTCOL", e.getMessage()); + } + + } + @Test public void testColumnDoesntAlreadyExist() throws SQLException { executeSql("create table SOMETABLE (PID bigint not null, myTextCol varchar(255))");