From a327a95c3552ffb01f13b49074a2937b1aa2c9e0 Mon Sep 17 00:00:00 2001
From: =?utf8?q?S=C3=A9bastien=20Lesaint?=
 <sebastien.lesaint@sonarsource.com>
Date: Wed, 8 Jun 2016 10:22:22 +0200
Subject: [PATCH] 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
---
 .../sonar/db/version/AlterColumnsBuilder.java | 12 ++++---
 .../java/org/sonar/db/version/DdlChange.java  | 35 +++++++++++++++++--
 .../db/version/AlterColumnsBuilderTest.java   | 12 +++++--
 3 files changed, 50 insertions(+), 9 deletions(-)

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 0ae0e72a7c7..da9765e428f 100644
--- 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 8c03055856f..7d2606fc563 100644
--- 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 f505b07b545..f38f225be97 100644
--- 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
-- 
2.39.5