Additional fixes to migration tasks for MySQL.

hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/JdbcUtils.java

@@ -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
 	 */

hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/AddColumnTask.java

@@ -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;

hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/AddForeignKeyTask.java

@@ -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:

hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/DropColumnTask.java

@@ -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);

hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/DropForeignKeyTask.java

@@ -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 foreign key " + theConstraintName);
-				sqls.add("alter table " + theTableName + " drop index " + theConstraintName);
 				break;
 			case MARIADB_10_1:
 			case POSTGRES_9_4:

hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/DropIndexTask.java

@@ -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

hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/ModifyColumnTask.java

@@ -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) {

hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/RenameColumnTask.java

@@ -20,6 +20,7 @@ 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 org.apache.commons.lang3.Validate;
@@ -30,6 +31,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 {
@@ -82,6 +84,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)) {
+					// Some DBs such as MYSQL require that foreign keys depending on the column be 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 +140,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:

hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/RenameIndexTask.java

@@ -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);

hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/tasks/HapiFhirJpaMigrationTasks.java

@@ -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");
 
 	}

hapi-fhir-jpaserver-migrate/src/test/java/ca/uhn/fhir/jpa/migrate/taskdef/DropColumnTest.java

@@ -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,56 @@ 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 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"));
+
+		DropColumnTask task = new DropColumnTask("1",  "1");
+		task.setTableName("CHILD");
+		task.setColumnName("PARENTREF");
+		getMigrator().addTask(task);
+
+		getMigrator().migrate();
+
+		assertThat(JdbcUtils.getColumnNames(getConnectionProperties(), "CHILD"), containsInAnyOrder("PID"));
+
+		assertThat(JdbcUtils.getForeignKeys(getConnectionProperties(), "PARENT", "CHILD"), empty());
+
+		// Do it again to make sure there is no error
+		getMigrator().migrate();
+		getMigrator().migrate();
+
+	}
+
+
+	/*
+			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"));
+
+		DropForeignKeyTask task = new DropForeignKeyTask("1", "1");
+		task.setTableName("CHILD");
+		task.setParentTableName("PARENT");
+		task.setConstraintName("FK_MOM");
+		getMigrator().addTask(task);
+
+		getMigrator().migrate();
+
+		assertThat(JdbcUtils.getForeignKeys(getConnectionProperties(), "PARENT", "CHILD"), empty());
+
+		// Make sure additional calls don't crash
+		getMigrator().migrate();
+		getMigrator().migrate();
+
+	 */
 }