From d37bf4634abf73bd4346f0b829fd1d0cc0eb35de Mon Sep 17 00:00:00 2001 From: Roland Praml Date: Wed, 16 Mar 2022 15:11:11 +0100 Subject: [PATCH] Fix: Migration problems --- .../ddlgeneration/platform/BaseTableDdl.java | 26 ++- .../ddlgeneration/platform/DB2Ddl.java | 47 +---- .../ddlgeneration/platform/PlatformDdl.java | 10 -- .../dbmigration/migration/ObjectFactory.java | 13 +- .../dbmigration/model/ModelContainer.java | 4 +- .../dbmigration/DbMigrationGenerateTest.java | 42 ++--- .../dbmigration/DbMigrationTest.java | 167 +++++++++++++++--- .../java/misc/migration/v1_1/EHistory2.java | 9 - 8 files changed, 185 insertions(+), 133 deletions(-) diff --git a/ebean-ddl-generator/src/main/java/io/ebeaninternal/dbmigration/ddlgeneration/platform/BaseTableDdl.java b/ebean-ddl-generator/src/main/java/io/ebeaninternal/dbmigration/ddlgeneration/platform/BaseTableDdl.java index da8c91d9f..87606c297 100644 --- a/ebean-ddl-generator/src/main/java/io/ebeaninternal/dbmigration/ddlgeneration/platform/BaseTableDdl.java +++ b/ebean-ddl-generator/src/main/java/io/ebeaninternal/dbmigration/ddlgeneration/platform/BaseTableDdl.java @@ -238,8 +238,7 @@ public class BaseTableDdl implements TableDdl { } apply.newLine().append(")"); if (createTable.getTablespace() != null) { - platformDdl.addTablespace(apply, createTable.getTablespace(), createTable.getIndexTablespace(), - createTable.getLobTablespace()); + platformDdl.addTablespace(apply, createTable.getTablespace(), createTable.getIndexTablespace(), createTable.getLobTablespace()); } addTableStorageEngine(apply, createTable); addTableCommentInline(apply, createTable); @@ -666,25 +665,22 @@ public class BaseTableDdl implements TableDdl { } } + /** + * Add table related changes to DDL (tableSpace,...) + */ @Override public void generate(DdlWrite writer, AlterTable alterTable) { - if (hasValue(alterTable.getTablespace()) || hasValue(alterTable.getIndexTablespace()) || hasValue(alterTable.getLobTablespace())) { + if (hasValue(alterTable.getTablespace()) + || hasValue(alterTable.getIndexTablespace()) + || hasValue(alterTable.getLobTablespace())) { + writer.apply().appendStatement(platformDdl.alterTableTablespace(alterTable.getName(), - DdlHelp.toTablespace(alterTable.getTablespace()), - DdlHelp.toTablespace(alterTable.getIndexTablespace()), - DdlHelp.toTablespace(alterTable.getLobTablespace()))); + DdlHelp.toTablespace(alterTable.getTablespace()), + DdlHelp.toTablespace(alterTable.getIndexTablespace()), + DdlHelp.toTablespace(alterTable.getLobTablespace()))); } } - protected void writeTablespaceChange(DdlBuffer buffer, String tablename, String tableSpace, String indexSpace, String lobSpace) { - buffer.appendStatement("-- TableSpace changed: Table: " + tablename + ", tableSpace " + tableSpace + ", indexSpace " - + indexSpace + ", lobSpace " + lobSpace); - if (strictMode) { - throw new UnsupportedOperationException( - "Tablespace change is not supported by this platform. Disable strict mode for migration and write migration manually"); - } - } - /** * Add drop column DDL. */ diff --git a/ebean-ddl-generator/src/main/java/io/ebeaninternal/dbmigration/ddlgeneration/platform/DB2Ddl.java b/ebean-ddl-generator/src/main/java/io/ebeaninternal/dbmigration/ddlgeneration/platform/DB2Ddl.java index 67c3eb133..616515e1e 100644 --- a/ebean-ddl-generator/src/main/java/io/ebeaninternal/dbmigration/ddlgeneration/platform/DB2Ddl.java +++ b/ebean-ddl-generator/src/main/java/io/ebeaninternal/dbmigration/ddlgeneration/platform/DB2Ddl.java @@ -12,28 +12,6 @@ import io.ebeaninternal.dbmigration.migration.Column; /** * DB2 platform specific DDL. - * - * according to the list - * https://datageek.blog/en/2014/05/06/db2-basics-what-is-a-reorg/ a reorg is - * necessary after - *
    - *
  1. Data type changes that increase the size of a varchar or vargraphic - * column - *
  2. Data type changes that decrease the size of a varchar or vargraphic - * column - *
  3. Altering a column to include NOT NULL - *
  4. Altering a column to inline LOBS - *
  5. Altering a column to compress the system default or turn off compression - * for the system default - *
  6. Altering a table to enable value compression - *
  7. Altering a table to drop a column - *
  8. Changing the PCTFREE for a table - *
  9. Altering a table to turn APEND mode off - *
  10. Altering a table or index to turn compression on - *
- * - * This is currently handled by BaseTableDdl - * */ public class DB2Ddl extends PlatformDdl { private static final String MOVE_TABLE = "CALL SYSPROC.ADMIN_MOVE_TABLE(CURRENT_SCHEMA,'%s','%s','%s','%s','','','','','','MOVE')"; @@ -62,39 +40,32 @@ public class DB2Ddl extends PlatformDdl { } @Override - public String alterTableAddUniqueConstraint(String tableName, String uqName, String[] columns, - String[] nullableColumns) { - StringBuilder sb = new StringBuilder(300); + public String alterTableAddUniqueConstraint(String tableName, String uqName, String[] columns, String[] nullableColumns) { if (nullableColumns == null || nullableColumns.length == 0) { - - sb.append("alter table ").append(lowerTableName(tableName)); - sb.append(" add constraint ").append(maxConstraintName(uqName)).append(" unique "); - appendColumns(columns, sb); - return sb.toString(); - } + return super.alterTableAddUniqueConstraint(tableName, uqName, columns, nullableColumns); + } if (uqName == null) { throw new NullPointerException(); } - sb = new StringBuilder("create unique index "); + StringBuilder sb = new StringBuilder("create unique index "); sb.append(maxConstraintName(uqName)).append(" on ").append(tableName).append('('); for (int i = 0; i < columns.length; i++) { if (i > 0) { sb.append(","); } - sb.append(lowerColumnName(columns[i])); + sb.append(columns[i]); } sb.append(") exclude null keys"); return sb.toString(); } - // @Override - // public void addTablespace(DdlBuffer apply, String tablespaceName, String indexTablespace, String lobTablespace) - // throws IOException { - // apply.append(" in ").append(tablespaceName).append(" index in ").append(indexTablespace).append(" long in ").append(lobTablespace); - // } + @Override + public void addTablespace(DdlBuffer apply, String tablespaceName, String indexTablespace, String lobTablespace) { + apply.append(" in ").append(tablespaceName).append(" index in ").append(indexTablespace).append(" long in ").append(lobTablespace); + } @Override public void alterTableAddColumn(DdlWrite writer, String tableName, Column column, boolean onHistoryTable, String defaultValue) { diff --git a/ebean-ddl-generator/src/main/java/io/ebeaninternal/dbmigration/ddlgeneration/platform/PlatformDdl.java b/ebean-ddl-generator/src/main/java/io/ebeaninternal/dbmigration/ddlgeneration/platform/PlatformDdl.java index 8156e67ae..dfae8815a 100644 --- a/ebean-ddl-generator/src/main/java/io/ebeaninternal/dbmigration/ddlgeneration/platform/PlatformDdl.java +++ b/ebean-ddl-generator/src/main/java/io/ebeaninternal/dbmigration/ddlgeneration/platform/PlatformDdl.java @@ -813,14 +813,4 @@ public class PlatformDdl { // now only supported for db2 } - /** - * Returns a statement to reorganize the table. This is required mainly for DB2. - * - * @param table the table name - * @param counter to make statements unique. - */ - public String reorgTable(String table, int counter) { - return null; - } - } diff --git a/ebean-ddl-generator/src/main/java/io/ebeaninternal/dbmigration/migration/ObjectFactory.java b/ebean-ddl-generator/src/main/java/io/ebeaninternal/dbmigration/migration/ObjectFactory.java index 0fb3cec99..38e530287 100644 --- a/ebean-ddl-generator/src/main/java/io/ebeaninternal/dbmigration/migration/ObjectFactory.java +++ b/ebean-ddl-generator/src/main/java/io/ebeaninternal/dbmigration/migration/ObjectFactory.java @@ -187,13 +187,12 @@ public class ObjectFactory { return new ChangeSet(); } - /** - * Create an instance of {@link AlterTable } - * - */ - public AlterTable createAlterTable() { - return new AlterTable(); - } + /** + * Create an instance of {@link AlterTable } + */ + public AlterTable createAlterTable() { + return new AlterTable(); + } /** * Create an instance of {@link AddHistoryTable } diff --git a/ebean-ddl-generator/src/main/java/io/ebeaninternal/dbmigration/model/ModelContainer.java b/ebean-ddl-generator/src/main/java/io/ebeaninternal/dbmigration/model/ModelContainer.java index 9e5749483..7588a7cdf 100644 --- a/ebean-ddl-generator/src/main/java/io/ebeaninternal/dbmigration/model/ModelContainer.java +++ b/ebean-ddl-generator/src/main/java/io/ebeaninternal/dbmigration/model/ModelContainer.java @@ -266,7 +266,9 @@ public class ModelContainer { tables.remove(dropTable.getName()); } - + /** + * Apply a AlterTable change to the model. + */ protected void applyChange(AlterTable alterTable) { MTable table = getTable(alterTable.getName()); if (table == null) { diff --git a/ebean-test/src/test/java/io/ebeaninternal/dbmigration/DbMigrationGenerateTest.java b/ebean-test/src/test/java/io/ebeaninternal/dbmigration/DbMigrationGenerateTest.java index d8b6a729e..e98a532a0 100644 --- a/ebean-test/src/test/java/io/ebeaninternal/dbmigration/DbMigrationGenerateTest.java +++ b/ebean-test/src/test/java/io/ebeaninternal/dbmigration/DbMigrationGenerateTest.java @@ -4,6 +4,8 @@ import io.ebean.Database; import io.ebean.DatabaseFactory; import io.ebean.annotation.Platform; import io.ebean.config.DatabaseConfig; +import io.ebeaninternal.api.DbOffline; + import org.junit.jupiter.api.Test; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -95,7 +97,7 @@ public class DbMigrationGenerateTest { config.getProperties().put("ebean.hana.generateUniqueDdl", "true"); // need to generate unique statements to prevent them from being filtered out as duplicates by the DdlRunner config.setPackages(Arrays.asList("misc.migration.v1_0")); - Database server = DatabaseFactory.create(config); + Database server = createServer(config); migration.setServer(server); // then we generate migration scripts for v1_0 @@ -106,43 +108,29 @@ public class DbMigrationGenerateTest { // and now for v1_1 config.setPackages(Arrays.asList("misc.migration.v1_1")); server.shutdown(); - server = DatabaseFactory.create(config); + server = createServer(config); migration.setServer(server); - assertThat(migration.generateMigration()).isEqualTo("1.1"); - assertThat(migration.generateMigration()).isNull(); // subsequent call - - - - System.setProperty("ddl.migration.pendingDropsFor", "1.1"); - assertThat(migration.generateMigration()).isEqualTo("1.2__dropsFor_1.1"); - - assertThatThrownBy(()->migration.generateMigration()) - .isInstanceOf(IllegalArgumentException.class) - .hasMessageContaining("No 'pendingDrops'"); // subsequent call - - System.clearProperty("ddl.migration.pendingDropsFor"); + assertThat(migration.generateMigration()).isEqualTo("1.1,1.2__dropsFor_1.1"); assertThat(migration.generateMigration()).isNull(); // subsequent call // and now for v1_2 with config.setPackages(Arrays.asList("misc.migration.v1_2")); server.shutdown(); - server = DatabaseFactory.create(config); + server = createServer(config); migration.setServer(server); - assertThat(migration.generateMigration()).isEqualTo("1.3"); - assertThat(migration.generateMigration()).isNull(); // subsequent call - - - System.setProperty("ddl.migration.pendingDropsFor", "1.3"); - assertThat(migration.generateMigration()).isEqualTo("1.4__dropsFor_1.3"); - assertThatThrownBy(migration::generateMigration) - .isInstanceOf(IllegalArgumentException.class) - .hasMessageContaining("No 'pendingDrops'"); // subsequent call - - System.clearProperty("ddl.migration.pendingDropsFor"); + assertThat(migration.generateMigration()).isEqualTo("1.3,1.4__dropsFor_1.3"); assertThat(migration.generateMigration()).isNull(); // subsequent call server.shutdown(); logger.info("end"); } + private static Database createServer(DatabaseConfig config) { + DbOffline.setGenerateMigration(); + Database server = DatabaseFactory.create(config); + DbOffline.reset(); + server.start(); + return server; + } + } diff --git a/ebean-test/src/test/java/io/ebeaninternal/dbmigration/DbMigrationTest.java b/ebean-test/src/test/java/io/ebeaninternal/dbmigration/DbMigrationTest.java index 5a23e0944..7e47a0cdb 100644 --- a/ebean-test/src/test/java/io/ebeaninternal/dbmigration/DbMigrationTest.java +++ b/ebean-test/src/test/java/io/ebeaninternal/dbmigration/DbMigrationTest.java @@ -1,28 +1,44 @@ package io.ebeaninternal.dbmigration; - -import io.ebean.BaseTestCase; -import io.ebean.SqlRow; -import io.ebean.SqlUpdate; +import io.ebean.*; import io.ebean.annotation.IgnorePlatform; import io.ebean.annotation.Platform; +import io.ebean.config.DatabaseConfig; +import io.ebean.config.dbplatform.DbHistorySupport; +import io.ebean.datasource.pool.ConnectionPool; +import misc.migration.v1_1.EHistory; +import misc.migration.v1_1.EHistory2; import org.junit.jupiter.api.Test; import java.io.File; import java.io.IOException; import java.net.URL; +import java.sql.SQLException; +import java.sql.Timestamp; +import java.util.Collections; import java.util.List; import static org.assertj.core.api.Assertions.assertThat; +/** + * This testcase tries to apply the migrationtests that are genearated by {@link DbMigrationGenerateTest}. + * + * It does also some basic checks, if the migration is applied correctly. + * + * Please note, that this test requires the scripts generated by DbMigrationGenerateTest. So you may have to execute this test + * first. + * + * @author Roland Praml, FOCONIS AG + * + */ public class DbMigrationTest extends BaseTestCase { - private void runScript(String scriptName) throws IOException { + private void runScript(String scriptName) { URL url = getClass().getResource("/migrationtest/dbmigration/" + server().platform().name().toLowerCase() + "/" + scriptName); assert url != null : scriptName + " not found"; server().script().run(url); } - + @Test public void lastVersion() { File d = new File("src/test/resources/migrationtest/dbmigration/h2"); @@ -31,19 +47,17 @@ public class DbMigrationTest extends BaseTestCase { assertThat(LastMigration.nextVersion(d, null, true)).isEqualTo("1.4"); } + @IgnorePlatform({ + // Yugabyte does not see column updates on table alters: + // update table T set C = 'value'; alter table T alter column C set not null -> error column C has null values + // do we need a commit after update? + Platform.YUGABYTE, + }) @Test - public void lastVersion_no_v_Prefix() { - File d = new File("src/test/resources/migrationtest-history/dbmigration"); - assertThat(LastMigration.lastVersion(d, null)).isEqualTo("1.2"); - } - - - @IgnorePlatform({Platform.ORACLE, Platform.NUODB, Platform.POSTGRES, Platform.YUGABYTE}) - // Note: Postgres locks up on build server - // Note: YUGABYTE complains on "alter table migtest_e_basic alter column status set not null;" - @Test - public void testRunMigration() throws IOException { - // first clean up previously created objects + public void testRunMigration() throws IOException, SQLException { + // Shutdown and reconnect - this prevents postgres from lock up + ((ConnectionPool)server().dataSource()).offline(); + ((ConnectionPool)server().dataSource()).online(); cleanup("migtest_ckey_assoc", "migtest_ckey_detail", "migtest_ckey_parent", @@ -71,8 +85,10 @@ public class DbMigrationTest extends BaseTestCase { "migtest_mtm_m_migtest_mtm_c", "migtest_oto_child", "migtest_oto_master"); + ((ConnectionPool)server().dataSource()).offline(); + ((ConnectionPool)server().dataSource()).online(); - if (isSqlServer() || isMariaDB()) { // || isMySql() + if (isSqlServer() || isMariaDB() || isMySql() || isHana()) { runScript("I__create_procs.sql"); } @@ -82,7 +98,11 @@ public class DbMigrationTest extends BaseTestCase { runScript("1.0__initial.sql"); - if (isOracle() || isHana()) { + if (isClickHouse()) { + // ClickHouse does not support transactions, so we cannot do update statements + // Add column is also not implemented. So exit here + return; + } else if (isOracle() || isHana()) { SqlUpdate update = server().sqlUpdate("insert into migtest_e_basic (id, old_boolean, user_id) values (1, :false, 1)"); update.setParameter("false", false); assertThat(server().execute(update)).isEqualTo(1); @@ -99,6 +119,15 @@ public class DbMigrationTest extends BaseTestCase { } createHistoryEntities(); + if (isOracle()) { + // Oracle does not like to convert varchar to integer + // ORA-01439. "column to be modified must be empty to change datatype". + // If the current table is not empty, you may have to create a temp-table + // with correct data types or do it with DBMS_REDEFINITION - to get the test + // working, we clear all data in the table + server().sqlUpdate("delete from migtest_e_history").execute(); + server().sqlUpdate("delete from migtest_e_history4").execute(); + } // Run migration runScript("1.1.sql"); @@ -125,28 +154,114 @@ public class DbMigrationTest extends BaseTestCase { assertThat(row.getBoolean("new_boolean_field2")).isTrue(); //assertThat(row.getTimestamp("some_date")).isCloseTo(new Date(), 60_000); // allow 1 minute delta + testVersioning(); + if (isSqLite()) { + // SqLite does not support drops on columns with foreign keys, so we end with the test here. + return; + } runScript("1.2__dropsFor_1.1.sql"); - // Some platforms (oracle, db2) caches the statement and does not detect schema change. - // so we must not perform the same query, again + // Oracle caches the statement and does not detect schema change. It fails with + // an ORA-01007 result = server().sqlQuery("select * from migtest_e_basic order by id,status").findList(); assertThat(result).hasSize(2); row = result.get(0); assertThat(row.keySet()).doesNotContain("old_boolean", "old_boolean2"); + if (isYugabyte()) { + // there are some unsupported alter commands in 1.3 - so we exit here + return; + } runScript("1.3.sql"); runScript("1.4__dropsFor_1.3.sql"); - // now DB structure shoud be the same as v1_0 - perform a diffent query. + // now DB structure should be the same as v1_0 - perform a diffent query. result = server().sqlQuery("select * from migtest_e_basic order by id,name").findList(); assertThat(result).hasSize(2); row = result.get(0); assertThat(row.keySet()).contains("old_boolean", "old_boolean2"); } - /** - * - */ + // do some history tests with V1.1 models + private void testVersioning() { + if (isOracle()) { + System.err.println("FIXME: Oracle history support seems to be broken"); + return; + } + DbHistorySupport history = server().pluginApi().databasePlatform().getHistorySupport(); + if (history == null) { + return; + } + DatabaseConfig config = new DatabaseConfig(); + config.setName(server().name()); + config.loadFromProperties(server().pluginApi().config().getProperties()); + config.setDataSource(server().dataSource()); + config.setReadOnlyDataSource(server().dataSource()); + config.setDdlGenerate(false); + config.setDdlRun(false); + config.setRegister(false); + config.setPackages(Collections.singletonList("misc.migration.v1_1")); + + Database tmpServer = DatabaseFactory.create(config); + try { + EHistory hist = new misc.migration.v1_1.EHistory(); + hist.setId(2); + hist.setTestString(42L); + tmpServer.save(hist); + hist = tmpServer.find(EHistory.class).where().eq("testString", 42L).findOne(); + assert hist != null; + hist.setTestString(45L); + tmpServer.save(hist); + + List> versions = tmpServer.find(EHistory.class).setId(hist.getId()) + .findVersionsBetween(Timestamp.valueOf("1970-01-01 00:00:00"), Timestamp.valueOf("2100-01-01 00:00:00")); + assertThat(versions).hasSize(2); + assertThat(versions.get(0).getDiff().toString()).as("using platform: %s", server().platform()) + .isEqualTo("{testString=45,42}"); + + EHistory2 hist2 = new misc.migration.v1_1.EHistory2(); + hist2.setId(2); + hist2.setTestString("foo1"); + hist2.setTestString2("bar1"); + hist2.setTestString3("baz1"); + tmpServer.save(hist2); + hist2.setTestString("foo2"); + hist2.setTestString2("bar2"); + tmpServer.save(hist2); + + List> versions2 = tmpServer.find(EHistory2.class).setId(hist.getId()) + .findVersionsBetween(Timestamp.valueOf("1970-01-01 00:00:00"), Timestamp.valueOf("2100-01-01 00:00:00")); + assertThat(versions2).hasSize(2); + + // not all platforms will support history exclusions + switch (server().platform()) { + case H2: // Trigger ignores HistoryExclude + case SQLSERVER17: // these DBs are 'standard based' so they also do not support HistoryExclude + case MARIADB: + case HANA: + case DB2LUW: + case DB2FORI: // not yet tested + case DB2ZOS: // not yet tested + assertThat(versions2.get(0).getDiff().toString()).as("using platform: %s, versions2:%s", server().platform(), versions2) + .contains("testString=foo2,foo1") + .contains("testString2=bar2,bar1"); + break; + case MYSQL: + case POSTGRES: + case YUGABYTE: + assertThat(versions2.get(0).getDiff().toString()).as("using platform: %s, versions2:%s", server().platform(), versions2) + .contains("testString=foo2,foo1") + .contains("testString2=bar2,null"); + break; + default: + throw new IllegalArgumentException(server().platform() + " not expected"); + } + + } finally { + tmpServer.shutdown(false, false); + } + } + private void createHistoryEntities() { SqlUpdate update = server().sqlUpdate("insert into migtest_e_history (id, test_string) values (1, '42')"); assertThat(server().execute(update)).isEqualTo(1); diff --git a/ebean-test/src/test/java/misc/migration/v1_1/EHistory2.java b/ebean-test/src/test/java/misc/migration/v1_1/EHistory2.java index 0c58ab84b..89cf970a7 100644 --- a/ebean-test/src/test/java/misc/migration/v1_1/EHistory2.java +++ b/ebean-test/src/test/java/misc/migration/v1_1/EHistory2.java @@ -2,11 +2,9 @@ package misc.migration.v1_1; import io.ebean.annotation.DbDefault; -import io.ebean.annotation.DbMigration; import io.ebean.annotation.History; import io.ebean.annotation.HistoryExclude; import io.ebean.annotation.NotNull; -import io.ebean.annotation.Platform; import javax.persistence.Entity; import javax.persistence.Id; @@ -22,13 +20,6 @@ public class EHistory2 { Integer id; @NotNull - // see: https://mariadb.com/de/resources/blog/use-cases-for-mariadb-data-versioning/ - @DbMigration(preAlter = { - "SET @@system_versioning_alter_history = 1", - "update ${table} set ${column} = 'unknown' where ${column} is null"}, platforms = Platform.MARIADB) - - // other platforms - @DbMigration(preAlter = "update ${table} set ${column} = 'unknown' where ${column} is null") @DbDefault("unknown") String testString;