Merge pull request #1928 from jamesagnew/im_20200615_mysql_migration_syntax

Im 20200615 mysql migration syntax
This commit is contained in:
IanMMarshall 2020-06-17 14:21:17 -04:00 committed by GitHub
commit abf0a0053d
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
13 changed files with 255 additions and 13 deletions

View File

@ -317,6 +317,47 @@ public class JdbcUtils {
}
}
/**
* Retrieve names of foreign keys that reference a specified foreign key column.
*/
public static Set<String> getForeignKeysForColumn(DriverTypeEnum.ConnectionProperties theConnectionProperties, String theForeignKeyColumn, String theForeignTable) throws SQLException {
DataSource dataSource = Objects.requireNonNull(theConnectionProperties.getDataSource());
try (Connection connection = dataSource.getConnection()) {
return theConnectionProperties.getTxTemplate().execute(t -> {
DatabaseMetaData metadata;
try {
metadata = connection.getMetaData();
String catalog = connection.getCatalog();
String schema = connection.getSchema();
List<String> parentTables = new ArrayList<>();
parentTables.addAll(JdbcUtils.getTableNames(theConnectionProperties));
String foreignTable = massageIdentifier(metadata, theForeignTable);
Set<String> fkNames = new HashSet<>();
for (String nextParentTable : parentTables) {
ResultSet indexes = metadata.getCrossReference(catalog, schema, nextParentTable, catalog, schema, foreignTable);
while (indexes.next()) {
if (theForeignKeyColumn.equals(indexes.getString("FKCOLUMN_NAME"))) {
String fkName = indexes.getString("FK_NAME");
fkName = toUpperCase(fkName, Locale.US);
fkNames.add(fkName);
}
}
}
return fkNames;
} catch (SQLException e) {
throw new InternalErrorException(e);
}
});
}
}
/**
* Retrieve all index names
*/

View File

@ -53,9 +53,12 @@ public class AddColumnTask extends BaseTableColumnTypeTask {
String sql;
switch (getDriverType()) {
case MYSQL_5_7:
// Quote the column name as "SYSTEM" is a reserved word in MySQL
sql = "alter table " + getTableName() + " add column `" + getColumnName() + "` " + typeStatement;
break;
case DERBY_EMBEDDED:
case MARIADB_10_1:
case MYSQL_5_7:
case POSTGRES_9_4:
sql = "alter table " + getTableName() + " add column " + getColumnName() + " " + typeStatement;
break;

View File

@ -78,7 +78,8 @@ public class AddForeignKeyTask extends BaseTableColumnTask {
switch (getDriverType()) {
case MARIADB_10_1:
case MYSQL_5_7:
sql = "alter table " + getTableName() + " add constraint " + myConstraintName + " foreign key (" + getColumnName() + ") references " + myForeignTableName + " (" + myForeignColumnName + ")";
// Quote the column names as "SYSTEM" is a reserved word in MySQL
sql = "alter table " + getTableName() + " add constraint " + myConstraintName + " foreign key (`" + getColumnName() + "`) references " + myForeignTableName + " (`" + myForeignColumnName + "`)";
break;
case POSTGRES_9_4:
case DERBY_EMBEDDED:

View File

@ -20,12 +20,14 @@ package ca.uhn.fhir.jpa.migrate.taskdef;
* #L%
*/
import ca.uhn.fhir.jpa.migrate.DriverTypeEnum;
import ca.uhn.fhir.jpa.migrate.JdbcUtils;
import org.intellij.lang.annotations.Language;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import java.sql.SQLException;
import java.util.List;
import java.util.Set;
public class DropColumnTask extends BaseTableColumnTask {
@ -50,6 +52,20 @@ public class DropColumnTask extends BaseTableColumnTask {
return;
}
if(getDriverType().equals(DriverTypeEnum.MYSQL_5_7)) {
// Some DBs such as MYSQL require that foreign keys depending on the column be dropped before the column itself is dropped.
logInfo(ourLog, "Dropping any foreign keys on table {} depending on column {}", getTableName(), getColumnName());
Set<String> foreignKeys = JdbcUtils.getForeignKeysForColumn(getConnectionProperties(), getColumnName(), getTableName());
if(foreignKeys != null) {
for (String foreignKey:foreignKeys) {
List<String> dropFkSqls = DropForeignKeyTask.generateSql(getTableName(), foreignKey, getDriverType());
for(String dropFkSql : dropFkSqls) {
executeSql(getTableName(), dropFkSql);
}
}
}
}
String tableName = getTableName();
String columnName = getColumnName();
String sql = createSql(tableName, columnName);

View File

@ -102,8 +102,7 @@ public class DropForeignKeyTask extends BaseTableTask {
switch (theDriverType) {
case MYSQL_5_7:
// Lousy MYQL....
sqls.add("alter table " + theTableName + " drop constraint " + theConstraintName);
sqls.add("alter table " + theTableName + " drop index " + theConstraintName);
sqls.add("alter table " + theTableName + " drop foreign key " + theConstraintName);
break;
case MARIADB_10_1:
case POSTGRES_9_4:

View File

@ -107,6 +107,9 @@ public class DropIndexTask extends BaseTableTask {
// Drop constraint
switch (theDriverType) {
case MYSQL_5_7:
// Need to quote the index name as the word "PRIMARY" is reserved in MySQL
sql.add("alter table " + theTableName + " drop index `" + theIndexName + "`");
break;
case MARIADB_10_1:
sql.add("alter table " + theTableName + " drop index " + theIndexName);
break;
@ -114,16 +117,14 @@ public class DropIndexTask extends BaseTableTask {
sql.add("drop index " + theIndexName);
break;
case DERBY_EMBEDDED:
case ORACLE_12C:
case MSSQL_2012:
sql.add("alter table " + theTableName + " drop constraint " + theIndexName);
break;
case POSTGRES_9_4:
sql.add("alter table " + theTableName + " drop constraint if exists " + theIndexName + " cascade");
sql.add("drop index if exists " + theIndexName + " cascade");
break;
case ORACLE_12C:
case MSSQL_2012:
sql.add("alter table " + theTableName + " drop constraint " + theIndexName);
break;
}
} else {
// Drop index

View File

@ -99,7 +99,8 @@ public class ModifyColumnTask extends BaseTableColumnTypeTask {
break;
case MARIADB_10_1:
case MYSQL_5_7:
sql = "alter table " + getTableName() + " modify column " + getColumnName() + " " + type + notNull;
// Quote the column name as "SYSTEM" is a reserved word in MySQL
sql = "alter table " + getTableName() + " modify column `" + getColumnName() + "` " + type + notNull;
break;
case POSTGRES_9_4:
if (!alreadyOfCorrectType) {

View File

@ -20,8 +20,10 @@ package ca.uhn.fhir.jpa.migrate.taskdef;
* #L%
*/
import ca.uhn.fhir.jpa.migrate.DriverTypeEnum;
import ca.uhn.fhir.jpa.migrate.JdbcUtils;
import ca.uhn.fhir.rest.server.exceptions.InternalErrorException;
import com.google.common.annotations.VisibleForTesting;
import org.apache.commons.lang3.Validate;
import org.apache.commons.lang3.builder.HashCodeBuilder;
import org.slf4j.Logger;
@ -30,6 +32,7 @@ import org.springframework.jdbc.core.ColumnMapRowMapper;
import org.springframework.jdbc.core.JdbcTemplate;
import java.sql.SQLException;
import java.util.List;
import java.util.Set;
public class RenameColumnTask extends BaseTableTask {
@ -40,6 +43,8 @@ public class RenameColumnTask extends BaseTableTask {
private boolean myIsOkayIfNeitherColumnExists;
private boolean myDeleteTargetColumnFirstIfBothExist;
private boolean mySimulateMySQLForTest = false;
public RenameColumnTask(String theProductVersion, String theSchemaVersion) {
super(theProductVersion, theSchemaVersion);
}
@ -82,6 +87,20 @@ public class RenameColumnTask extends BaseTableTask {
throw new SQLException("Can not rename " + getTableName() + "." + myOldName + " to " + myNewName + " because both columns exist and data exists in " + myNewName);
}
if (getDriverType().equals(DriverTypeEnum.MYSQL_5_7) || mySimulateMySQLForTest) {
// Some DBs such as MYSQL require that foreign keys depending on the column be explicitly dropped before the column itself is dropped.
logInfo(ourLog, "Table {} has columns {} and {} - Going to drop any foreign keys depending on column {} before renaming", getTableName(), myOldName, myNewName, myNewName);
Set<String> foreignKeys = JdbcUtils.getForeignKeysForColumn(getConnectionProperties(), myNewName, getTableName());
if(foreignKeys != null) {
for (String foreignKey:foreignKeys) {
List<String> dropFkSqls = DropForeignKeyTask.generateSql(getTableName(), foreignKey, getDriverType());
for(String dropFkSql : dropFkSqls) {
executeSql(getTableName(), dropFkSql);
}
}
}
}
logInfo(ourLog, "Table {} has columns {} and {} - Going to drop {} before renaming", getTableName(), myOldName, myNewName, myNewName);
String sql = DropColumnTask.createSql(getTableName(), myNewName);
executeSql(getTableName(), sql);
@ -124,7 +143,8 @@ public class RenameColumnTask extends BaseTableTask {
sql = "ALTER TABLE " + getTableName() + " CHANGE COLUMN " + myOldName + " TO " + myNewName;
break;
case MYSQL_5_7:
sql = "ALTER TABLE " + getTableName() + " CHANGE COLUMN " + myOldName + " " + myNewName + " " + theExistingType + " " + theExistingNotNull;
// Quote the column names as "SYSTEM" is a reserved word in MySQL
sql = "ALTER TABLE " + getTableName() + " CHANGE COLUMN `" + myOldName + "` `" + myNewName + "` " + theExistingType + " " + theExistingNotNull;
break;
case POSTGRES_9_4:
case ORACLE_12C:
@ -156,4 +176,9 @@ public class RenameColumnTask extends BaseTableTask {
theBuilder.append(myOldName);
theBuilder.append(myNewName);
}
@VisibleForTesting
void setSimulateMySQLForTest(boolean theSimulateMySQLForTest) {
mySimulateMySQLForTest = theSimulateMySQLForTest;
}
}

View File

@ -110,6 +110,9 @@ public class RenameIndexTask extends BaseTableTask {
// Drop constraint
switch (theDriverType) {
case MYSQL_5_7:
// Quote the index names as "PRIMARY" is a reserved word in MySQL
sql.add("rename index `" + theOldIndexName + "` to `" + theNewIndexName + "`");
break;
case MARIADB_10_1:
case DERBY_EMBEDDED:
sql.add("rename index " + theOldIndexName + " to " + theNewIndexName);

View File

@ -118,7 +118,7 @@ public class HapiFhirJpaMigrationTasks extends BaseMigrationTasks<VersionEnum> {
pkgVerRes.addColumn("RES_SIZE_BYTES").nonNullable().type(ColumnTypeEnum.LONG);
pkgVerRes.addColumn("UPDATED_TIME").nonNullable().type(ColumnTypeEnum.DATE_TIMESTAMP);
pkgVerRes.addForeignKey("20200610.11", "FK_NPM_PACKVERRES_PACKVER").toColumn("PACKVER_PID").references("NPM_PACKAGE_VER", "PID");
pkgVerRes.addForeignKey("20200610.12", "FK_NPM_PKVR_RESID").toColumn("BINARY_RES_ID").references("HFJ_RESOURCE", "PID");
pkgVerRes.addForeignKey("20200610.12", "FK_NPM_PKVR_RESID").toColumn("BINARY_RES_ID").references("HFJ_RESOURCE", "RES_ID");
pkgVerRes.addIndex("20200610.13", "IDX_PACKVERRES_URL").unique(false).withColumns("CANONICAL_URL");
}

View File

@ -7,6 +7,8 @@ import java.sql.SQLException;
import java.util.function.Supplier;
import static org.hamcrest.Matchers.containsInAnyOrder;
import static org.hamcrest.Matchers.empty;
import static org.hamcrest.Matchers.hasSize;
import static org.junit.Assert.assertThat;
public class DropColumnTest extends BaseTest {
@ -34,4 +36,39 @@ public class DropColumnTest extends BaseTest {
}
@Test
public void testDropForeignKeyColumn() throws SQLException {
executeSql("create table PARENT (PID bigint not null, TEXTCOL varchar(255), primary key (PID))");
executeSql("create table SIBLING (PID bigint not null, TEXTCOL varchar(255), primary key (PID))");
executeSql("create table CHILD (PID bigint not null, PARENTREF bigint, SIBLINGREF bigint)");
executeSql("alter table CHILD add constraint FK_MOM foreign key (PARENTREF) references PARENT(PID)");
executeSql("alter table CHILD add constraint FK_BROTHER foreign key (SIBLINGREF) references SIBLING(PID)");
assertThat(JdbcUtils.getForeignKeys(getConnectionProperties(), "PARENT", "CHILD"), hasSize(1));
assertThat(JdbcUtils.getForeignKeys(getConnectionProperties(), "SIBLING", "CHILD"), hasSize(1));
assertThat(JdbcUtils.getForeignKeysForColumn(getConnectionProperties(), "PARENTREF", "CHILD"), containsInAnyOrder("FK_MOM"));
assertThat(JdbcUtils.getForeignKeysForColumn(getConnectionProperties(), "SIBLINGREF", "CHILD"), containsInAnyOrder("FK_BROTHER"));
DropColumnTask task = new DropColumnTask("1", "1");
task.setTableName("CHILD");
task.setColumnName("PARENTREF");
getMigrator().addTask(task);
getMigrator().migrate();
assertThat(JdbcUtils.getColumnNames(getConnectionProperties(), "CHILD"), containsInAnyOrder("PID", "SIBLINGREF"));
assertThat(JdbcUtils.getForeignKeys(getConnectionProperties(), "PARENT", "CHILD"), empty());
assertThat(JdbcUtils.getForeignKeys(getConnectionProperties(), "SIBLING", "CHILD"), hasSize(1));
assertThat(JdbcUtils.getForeignKeysForColumn(getConnectionProperties(), "PARENTREF", "CHILD"), empty());
assertThat(JdbcUtils.getForeignKeysForColumn(getConnectionProperties(), "SIBLINGREF", "CHILD"), containsInAnyOrder("FK_BROTHER"));
// Do it again to make sure there is no error
getMigrator().migrate();
getMigrator().migrate();
}
}

View File

@ -8,7 +8,7 @@ public class RenameColumnTaskDbSpecificTest {
@Test
public void testBuildSqlStatementForMySql() {
assertEquals("ALTER TABLE SOMETABLE CHANGE COLUMN myTextCol TEXTCOL integer null", createRenameColumnSql(DriverTypeEnum.MYSQL_5_7));
assertEquals("ALTER TABLE SOMETABLE CHANGE COLUMN `myTextCol` `TEXTCOL` integer null", createRenameColumnSql(DriverTypeEnum.MYSQL_5_7));
}
private String createRenameColumnSql(DriverTypeEnum theDriverTypeEnum) {

View File

@ -9,6 +9,8 @@ import java.util.Set;
import java.util.function.Supplier;
import static org.hamcrest.Matchers.containsInAnyOrder;
import static org.hamcrest.Matchers.empty;
import static org.hamcrest.Matchers.hasSize;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertThat;
import static org.junit.Assert.fail;
@ -35,6 +37,41 @@ public class RenameColumnTaskTest extends BaseTest {
assertThat(JdbcUtils.getColumnNames(getConnectionProperties(), "SOMETABLE"), containsInAnyOrder("PID", "TEXTCOL"));
}
@Test
public void testForeignKeyColumnAlreadyExists_MySql() throws SQLException {
testForeignKeyColumnAlreadyExists(true);
}
private void testForeignKeyColumnAlreadyExists(boolean isMySql) throws SQLException {
executeSql("create table PARENT (PID bigint not null, TEXTCOL varchar(255), primary key (PID))");
executeSql("create table CHILD (PID bigint not null, PARENTREF bigint)");
executeSql("alter table CHILD add constraint FK_MOM foreign key (PARENTREF) references PARENT(PID)");
assertThat(JdbcUtils.getForeignKeys(getConnectionProperties(), "PARENT", "CHILD"), hasSize(1));
assertThat(JdbcUtils.getForeignKeysForColumn(getConnectionProperties(), "PARENTREF", "CHILD"), containsInAnyOrder("FK_MOM"));
RenameColumnTask task = new RenameColumnTask("1", "1");
task.setTableName("CHILD");
task.setOldName("myParentRef");
task.setNewName("PARENTREF");
task.setSimulateMySQLForTest(isMySql);
getMigrator().addTask(task);
getMigrator().migrate();
assertThat(JdbcUtils.getForeignKeys(getConnectionProperties(), "PARENT", "CHILD"), hasSize(1));
assertThat(JdbcUtils.getColumnNames(getConnectionProperties(), "CHILD"), containsInAnyOrder("PID", "PARENTREF"));
assertThat(JdbcUtils.getForeignKeysForColumn(getConnectionProperties(), "PARENTREF", "CHILD"), containsInAnyOrder("FK_MOM"));
}
@Test
public void testForeignKeyColumnAlreadyExists_OtherDB() throws SQLException {
testForeignKeyColumnAlreadyExists(false);
}
@Test
public void testBothExistDeleteTargetFirst() throws SQLException {
executeSql("create table SOMETABLE (PID bigint not null, TEXTCOL varchar(255), myTextCol varchar(255))");
@ -54,7 +91,49 @@ public class RenameColumnTaskTest extends BaseTest {
}
@Test
public void testBothExistDeleteTargetFirstDataExistsInSourceAndTarget() throws SQLException {
public void testForeignKeyColumnBothExistDeleteTargetFirst_MySql() throws SQLException {
testForeignKeyColumnBothExistDeleteTargetFirst(true);
}
private void testForeignKeyColumnBothExistDeleteTargetFirst(boolean isMySql) throws SQLException {
executeSql("create table PARENT (PARENTID bigint not null, TEXTCOL varchar(255), primary key (PARENTID))");
executeSql("create table RELATION (RELATIONID bigint not null, TEXTCOL varchar(255), primary key (RELATIONID))");
executeSql("create table CHILD (PID bigint not null, PARENTREF bigint, NOKREF bigint)");
executeSql("alter table CHILD add constraint FK_MOM foreign key (PARENTREF) references PARENT(PARENTID)");
executeSql("alter table CHILD add constraint FK_NOK foreign key (NOKREF) references RELATION(RELATIONID)");
assertThat(JdbcUtils.getForeignKeys(getConnectionProperties(), "PARENT", "CHILD"), hasSize(1));
assertThat(JdbcUtils.getForeignKeys(getConnectionProperties(), "RELATION", "CHILD"), hasSize(1));
assertThat(JdbcUtils.getForeignKeysForColumn(getConnectionProperties(), "PARENTREF", "CHILD"), containsInAnyOrder("FK_MOM"));
assertThat(JdbcUtils.getForeignKeysForColumn(getConnectionProperties(), "NOKREF", "CHILD"), containsInAnyOrder("FK_NOK"));
RenameColumnTask task = new RenameColumnTask("1", "1");
task.setTableName("CHILD");
task.setOldName("PARENTREF");
task.setNewName("NOKREF");
task.setDeleteTargetColumnFirstIfBothExist(true);
task.setSimulateMySQLForTest(isMySql);
getMigrator().addTask(task);
getMigrator().migrate();
assertThat(JdbcUtils.getForeignKeys(getConnectionProperties(), "RELATION", "CHILD"), empty());
assertThat(JdbcUtils.getForeignKeys(getConnectionProperties(), "PARENT", "CHILD"), hasSize(1));
assertThat(JdbcUtils.getColumnNames(getConnectionProperties(), "CHILD"), containsInAnyOrder("PID", "NOKREF"));
assertThat(JdbcUtils.getForeignKeysForColumn(getConnectionProperties(), "NOKREF", "CHILD"), containsInAnyOrder("FK_MOM"));
}
@Test
public void testForeignKeyColumnBothExistDeleteTargetFirst_OtherDB() throws SQLException {
testForeignKeyColumnBothExistDeleteTargetFirst(false);
}
@Test
public void testBothExistDeleteTargetFirstDataExistsInSourceAndTarget() {
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')");
@ -91,6 +170,42 @@ public class RenameColumnTaskTest extends BaseTest {
assertThat(JdbcUtils.getColumnNames(getConnectionProperties(), "SOMETABLE"), containsInAnyOrder("PID", "TEXTCOL"));
}
@Test
public void testForeignKeyColumnDoesntAlreadyExist_MySql() throws SQLException {
testForeignKeyColumnDoesntAlreadyExist(true);
}
private void testForeignKeyColumnDoesntAlreadyExist(boolean isMySql) throws SQLException {
executeSql("create table PARENT (PARENTID bigint not null, TEXTCOL varchar(255), primary key (PARENTID))");
executeSql("create table CHILD (PID bigint not null, PARENTREF bigint)");
executeSql("alter table CHILD add constraint FK_MOM foreign key (PARENTREF) references PARENT(PARENTID)");
assertThat(JdbcUtils.getForeignKeys(getConnectionProperties(), "PARENT", "CHILD"), hasSize(1));
assertThat(JdbcUtils.getForeignKeysForColumn(getConnectionProperties(), "PARENTREF", "CHILD"), containsInAnyOrder("FK_MOM"));
RenameColumnTask task = new RenameColumnTask("1", "1");
task.setTableName("CHILD");
task.setOldName("PARENTREF");
task.setNewName("MOMREF");
task.setSimulateMySQLForTest(isMySql);
getMigrator().addTask(task);
getMigrator().migrate();
assertThat(JdbcUtils.getForeignKeys(getConnectionProperties(), "PARENT", "CHILD"), hasSize(1));
assertThat(JdbcUtils.getColumnNames(getConnectionProperties(), "CHILD"), containsInAnyOrder("PID", "MOMREF"));
assertThat(JdbcUtils.getForeignKeysForColumn(getConnectionProperties(), "MOMREF", "CHILD"), containsInAnyOrder("FK_MOM"));
}
@Test
public void testForeignKeyColumnDoesntAlreadyExist_OtherDB() throws SQLException {
testForeignKeyColumnDoesntAlreadyExist(false);
}
@Test
public void testNeitherColumnExists() {
executeSql("create table SOMETABLE (PID bigint not null)");