DB migration: make alter column re-entrant on Oracle

this is achieved by handling errors ORA-01451 and ORA-01442 when statement is executed and executing it again without the confliction NULL or NOT NULL
also, altering multiple columns in the same statement on Oracle had to be removed to ensure correct handling of these errors

diff --git a/sonar-db/src/main/java/org/sonar/db/version/AlterColumnsBuilder.java b/sonar-db/src/main/java/org/sonar/db/version/AlterColumnsBuilder.java
index 0ae0e72a7c71d535a9dafd45b1204b6748e6cc4b..da9765e428feecbc76b9ccd13ddd62bd046cc711 100644 (file)
--- a/sonar-db/src/main/java/org/sonar/db/version/AlterColumnsBuilder.java
+++ b/sonar-db/src/main/java/org/sonar/db/version/AlterColumnsBuilder.java
@@ -98,10 +98,14 @@ public class AlterColumnsBuilder {
   }
 
   private List<String> createOracleQuery() {
-    StringBuilder sql = new StringBuilder(ALTER_TABLE + tableName + " ").append("MODIFY (");
-    addColumns(sql, "", "", true);
-    sql.append(")");
-    return Collections.singletonList(sql.toString());
+    List<String> sqls = new ArrayList<>();
+    for (ColumnDef columnDef : columnDefs) {
+      StringBuilder sql = new StringBuilder(ALTER_TABLE + tableName + " ").append("MODIFY (");
+      addColumn(sql, columnDef, "", true);
+      sql.append(")");
+      sqls.add(sql.toString());
+    }
+    return sqls;
   }
 
   private List<String> createMsSqlAndH2Queries() {

diff --git a/sonar-db/src/main/java/org/sonar/db/version/DdlChange.java b/sonar-db/src/main/java/org/sonar/db/version/DdlChange.java
index 8c03055856f46e4a82df7e96b2d61b14de4671a4..7d2606fc5632ccbbd68486490f1142060509bbdd 100644 (file)
--- a/sonar-db/src/main/java/org/sonar/db/version/DdlChange.java
+++ b/sonar-db/src/main/java/org/sonar/db/version/DdlChange.java
@@ -22,11 +22,12 @@ package org.sonar.db.version;
 import java.sql.Connection;
 import java.sql.SQLException;
 import java.util.List;
+import java.util.regex.Pattern;
 import org.apache.commons.dbutils.DbUtils;
 import org.sonar.db.Database;
 import org.sonar.db.dialect.Dialect;
 
-import static java.lang.String.*;
+import static java.lang.String.format;
 import static java.util.Arrays.asList;
 
 public abstract class DdlChange implements MigrationStep {
@@ -62,6 +63,10 @@ public abstract class DdlChange implements MigrationStep {
   }
 
   public static class Context {
+    private static final int ERROR_HANDLING_THRESHOLD = 10;
+    // the tricky regexp is required to match "NULL" but not "NOT NULL"
+    private final Pattern nullPattern = Pattern.compile("\\h?(?<!NOT )NULL");
+    private final Pattern notNullPattern = Pattern.compile("\\h?NOT NULL");
     private final Connection writeConnection;
 
     public Context(Connection writeConnection) {
@@ -69,10 +74,36 @@ public abstract class DdlChange implements MigrationStep {
     }
 
     public void execute(String sql) throws SQLException {
+      execute(sql, sql, 0);
+    }
+
+    public void execute(String original, String sql, int errorCount) throws SQLException {
       try {
         UpsertImpl.create(writeConnection, sql).execute().commit();
+      } catch (SQLException e) {
+        if (errorCount < ERROR_HANDLING_THRESHOLD) {
+          String message = e.getMessage();
+          if (message.contains("ORA-01451")) {
+            String newSql = nullPattern.matcher(sql).replaceFirst("");
+            execute(original, newSql, errorCount + 1);
+            return;
+          } else if (message.contains("ORA-01442")) {
+            String newSql = notNullPattern.matcher(sql).replaceFirst("");
+            execute(original, newSql, errorCount + 1);
+            return;
+          }
+        }
+        throw new IllegalStateException(messageForIseOf(original, sql, errorCount), e);
       } catch (Exception e) {
-        throw new IllegalStateException(format("Fail to execute %s", sql), e);
+        throw new IllegalStateException(messageForIseOf(original, sql, errorCount), e);
+      }
+    }
+
+    private static String messageForIseOf(String original, String sql, int errorCount) {
+      if (!original.equals(sql) || errorCount > 0) {
+        return format("Fail to execute %s %n (caught %s error, original was %s)", sql, errorCount, original);
+      } else {
+        return format("Fail to execute %s", sql);
       }
     }
 

diff --git a/sonar-db/src/test/java/org/sonar/db/version/AlterColumnsBuilderTest.java b/sonar-db/src/test/java/org/sonar/db/version/AlterColumnsBuilderTest.java
index f505b07b545c719b867055607ec62924f0425d3d..f38f225be97196e71864128b61ed89ae4dfc6b30 100644 (file)
--- a/sonar-db/src/test/java/org/sonar/db/version/AlterColumnsBuilderTest.java
+++ b/sonar-db/src/test/java/org/sonar/db/version/AlterColumnsBuilderTest.java
@@ -42,7 +42,9 @@ public class AlterColumnsBuilderTest {
   @Test
   public void update_columns_on_h2() {
     assertThat(createSampleBuilder(new H2()).build())
-      .containsOnly("ALTER TABLE issues ALTER COLUMN value DOUBLE NULL", "ALTER TABLE issues ALTER COLUMN name VARCHAR (10) NULL");
+      .containsOnly(
+        "ALTER TABLE issues ALTER COLUMN value DOUBLE NULL",
+        "ALTER TABLE issues ALTER COLUMN name VARCHAR (10) NULL");
   }
 
   @Test
@@ -54,7 +56,9 @@ public class AlterColumnsBuilderTest {
   @Test
   public void update_columns_on_mssql() {
     assertThat(createSampleBuilder(new MsSql()).build())
-      .containsOnly("ALTER TABLE issues ALTER COLUMN value DECIMAL (30,20) NULL", "ALTER TABLE issues ALTER COLUMN name NVARCHAR (10) NULL");
+      .containsOnly(
+        "ALTER TABLE issues ALTER COLUMN value DECIMAL (30,20) NULL",
+        "ALTER TABLE issues ALTER COLUMN name NVARCHAR (10) NULL");
   }
 
   @Test
@@ -92,7 +96,9 @@ public class AlterColumnsBuilderTest {
   @Test
   public void update_columns_on_oracle() {
     assertThat(createSampleBuilder(new Oracle()).build())
-      .containsOnly("ALTER TABLE issues MODIFY (value NUMERIC (30,20) NULL, name VARCHAR (10) NULL)");
+      .containsOnly(
+        "ALTER TABLE issues MODIFY (value NUMERIC (30,20) NULL)",
+        "ALTER TABLE issues MODIFY (name VARCHAR (10) NULL)");
   }
 
   @Test