Repository: ambari Updated Branches: refs/heads/branch-2.2 5edef8a68 -> 4d8bc0f63 refs/heads/trunk ed954bbe8 -> c82f1337e
AMBARI-15159. DBAccessor metadata API for schema check calls can return more than 1 result. (mpapirkovskyy) Project: http://git-wip-us.apache.org/repos/asf/ambari/repo Commit: http://git-wip-us.apache.org/repos/asf/ambari/commit/c82f1337 Tree: http://git-wip-us.apache.org/repos/asf/ambari/tree/c82f1337 Diff: http://git-wip-us.apache.org/repos/asf/ambari/diff/c82f1337 Branch: refs/heads/trunk Commit: c82f1337e406b37d5515c5de4fc1a9d4b541f843 Parents: ed954bb Author: Myroslav Papirkovskyi <mpapyrkovs...@hortonworks.com> Authored: Wed Feb 24 17:46:53 2016 +0200 Committer: Myroslav Papirkovskyi <mpapyrkovs...@hortonworks.com> Committed: Thu Feb 25 18:13:49 2016 +0200 ---------------------------------------------------------------------- .../server/configuration/Configuration.java | 27 +++++++++- .../ambari/server/orm/DBAccessorImpl.java | 54 ++++++++++++++----- .../ambari/server/orm/DBAccessorImplTest.java | 57 +++++++++++++++++++- 3 files changed, 121 insertions(+), 17 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/ambari/blob/c82f1337/ambari-server/src/main/java/org/apache/ambari/server/configuration/Configuration.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/configuration/Configuration.java b/ambari-server/src/main/java/org/apache/ambari/server/configuration/Configuration.java index 882adb2..17fb42d 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/configuration/Configuration.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/configuration/Configuration.java @@ -234,7 +234,8 @@ public class Configuration { public static final String JDBC_UNIT_NAME = "ambari-server"; public static final String JDBC_LOCAL_URL = "jdbc:postgresql://localhost/"; public static final String JDBC_LOCAL_DRIVER = "org.postgresql.Driver"; - public static final String JDBC_IN_MEMORY_URL = "jdbc:derby:memory:myDB/ambari;create=true"; + public static final String DEFAULT_DERBY_SCHEMA = "ambari"; + public static final String JDBC_IN_MEMORY_URL = String.format("jdbc:derby:memory:myDB/%s;create=true", DEFAULT_DERBY_SCHEMA); public static final String JDBC_IN_MEMROY_DRIVER = "org.apache.derby.jdbc.EmbeddedDriver"; public static final String HOSTNAME_MACRO = "{hostname}"; public static final String JDBC_RCA_LOCAL_URL = "jdbc:postgresql://" + HOSTNAME_MACRO + "/ambarirca"; @@ -2258,6 +2259,30 @@ public class Configuration { } /** + * Gets the schema name of database + * + * @return the database schema name (can return {@code null} for any DB besides Postgres, MySQL, Oracle). + */ + public String getDatabaseSchema() { + DatabaseType databaseType = getDatabaseType(); + String databaseSchema; + + if (databaseType.equals(DatabaseType.POSTGRES)) { + databaseSchema = getServerJDBCPostgresSchemaName(); + } else if (databaseType.equals(DatabaseType.MYSQL)) { + databaseSchema = getServerDBName(); + } else if (databaseType.equals(DatabaseType.ORACLE)) { + databaseSchema = getDatabaseUser(); + } else if (databaseType.equals(DatabaseType.DERBY)) { + databaseSchema = DEFAULT_DERBY_SCHEMA; + } else { + databaseSchema = null; + } + + return databaseSchema; + } + + /** * Gets the type of connection pool that EclipseLink should use. * * @return default of {@link ConnectionPoolType#INTERNAL}. http://git-wip-us.apache.org/repos/asf/ambari/blob/c82f1337/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 188efa7..b10d32a 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 @@ -33,7 +33,9 @@ import java.sql.Statement; import java.sql.Types; import java.util.ArrayList; import java.util.Arrays; +import java.util.HashSet; import java.util.List; +import java.util.Set; import org.apache.ambari.server.configuration.Configuration; import org.apache.ambari.server.configuration.Configuration.DatabaseType; @@ -73,6 +75,7 @@ public class DBAccessorImpl implements DBAccessor { private DatabaseMetaData databaseMetaData; private static final String dbURLPatternString = "jdbc:(.*?):.*"; private DbType dbType; + private final String dbSchema; @Inject public DBAccessorImpl(Configuration configuration) { @@ -98,6 +101,7 @@ public class DBAccessorImpl implements DBAccessor { }); databasePlatform = (DatabasePlatform) Class.forName(dbPlatform).newInstance(); dbmsHelper = loadHelper(databasePlatform); + dbSchema = convertObjectName(configuration.getDatabaseSchema()); } catch (Exception e) { String message = "Error while creating database accessor "; LOG.error(message, e); @@ -190,12 +194,17 @@ public class DBAccessorImpl implements DBAccessor { boolean result = false; DatabaseMetaData metaData = getDatabaseMetaData(); - ResultSet res = metaData.getTables(null, null, convertObjectName(tableName), new String[]{"TABLE"}); + ResultSet res = metaData.getTables(null, dbSchema, convertObjectName(tableName), new String[]{"TABLE"}); if (res != null) { try { if (res.next()) { - return res.getString("TABLE_NAME") != null && res.getString("TABLE_NAME").equalsIgnoreCase(tableName); + result = res.getString("TABLE_NAME") != null && res.getString("TABLE_NAME").equalsIgnoreCase(tableName); + } + if (res.next()) { + throw new IllegalStateException( + String.format("Request for table [%s] existing returned more than one results", + tableName)); } } finally { res.close(); @@ -238,21 +247,27 @@ public class DBAccessorImpl implements DBAccessor { @Override public boolean tableHasColumn(String tableName, String columnName) throws SQLException { + boolean result = false; DatabaseMetaData metaData = getDatabaseMetaData(); - ResultSet rs = metaData.getColumns(null, null, convertObjectName(tableName), convertObjectName(columnName)); + ResultSet rs = metaData.getColumns(null, dbSchema, convertObjectName(tableName), convertObjectName(columnName)); if (rs != null) { try { if (rs.next()) { - return rs.getString("COLUMN_NAME") != null && rs.getString("COLUMN_NAME").equalsIgnoreCase(columnName); + result = rs.getString("COLUMN_NAME") != null && rs.getString("COLUMN_NAME").equalsIgnoreCase(columnName); + } + if (rs.next()) { + throw new IllegalStateException( + String.format("Request for column [%s] existing in table [%s] returned more than one results", + columnName, tableName)); } } finally { rs.close(); } } - return false; + return result; } @Override @@ -261,19 +276,30 @@ public class DBAccessorImpl implements DBAccessor { DatabaseMetaData metaData = getDatabaseMetaData(); CustomStringUtils.toUpperCase(columnsList); - ResultSet rs = metaData.getColumns(null, null, convertObjectName(tableName), null); + Set<String> columnsListToCheckCopies = new HashSet<>(columnsList); + List<String> duplicatedColumns = new ArrayList<>(); + ResultSet rs = metaData.getColumns(null, dbSchema, convertObjectName(tableName), null); if (rs != null) { try { while (rs.next()) { - if (rs.getString("COLUMN_NAME") != null) { - columnsList.remove(rs.getString("COLUMN_NAME").toUpperCase()); + String actualColumnName = rs.getString("COLUMN_NAME"); + if (actualColumnName != null) { + boolean removingResult = columnsList.remove(actualColumnName.toUpperCase()); + if (!removingResult && columnsListToCheckCopies.contains(actualColumnName.toUpperCase())) { + duplicatedColumns.add(actualColumnName.toUpperCase()); + } } } } finally { rs.close(); } } + if (!duplicatedColumns.isEmpty()) { + throw new IllegalStateException( + String.format("Request for columns [%s] existing in table [%s] returned too many results [%s] for columns [%s]", + columnName, tableName, duplicatedColumns.size(), duplicatedColumns.toString())); + } return columnsList.size() == 0; } @@ -282,7 +308,7 @@ public class DBAccessorImpl implements DBAccessor { public boolean tableHasForeignKey(String tableName, String fkName) throws SQLException { DatabaseMetaData metaData = getDatabaseMetaData(); - ResultSet rs = metaData.getImportedKeys(null, null, convertObjectName(tableName)); + ResultSet rs = metaData.getImportedKeys(null, dbSchema, convertObjectName(tableName)); if (rs != null) { try { @@ -304,7 +330,7 @@ public class DBAccessorImpl implements DBAccessor { public String getCheckedForeignKey(String tableName, String fkName) throws SQLException { DatabaseMetaData metaData = getDatabaseMetaData(); - ResultSet rs = metaData.getImportedKeys(null, null, convertObjectName(tableName)); + ResultSet rs = metaData.getImportedKeys(null, dbSchema, convertObjectName(tableName)); if (rs != null) { try { @@ -334,8 +360,8 @@ public class DBAccessorImpl implements DBAccessor { DatabaseMetaData metaData = getDatabaseMetaData(); //NB: reference table contains pk columns while key table contains fk columns - ResultSet rs = metaData.getCrossReference(null, null, convertObjectName(referenceTableName), - null, null, convertObjectName(tableName)); + ResultSet rs = metaData.getCrossReference(null, dbSchema, convertObjectName(referenceTableName), + null, dbSchema, convertObjectName(tableName)); List<String> pkColumns = new ArrayList<String>(referenceColumns.length); for (String referenceColumn : referenceColumns) { @@ -914,7 +940,7 @@ public class DBAccessorImpl implements DBAccessor { @Override public boolean tableHasPrimaryKey(String tableName, String columnName) throws SQLException { - ResultSet rs = getDatabaseMetaData().getPrimaryKeys(null, null, convertObjectName(tableName)); + ResultSet rs = getDatabaseMetaData().getPrimaryKeys(null, dbSchema, convertObjectName(tableName)); boolean res = false; try { if (rs != null && columnName != null) { @@ -1054,7 +1080,7 @@ public class DBAccessorImpl implements DBAccessor { @Override public List<String> getIndexesList(String tableName, boolean unique) throws SQLException{ - ResultSet rs = getDatabaseMetaData().getIndexInfo(null, null, convertObjectName(tableName), unique, false); + ResultSet rs = getDatabaseMetaData().getIndexInfo(null, dbSchema, convertObjectName(tableName), unique, false); List<String> indexList = new ArrayList<String>(); if (rs != null){ try{ http://git-wip-us.apache.org/repos/asf/ambari/blob/c82f1337/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 c867c9f..ac8bea1 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 @@ -51,7 +51,8 @@ import java.sql.PreparedStatement; public class DBAccessorImplTest { private Injector injector; - private static final AtomicInteger counter = new AtomicInteger(1); + private static final AtomicInteger tables_counter = new AtomicInteger(1); + private static final AtomicInteger schemas_counter = new AtomicInteger(1); @Rule public ExpectedException exception = ExpectedException.none(); @@ -67,7 +68,11 @@ public class DBAccessorImplTest { } private static String getFreeTableName() { - return "test_table_" + counter.getAndIncrement(); + return "test_table_" + tables_counter.getAndIncrement(); + } + + private static String getFreeSchamaName() { + return "test_schema_" + schemas_counter.getAndIncrement(); } private void createMyTable(String tableName) throws Exception { @@ -352,6 +357,19 @@ public class DBAccessorImplTest { } @Test + public void testTableExistsMultipleSchemas() throws Exception { + DBAccessorImpl dbAccessor = injector.getInstance(DBAccessorImpl.class); + + String tableName = getFreeTableName(); + createMyTable(tableName); + + // create table with the same name but in custom schema + createTableUnderNewSchema(dbAccessor, tableName); + + Assert.assertTrue(dbAccessor.tableExists(tableName)); + } + + @Test public void testColumnExists() throws Exception { String tableName = getFreeTableName(); createMyTable(tableName); @@ -362,6 +380,32 @@ public class DBAccessorImplTest { } @Test + public void testColumnExistsMultipleSchemas() throws Exception { + DBAccessorImpl dbAccessor = injector.getInstance(DBAccessorImpl.class); + + String tableName = getFreeTableName(); + createMyTable(tableName); + + // create table with the same name and same field (id) but in custom schema + createTableUnderNewSchema(dbAccessor, tableName); + + Assert.assertTrue(dbAccessor.tableHasColumn(tableName, "id")); + } + + @Test + public void testColumnsExistsMultipleSchemas() throws Exception { + DBAccessorImpl dbAccessor = injector.getInstance(DBAccessorImpl.class); + + String tableName = getFreeTableName(); + createMyTable(tableName); + + // create table with the same name and same field (id) but in custom schema + createTableUnderNewSchema(dbAccessor, tableName); + + Assert.assertTrue(dbAccessor.tableHasColumn(tableName, "id", "time")); + } + + @Test public void testRenameColumn() throws Exception { String tableName = getFreeTableName(); createMyTable(tableName); @@ -479,4 +523,13 @@ public class DBAccessorImplTest { statement.close(); } + + private void createTableUnderNewSchema(DBAccessorImpl dbAccessor, String tableName) throws SQLException { + Statement schemaCreation = dbAccessor.getConnection().createStatement(); + String schemaName = getFreeSchamaName(); + schemaCreation.execute("create schema " + schemaName); + + Statement customSchemaTableCreation = dbAccessor.getConnection().createStatement(); + customSchemaTableCreation.execute(toString().format("Create table %s.%s (id int, time int)", schemaName, tableName)); + } }
