ddl: Fix rename come after database has been dropped (#9274) (#9277)

close #9266

Co-authored-by: JaySon-Huang <[email protected]>
Co-authored-by: JaySon <[email protected]>

dbms/src/Debug/MockTiDB.cpp

@@ -877,17 +877,18 @@ TiDB::TableInfoPtr MockTiDB::getTableInfoByID(TableID table_id)
 
 TiDB::DBInfoPtr MockTiDB::getDBInfoByID(DatabaseID db_id)
 {
-    TiDB::DBInfoPtr db_ptr = std::make_shared<TiDB::DBInfo>(TiDB::DBInfo());
-    db_ptr->id = db_id;
     for (const auto & database : databases)
     {
         if (database.second == db_id)
         {
+            TiDB::DBInfoPtr db_ptr = std::make_shared<TiDB::DBInfo>(TiDB::DBInfo());
+            db_ptr->id = db_id;
             db_ptr->name = database.first;
-            break;
+            return db_ptr;
         }
     }
-    return db_ptr;
+    // If the database has been dropped in TiKV, TiFlash get a nullptr
+    return nullptr;
 }
 
 std::pair<bool, DatabaseID> MockTiDB::getDBIDByName(const String & database_name)

dbms/src/TiDB/Schema/SchemaBuilder.cpp

@@ -26,16 +26,14 @@
 #include <IO/FileProvider/FileProvider.h>
 #include <IO/WriteHelpers.h>
 #include <Interpreters/Context.h>
-#include <Interpreters/InterpreterAlterQuery.h>
 #include <Interpreters/InterpreterCreateQuery.h>
-#include <Interpreters/InterpreterDropQuery.h>
 #include <Interpreters/InterpreterRenameQuery.h>
 #include <Parsers/ASTCreateQuery.h>
-#include <Parsers/ASTDropQuery.h>
 #include <Parsers/ASTLiteral.h>
 #include <Parsers/ASTRenameQuery.h>
 #include <Parsers/ParserCreateQuery.h>
 #include <Parsers/parseQuery.h>
+#include <Storages/AlterCommands.h>
 #include <Storages/IManageableStorage.h>
 #include <Storages/KVStore/TMTContext.h>
 #include <Storages/MutableSupport.h>
@@ -723,6 +721,11 @@ void SchemaBuilder<Getter, NameMapper>::applyRenamePhysicalTable(
         return;
     }
 
+    // There could be a chance that the target database has been dropped in TiKV before
+    // TiFlash accepts the "create database" schema diff. We need to ensure the local
+    // database exist before executing renaming.
+    const auto action = fmt::format("applyRenamePhysicalTable-table_id={}", new_table_info.id);
+    ensureLocalDatabaseExist(new_database_id, new_mapped_db_name, action);
     const auto old_mapped_tbl_name = storage->getTableName();
     GET_METRIC(tiflash_schema_internal_ddl_count, type_rename_table).Increment();
     LOG_INFO(
@@ -1167,27 +1170,7 @@ void SchemaBuilder<Getter, NameMapper>::applyCreateStorageInstance(
     // in TiDB, then TiFlash may not create the IDatabase instance. Make sure we can access
     // to the IDatabase when creating IStorage.
     const auto database_mapped_name = name_mapper.mapDatabaseName(database_id, keyspace_id);
-    if (!context.isDatabaseExist(database_mapped_name))
-    {
-        LOG_WARNING(
-            log,
-            "database instance is not exist (applyCreateStorageInstance), may has been dropped, create a database "
-            "with fake DatabaseInfo for it, database_id={} database_name={} action={}",
-            database_id,
-            database_mapped_name,
-            action);
-        // The database is dropped in TiKV and we can not fetch it. Generate a fake
-        // DatabaseInfo for it. It is OK because the DatabaseInfo will be updated
-        // when the database is `FLASHBACK`.
-        TiDB::DBInfoPtr database_info = std::make_shared<TiDB::DBInfo>();
-        database_info->id = database_id;
-        database_info->keyspace_id = keyspace_id;
-        database_info->name = database_mapped_name; // use the mapped name because we done known the actual name
-        database_info->charset = "utf8mb4"; // default value
-        database_info->collate = "utf8mb4_bin"; // default value
-        database_info->state = TiDB::StateNone; // special state
-        applyCreateDatabaseByInfo(database_info);
-    }
+    ensureLocalDatabaseExist(database_id, database_mapped_name, action);
 
     ParserCreateQuery parser;
     ASTPtr ast = parseQuery(parser, stmt.data(), stmt.data() + stmt.size(), "from syncSchema " + table_info->name, 0);
@@ -1203,13 +1186,41 @@ void SchemaBuilder<Getter, NameMapper>::applyCreateStorageInstance(
     interpreter.execute();
     LOG_INFO(
         log,
-        "Creat table {} end, database_id={} table_id={} action={}",
+        "Create table {} end, database_id={} table_id={} action={}",
         name_mapper.debugCanonicalName(*table_info, database_id, keyspace_id),
         database_id,
         table_info->id,
         action);
 }
 
+template <typename Getter, typename NameMapper>
+void SchemaBuilder<Getter, NameMapper>::ensureLocalDatabaseExist(
+    DatabaseID database_id,
+    const String & database_mapped_name,
+    std::string_view action)
+{
+    if (likely(context.isDatabaseExist(database_mapped_name)))
+        return;
+
+    LOG_WARNING(
+        log,
+        "database instance is not exist, may has been dropped, create a database "
+        "with fake DatabaseInfo for it, database_id={} database_name={} action={}",
+        database_id,
+        database_mapped_name,
+        action);
+    // The database is dropped in TiKV and we can not fetch it. Generate a fake
+    // DatabaseInfo for it. It is OK because the DatabaseInfo will be updated
+    // when the database is `FLASHBACK`.
+    TiDB::DBInfoPtr database_info = std::make_shared<TiDB::DBInfo>();
+    database_info->id = database_id;
+    database_info->keyspace_id = keyspace_id;
+    database_info->name = database_mapped_name; // use the mapped name because we don't known the actual name
+    database_info->charset = "utf8mb4"; // default value
+    database_info->collate = "utf8mb4_bin"; // default value
+    database_info->state = TiDB::StateNone; // special state
+    applyCreateDatabaseByInfo(database_info);
+}
 
 template <typename Getter, typename NameMapper>
 void SchemaBuilder<Getter, NameMapper>::applyDropPhysicalTable(
@@ -1773,5 +1784,4 @@ template struct SchemaBuilder<SchemaGetter, SchemaNameMapper>;
 template struct SchemaBuilder<MockSchemaGetter, MockSchemaNameMapper>;
 // unit test
 template struct SchemaBuilder<MockSchemaGetter, SchemaNameMapper>;
-// end namespace
 } // namespace DB

dbms/src/TiDB/Schema/SchemaBuilder.h

@@ -72,6 +72,7 @@ struct SchemaBuilder
 
     bool applyCreateDatabase(DatabaseID database_id);
     void applyCreateDatabaseByInfo(const TiDB::DBInfoPtr & db_info);
+    void ensureLocalDatabaseExist(DatabaseID database_id, const String & database_mapped_name, std::string_view action);
 
     void applyRecoverDatabase(DatabaseID database_id);
 

tests/fullstack-test2/ddl/rename_table_across_databases.test

@@ -126,3 +126,25 @@ mysql> set session tidb_isolation_read_engines='tiflash'; select * from test_new
 +----+----------+------+
 
 mysql> drop table if exists test_new.part4
+
+# (case 4) rename partitioned table across database
+# (required) stop regular schema sync
+=> DBGInvoke __enable_schema_sync_service('false')
+mysql> drop database if exists test_new;
+mysql> drop table if exists test.part5;
+mysql> CREATE TABLE test.part5 (id INT NOT NULL,store_id INT NOT NULL)PARTITION BY RANGE (store_id) (PARTITION p0 VALUES LESS THAN (6),PARTITION p1 VALUES LESS THAN (11),PARTITION p2 VALUES LESS THAN (16),PARTITION p3 VALUES LESS THAN (21));
+# (1,1),(2,2),(3,3) => p0; p1 is empty;(11,11) => p2;(16,16) => p3
+mysql> alter table test.part5 set tiflash replica 1;
+func> wait_table test part5
+
+>> DBGInvoke __enable_fail_point(pause_before_apply_raft_cmd)
+mysql> insert into test.part5(id, store_id) values(1,1),(2,2),(3,3),(11,11),(16,16);
+
+# create target db, rename table to target db, then drop target db
+mysql> create database if not exists test_new;
+mysql> rename table test.part5 to test_new.part5;
+mysql> alter table test_new.part5 add column c1 int;
+mysql> drop database if exists test_new;
+# raft command comes after target db dropped, no crashes
+>> DBGInvoke __disable_fail_point(pause_before_apply_raft_cmd)
+>> DBGInvoke __refresh_schemas()