Merge pull request #2603 from FOCONIS/remove-lowertablename

Refactored DDL generation to correctly handle quoted identifiers

ebean-api/src/main/java/io/ebean/config/DbConstraintNormalise.java

@@ -58,30 +58,16 @@ private String trimBrackets(String value) {
   /**
    * Trim off the platform quoted identifier quotes like [ ' and ".
    */
-  public boolean notQuoted(String tableName) {
+  public String trimQuotes(String identifier) {
 
-    // remove quoted identifier characters
-    for (String quotedIdentifier : quotedIdentifiers) {
-      if (tableName.contains(quotedIdentifier)) {
-        return false;
-      }
-    }
-    return true;
-  }
-
-  /**
-   * Trim off the platform quoted identifier quotes like [ ' and ".
-   */
-  public String trimQuotes(String tableName) {
-
-    if (tableName == null) {
+    if (identifier == null) {
       return "";
     }
     // remove quoted identifier characters
     for (String quotedIdentifier : quotedIdentifiers) {
-      tableName = tableName.replace(quotedIdentifier, "");
+      identifier = identifier.replace(quotedIdentifier, "");
     }
-    return tableName;
+    return identifier;
   }
 
 

ebean-api/src/main/java/io/ebean/config/dbplatform/DatabasePlatform.java

@@ -165,7 +165,7 @@ public enum OnQueryOnly {
    * want to use quoted identifiers for. The backticks get converted to the
    * appropriate characters in convertQuotedIdentifiers
    */
-  private static final char BACK_TICK = '`';
+  private static final char[] QUOTED_IDENTIFIERS = new char[] { '"', '\'', '[', ']', '`' };
 
   /**
    * The non-escaped like clause (to stop slash being escaped on some platforms).
@@ -662,8 +662,8 @@ public void setDbFalseLiteral(String dbFalseLiteral) {
   public String convertQuotedIdentifiers(String dbName) {
     // Ignore null values e.g. schema name or catalog
     if (dbName != null && !dbName.isEmpty()) {
-      if (dbName.charAt(0) == BACK_TICK) {
-        if (dbName.charAt(dbName.length() - 1) == BACK_TICK) {
+      if (isQuote(dbName.charAt(0))) {
+        if (isQuote(dbName.charAt(dbName.length() - 1))) {
           return openQuote + dbName.substring(1, dbName.length() - 1) + closeQuote;
         } else {
           log.error("Missing backquote on [" + dbName + "]");
@@ -675,6 +675,15 @@ public String convertQuotedIdentifiers(String dbName) {
     return dbName;
   }
 
+  private boolean isQuote(char ch) {
+    for (char identifer : QUOTED_IDENTIFIERS) {
+      if (identifer == ch) {
+        return true;
+      }
+    }
+    return false;
+  }
+
   /**
    * Remove quoted identifier quotes from the table or column name if present.
    */

ebean-ddl-generator/src/main/java/io/ebeaninternal/dbmigration/ddlgeneration/platform/AbstractHanaDdl.java

@@ -200,10 +200,10 @@ protected DdlAlterTable alterTable(DdlWrite writer, String tableName) {
   /**
    * Joins alter table commands and add open/closing brackets for the alter statements
    */
-  private static class HanaAlterTableWrite extends BaseAlterTableWrite {
+  class HanaAlterTableWrite extends BaseAlterTableWrite {
 
     public HanaAlterTableWrite(String tableName) {
-      super(tableName);
+      super(tableName, AbstractHanaDdl.this);
     }
 
     @Override

ebean-ddl-generator/src/main/java/io/ebeaninternal/dbmigration/ddlgeneration/platform/BaseAlterTableWrite.java

@@ -17,6 +17,8 @@ public class BaseAlterTableWrite implements DdlAlterTable {
 
   protected static final String RAW_OPERATION = "$RAW";
 
+  protected final PlatformDdl platformDdl;
+
   public class AlterCmd {
     // the command (e.g. "alter", "modify"
     private final String operation;
@@ -58,9 +60,9 @@ protected void write(Appendable target) throws IOException {
         // of all alter commands
         target.append(getAlternation());
       } else {
-        target.append("alter table ").append(tableName).append(' ').append(operation);
+        target.append("alter table ").append(platformDdl.quote(tableName)).append(' ').append(operation);
         if (column != null) {
-          target.append(' ').append(column);
+          target.append(' ').append(platformDdl.quote(column));
         }
         if (!getAlternation().isEmpty()) {
           target.append(' ').append(getAlternation());
@@ -85,8 +87,9 @@ public String toString() {
 
   private boolean historyHandled;
 
-  public BaseAlterTableWrite(String tableName) {
+  public BaseAlterTableWrite(String tableName, PlatformDdl platformDdl) {
     this.tableName = tableName;
+    this.platformDdl = platformDdl;
   }
 
   public String tableName() {

ebean-ddl-generator/src/main/java/io/ebeaninternal/dbmigration/ddlgeneration/platform/BaseTableDdl.java

@@ -222,7 +222,7 @@ public void generate(DdlWrite writer, CreateTable createTable) {
     String partitionMode = createTable.getPartitionMode();
 
     DdlBuffer apply = writer.apply();
-    apply.append(platformDdl.getCreateTableCommandPrefix()).append(" ").append(tableName).append(" (");
+    apply.append(platformDdl.getCreateTableCommandPrefix()).append(" ").append(platformDdl.quote(tableName)).append(" (");
     writeTableColumns(apply, columns, identity);
     writeUniqueConstraints(apply, createTable);
     writeCompoundUniqueConstraints(apply, createTable);
@@ -423,7 +423,7 @@ protected void appendColumns(String[] columns, DdlBuffer buffer) {
       if (i > 0) {
         buffer.append(",");
       }
-      buffer.append(columns[i].trim());
+      buffer.append(platformDdl.quote(columns[i].trim()));
     }
     buffer.append(")");
   }
@@ -490,7 +490,7 @@ protected void inlineUniqueConstraintSingle(DdlBuffer buffer, Column column) {
     buffer.append(",").newLine();
     buffer.append("  constraint ").append(uqName).append(" unique ");
     buffer.append("(");
-    buffer.append(column.getName());
+    buffer.append(platformDdl.quote(column.getName()));
     buffer.append(")");
   }
 
@@ -741,7 +741,7 @@ private void alterColumnComment(DdlWrite writer, AlterColumn alterColumn) {
    * Return the name of the history table given the base table name.
    */
   protected String historyTable(String baseTable) {
-    return baseTable + historyTableSuffix;
+    return naming.normaliseTable(baseTable) + historyTableSuffix;
   }
 
   /**

ebean-ddl-generator/src/main/java/io/ebeaninternal/dbmigration/ddlgeneration/platform/DB2Ddl.java

@@ -161,10 +161,10 @@ protected DdlAlterTable alterTable(DdlWrite writer, String tableName) {
     return writer.applyAlterTable(tableName, Db2AlterTableWrite::new);
   };
 
-  static class Db2AlterTableWrite extends BaseAlterTableWrite {
+  class Db2AlterTableWrite extends BaseAlterTableWrite {
 
     public Db2AlterTableWrite(String tableName) {
-      super(tableName);
+      super(tableName, DB2Ddl.this);
     }
 
     @Override

ebean-ddl-generator/src/main/java/io/ebeaninternal/dbmigration/ddlgeneration/platform/Db2HistoryDdl.java

@@ -1,14 +1,12 @@
 package io.ebeaninternal.dbmigration.ddlgeneration.platform;
 
-import java.util.Collection;
-
 import io.ebean.config.DatabaseConfig;
+import io.ebean.config.DbConstraintNaming;
 import io.ebeaninternal.dbmigration.ddlgeneration.DdlAlterTable;
 import io.ebeaninternal.dbmigration.ddlgeneration.DdlBuffer;
 import io.ebeaninternal.dbmigration.ddlgeneration.DdlWrite;
 import io.ebeaninternal.dbmigration.migration.AddHistoryTable;
 import io.ebeaninternal.dbmigration.migration.DropHistoryTable;
-import io.ebeaninternal.dbmigration.model.MColumn;
 import io.ebeaninternal.dbmigration.model.MTable;
 
 /**
@@ -22,6 +20,7 @@ public class Db2HistoryDdl implements PlatformHistoryDdl {
   private String systemPeriodEnd;
   private String transactionId;
   private PlatformDdl platformDdl;
+  private DbConstraintNaming constraintNaming;
   private String historySuffix;
 
   @Override
@@ -30,35 +29,21 @@ public void configure(DatabaseConfig config, PlatformDdl platformDdl) {
     this.systemPeriodEnd = config.getAsOfSysPeriod() + "_end";
     this.transactionId = config.getAsOfSysPeriod() + "_txn"; // required for DB2
     this.platformDdl = platformDdl;
+    this.constraintNaming = config.getConstraintNaming();
     this.historySuffix = config.getHistoryTableSuffix();
   }
 
   @Override
   public void createWithHistory(DdlWrite writer, MTable table) {
     String tableName = table.getName();
-    String historyTableName = tableName + historySuffix;
-
-    DdlBuffer apply = writer.applyPostAlter();
-    apply.append(platformDdl.getCreateTableCommandPrefix()).append(" ").append(historyTableName).append(" (").newLine();
-
-    // create history table
-    Collection<MColumn> cols = table.allColumns();
-    for (MColumn column : cols) {
-      if (!column.isDraftOnly()) {
-        writeColumnDefinition(apply, column.getName(), column.getType(), column.isNotnull() || column.isPrimaryKey());
-        apply.append(",").newLine();
-      }
-    }
-    writeColumnDefinition(apply, systemPeriodStart, "timestamp(12)", true);
-    apply.append(",").newLine();
-    writeColumnDefinition(apply, systemPeriodEnd, "timestamp(12)", true);
-    apply.append(",").newLine();
-    writeColumnDefinition(apply, transactionId, "timestamp(12)", false);
-    apply.newLine().append(")").endOfStatement();
-
-    // enable system versioning
+    String historyTableName = historyTable(tableName);
+
+    // DB2 requires an EXACT copy (same column types with null/non-null, same order)
     addSysPeriodColumns(writer, tableName);
-    enableSystemVersioning(apply, tableName);
+    writer.applyPostAlter().append("create table ").append(historyTableName)
+      .append(" as (select * from ").append(tableName).append(") with no data").endOfStatement();
+
+    enableSystemVersioning(writer.applyPostAlter(), tableName);
     platformDdl.alterTable(writer, tableName).setHistoryHandled();
 
     // drop all: We do not drop columns here, as the whole table will be dropped
@@ -76,10 +61,10 @@ void addSysPeriodColumns(DdlWrite writer, String baseTable) {
 
   @Override
   public void dropHistoryTable(DdlWrite writer, DropHistoryTable dropHistoryTable) {
-    dropHistoryTable(writer, dropHistoryTable.getBaseTable(), dropHistoryTable.getBaseTable() + historySuffix);
+    dropHistoryTable(writer, dropHistoryTable.getBaseTable());
   }
 
-  protected void dropHistoryTable(DdlWrite writer, String baseTable, String historyTable) {
+  protected void dropHistoryTable(DdlWrite writer, String baseTable) {
     disableSystemVersioning(writer.apply(), baseTable);
     writer.apply().append("alter table ").append(baseTable).append(" drop period system_time").endOfStatement();
 
@@ -89,7 +74,7 @@ protected void dropHistoryTable(DdlWrite writer, String baseTable, String histor
     platformDdl.alterTableDropColumn(writer, baseTable, transactionId);
 
     // drop the history table
-    writer.applyPostAlter().append("drop table ").append(historyTable).endOfStatement();
+    writer.applyPostAlter().append("drop table ").append(historyTable(baseTable)).endOfStatement();
   }
 
   @Override
@@ -117,22 +102,16 @@ public void updateTriggers(DdlWrite writer, String tableName) {
     }
   }
 
-  protected void writeColumnDefinition(DdlBuffer buffer, String columnName, String type, boolean isNotNull) {
-
-    String platformType = platformDdl.convert(type);
-    buffer.append(" ").append(columnName);
-    buffer.append(" ").append(platformType);
-    if (isNotNull) {
-      buffer.append(" not null");
-    }
-  }
-
   public void disableSystemVersioning(DdlBuffer apply, String tableName) {
     apply.append("alter table ").append(tableName).append(" drop versioning").endOfStatement();
   }
 
   public void enableSystemVersioning(DdlBuffer apply, String tableName) {
-    apply.append("alter table ").append(tableName).append(" add versioning use history table ").append(tableName).append(historySuffix).endOfStatement();
+    apply.append("alter table ").append(tableName).append(" add versioning use history table ").append(historyTable(tableName)).endOfStatement();
+  }
+
+  protected String historyTable(String tableName) {
+    return constraintNaming.normaliseTable(tableName) + historySuffix;
   }
 
 }

ebean-ddl-generator/src/main/java/io/ebeaninternal/dbmigration/ddlgeneration/platform/DbTriggerBasedHistoryDdl.java

@@ -127,11 +127,15 @@ protected String normalise(String tableName) {
   }
 
   protected String historyTableName(String baseTableName) {
-    return baseTableName + historySuffix;
+    return quote(normalise(baseTableName) + historySuffix);
+  }
+
+  protected String historyViewName(String baseTableName) {
+    return quote(normalise(baseTableName) + viewSuffix);
   }
 
   protected String procedureName(String baseTableName) {
-    return baseTableName + "_history_version";
+    return normalise(baseTableName) + "_history_version";
   }
 
   protected String triggerName(String baseTableName) {
@@ -146,16 +150,6 @@ protected String deleteTriggerName(String baseTableName) {
     return normalise(baseTableName) + "_history_del";
   }
 
-  protected void addHistoryTable(DdlWrite writer, MTable table, String whenCreatedColumn) {
-
-    String baseTableName = table.getName();
-
-
-    addSysPeriodColumns(writer, baseTableName, whenCreatedColumn);
-    createHistoryTable(writer.applyPostAlter(), table);
-    createWithHistoryView(writer.applyPostAlter(), baseTableName);
-  }
-
   protected void addSysPeriodColumns(DdlWrite writer, String baseTableName, String whenCreatedColumn) {
 
     platformDdl.alterTableAddColumn(writer, baseTableName, sysPeriodStart, sysPeriodType, now);
@@ -172,13 +166,17 @@ protected void createHistoryTable(DdlBuffer apply, MTable table) {
   }
 
   protected void createHistoryTableAs(DdlBuffer apply, MTable table) {
-    apply.append(platformDdl.getCreateTableCommandPrefix()).append(" ").append(table.getName()).append(historySuffix).append("(").newLine();
+    apply.append(platformDdl.getCreateTableCommandPrefix()).append(" ").append(historyTableName(table.getName())).append("(").newLine();
     for (MColumn column : table.allColumns()) {
       if (!column.isDraftOnly()) {
         writeColumnDefinition(apply, column.getName(), column.getType());
         apply.append(",").newLine();
       }
     }
+    // TODO: We must apply also pending dropped columns. Let's do that in a later step
+    if (table.hasDroppedColumns()) {
+      throw new IllegalStateException(table.getName() + " has dropped columns. Please generate drop script before enabling history");
+    }
   }
 
   protected void createHistoryTableWithPeriod(DdlBuffer apply) {
@@ -195,16 +193,16 @@ protected void writeColumnDefinition(DdlBuffer buffer, String columnName, String
 
     String platformType = platformDdl.convert(type);
     buffer.append("  ");
-    buffer.append(columnName, 29);
+    buffer.append(quote(columnName), 29);
     buffer.append(platformType);
   }
 
   protected void createWithHistoryView(DdlBuffer apply, String baseTableName) {
 
     apply
-      .append("create view ").append(baseTableName).append(viewSuffix)
-      .append(" as select * from ").append(baseTableName)
-      .append(" union all select * from ").append(baseTableName).append(historySuffix)
+      .append("create view ").append(historyViewName(baseTableName))
+      .append(" as select * from ").append(quote(baseTableName))
+      .append(" union all select * from ").append(historyTableName(baseTableName))
       .endOfStatement();
   }
 
@@ -214,11 +212,11 @@ protected void appendSysPeriodColumns(DdlBuffer apply, String prefix) {
   }
 
   protected void dropWithHistoryView(DdlBuffer apply, String baseTableName) {
-    apply.append("drop view ").append(baseTableName).append(viewSuffix).endOfStatement();
+    apply.append("drop view ").append(historyViewName(baseTableName)).endOfStatement();
   }
 
   protected void dropHistoryTable(DdlBuffer apply, String baseTableName) {
-    apply.append("drop table ").append(baseTableName).append(historySuffix).endOfStatement().end();
+    apply.append("drop table ").append(historyTableName(baseTableName)).endOfStatement().end();
   }
 
   protected void dropSysPeriodColumns(DdlWrite writer, String baseTableName) {
@@ -228,7 +226,7 @@ protected void dropSysPeriodColumns(DdlWrite writer, String baseTableName) {
 
   protected void appendInsertIntoHistory(DdlBuffer buffer, String baseTable, List<String> columns) {
 
-    buffer.append("    insert into ").append(baseTable).append(historySuffix).append(" (").append(sysPeriodStart).append(",").append(sysPeriodEnd).append(",");
+    buffer.append("    insert into ").append(historyTableName(baseTable)).append(" (").append(sysPeriodStart).append(",").append(sysPeriodEnd).append(",");
     appendColumnNames(buffer, columns, "");
     buffer.append(") values (OLD.").append(sysPeriodStart).append(", ").append(sysPeriodEndValue).append(",");
     appendColumnNames(buffer, columns, "OLD.");
@@ -270,4 +268,8 @@ List<String> columnNamesForApply(MTable table) {
   public boolean alterHistoryTables() {
     return true;
   }
+
+  protected String quote(String dbName) {
+    return platformDdl.quote(dbName);
+  }
 }

ebean-ddl-generator/src/main/java/io/ebeaninternal/dbmigration/ddlgeneration/platform/H2HistoryDdl.java

@@ -31,7 +31,7 @@ private void addCreateTrigger(DdlBuffer apply, String triggerName, String baseTa
     // Note that this does not take into account the historyTable name (excepts _history suffix) and
     // does not take into account excluded columns (all columns included in history)
     apply
-      .append("create trigger ").append(triggerName).append(" before update,delete on ").append(baseTable)
+      .append("create trigger ").append(triggerName).append(" before update,delete on ").append(quote(baseTable))
       .append(" for each row call \"" + TRIGGER_CLASS + "\";").newLine();
   }