Skip to content

Commit

Permalink
0001369: Alter generated by table upgrade that requires and rebuild o…
Browse files Browse the repository at this point in the history 
…f the table and includes the addition or removal of columns fails
  • Loading branch information
@chenson42
chenson42 committed Aug 6, 2013
1 parent 8530b50 commit d92aff2
Show file tree
Hide file tree
Showing 4 changed files with 96 additions and 56 deletions.
6 changes: 4 additions & 2 deletions symmetric-db/src/main/java/org/jumpmind/db/model/Database.java
View file
Expand Up @@ -549,13 +549,15 @@ public void resetTableIndexCache() {
/**
* {@inheritDoc}
*/
@SuppressWarnings("unchecked")
public Object clone() throws CloneNotSupportedException {
Database result = (Database) super.clone();
result.name = name;
result.idMethod = idMethod;
result.version = version;
result.tables = (ArrayList<Table>) tables.clone();
result.tables = new ArrayList<Table>(tables.size());
for (Table table : tables) {
result.tables.add((Table)table.clone());
}

return result;
}
Expand Down
48 changes: 19 additions & 29 deletions symmetric-db/src/main/java/org/jumpmind/db/model/Table.java
View file
Expand Up @@ -848,18 +848,31 @@ public int compare(ForeignKey obj1, ForeignKey obj2) {
}
}

@SuppressWarnings("unchecked")
@Override
public Object clone() throws CloneNotSupportedException {
Table result = (Table) super.clone();
result.catalog = catalog;
result.schema = schema;
result.name = name;
result.type = type;
result.primaryKeyConstraintName = primaryKeyConstraintName;
result.columns = (ArrayList<Column>) columns.clone();
result.foreignKeys = (ArrayList<ForeignKey>) foreignKeys.clone();
result.indices = (ArrayList<IIndex>) indices.clone();
result.columns = new ArrayList<Column>(columns.size());
for (Column col : columns) {
if (col != null) {
result.columns.add((Column) col.clone());
}
}
result.foreignKeys = new ArrayList<ForeignKey>(foreignKeys.size());
for (ForeignKey fk : foreignKeys) {
if (fk != null) {
result.foreignKeys.add((ForeignKey) fk.clone());
}
}
result.indices = new ArrayList<IIndex>(indices.size());
for (IIndex i : indices) {
if (i != null) {
result.indices.add((IIndex) i.clone());
}
}
return result;
}

Expand Down Expand Up @@ -1074,30 +1087,7 @@ public static Column[] orderColumns(String[] columnNames, Table table) {

public Table copy() {
try {
Table result = (Table) super.clone();
result.catalog = catalog;
result.schema = schema;
result.name = name;
result.type = type;
result.columns = new ArrayList<Column>(columns.size());
for (Column col : columns) {
if (col != null) {
result.columns.add((Column) col.clone());
}
}
result.foreignKeys = new ArrayList<ForeignKey>(foreignKeys.size());
for (ForeignKey fk : foreignKeys) {
if (fk != null) {
result.foreignKeys.add((ForeignKey) fk.clone());
}
}
result.indices = new ArrayList<IIndex>(indices.size());
for (IIndex i : indices) {
if (i != null) {
result.indices.add((IIndex) i.clone());
}
}
return result;
return (Table)this.clone();
} catch (CloneNotSupportedException ex) {
throw new RuntimeException(ex);
}
Expand Down
46 changes: 22 additions & 24 deletions symmetric-db/src/main/java/org/jumpmind/db/platform/AbstractDdlBuilder.java
View file
Expand Up @@ -603,13 +603,7 @@ protected void processTableStructureChanges(Database currentModel, Database desi

// We're using a copy of the current model so that the table structure
// changes can modify it
Database copyOfCurrentModel = null;

try {
copyOfCurrentModel = (Database) currentModel.clone();
} catch (CloneNotSupportedException ex) {
throw new DdlException(ex);
}
Database copyOfCurrentModel = copy(currentModel);

for (Iterator<Map.Entry<String, List<TableChange>>> tableChangeIt = changesPerTable
.entrySet().iterator(); tableChangeIt.hasNext();) {
Expand Down Expand Up @@ -718,7 +712,8 @@ protected void processTableStructureChanges(Database currentModel, Database desi

StringBuilder tableDdl = new StringBuilder();

Table sourceTable = currentModel.findTable(tableName, delimitedIdentifierModeOn);
Database originalModel = copy(currentModel);
Table sourceTable = originalModel.findTable(tableName, delimitedIdentifierModeOn);
Table targetTable = desiredModel.findTable(tableName, delimitedIdentifierModeOn);

// we're enforcing a full rebuild in case of the addition of a required
Expand Down Expand Up @@ -770,10 +765,9 @@ protected void processTableStructureChanges(Database currentModel, Database desi
Table realTargetTable = getRealTargetTableFor(desiredModel, sourceTable, targetTable);

if (canMigrateData) {
Table tempTable = getTemporaryTableFor(desiredModel, targetTable);

Table tempTable = getTemporaryTableFor(sourceTable);
dropTemporaryTable(tempTable, ddl);
createTemporaryTable(desiredModel, tempTable, ddl);
createTemporaryTable(tempTable, ddl);
writeCopyDataStatement(sourceTable, tempTable, ddl);
/*
* Note that we don't drop the indices here because the DROP
Expand All @@ -793,6 +787,15 @@ protected void processTableStructureChanges(Database currentModel, Database desi
ddl.append(tableDdl);
}
}

protected Database copy(Database currentModel) {
try {
return (Database) currentModel.clone();
} catch (CloneNotSupportedException ex) {
throw new DdlException(ex);
}
}


/**
* Allows database-specific implementations to handle changes in a database
Expand Down Expand Up @@ -862,23 +865,21 @@ protected boolean writeAlterColumnDataType(ColumnDataTypeChange change, StringBu
* database directly supports temporary tables. The default implementation
* simply appends an underscore to the table name and uses that as the table
* name.
*
* @param targetModel
* The target database
* @param targetTable
* The target table
*
* @return The temporary table
*/
protected Table getTemporaryTableFor(Database targetModel, Table targetTable) {
return getTemporaryTableFor(targetModel, targetTable, "_");
protected Table getTemporaryTableFor(Table targetTable) {
return getTemporaryTableFor(targetTable, "_");
}

public Table getBackupTableFor(Database model, Table sourceTable) {
return getTemporaryTableFor(model, sourceTable, "x");
public Table getBackupTableFor(Table sourceTable) {
return getTemporaryTableFor(sourceTable, "x");
}

public Table createBackupTableFor(Database model, Table sourceTable, StringBuilder ddl) {
Table backupTable = getBackupTableFor(model, sourceTable);
Table backupTable = getBackupTableFor(sourceTable);
writeTableCreationStmt(backupTable, ddl);
printEndOfStatement(ddl);
writeCopyDataStatement(sourceTable, backupTable, ddl);
Expand All @@ -893,7 +894,7 @@ public void restoreTableFromBackup(Table backupTable, Table targetTable,
writeCopyDataStatement(backupTable, targetTable, columnMap, ddl);
}

protected Table getTemporaryTableFor(Database targetModel, Table targetTable, String suffix) {
protected Table getTemporaryTableFor(Table targetTable, String suffix) {
Table table = new Table();

table.setCatalog(targetTable.getCatalog());
Expand All @@ -914,13 +915,10 @@ protected Table getTemporaryTableFor(Database targetModel, Table targetTable, St
/**
* Outputs the DDL to create the given temporary table. Per default this is
* simply a call to {@link #createTable(Table, Map)}.
*
* @param database
* The database model
* @param table
* The table
*/
protected void createTemporaryTable(Database database, Table table, StringBuilder ddl) {
protected void createTemporaryTable(Table table, StringBuilder ddl) {
createTable(table, ddl, true, false);
}

Expand Down
52 changes: 51 additions & 1 deletion symmetric-jdbc/src/test/java/org/jumpmind/db/DatabasePlatformTest.java
View file
Expand Up @@ -32,6 +32,7 @@
import org.jumpmind.db.platform.DatabaseNamesConstants;
import org.jumpmind.db.platform.IDatabasePlatform;
import org.jumpmind.db.platform.IDdlBuilder;
import org.jumpmind.db.sql.ISqlTemplate;
import org.jumpmind.db.sql.SqlScript;
import org.junit.Assert;
import org.junit.Before;
Expand Down Expand Up @@ -65,6 +66,55 @@ public void turnOffDebug() {
Logger logger = Logger.getLogger("org.jumpmind.db");
logger.setLevel(originalLevel);
}

@Test
public void testTableRebuild() throws Exception {
Table table = new Table("TEST_REBUILD");
table.addColumn(new Column("ID1", true));
table.getColumnWithName("ID1").setTypeCode(Types.INTEGER);
table.getColumnWithName("ID1").setRequired(true);

table.addColumn(new Column("NOTES"));
table.getColumnWithName("NOTES").setTypeCode(Types.VARCHAR);
table.getColumnWithName("NOTES").setSize("20");
table.getColumnWithName("NOTES").setDefaultValue("1234");

Table origTable = (Table)table.clone();

Table tableFromDatabase = dropCreateAndThenReadTable(table);

Assert.assertNotNull(tableFromDatabase);
Assert.assertEquals(2, tableFromDatabase.getColumnCount());

ISqlTemplate template = platform.getSqlTemplate();
Assert.assertEquals(1, template.update(String.format("insert into %s values(?,?)", tableFromDatabase.getName()), 1, "test"));

table.addColumn(new Column("ID2", true));
table.getColumnWithName("ID2").setTypeCode(Types.VARCHAR);
table.getColumnWithName("ID2").setSize("20");
table.getColumnWithName("ID2").setRequired(true);
table.getColumnWithName("ID2").setDefaultValue("value");

table.addColumn(new Column("NOTES2"));
table.getColumnWithName("NOTES2").setTypeCode(Types.VARCHAR);
table.getColumnWithName("NOTES2").setSize("20");
table.getColumnWithName("NOTES2").setDefaultValue("1234");

// alter to add two columns that will cause a table rebuild
platform.alterTables(false, table);
tableFromDatabase = platform.getTableFromCache(table.getName(), true);
Assert.assertNotNull(tableFromDatabase);
Assert.assertEquals(4, tableFromDatabase.getColumnCount());
Assert.assertEquals(1, template.queryForLong(String.format("select count(*) from %s", tableFromDatabase.getName())));

// alter to remove two columns that will cause a table rebuild
platform.alterTables(false, origTable);
tableFromDatabase = platform.getTableFromCache(origTable.getName(), true);
Assert.assertNotNull(tableFromDatabase);
Assert.assertEquals(2, tableFromDatabase.getColumnCount());
Assert.assertEquals(1, template.queryForLong(String.format("select count(*) from %s", tableFromDatabase.getName())));

}

@Test
public void testExportDefaultValueWithUnderscores() {
Expand Down Expand Up @@ -143,7 +193,7 @@ public void testUpgradePrimaryKeyAutoIncrementFromIntToBigInt() throws Exception
Assert.assertNotSame(id1, id2);
}
}

protected String getSequenceName(IDatabasePlatform platform) {
if (platform.getName().equals(DatabaseNamesConstants.ORACLE)) {
return "TEST_UPGRADE_ID";
Expand Down

0 comments on commit d92aff2

Please sign in to comment.