ambari-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From jonathanhur...@apache.org
Subject ambari git commit: AMBARI-18057 - NullPointerException When Retrieving Cluster via API Due To Database Inconsistency (jonathanhurley)
Date Mon, 08 Aug 2016 04:03:31 GMT
Repository: ambari
Updated Branches:
  refs/heads/branch-2.4 0f89b863b -> 022285dd3


AMBARI-18057 - NullPointerException When Retrieving Cluster via API Due To Database Inconsistency
(jonathanhurley)


Project: http://git-wip-us.apache.org/repos/asf/ambari/repo
Commit: http://git-wip-us.apache.org/repos/asf/ambari/commit/022285dd
Tree: http://git-wip-us.apache.org/repos/asf/ambari/tree/022285dd
Diff: http://git-wip-us.apache.org/repos/asf/ambari/diff/022285dd

Branch: refs/heads/branch-2.4
Commit: 022285dd3bf7bc6aaf012b96f607922a0abe7c73
Parents: 0f89b86
Author: Jonathan Hurley <jhurley@hortonworks.com>
Authored: Sun Aug 7 16:07:08 2016 -0400
Committer: Jonathan Hurley <jhurley@hortonworks.com>
Committed: Mon Aug 8 00:01:57 2016 -0400

----------------------------------------------------------------------
 .../apache/ambari/server/orm/DBAccessor.java    |  29 ++++-
 .../ambari/server/orm/DBAccessorImpl.java       | 114 +++++++++++++++----
 .../server/orm/entities/AlertCurrentEntity.java |   8 --
 .../server/orm/helpers/dbms/DbmsHelper.java     |  10 ++
 .../orm/helpers/dbms/GenericDbmsHelper.java     |  34 ++++++
 .../server/orm/helpers/dbms/OracleHelper.java   |  16 +++
 .../ambari/server/orm/DBAccessorImplTest.java   |  75 +++++++++---
 7 files changed, 241 insertions(+), 45 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/ambari/blob/022285dd/ambari-server/src/main/java/org/apache/ambari/server/orm/DBAccessor.java
----------------------------------------------------------------------
diff --git a/ambari-server/src/main/java/org/apache/ambari/server/orm/DBAccessor.java b/ambari-server/src/main/java/org/apache/ambari/server/orm/DBAccessor.java
index 5f126f6..488c01e 100644
--- a/ambari-server/src/main/java/org/apache/ambari/server/orm/DBAccessor.java
+++ b/ambari-server/src/main/java/org/apache/ambari/server/orm/DBAccessor.java
@@ -23,6 +23,7 @@ import java.sql.SQLException;
 import java.util.List;
 
 import org.apache.ambari.server.configuration.Configuration.DatabaseType;
+import org.eclipse.jdt.internal.compiler.ast.FieldDeclaration;
 import org.eclipse.persistence.internal.databaseaccess.FieldTypeDefinition;
 import org.eclipse.persistence.sessions.DatabaseSession;
 
@@ -138,8 +139,22 @@ public interface DBAccessor {
                               String referenceTableName,
                               String[] referenceColumns,
                               boolean ignoreFailure) throws SQLException;
+
   /**
-   * Add column to existing table
+   * Adds a column to an existing table. This method will honor the
+   * {@link DBColumnInfo#isNullable} and {@link DBColumnInfo#getDefaultValue()}
+   * methods and create a new column that has a {@code DEFAULT} constraint.
+   * <p/>
+   * Oracle is, of course, the exception here since their syntax doesn't conform
+   * to widely accepted SQL standards. Because they switch the order of the
+   * {@code NULL} constraint and the {@code DEFAULT} constraint, we need to
+   * create the table in a non-atomic fashion. This is because the EclipseLink
+   * {@link FieldDeclaration} class hard codes the order of the constraints. In
+   * the case of Oracle, we alter the nullability of the table after its
+   * creation.
+   * <p/>
+   * No work is performed if the column already exists.
+   *
    * @param tableName
    * @param columnInfo
    * @throws SQLException
@@ -580,6 +595,18 @@ public interface DBAccessor {
    */
   void dropPKConstraint(String tableName, String defaultConstraintName) throws SQLException;
 
+  /**
+   * Adds a default constraint to an existing column.
+   *
+   * @param tableName
+   *          the table where the column is defined (not {@code null}).
+   * @param column
+   *          the column information which contains the default value (not
+   *          {@code null}).
+   * @throws SQLException
+   */
+  void addDefaultConstraint(String tableName, DBColumnInfo column) throws SQLException;
+
   enum DbType {
     ORACLE,
     MYSQL,

http://git-wip-us.apache.org/repos/asf/ambari/blob/022285dd/ambari-server/src/main/java/org/apache/ambari/server/orm/DBAccessorImpl.java
----------------------------------------------------------------------
diff --git a/ambari-server/src/main/java/org/apache/ambari/server/orm/DBAccessorImpl.java
b/ambari-server/src/main/java/org/apache/ambari/server/orm/DBAccessorImpl.java
index ea5f496..bb3f826 100644
--- a/ambari-server/src/main/java/org/apache/ambari/server/orm/DBAccessorImpl.java
+++ b/ambari-server/src/main/java/org/apache/ambari/server/orm/DBAccessorImpl.java
@@ -550,18 +550,52 @@ public class DBAccessorImpl implements DBAccessor {
 
   }
 
+  /**
+   * {@inheritDoc}
+   */
   @Override
   public void addColumn(String tableName, DBColumnInfo columnInfo) throws SQLException {
-    if (!tableHasColumn(tableName, columnInfo.getName())) {
-      //TODO workaround for default values, possibly we will have full support later
-      if (columnInfo.getDefaultValue() != null) {
-        columnInfo.setNullable(true);
-      }
-      String query = dbmsHelper.getAddColumnStatement(tableName, columnInfo);
-      executeQuery(query);
+    if (tableHasColumn(tableName, columnInfo.getName())) {
+      return;
+    }
+
+    DatabaseType databaseType = configuration.getDatabaseType();
+    switch (databaseType) {
+      case ORACLE: {
+        // capture the original null value and set the column to nullable if
+        // there is a default value
+        boolean originalNullable = columnInfo.isNullable();
+        if (columnInfo.getDefaultValue() != null) {
+          columnInfo.setNullable(true);
+        }
+
+        String query = dbmsHelper.getAddColumnStatement(tableName, columnInfo);
+        executeQuery(query);
+
+        // update the column after it's been created with the default value and
+        // then set the nullable field back to the specified value
+        if (columnInfo.getDefaultValue() != null) {
+          updateTable(tableName, columnInfo.getName(), columnInfo.getDefaultValue(), "");
 
-      if (columnInfo.getDefaultValue() != null) {
-        updateTable(tableName, columnInfo.getName(), columnInfo.getDefaultValue(), "");
+          // if the column wasn't originally nullable, then set that here
+          if (!originalNullable) {
+            setColumnNullable(tableName, columnInfo, originalNullable);
+          }
+
+          // finally, add the DEFAULT constraint to the table
+          addDefaultConstraint(tableName, columnInfo);
+        }
+        break;
+      }
+      case DERBY:
+      case MYSQL:
+      case POSTGRES:
+      case SQL_ANYWHERE:
+      case SQL_SERVER:
+      default: {
+        String query = dbmsHelper.getAddColumnStatement(tableName, columnInfo);
+        executeQuery(query);
+        break;
       }
     }
   }
@@ -733,16 +767,7 @@ public class DBAccessorImpl implements DBAccessor {
           String whereClause) throws SQLException {
 
     StringBuilder query = new StringBuilder(String.format("UPDATE %s SET %s = ", tableName,
columnName));
-
-    // Only String and number supported.
-    // Taken from: org.eclipse.persistence.internal.databaseaccess.appendParameterInternal
-    Object dbValue = databasePlatform.convertToDatabaseType(value);
-    String valueString = value.toString();
-    if (dbValue instanceof String) {
-      valueString = "'" + value.toString() + "'";
-    }
-
-    query.append(valueString);
+    query.append(escapeParameter(value));
     query.append(" ");
     query.append(whereClause);
 
@@ -1200,4 +1225,55 @@ public class DBAccessorImpl implements DBAccessor {
       dropPKConstraint(tableName, primaryKeyConstraintName, true);
     }
   }
+
+  /**
+   * {@inheritDoc}
+   */
+  @Override
+  public void addDefaultConstraint(String tableName, DBColumnInfo column) throws SQLException
{
+    String defaultValue = escapeParameter(column.getDefaultValue());
+    StringBuilder builder = new StringBuilder(String.format("ALTER TABLE %s ", tableName));
+
+    DatabaseType databaseType = configuration.getDatabaseType();
+    switch (databaseType) {
+      case DERBY:
+      case MYSQL:
+      case POSTGRES:
+      case SQL_ANYWHERE:
+        builder.append(String.format("ALTER %s SET DEFAULT %s", column.getName(), defaultValue));
+      case ORACLE:
+        builder.append(String.format("MODIFY %s DEFAULT %s", column.getName(), defaultValue));
+        break;
+      case SQL_SERVER:
+        builder.append(
+            String.format("ALTER COLUMN %s SET DEFAULT %s", column.getName(), defaultValue));
+        break;
+      default:
+        builder.append(String.format("ALTER %s SET DEFAULT %s", column.getName(), defaultValue));
+        break;
+    }
+
+    executeQuery(builder.toString());
+  }
+
+  /**
+   * Gets an escaped version of the specified value suitable for including as a
+   * parameter when building statements.
+   *
+   * @param value
+   *          the value to escape
+   * @return the escaped value
+   */
+  protected String escapeParameter(Object value) {
+    // Only String and number supported.
+    // Taken from:
+    // org.eclipse.persistence.internal.databaseaccess.appendParameterInternal
+    Object dbValue = databasePlatform.convertToDatabaseType(value);
+    String valueString = value.toString();
+    if (dbValue instanceof String) {
+      valueString = "'" + value.toString() + "'";
+    }
+
+    return valueString;
+  }
 }

http://git-wip-us.apache.org/repos/asf/ambari/blob/022285dd/ambari-server/src/main/java/org/apache/ambari/server/orm/entities/AlertCurrentEntity.java
----------------------------------------------------------------------
diff --git a/ambari-server/src/main/java/org/apache/ambari/server/orm/entities/AlertCurrentEntity.java
b/ambari-server/src/main/java/org/apache/ambari/server/orm/entities/AlertCurrentEntity.java
index 77f5acc..b30b100 100644
--- a/ambari-server/src/main/java/org/apache/ambari/server/orm/entities/AlertCurrentEntity.java
+++ b/ambari-server/src/main/java/org/apache/ambari/server/orm/entities/AlertCurrentEntity.java
@@ -284,14 +284,6 @@ public class AlertCurrentEntity {
   }
 
   /**
-   * @param occurrences
-   *          the occurrences to set
-   */
-  public void setOccurrences(Long occurrences) {
-    this.occurrences = occurrences;
-  }
-
-  /**
    * Gets the firmness of the alert, indicating whether or not it could be a
    * potential false positive.
    *

http://git-wip-us.apache.org/repos/asf/ambari/blob/022285dd/ambari-server/src/main/java/org/apache/ambari/server/orm/helpers/dbms/DbmsHelper.java
----------------------------------------------------------------------
diff --git a/ambari-server/src/main/java/org/apache/ambari/server/orm/helpers/dbms/DbmsHelper.java
b/ambari-server/src/main/java/org/apache/ambari/server/orm/helpers/dbms/DbmsHelper.java
index 30c06fb..1fe65db 100644
--- a/ambari-server/src/main/java/org/apache/ambari/server/orm/helpers/dbms/DbmsHelper.java
+++ b/ambari-server/src/main/java/org/apache/ambari/server/orm/helpers/dbms/DbmsHelper.java
@@ -123,4 +123,14 @@ public interface DbmsHelper {
    * @return the statement (never {@code null}).
    */
   String getSetNullableStatement(String tableName, DBAccessor.DBColumnInfo columnInfo, boolean
nullable);
+
+  /**
+   * Gets whether the database platform supports adding contraints after the
+   * {@code NULL} constraint. Some database, such as Oracle, don't allow this.
+   * Unfortunately, EclipsLink hard codes the order of constraints.
+   *
+   * @return {@code true} if contraints can be added after the {@code NULL}
+   *         constraint, {@code false} otherwise.
+   */
+  boolean isConstraintSupportedAfterNullability();
 }

http://git-wip-us.apache.org/repos/asf/ambari/blob/022285dd/ambari-server/src/main/java/org/apache/ambari/server/orm/helpers/dbms/GenericDbmsHelper.java
----------------------------------------------------------------------
diff --git a/ambari-server/src/main/java/org/apache/ambari/server/orm/helpers/dbms/GenericDbmsHelper.java
b/ambari-server/src/main/java/org/apache/ambari/server/orm/helpers/dbms/GenericDbmsHelper.java
index 21fa361..042b4d2 100644
--- a/ambari-server/src/main/java/org/apache/ambari/server/orm/helpers/dbms/GenericDbmsHelper.java
+++ b/ambari-server/src/main/java/org/apache/ambari/server/orm/helpers/dbms/GenericDbmsHelper.java
@@ -205,6 +205,11 @@ public class GenericDbmsHelper implements DbmsHelper {
     int length = columnInfo.getLength() != null ? columnInfo.getLength() : 0;
     FieldDefinition fieldDefinition = new FieldDefinition(columnInfo.getName(), columnInfo.getType(),
length);
     fieldDefinition.setShouldAllowNull(columnInfo.isNullable());
+
+    if (null != columnInfo.getDefaultValue() && isConstraintSupportedAfterNullability())
{
+      fieldDefinition.setConstraint("DEFAULT " + escapeParameter(columnInfo.getDefaultValue()));
+    }
+
     return fieldDefinition;
   }
 
@@ -384,4 +389,33 @@ public class GenericDbmsHelper implements DbmsHelper {
       }
     };
   }
+
+  /**
+   * {@inheritDoc}
+   */
+  @Override
+  public boolean isConstraintSupportedAfterNullability() {
+    return true;
+  }
+
+  /**
+   * Gets an escaped version of the specified value suitable for including as a
+   * parameter when building statements.
+   *
+   * @param value
+   *          the value to escape
+   * @return the escaped value
+   */
+  protected String escapeParameter(Object value) {
+    // Only String and number supported.
+    // Taken from:
+    // org.eclipse.persistence.internal.databaseaccess.appendParameterInternal
+    Object dbValue = databasePlatform.convertToDatabaseType(value);
+    String valueString = value.toString();
+    if (dbValue instanceof String) {
+      valueString = "'" + value.toString() + "'";
+    }
+
+    return valueString;
+  }
 }

http://git-wip-us.apache.org/repos/asf/ambari/blob/022285dd/ambari-server/src/main/java/org/apache/ambari/server/orm/helpers/dbms/OracleHelper.java
----------------------------------------------------------------------
diff --git a/ambari-server/src/main/java/org/apache/ambari/server/orm/helpers/dbms/OracleHelper.java
b/ambari-server/src/main/java/org/apache/ambari/server/orm/helpers/dbms/OracleHelper.java
index 88ef8fe..b5955b4 100644
--- a/ambari-server/src/main/java/org/apache/ambari/server/orm/helpers/dbms/OracleHelper.java
+++ b/ambari-server/src/main/java/org/apache/ambari/server/orm/helpers/dbms/OracleHelper.java
@@ -63,4 +63,20 @@ public class OracleHelper extends GenericDbmsHelper {
     return statement.toString();
   }
 
+  /**
+   * {@inheritDoc}
+   * <p/>
+   * Oracle supports the format:
+   *
+   * <pre>
+   * ALTER TABLE foo ADD COLUMN bar varchar2(32) DEFAULT 'baz' NOT NULL
+   * </pre>
+   *
+   * This syntax doesn't allow contraints added after the {@code NULL}
+   * constraint.
+   */
+  @Override
+  public boolean isConstraintSupportedAfterNullability() {
+    return false;
+  }
 }

http://git-wip-us.apache.org/repos/asf/ambari/blob/022285dd/ambari-server/src/test/java/org/apache/ambari/server/orm/DBAccessorImplTest.java
----------------------------------------------------------------------
diff --git a/ambari-server/src/test/java/org/apache/ambari/server/orm/DBAccessorImplTest.java
b/ambari-server/src/test/java/org/apache/ambari/server/orm/DBAccessorImplTest.java
index ac8bea1..9dcaeef 100644
--- a/ambari-server/src/test/java/org/apache/ambari/server/orm/DBAccessorImplTest.java
+++ b/ambari-server/src/test/java/org/apache/ambari/server/orm/DBAccessorImplTest.java
@@ -19,9 +19,13 @@
 package org.apache.ambari.server.orm;
 
 import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertTrue;
 import static org.junit.matchers.JUnitMatchers.containsString;
 
+import java.io.ByteArrayInputStream;
+import java.sql.Clob;
+import java.sql.Connection;
+import java.sql.DatabaseMetaData;
+import java.sql.PreparedStatement;
 import java.sql.ResultSet;
 import java.sql.ResultSetMetaData;
 import java.sql.SQLException;
@@ -33,8 +37,6 @@ import java.util.Map;
 import java.util.Vector;
 import java.util.concurrent.atomic.AtomicInteger;
 
-import junit.framework.Assert;
-
 import org.apache.ambari.server.orm.DBAccessor.DBColumnInfo;
 import org.eclipse.persistence.sessions.DatabaseSession;
 import org.junit.After;
@@ -45,9 +47,8 @@ import org.junit.rules.ExpectedException;
 
 import com.google.inject.Guice;
 import com.google.inject.Injector;
-import java.io.ByteArrayInputStream;
-import java.sql.Clob;
-import java.sql.PreparedStatement;
+
+import junit.framework.Assert;
 
 public class DBAccessorImplTest {
   private Injector injector;
@@ -494,32 +495,32 @@ public class DBAccessorImplTest {
     createMyTable(tableName);
     DBAccessorImpl dbAccessor = injector.getInstance(DBAccessorImpl.class);
 
-    dbAccessor.addColumn(tableName, new DBColumnInfo("isNullable",
-        String.class, 1000, "test", false));
+    // create a column with a non-NULL constraint
+    DBColumnInfo dbColumnInfo = new DBColumnInfo("isNullable", String.class, 1000, "test",
false);
+    dbAccessor.addColumn(tableName, dbColumnInfo);
 
     Statement statement = dbAccessor.getConnection().createStatement();
-    ResultSet resultSet = statement.executeQuery("SELECT isNullable FROM "
-        + tableName);
+    ResultSet resultSet = statement.executeQuery("SELECT isNullable FROM " + tableName);
     ResultSetMetaData rsmd = resultSet.getMetaData();
-    assertEquals(ResultSetMetaData.columnNullable, rsmd.isNullable(1));
+    assertEquals(ResultSetMetaData.columnNoNulls, rsmd.isNullable(1));
 
     statement.close();
 
-    dbAccessor.setColumnNullable(tableName, new DBColumnInfo("isNullable",
-                                                              String.class, 1000, "test",
false), false);
+    // set it to nullable
+    dbAccessor.setColumnNullable(tableName, dbColumnInfo, true);
     statement = dbAccessor.getConnection().createStatement();
     resultSet = statement.executeQuery("SELECT isNullable FROM " + tableName);
     rsmd = resultSet.getMetaData();
-    assertEquals(ResultSetMetaData.columnNoNulls, rsmd.isNullable(1));
+    assertEquals(ResultSetMetaData.columnNullable, rsmd.isNullable(1));
 
     statement.close();
 
-    dbAccessor.setColumnNullable(tableName, new DBColumnInfo("isNullable",
-                                                              String.class, 1000, "test",
false), true);
+    // set it back to non-NULL
+    dbAccessor.setColumnNullable(tableName, dbColumnInfo, false);
     statement = dbAccessor.getConnection().createStatement();
     resultSet = statement.executeQuery("SELECT isNullable FROM " + tableName);
     rsmd = resultSet.getMetaData();
-    assertEquals(ResultSetMetaData.columnNullable, rsmd.isNullable(1));
+    assertEquals(ResultSetMetaData.columnNoNulls, rsmd.isNullable(1));
 
     statement.close();
   }
@@ -532,4 +533,44 @@ public class DBAccessorImplTest {
     Statement customSchemaTableCreation = dbAccessor.getConnection().createStatement();
     customSchemaTableCreation.execute(toString().format("Create table %s.%s (id int, time
int)", schemaName, tableName));
   }
+
+  /**
+   * Checks to ensure that columns created with a default have the correct
+   * DEFAULT constraint value set in.
+   *
+   * @throws Exception
+   */
+  @Test
+  public void testDefaultColumnConstraintOnAddColumn() throws Exception {
+    String tableName = getFreeTableName().toUpperCase();
+    String columnName = "COLUMN_WITH_DEFAULT_VALUE";
+
+    createMyTable(tableName);
+    DBAccessorImpl dbAccessor = injector.getInstance(DBAccessorImpl.class);
+
+    // create a column with a non-NULL constraint that has a default value
+    DBColumnInfo dbColumnInfo = new DBColumnInfo(columnName, String.class, 32, "foo", false);
+    dbAccessor.addColumn(tableName, dbColumnInfo);
+
+    String schema = null;
+    Connection connection = dbAccessor.getConnection();
+    DatabaseMetaData databaseMetadata = connection.getMetaData();
+    ResultSet schemaResultSet = databaseMetadata.getSchemas();
+    if (schemaResultSet.next()) {
+      schema = schemaResultSet.getString(1);
+    }
+
+    schemaResultSet.close();
+
+    String columnDefaultVal = null;
+    ResultSet rs = databaseMetadata.getColumns(null, schema, tableName, columnName);
+
+    if (rs.next()) {
+      columnDefaultVal = rs.getString("COLUMN_DEF");
+    }
+
+    rs.close();
+
+    assertEquals("'foo'", columnDefaultVal);
+   }
 }


Mime
View raw message