Backport to branch(3) : Refactor JdbcAdmin (Part 2)

core/src/integration-test/java/com/scalar/db/storage/jdbc/JdbcAdminTestUtils.java

@@ -111,7 +111,7 @@ public boolean namespaceExists(String namespace) throws SQLException {
   @Override
   public boolean tableExists(String namespace, String table) throws Exception {
     String fullTableName = rdbEngine.encloseFullTableName(namespace, table);
-    String sql = rdbEngine.tableExistsInternalTableCheckSql(fullTableName);
+    String sql = rdbEngine.internalTableExistsCheckSql(fullTableName);
     try (Connection connection = dataSource.getConnection();
         Statement statement = connection.createStatement()) {
       statement.execute(sql);

core/src/integration-test/java/com/scalar/db/storage/multistorage/MultiStorageAdminTestUtils.java

@@ -164,7 +164,7 @@ private boolean tableExistsOnCassandra(String namespace, String table) {
 
   private boolean tableExistsOnJdbc(String namespace, String table) throws Exception {
     String fullTableName = rdbEngine.encloseFullTableName(namespace, table);
-    String sql = rdbEngine.tableExistsInternalTableCheckSql(fullTableName);
+    String sql = rdbEngine.internalTableExistsCheckSql(fullTableName);
     try (Connection connection = dataSource.getConnection();
         Statement statement = connection.createStatement()) {
       statement.execute(sql);

core/src/main/java/com/scalar/db/storage/jdbc/JdbcAdmin.java

@@ -117,6 +117,7 @@ public void createTable(
       String namespace, String table, TableMetadata metadata, Map<String, String> options)
       throws ExecutionException {
     try (Connection connection = dataSource.getConnection()) {
+      createMetadataSchemaIfNotExists(connection);
       createTableInternal(connection, namespace, table, metadata, false);
       addTableMetadata(connection, namespace, table, metadata, true, false);
     } catch (SQLException e) {
@@ -197,6 +198,7 @@ public void dropTable(String namespace, String table) throws ExecutionException
     try (Connection connection = dataSource.getConnection()) {
       dropTableInternal(connection, namespace, table);
       tableMetadataService.deleteTableMetadata(connection, namespace, table, true);
+      deleteMetadataSchemaIfEmpty(connection);
     } catch (SQLException e) {
       throw new ExecutionException(
           "Dropping the " + getFullTableName(namespace, table) + " table failed", e);
@@ -280,7 +282,7 @@ TableMetadata getImportTableMetadata(
       String catalogName = rdbEngine.getCatalogName(namespace);
       String schemaName = rdbEngine.getSchemaName(namespace);
 
-      if (!tableExistsInternal(connection, namespace, table)) {
+      if (!internalTableExists(connection, namespace, table)) {
         throw new IllegalArgumentException(
             CoreError.TABLE_NOT_FOUND.buildMessage(getFullTableName(namespace, table)));
       }
@@ -335,6 +337,7 @@ public void importTable(
 
     try (Connection connection = dataSource.getConnection()) {
       TableMetadata tableMetadata = getImportTableMetadata(namespace, table, overrideColumnsType);
+      createMetadataSchemaIfNotExists(connection);
       addTableMetadata(connection, namespace, table, tableMetadata, true, false);
     } catch (SQLException | ExecutionException e) {
       throw new ExecutionException(
@@ -484,11 +487,12 @@ public void repairTable(
     rdbEngine.throwIfInvalidNamespaceName(table);
 
     try (Connection connection = dataSource.getConnection()) {
-      if (!tableExistsInternal(connection, namespace, table)) {
+      if (!internalTableExists(connection, namespace, table)) {
         throw new IllegalArgumentException(
             CoreError.TABLE_NOT_FOUND.buildMessage(getFullTableName(namespace, table)));
       }
 
+      createMetadataSchemaIfNotExists(connection);
       addTableMetadata(connection, namespace, table, metadata, true, true);
     } catch (SQLException e) {
       throw new ExecutionException(
@@ -736,6 +740,21 @@ void createTableMetadataTableIfNotExists(Connection connection) throws SQLExcept
     tableMetadataService.createTableMetadataTableIfNotExists(connection);
   }
 
+  @VisibleForTesting
+  void createMetadataSchemaIfNotExists(Connection connection) throws SQLException {
+    createSchemaIfNotExists(connection, metadataSchema);
+  }
+
+  private void deleteMetadataSchemaIfEmpty(Connection connection) throws SQLException {
+    Set<String> internalTableNames = getInternalTableNames(connection, metadataSchema);
+    if (!internalTableNames.isEmpty()) {
+      return;
+    }
+
+    String sql = rdbEngine.dropNamespaceSql(metadataSchema);
+    execute(connection, sql);
+  }
+
   private void createTable(Connection connection, String createTableStatement, boolean ifNotExists)
       throws SQLException {
     String stmt = createTableStatement;
@@ -752,10 +771,10 @@ private void createTable(Connection connection, String createTableStatement, boo
     }
   }
 
-  private boolean tableExistsInternal(Connection connection, String namespace, String table)
+  private boolean internalTableExists(Connection connection, String namespace, String table)
       throws ExecutionException {
     String fullTableName = encloseFullTableName(namespace, table);
-    String sql = rdbEngine.tableExistsInternalTableCheckSql(fullTableName);
+    String sql = rdbEngine.internalTableExistsCheckSql(fullTableName);
     try {
       execute(connection, sql);
       return true;
@@ -772,6 +791,18 @@ private boolean tableExistsInternal(Connection connection, String namespace, Str
     }
   }
 
+  private void createSchemaIfNotExists(Connection connection, String schema) throws SQLException {
+    String[] sqls = rdbEngine.createSchemaIfNotExistsSqls(schema);
+    try {
+      execute(connection, sqls);
+    } catch (SQLException e) {
+      // Suppress exceptions indicating the duplicate metadata schema
+      if (!rdbEngine.isCreateMetadataSchemaDuplicateSchemaError(e)) {
+        throw e;
+      }
+    }
+  }
+
   private String enclose(String name) {
     return rdbEngine.enclose(name);
   }

core/src/main/java/com/scalar/db/storage/jdbc/RdbEngineDb2.java

@@ -283,7 +283,7 @@ public String[] alterColumnTypeSql(
   }
 
   @Override
-  public String tableExistsInternalTableCheckSql(String fullTableName) {
+  public String internalTableExistsCheckSql(String fullTableName) {
     return "SELECT 1 FROM " + fullTableName + " LIMIT 1";
   }
 

core/src/main/java/com/scalar/db/storage/jdbc/RdbEngineMysql.java

@@ -157,7 +157,7 @@ public String[] alterColumnTypeSql(
   }
 
   @Override
-  public String tableExistsInternalTableCheckSql(String fullTableName) {
+  public String internalTableExistsCheckSql(String fullTableName) {
     return "SELECT 1 FROM " + fullTableName + " LIMIT 1";
   }
 

core/src/main/java/com/scalar/db/storage/jdbc/RdbEngineOracle.java

@@ -159,7 +159,7 @@ public String[] alterColumnTypeSql(
   }
 
   @Override
-  public String tableExistsInternalTableCheckSql(String fullTableName) {
+  public String internalTableExistsCheckSql(String fullTableName) {
     return "SELECT 1 FROM " + fullTableName + " FETCH FIRST 1 ROWS ONLY";
   }
 

core/src/main/java/com/scalar/db/storage/jdbc/RdbEnginePostgresql.java

@@ -137,7 +137,7 @@ public String[] alterColumnTypeSql(
   }
 
   @Override
-  public String tableExistsInternalTableCheckSql(String fullTableName) {
+  public String internalTableExistsCheckSql(String fullTableName) {
     return "SELECT 1 FROM " + fullTableName + " LIMIT 1";
   }
 

core/src/main/java/com/scalar/db/storage/jdbc/RdbEngineSqlServer.java

@@ -144,7 +144,7 @@ public String[] alterColumnTypeSql(
   }
 
   @Override
-  public String tableExistsInternalTableCheckSql(String fullTableName) {
+  public String internalTableExistsCheckSql(String fullTableName) {
     return "SELECT TOP 1 1 FROM " + fullTableName;
   }
 

core/src/main/java/com/scalar/db/storage/jdbc/RdbEngineSqlite.java

@@ -284,7 +284,7 @@ public String[] alterColumnTypeSql(
   }
 
   @Override
-  public String tableExistsInternalTableCheckSql(String fullTableName) {
+  public String internalTableExistsCheckSql(String fullTableName) {
     return "SELECT 1 FROM " + fullTableName + " LIMIT 1";
   }
 

core/src/main/java/com/scalar/db/storage/jdbc/RdbEngineStrategy.java

@@ -133,7 +133,7 @@ default String renameColumnSql(
 
   String[] alterColumnTypeSql(String namespace, String table, String columnName, String columnType);
 
-  String tableExistsInternalTableCheckSql(String fullTableName);
+  String internalTableExistsCheckSql(String fullTableName);
 
   default String createIndexSql(
       String schema, String table, String indexName, String indexedColumn) {

core/src/main/java/com/scalar/db/storage/jdbc/TableMetadataService.java

@@ -64,8 +64,6 @@ void addTableMetadata(
 
   @VisibleForTesting
   void createTableMetadataTableIfNotExists(Connection connection) throws SQLException {
-    createSchemaIfNotExists(connection, metadataSchema);
-
     String createTableStatement =
         "CREATE TABLE "
             + encloseFullTableName(metadataSchema, TABLE_NAME)
@@ -398,18 +396,6 @@ private void deleteTable(Connection connection, String fullTableName) throws SQL
     execute(connection, dropTableStatement);
   }
 
-  private void createSchemaIfNotExists(Connection connection, String schema) throws SQLException {
-    String[] sqls = rdbEngine.createSchemaIfNotExistsSqls(schema);
-    try {
-      execute(connection, sqls);
-    } catch (SQLException e) {
-      // Suppress exceptions indicating the duplicate metadata schema
-      if (!rdbEngine.isCreateMetadataSchemaDuplicateSchemaError(e)) {
-        throw e;
-      }
-    }
-  }
-
   private String enclose(String name) {
     return rdbEngine.enclose(name);
   }

core/src/test/java/com/scalar/db/storage/jdbc/JdbcAdminTest.java

@@ -829,7 +829,6 @@ public void addTableMetadata_ifNotExistsAndOverwriteMetadataForMysql_ShouldWorkP
       throws Exception {
     addTableMetadata_createMetadataTableIfNotExistsForXAndOverwriteMetadata_ShouldWorkProperly(
         RdbEngine.MYSQL,
-        "CREATE SCHEMA IF NOT EXISTS `" + METADATA_SCHEMA + "`",
         "CREATE TABLE IF NOT EXISTS `"
             + METADATA_SCHEMA
             + "`.`metadata`("
@@ -857,7 +856,6 @@ public void addTableMetadata_ifNotExistsAndOverwriteMetadataForPostgresql_Should
       throws Exception {
     addTableMetadata_createMetadataTableIfNotExistsForXAndOverwriteMetadata_ShouldWorkProperly(
         RdbEngine.POSTGRESQL,
-        "CREATE SCHEMA IF NOT EXISTS \"" + METADATA_SCHEMA + "\"",
         "CREATE TABLE IF NOT EXISTS \""
             + METADATA_SCHEMA
             + "\".\"metadata\"("
@@ -885,7 +883,6 @@ public void addTableMetadata_ifNotExistsAndOverwriteMetadataForSqlServer_ShouldW
       throws Exception {
     addTableMetadata_createMetadataTableIfNotExistsForXAndOverwriteMetadata_ShouldWorkProperly(
         RdbEngine.SQL_SERVER,
-        "CREATE SCHEMA [" + METADATA_SCHEMA + "]",
         "CREATE TABLE ["
             + METADATA_SCHEMA
             + "].[metadata]("
@@ -913,8 +910,6 @@ public void addTableMetadata_ifNotExistsAndOverwriteMetadataForOracle_ShouldWork
       throws Exception {
     addTableMetadata_createMetadataTableIfNotExistsForXAndOverwriteMetadata_ShouldWorkProperly(
         RdbEngine.ORACLE,
-        "CREATE USER \"" + METADATA_SCHEMA + "\" IDENTIFIED BY \"Oracle1234!@#$\"",
-        "ALTER USER \"" + METADATA_SCHEMA + "\" quota unlimited on USERS",
         "CREATE TABLE \""
             + METADATA_SCHEMA
             + "\".\"metadata\"(\"full_table_name\" VARCHAR2(128),\"column_name\" VARCHAR2(128),\"data_type\" VARCHAR2(20) NOT NULL,\"key_type\" VARCHAR2(20),\"clustering_order\" VARCHAR2(10),\"indexed\" NUMBER(1) NOT NULL,\"ordinal_position\" INTEGER NOT NULL,PRIMARY KEY (\"full_table_name\", \"column_name\"))",
@@ -961,7 +956,6 @@ public void addTableMetadata_ifNotExistsAndOverwriteMetadataForDb2_ShouldWorkPro
       throws Exception {
     addTableMetadata_createMetadataTableIfNotExistsForXAndOverwriteMetadata_ShouldWorkProperly(
         RdbEngine.DB2,
-        "CREATE SCHEMA \"" + METADATA_SCHEMA + "\"",
         "CREATE TABLE IF NOT EXISTS \""
             + METADATA_SCHEMA
             + "\".\"metadata\"("
@@ -1023,7 +1017,6 @@ public void addTableMetadata_ifNotExistsAndDoNotOverwriteMetadataForMysql_Should
       throws Exception {
     addTableMetadata_createMetadataTableIfNotExistsForX_ShouldWorkProperly(
         RdbEngine.MYSQL,
-        "CREATE SCHEMA IF NOT EXISTS `" + METADATA_SCHEMA + "`",
         "CREATE TABLE IF NOT EXISTS `"
             + METADATA_SCHEMA
             + "`.`metadata`("
@@ -1068,7 +1061,6 @@ public void addTableMetadata_ifNotExistsAndDoNotOverwriteMetadataForMysql_Should
           throws Exception {
     addTableMetadata_createMetadataTableIfNotExistsForX_ShouldWorkProperly(
         RdbEngine.POSTGRESQL,
-        "CREATE SCHEMA IF NOT EXISTS \"" + METADATA_SCHEMA + "\"",
         "CREATE TABLE IF NOT EXISTS \""
             + METADATA_SCHEMA
             + "\".\"metadata\"("
@@ -1112,7 +1104,6 @@ public void addTableMetadata_ifNotExistsAndDoNotOverwriteMetadataForSqlServer_Sh
       throws Exception {
     addTableMetadata_createMetadataTableIfNotExistsForX_ShouldWorkProperly(
         RdbEngine.SQL_SERVER,
-        "CREATE SCHEMA [" + METADATA_SCHEMA + "]",
         "CREATE TABLE ["
             + METADATA_SCHEMA
             + "].[metadata]("
@@ -1156,8 +1147,6 @@ public void addTableMetadata_ifNotExistsAndDoNotOverwriteMetadataForOracle_Shoul
       throws Exception {
     addTableMetadata_createMetadataTableIfNotExistsForX_ShouldWorkProperly(
         RdbEngine.ORACLE,
-        "CREATE USER \"" + METADATA_SCHEMA + "\" IDENTIFIED BY \"Oracle1234!@#$\"",
-        "ALTER USER \"" + METADATA_SCHEMA + "\" quota unlimited on USERS",
         "CREATE TABLE \""
             + METADATA_SCHEMA
             + "\".\"metadata\"(\"full_table_name\" VARCHAR2(128),\"column_name\" VARCHAR2(128),\"data_type\" VARCHAR2(20) NOT NULL,\"key_type\" VARCHAR2(20),\"clustering_order\" VARCHAR2(10),\"indexed\" NUMBER(1) NOT NULL,\"ordinal_position\" INTEGER NOT NULL,PRIMARY KEY (\"full_table_name\", \"column_name\"))",
@@ -1236,7 +1225,6 @@ public void addTableMetadata_ifNotExistsAndDoNotOverwriteMetadataForDb2_ShouldWo
       throws Exception {
     addTableMetadata_createMetadataTableIfNotExistsForX_ShouldWorkProperly(
         RdbEngine.DB2,
-        "CREATE SCHEMA \"" + METADATA_SCHEMA + "\"",
         "CREATE TABLE IF NOT EXISTS \""
             + METADATA_SCHEMA
             + "\".\"metadata\"("
@@ -1356,6 +1344,7 @@ public void createTable_ShouldCallCreateTableAndAddTableMetadataCorrectly(RdbEng
     adminSpy.createTable(namespace, table, metadata, Collections.emptyMap());
 
     // Assert
+    verify(adminSpy).createMetadataSchemaIfNotExists(connection);
     verify(adminSpy).createTableInternal(connection, namespace, table, metadata, false);
     verify(adminSpy).addTableMetadata(connection, namespace, table, metadata, true, false);
   }
@@ -1726,7 +1715,8 @@ public void dropTable_forMysqlWithNoMoreMetadataAfterDeletion_shouldDropTableAnd
             + METADATA_SCHEMA
             + "`.`metadata` WHERE `full_table_name` = 'my_ns.foo_table'",
         "SELECT DISTINCT `full_table_name` FROM `" + METADATA_SCHEMA + "`.`metadata`",
-        "DROP TABLE `" + METADATA_SCHEMA + "`.`metadata`");
+        "DROP TABLE `" + METADATA_SCHEMA + "`.`metadata`",
+        "DROP SCHEMA `" + METADATA_SCHEMA + "`");
   }
 
   @Test
@@ -1740,7 +1730,8 @@ public void dropTable_forMysqlWithNoMoreMetadataAfterDeletion_shouldDropTableAnd
             + METADATA_SCHEMA
             + "\".\"metadata\" WHERE \"full_table_name\" = 'my_ns.foo_table'",
         "SELECT DISTINCT \"full_table_name\" FROM \"" + METADATA_SCHEMA + "\".\"metadata\"",
-        "DROP TABLE \"" + METADATA_SCHEMA + "\".\"metadata\"");
+        "DROP TABLE \"" + METADATA_SCHEMA + "\".\"metadata\"",
+        "DROP SCHEMA \"" + METADATA_SCHEMA + "\"");
   }
 
   @Test
@@ -1754,7 +1745,8 @@ public void dropTable_forMysqlWithNoMoreMetadataAfterDeletion_shouldDropTableAnd
             + METADATA_SCHEMA
             + "].[metadata] WHERE [full_table_name] = 'my_ns.foo_table'",
         "SELECT DISTINCT [full_table_name] FROM [" + METADATA_SCHEMA + "].[metadata]",
-        "DROP TABLE [" + METADATA_SCHEMA + "].[metadata]");
+        "DROP TABLE [" + METADATA_SCHEMA + "].[metadata]",
+        "DROP SCHEMA [" + METADATA_SCHEMA + "]");
   }
 
   @Test
@@ -1767,7 +1759,8 @@ public void dropTable_forOracleWithNoMoreMetadataAfterDeletion_shouldDropTableAn
             + METADATA_SCHEMA
             + "\".\"metadata\" WHERE \"full_table_name\" = 'my_ns.foo_table'",
         "SELECT DISTINCT \"full_table_name\" FROM \"" + METADATA_SCHEMA + "\".\"metadata\"",
-        "DROP TABLE \"" + METADATA_SCHEMA + "\".\"metadata\"");
+        "DROP TABLE \"" + METADATA_SCHEMA + "\".\"metadata\"",
+        "DROP USER \"" + METADATA_SCHEMA + "\"");
   }
 
   @Test
@@ -1793,7 +1786,8 @@ public void dropTable_forDb2WithNoMoreMetadataAfterDeletion_shouldDropTableAndDe
             + METADATA_SCHEMA
             + "\".\"metadata\" WHERE \"full_table_name\" = 'my_ns.foo_table'",
         "SELECT DISTINCT \"full_table_name\" FROM \"" + METADATA_SCHEMA + "\".\"metadata\"",
-        "DROP TABLE \"" + METADATA_SCHEMA + "\".\"metadata\"");
+        "DROP TABLE \"" + METADATA_SCHEMA + "\".\"metadata\"",
+        "DROP SCHEMA \"" + METADATA_SCHEMA + "\" RESTRICT");
   }
 
   private void dropTable_forXWithNoMoreMetadataAfterDeletion_shouldDropTableAndDeleteMetadata(
@@ -1805,6 +1799,10 @@ private void dropTable_forXWithNoMoreMetadataAfterDeletion_shouldDropTableAndDel
     ResultSet resultSet = mock(ResultSet.class);
     when(resultSet.next()).thenReturn(false);
 
+    PreparedStatement preparedStatement = mock(PreparedStatement.class);
+    when(preparedStatement.executeQuery()).thenReturn(resultSet);
+    when(connection.prepareStatement(any())).thenReturn(preparedStatement);
+
     List<Statement> mockedStatements = new ArrayList<>();
     for (String expectedSqlStatement : expectedSqlStatements) {
       Statement mock = mock(Statement.class);
@@ -1921,7 +1919,11 @@ private void dropTable_forXWithNoMoreMetadataAfterDeletion_shouldDropTableAndDel
     String table = "foo_table";
 
     ResultSet resultSet = mock(ResultSet.class);
-    when(resultSet.next()).thenReturn(true);
+    when(resultSet.next()).thenReturn(true, false);
+
+    PreparedStatement preparedStatement = mock(PreparedStatement.class);
+    when(preparedStatement.executeQuery()).thenReturn(resultSet);
+    when(connection.prepareStatement(any())).thenReturn(preparedStatement);
 
     List<Statement> mockedStatements = new ArrayList<>();
     for (String expectedSqlStatement : expectedSqlStatements) {
@@ -3922,6 +3924,8 @@ public void importTable_ForXBesidesSqlite_ShouldWorkProperly(RdbEngine rdbEngine
     // Arrange
     JdbcAdmin adminSpy = spy(createJdbcAdminFor(rdbEngine));
 
+    Statement statement = mock(Statement.class);
+    when(connection.createStatement()).thenReturn(statement);
     when(dataSource.getConnection()).thenReturn(connection);
     TableMetadata importedTableMetadata = mock(TableMetadata.class);
     doReturn(importedTableMetadata)
@@ -3936,6 +3940,7 @@ public void importTable_ForXBesidesSqlite_ShouldWorkProperly(RdbEngine rdbEngine
 
     // Assert
     verify(adminSpy).getImportTableMetadata(NAMESPACE, TABLE, Collections.emptyMap());
+    verify(adminSpy).createMetadataSchemaIfNotExists(connection);
     verify(adminSpy)
         .addTableMetadata(connection, NAMESPACE, TABLE, importedTableMetadata, true, false);
   }