AMBARI-11905. Fix idempotent issue for Oracle.(vbrodetskyi)

ambari-server/src/main/java/org/apache/ambari/server/orm/DBAccessor.java

@@ -238,6 +238,23 @@ public interface DBAccessor {
    */
   void executeScript(String filePath) throws SQLException, IOException;
 
+  /**
+   *
+   * @param query update query
+   * @return same like {@code java.sql.Statement}
+   * @throws SQLException
+   */
+  int executeUpdate(String query) throws SQLException;
+
+  /**
+   *
+   * @param query update query
+   * @param ignoreErrors true to ignore errors
+   * @return same like {@code java.sql.Statement}
+   * @throws SQLException
+   */
+  int executeUpdate(String query, boolean ignoreErrors) throws SQLException;
+
   /**
    * Conditional ad-hoc query on DB
    * @param query
@@ -488,8 +505,25 @@ public interface DBAccessor {
   int getColumnType(String tableName, String columnName)
       throws SQLException;
 
+  /**
+   * Get type class of the column
+   * @param tableName name of the table
+   * @param columnName name of the column
+   * @return
+   * @throws SQLException
+   * @throws ClassNotFoundException
+   */
   Class getColumnClass(String tableName, String columnName) throws SQLException, ClassNotFoundException;
 
+  /**
+   * Check if column could be nullable
+   * @param tableName name of the table
+   * @param columnName name of the column
+   * @return true if column could be nullable
+   * @throws SQLException
+   */
+  boolean isColumnNullable(String tableName, String columnName) throws SQLException;
+
   /**
    * Sets the specified column to either allow or prohibit {@code NULL}.
    * 
@@ -508,6 +542,16 @@ public interface DBAccessor {
   void setColumnNullable(String tableName, String columnName, boolean nullable)
     throws SQLException;
 
+  /**
+   * Alter column wrapper, which handle DB specific type conversion
+   * @param tableName name of the table
+   * @param columnName name of the column
+   * @param fromType previous type
+   * @param toType new desired type
+   * @throws SQLException
+   */
+  void changeColumnType(String tableName, String columnName, Class fromType, Class toType) throws SQLException;
+
   enum DbType {
     ORACLE,
     MYSQL,

ambari-server/src/main/java/org/apache/ambari/server/orm/DBAccessorImpl.java

@@ -610,6 +610,26 @@ public class DBAccessorImpl implements DBAccessor {
     return statement.executeUpdate(query.toString());
   }
 
+  @Override
+  public int executeUpdate(String query) throws SQLException{
+    return  executeUpdate(query, false);
+  }
+
+  @Override
+  public int executeUpdate(String query, boolean ignoreErrors) throws SQLException{
+    Statement statement = getConnection().createStatement();
+    try {
+      return statement.executeUpdate(query);
+    } catch (SQLException e){
+      LOG.warn("Error executing query: " + query + ", " +
+                 "errorCode = " + e.getErrorCode() + ", message = " + e.getMessage());
+      if (!ignoreErrors){
+        throw e;
+      }
+    }
+    return 0;  // If error appears and ignoreError is set, return 0 (no changes was made)
+  }
+
   @Override
   public void executeQuery(String query, String tableName, String hasColumnName) throws SQLException{
     if (tableHasColumn(tableName, hasColumnName)){
@@ -755,7 +775,6 @@ public class DBAccessorImpl implements DBAccessor {
     login.setDatabaseURL(configuration.getDatabaseUrl());
     login.setDriverClassName(configuration.getDatabaseDriver());
 
-
     return new DatabaseSessionImpl(login);
   }
 
@@ -771,7 +790,6 @@ public class DBAccessorImpl implements DBAccessor {
     } else if (rs != null){
       return rs.next();
     }
-
     return false;
   }
 
@@ -781,27 +799,35 @@ public class DBAccessorImpl implements DBAccessor {
     // We doesn't require any actual result except metadata, so WHERE clause shouldn't match
     String query = String.format("SELECT %s FROM %s WHERE 1=2", columnName, convertObjectName(tableName));
     ResultSet rs = executeSelect(query);
-
     ResultSetMetaData rsmd = rs.getMetaData();
     return rsmd.getColumnType(1);
   }
 
+  private ResultSetMetaData getColumnMetadata(String tableName, String columnName) throws SQLException{
+    // We doesn't require any actual result except metadata, so WHERE clause shouldn't match
+    String query = String.format("SELECT %s FROM %s WHERE 1=2", convertObjectName(columnName), convertObjectName(tableName));
+    ResultSet rs = executeSelect(query);
+    return rs.getMetaData();
+  }
+
   @Override
   public Class getColumnClass(String tableName, String columnName)
           throws SQLException, ClassNotFoundException{
-      // We doesn't require any actual result except metadata, so WHERE clause shouldn't match
-      String query = String.format("SELECT %s FROM %s WHERE 1=2", columnName, convertObjectName(tableName));
-      ResultSet rs = executeSelect(query);
-
-      ResultSetMetaData rsmd = rs.getMetaData();
+      ResultSetMetaData rsmd = getColumnMetadata(tableName, columnName);
       return Class.forName(rsmd.getColumnClassName(1));
   }
 
+  @Override
+  public boolean isColumnNullable(String tableName, String columnName) throws SQLException{
+    ResultSetMetaData rsmd = getColumnMetadata(tableName, columnName);
+    return !(rsmd.isNullable(1) == ResultSetMetaData.columnNoNulls);
+  }
+
   @Override
   public void setColumnNullable(String tableName, DBAccessor.DBColumnInfo columnInfo, boolean nullable)
       throws SQLException {
-    String statement = dbmsHelper.getSetNullableStatement(tableName, columnInfo, nullable);
 
+    String statement = dbmsHelper.getSetNullableStatement(tableName, columnInfo, nullable);
     executeQuery(statement);
   }
 
@@ -809,12 +835,46 @@ public class DBAccessorImpl implements DBAccessor {
   public void setColumnNullable(String tableName, String columnName, boolean nullable)
           throws SQLException {
     try {
-      Class columnClass = getColumnClass(tableName, columnName);
-      String query = dbmsHelper.getSetNullableStatement(tableName, new DBColumnInfo(columnName, columnClass), nullable);
-      executeQuery(query);
+      // if column is already in nullable state, we shouldn't do anything. This is important for Oracle
+      if (isColumnNullable(tableName, columnName) != nullable) {
+        Class columnClass = getColumnClass(tableName, columnName);
+        String query = dbmsHelper.getSetNullableStatement(tableName, new DBColumnInfo(columnName, columnClass), nullable);
+        executeQuery(query);
+      } else {
+        LOG.info("Column nullability property is not changed due to {} column from {} table is already in {} state, skipping",
+                   columnName, tableName, (nullable)?"nullable":"not nullable");
+      }
     } catch (ClassNotFoundException e) {
       LOG.error("Could not modify table=[], column={}, error={}", tableName, columnName, e.getMessage());
     }
   }
 
+  @Override
+  public void changeColumnType(String tableName, String columnName, Class fromType, Class toType) throws SQLException {
+    // ToDo: create column with more random name
+    String tempColumnName = columnName + "_temp";
+
+    switch (configuration.getDatabaseType()){
+      case ORACLE:
+        // ToDo: add check, if target column is a part of constraint.
+        // oracle doesn't support direct type change from varchar2 -> clob
+        if (String.class.equals(fromType) && (Character[].class.equals(toType) || char[].class.equals(toType))){
+          addColumn(tableName, new DBColumnInfo(tempColumnName, toType));
+          executeUpdate(String.format("UPDATE %s SET %s = %s", convertObjectName(tableName),
+                                       convertObjectName(tempColumnName), convertObjectName(columnName)));
+          dropColumn(tableName, columnName);
+          renameColumn(tableName,tempColumnName, new DBColumnInfo(columnName, toType));
+          return;
+        }
+    }
+
+    alterColumn(tableName, new DBColumnInfo(columnName, toType, null));
+  }
+
+
+
+
+
+
+
 }

ambari-server/src/main/java/org/apache/ambari/server/orm/helpers/dbms/OracleHelper.java

@@ -52,4 +52,15 @@ public class OracleHelper extends GenericDbmsHelper {
     builder.append(nullStatement);
     return builder;
   }
+
+  @Override
+  public String writeGetTableConstraints(String databaseName, String tableName) {
+    StringBuilder statement = new StringBuilder()
+                                .append("SELECT CONSTRAINT_NAME as constraint_name, CONSTRAINT_TYPE as constraint_type ")
+                                .append("FROM USER_CONSTRAINTS ")
+                                .append("WHERE ")
+                                .append("USER_CONSTRAINTS.TABLE_NAME='").append(tableName.toUpperCase()).append("'");
+    return statement.toString();
+  }
+
 }

ambari-server/src/main/java/org/apache/ambari/server/upgrade/UpgradeCatalog210.java

@@ -269,12 +269,14 @@ public class UpgradeCatalog210 extends AbstractUpgradeCatalog {
   private void executeAlertDDLUpdates() throws AmbariException, SQLException {
     //Fix latest_text column type to match for all DBMS
     Configuration.DatabaseType databaseType = configuration.getDatabaseType();
+
+    // MySQL columns are already TEXT, but we need to be sure in that, since LONGTEXT will really slowdown database when querying the alerts too often
     if (Configuration.DatabaseType.MYSQL == databaseType) {
       dbAccessor.alterColumn("alert_current", new DBColumnInfo("latest_text", new FieldTypeDefinition("TEXT"), null));
       dbAccessor.alterColumn("alert_history", new DBColumnInfo("alert_text", new FieldTypeDefinition("TEXT"), null));
     } else {
-      dbAccessor.alterColumn("alert_current", new DBColumnInfo("latest_text", Character[].class, null));
-      dbAccessor.alterColumn("alert_history", new DBColumnInfo("alert_text", Character[].class, null));
+      dbAccessor.changeColumnType("alert_current", "latest_text", String.class, char[].class);
+      dbAccessor.changeColumnType("alert_history", "alert_text", String.class, char[].class);
     }
 
   }

ambari-server/src/test/java/org/apache/ambari/server/upgrade/UpgradeCatalog210Test.java

@@ -623,17 +623,27 @@ public class UpgradeCatalog210Test {
    * Verify alert changes
    */
   class AlertSectionDDL implements SectionDDL {
+    HashMap<String, Capture<String>> stringCaptures;
+    HashMap<String, Capture<Class>> classCaptures;
 
-    HashMap<String, Capture<DBColumnInfo>> captures;
 
     public AlertSectionDDL() {
-      captures = new HashMap<String, Capture<DBColumnInfo>>();
-
-      Capture<DBAccessor.DBColumnInfo> alertCurrentColumnCapture = new Capture<DBAccessor.DBColumnInfo>();
-      Capture<DBAccessor.DBColumnInfo> alertHistoryColumnCapture = new Capture<DBAccessor.DBColumnInfo>();
-
-      captures.put("alert_current", alertCurrentColumnCapture);
-      captures.put("alert_history", alertHistoryColumnCapture);
+      stringCaptures = new HashMap<String, Capture<String>>();
+      classCaptures = new HashMap<String, Capture<Class>>();
+
+      Capture<String> textCaptureC = new Capture<String>();
+      Capture<String> textCaptureH = new Capture<String>();
+      Capture<Class>  classFromC = new Capture<Class>();
+      Capture<Class>  classFromH = new Capture<Class>();
+      Capture<Class>  classToC = new Capture<Class>();
+      Capture<Class>  classToH = new Capture<Class>();
+
+      stringCaptures.put("textCaptureC", textCaptureC);
+      stringCaptures.put("textCaptureH", textCaptureH);
+      classCaptures.put("classFromC", classFromC);
+      classCaptures.put("classFromH", classFromH);
+      classCaptures.put("classToC", classToC);
+      classCaptures.put("classToH", classToH);
     }
 
     /**
@@ -641,11 +651,15 @@ public class UpgradeCatalog210Test {
      */
     @Override
     public void execute(DBAccessor dbAccessor) throws SQLException {
-      Capture<DBColumnInfo> alertCurrentColumnCapture = captures.get("alert_current");
-      Capture<DBColumnInfo> alertHistoryColumnCapture = captures.get("alert_history");
-
-      dbAccessor.alterColumn(eq("alert_current"), capture(alertCurrentColumnCapture));
-      dbAccessor.alterColumn(eq("alert_history"), capture(alertHistoryColumnCapture));
+      Capture<String> textCaptureC = stringCaptures.get("textCaptureC");
+      Capture<String> textCaptureH = stringCaptures.get("textCaptureH");
+      Capture<Class>  classFromC = classCaptures.get("classFromC");
+      Capture<Class>  classFromH = classCaptures.get("classFromH");
+      Capture<Class>  classToC = classCaptures.get("classToC");
+      Capture<Class>  classToH = classCaptures.get("classToH");
+
+      dbAccessor.changeColumnType(eq("alert_current"), capture(textCaptureC), capture(classFromC), capture(classToC));
+      dbAccessor.changeColumnType(eq("alert_history"), capture(textCaptureH), capture(classFromH), capture(classToH));
     }
 
     /**
@@ -653,20 +667,20 @@ public class UpgradeCatalog210Test {
      */
     @Override
     public void verify(DBAccessor dbAccessor) throws SQLException {
-      verifyAlertCurrent(captures.get("alert_current"));
-      verifyAlertHistory(captures.get("alert_history"));
-    }
-
-    private void verifyAlertCurrent(Capture<DBAccessor.DBColumnInfo> alertCurrentColumnCapture) {
-      DBColumnInfo latestTextColumn = alertCurrentColumnCapture.getValue();
-      Assert.assertEquals(Character[].class, latestTextColumn.getType());
-      Assert.assertEquals("latest_text", latestTextColumn.getName());
-    }
-
-    private void verifyAlertHistory(Capture<DBAccessor.DBColumnInfo> alertHistoryColumnCapture) {
-      DBColumnInfo alertTextColumn = alertHistoryColumnCapture.getValue();
-      Assert.assertEquals(Character[].class, alertTextColumn.getType());
-      Assert.assertEquals("alert_text", alertTextColumn.getName());
+      Capture<String> textCaptureC = stringCaptures.get("textCaptureC");
+      Capture<String> textCaptureH = stringCaptures.get("textCaptureH");
+      Capture<Class>  classFromC = classCaptures.get("classFromC");
+      Capture<Class>  classFromH = classCaptures.get("classFromH");
+      Capture<Class>  classToC = classCaptures.get("classToC");
+      Capture<Class>  classToH = classCaptures.get("classToH");
+
+      Assert.assertEquals("latest_text", textCaptureC.getValue());
+      Assert.assertEquals(String.class, classFromC.getValue());
+      Assert.assertEquals(char[].class, classToC.getValue());
+
+      Assert.assertEquals("alert_text", textCaptureH.getValue());
+      Assert.assertEquals(String.class, classFromH.getValue());
+      Assert.assertEquals(char[].class, classToH.getValue());
     }
   }
 }