AlterColumnsTypeBuilder: support change nullity on postgre and oracle too

diff --git a/sonar-db/src/main/java/org/sonar/db/version/AlterColumnsTypeBuilder.java b/sonar-db/src/main/java/org/sonar/db/version/AlterColumnsTypeBuilder.java
index 84bdb938c6a9f6b321556413cfd8a6962f48ddbd..caf9def91b3c4bfedce4624b6c1b8d0e4c3926a5 100644 (file)
--- a/sonar-db/src/main/java/org/sonar/db/version/AlterColumnsTypeBuilder.java
+++ b/sonar-db/src/main/java/org/sonar/db/version/AlterColumnsTypeBuilder.java
@@ -33,14 +33,17 @@ import static com.google.common.collect.Lists.newArrayList;
 /**
  * Generate SQL queries to update multiple column types from a table.
  *
- * This class should only be used to change the type of a column, as the nullable state is only used to not loose the nullable  information.
- * Indeed, on MsSQL, not setting the nullable state will set the column to NULL, but on Oracle setting a column NOT NULL when it's already NOT NULL will fail).
- *
- * The nullable information will then be ignored on Oracle.
- *
+ * Note that this operation will not be re-entrant on:
+ * <ul>
+ *   <li>Oracle 11G (may raise {@code ORA-01442: column to be modified to NOT NULL is already NOT NULL} or
+ *   {@code ORA-01451: column to be modified to NULL cannot be modified to NULL})</li>
+ * </ul>
  */
 public class AlterColumnsTypeBuilder {
 
+  private static final String ALTER_TABLE = "ALTER TABLE ";
+  private static final String ALTER_COLUMN = "ALTER COLUMN ";
+
   private final Dialect dialect;
   private final String tableName;
   private final List<ColumnDef> columnDefs = newArrayList();
@@ -72,20 +75,31 @@ public class AlterColumnsTypeBuilder {
   }
 
   private List<String> createPostgresQuery() {
-    StringBuilder sql = new StringBuilder("ALTER TABLE " + tableName + " ");
-    addColumns(sql, "ALTER COLUMN ", "TYPE ", true);
+    StringBuilder sql = new StringBuilder(ALTER_TABLE + tableName + " ");
+    for (Iterator<ColumnDef> columnDefIterator = columnDefs.iterator(); columnDefIterator.hasNext();) {
+      ColumnDef columnDef = columnDefIterator.next();
+      sql.append(ALTER_COLUMN);
+      addColumn(sql, columnDef, "TYPE ", false);
+      sql.append(", ");
+      sql.append(ALTER_COLUMN);
+      sql.append(columnDef.getName());
+      sql.append(' ').append(columnDef.isNullable() ? "DROP" : "SET").append(" NOT NULL");
+      if (columnDefIterator.hasNext()) {
+        sql.append(", ");
+      }
+    }
     return Collections.singletonList(sql.toString());
   }
 
   private List<String> createMySqlQuery() {
-    StringBuilder sql = new StringBuilder("ALTER TABLE " + tableName + " ");
+    StringBuilder sql = new StringBuilder(ALTER_TABLE + tableName + " ");
     addColumns(sql, "MODIFY COLUMN ", "", true);
     return Collections.singletonList(sql.toString());
   }
 
   private List<String> createOracleQuery() {
-    StringBuilder sql = new StringBuilder("ALTER TABLE " + tableName + " ").append("MODIFY (");
-    addColumns(sql, "", "", false);
+    StringBuilder sql = new StringBuilder(ALTER_TABLE + tableName + " ").append("MODIFY (");
+    addColumns(sql, "", "", true);
     sql.append(")");
     return Collections.singletonList(sql.toString());
   }
@@ -93,8 +107,8 @@ public class AlterColumnsTypeBuilder {
   private List<String> createMsSqlAndH2Queries() {
     List<String> sqls = new ArrayList<>();
     for (ColumnDef columnDef : columnDefs) {
-      StringBuilder defaultQuery = new StringBuilder("ALTER TABLE " + tableName + " ");
-      defaultQuery.append("ALTER COLUMN ");
+      StringBuilder defaultQuery = new StringBuilder(ALTER_TABLE + tableName + " ");
+      defaultQuery.append(ALTER_COLUMN);
       addColumn(defaultQuery, columnDef, "", true);
       sqls.add(defaultQuery.toString());
     }
@@ -116,8 +130,8 @@ public class AlterColumnsTypeBuilder {
       .append(" ")
       .append(typePrefix)
       .append(columnDef.generateSqlType(dialect));
-    if (!columnDef.isNullable() && addNotNullableProperty) {
-      sql.append(" NOT NULL");
+    if (addNotNullableProperty) {
+      sql.append(columnDef.isNullable() ? " NULL" : " NOT NULL");
     }
   }
 

diff --git a/sonar-db/src/test/java/org/sonar/db/version/AlterColumnsTypeBuilderTest.java b/sonar-db/src/test/java/org/sonar/db/version/AlterColumnsTypeBuilderTest.java
index d8dc1903577b00a5480f8f3b6a94c88596f85ad4..816ada98606aa9712937a5dbf9ac58220f09bac0 100644 (file)
--- a/sonar-db/src/test/java/org/sonar/db/version/AlterColumnsTypeBuilderTest.java
+++ b/sonar-db/src/test/java/org/sonar/db/version/AlterColumnsTypeBuilderTest.java
@@ -42,7 +42,7 @@ public class AlterColumnsTypeBuilderTest {
   @Test
   public void update_columns_on_h2() {
     assertThat(createSampleBuilder(new H2()).build())
-      .containsOnly("ALTER TABLE issues ALTER COLUMN value DOUBLE", "ALTER TABLE issues ALTER COLUMN name VARCHAR (10)");
+      .containsOnly("ALTER TABLE issues ALTER COLUMN value DOUBLE NULL", "ALTER TABLE issues ALTER COLUMN name VARCHAR (10) NULL");
   }
 
   @Test
@@ -54,7 +54,7 @@ public class AlterColumnsTypeBuilderTest {
   @Test
   public void update_columns_on_mssql() {
     assertThat(createSampleBuilder(new MsSql()).build())
-      .containsOnly("ALTER TABLE issues ALTER COLUMN value DECIMAL (30,20)", "ALTER TABLE issues ALTER COLUMN name NVARCHAR (10)");
+      .containsOnly("ALTER TABLE issues ALTER COLUMN value DECIMAL (30,20) NULL", "ALTER TABLE issues ALTER COLUMN name NVARCHAR (10) NULL");
   }
 
   @Test
@@ -66,19 +66,21 @@ public class AlterColumnsTypeBuilderTest {
   @Test
   public void update_columns_on_postgres() {
     assertThat(createSampleBuilder(new PostgreSql()).build())
-      .containsOnly("ALTER TABLE issues ALTER COLUMN value TYPE NUMERIC (30,20), ALTER COLUMN name TYPE VARCHAR (10)");
+      .containsOnly("ALTER TABLE issues " +
+        "ALTER COLUMN value TYPE NUMERIC (30,20), ALTER COLUMN value DROP NOT NULL, " +
+        "ALTER COLUMN name TYPE VARCHAR (10), ALTER COLUMN name DROP NOT NULL");
   }
 
   @Test
   public void update_not_nullable_column_on_postgres() {
     assertThat(createNotNullableBuilder(new PostgreSql()).build())
-      .containsOnly("ALTER TABLE issues ALTER COLUMN name TYPE VARCHAR (10) NOT NULL");
+      .containsOnly("ALTER TABLE issues ALTER COLUMN name TYPE VARCHAR (10), ALTER COLUMN name SET NOT NULL");
   }
 
   @Test
   public void update_columns_on_mysql() {
     assertThat(createSampleBuilder(new MySql()).build())
-      .containsOnly("ALTER TABLE issues MODIFY COLUMN value DECIMAL (30,20), MODIFY COLUMN name VARCHAR (10)");
+      .containsOnly("ALTER TABLE issues MODIFY COLUMN value DECIMAL (30,20) NULL, MODIFY COLUMN name VARCHAR (10) NULL");
   }
 
   @Test
@@ -90,13 +92,13 @@ public class AlterColumnsTypeBuilderTest {
   @Test
   public void update_columns_on_oracle() {
     assertThat(createSampleBuilder(new Oracle()).build())
-      .containsOnly("ALTER TABLE issues MODIFY (value NUMERIC (30,20), name VARCHAR (10))");
+      .containsOnly("ALTER TABLE issues MODIFY (value NUMERIC (30,20) NULL, name VARCHAR (10) NULL)");
   }
 
   @Test
-  public void not_nullable_column_are_ignored_on_oracle() {
+  public void update_not_nullable_column_on_oracle() {
     assertThat(createNotNullableBuilder(new Oracle()).build())
-      .containsOnly("ALTER TABLE issues MODIFY (name VARCHAR (10))");
+      .containsOnly("ALTER TABLE issues MODIFY (name VARCHAR (10) NOT NULL)");
   }
 
   @Test
@@ -127,11 +129,11 @@ public class AlterColumnsTypeBuilderTest {
   private AlterColumnsTypeBuilder createNotNullableBuilder(Dialect dialect) {
     return new AlterColumnsTypeBuilder(dialect, TABLE_NAME)
       .updateColumn(
-      newVarcharColumnDefBuilder()
-        .setColumnName("name")
-        .setLimit(10)
-        .setIsNullable(false)
-        .build());
+        newVarcharColumnDefBuilder()
+          .setColumnName("name")
+          .setLimit(10)
+          .setIsNullable(false)
+          .build());
   }
 
 }