ClickHouse · alesapin · Dec 26, 2019 · Oct 25, 2019 · Oct 28, 2019 · Oct 30, 2019
diff --git a/dbms/src/Databases/DatabaseLazy.cpp b/dbms/src/Databases/DatabaseLazy.cpp
@@ -30,22 +30,22 @@ namespace ErrorCodes
 
 
 
-DatabaseLazy::DatabaseLazy(const String & name_, const String & metadata_path_, time_t expiration_time_, const Context & context)
+DatabaseLazy::DatabaseLazy(const String & name_, const String & metadata_path_, time_t expiration_time_, const Context & context_)
     : name(name_)
     , metadata_path(metadata_path_)
-    , data_path(context.getPath() + "data/" + escapeForFileName(name) + "/")
+    , data_path("data/" + escapeForFileName(name) + "/")
     , expiration_time(expiration_time_)
     , log(&Logger::get("DatabaseLazy (" + name + ")"))
 {
-    Poco::File(getDataPath()).createDirectories();
+    Poco::File(context_.getPath() + getDataPath()).createDirectories();
 }
 
 
 void DatabaseLazy::loadStoredObjects(
-    Context & /* context */,
+    Context & context,
     bool /* has_force_restore_data_flag */)
 {
-    DatabaseOnDisk::iterateMetadataFiles(*this, log, [this](const String & file_name)
+    DatabaseOnDisk::iterateMetadataFiles(*this, log, context, [this](const String & file_name)
     {
         const std::string table_name = file_name.substr(0, file_name.size() - 4);
         attachTable(table_name, nullptr);
@@ -185,9 +185,9 @@ void DatabaseLazy::alterTable(
 }
 
 
-void DatabaseLazy::drop()
+void DatabaseLazy::drop(const Context & context)
 {
-    DatabaseOnDisk::drop(*this);
+    DatabaseOnDisk::drop(*this, context);
 }
 
 bool DatabaseLazy::isTableExist(

diff --git a/dbms/src/Databases/DatabaseLazy.h b/dbms/src/Databases/DatabaseLazy.h
@@ -18,7 +18,7 @@ class DatabaseLazyIterator;
 class DatabaseLazy : public IDatabase
 {
 public:
-    DatabaseLazy(const String & name_, const String & metadata_path_, time_t expiration_time_, const Context & context);
+    DatabaseLazy(const String & name_, const String & metadata_path_, time_t expiration_time_, const Context & context_);
 
     String getEngineName() const override { return "Lazy"; }
 
@@ -87,7 +87,7 @@ class DatabaseLazy : public IDatabase
     String getMetadataPath() const override;
     String getObjectMetadataPath(const String & table_name) const override;
 
-    void drop() override;
+    void drop(const Context & context) override;
 
     bool isTableExist(
         const Context & context,

diff --git a/dbms/src/Databases/DatabaseMySQL.cpp b/dbms/src/Databases/DatabaseMySQL.cpp
@@ -332,7 +332,7 @@ void DatabaseMySQL::shutdown()
     local_tables_cache.clear();
 }
 
-void DatabaseMySQL::drop()
+void DatabaseMySQL::drop(const Context & /*context*/)
 {
     Poco::File(getMetadataPath()).remove(true);
 }

diff --git a/dbms/src/Databases/DatabaseMySQL.h b/dbms/src/Databases/DatabaseMySQL.h
@@ -60,7 +60,7 @@ class DatabaseMySQL : public IDatabase
 
     void shutdown() override;
 
-    void drop() override;
+    void drop(const Context & /*context*/) override;
 
     String getMetadataPath() const override;
 

diff --git a/dbms/src/Databases/DatabaseOnDisk.cpp b/dbms/src/Databases/DatabaseOnDisk.cpp
@@ -129,7 +129,7 @@ ASTPtr parseCreateQueryFromMetadataFile(const String & filepath, Poco::Logger *
 std::pair<String, StoragePtr> createTableFromAST(
     ASTCreateQuery ast_create_query,
     const String & database_name,
-    const String & database_data_path,
+    const String & database_data_path_relative,
     Context & context,
     bool has_force_restore_data_flag)
 {
@@ -152,12 +152,13 @@ std::pair<String, StoragePtr> createTableFromAST(
     ColumnsDescription columns = InterpreterCreateQuery::getColumnsDescription(*ast_create_query.columns_list->columns, context);
     ConstraintsDescription constraints = InterpreterCreateQuery::getConstraintsDescription(ast_create_query.columns_list->constraints);
 
+    String table_data_path_relative = database_data_path_relative + escapeForFileName(ast_create_query.table) + '/';
     return
     {
         ast_create_query.table,
         StorageFactory::instance().get(
             ast_create_query,
-            database_data_path, ast_create_query.table, database_name, context, context.getGlobalContext(),
+            table_data_path_relative, ast_create_query.table, database_name, context, context.getGlobalContext(),
             columns, constraints,
             true, has_force_restore_data_flag)
     };
@@ -495,9 +496,9 @@ ASTPtr DatabaseOnDisk::getCreateDatabaseQuery(const IDatabase & database, const
     return ast;
 }
 
-void DatabaseOnDisk::drop(const IDatabase & database)
+void DatabaseOnDisk::drop(const IDatabase & database, const Context & context)
 {
-    Poco::File(database.getDataPath()).remove(false);
+    Poco::File(context.getPath() + database.getDataPath()).remove(false);
     Poco::File(database.getMetadataPath()).remove(false);
 }
 
@@ -519,7 +520,7 @@ time_t DatabaseOnDisk::getObjectMetadataModificationTime(
         return static_cast<time_t>(0);
 }
 
-void DatabaseOnDisk::iterateMetadataFiles(const IDatabase & database, Poco::Logger * log, const IteratingFunction & iterating_function)
+void DatabaseOnDisk::iterateMetadataFiles(const IDatabase & database, Poco::Logger * log, const Context & context, const IteratingFunction & iterating_function)
 {
     Poco::DirectoryIterator dir_end;
     for (Poco::DirectoryIterator dir_it(database.getMetadataPath()); dir_it != dir_end; ++dir_it)
@@ -537,7 +538,7 @@ void DatabaseOnDisk::iterateMetadataFiles(const IDatabase & database, Poco::Logg
         if (endsWith(dir_it.name(), tmp_drop_ext))
         {
             const std::string object_name = dir_it.name().substr(0, dir_it.name().size() - strlen(tmp_drop_ext));
-            if (Poco::File(database.getDataPath() + '/' + object_name).exists())
+            if (Poco::File(context.getPath() + database.getDataPath() + '/' + object_name).exists())
             {
                 /// TODO maybe complete table drop and remove all table data (including data on other volumes and metadata in ZK)
                 Poco::File(dir_it->path()).renameTo(database.getMetadataPath() + object_name + ".sql");

diff --git a/dbms/src/Databases/DatabaseOnDisk.h b/dbms/src/Databases/DatabaseOnDisk.h
@@ -24,7 +24,7 @@ ASTPtr parseCreateQueryFromMetadataFile(const String & filepath, Poco::Logger *
 std::pair<String, StoragePtr> createTableFromAST(
     ASTCreateQuery ast_create_query,
     const String & database_name,
-    const String & database_data_path,
+    const String & database_data_path_relative,
     Context & context,
     bool has_force_restore_data_flag);
 
@@ -98,7 +98,7 @@ class DatabaseOnDisk
         const IDatabase & database,
         const Context & context);
 
-    static void drop(const IDatabase & database);
+    static void drop(const IDatabase & database, const Context & context);
 
     static String getObjectMetadataPath(
         const IDatabase & database,
@@ -110,7 +110,7 @@ class DatabaseOnDisk
 
 
     using IteratingFunction = std::function<void(const String &)>;
-    static void iterateMetadataFiles(const IDatabase & database, Poco::Logger * log, const IteratingFunction & iterating_function);
+    static void iterateMetadataFiles(const IDatabase & database, Poco::Logger * log, const Context & context, const IteratingFunction & iterating_function);
 
 private:
     static ASTPtr getCreateTableQueryImpl(
@@ -156,7 +156,7 @@ void DatabaseOnDisk::renameTable(
     /// Notify the table that it is renamed. If the table does not support renaming, exception is thrown.
     try
     {
-        table->rename(context.getPath() + "/data/" + escapeForFileName(to_database_concrete->getDatabaseName()) + "/",
+        table->rename("/data/" + escapeForFileName(to_database_concrete->getDatabaseName()) + "/" + escapeForFileName(to_table_name) + '/',
             to_database_concrete->getDatabaseName(),
             to_table_name, lock);
     }

diff --git a/dbms/src/Databases/DatabaseOrdinary.cpp b/dbms/src/Databases/DatabaseOrdinary.cpp
@@ -59,7 +59,6 @@ namespace
         Context & context,
         const ASTCreateQuery & query,
         DatabaseOrdinary & database,
-        const String database_data_path,
         const String & database_name,
         bool has_force_restore_data_flag)
     {
@@ -69,7 +68,7 @@ namespace
             String table_name;
             StoragePtr table;
             std::tie(table_name, table)
-                = createTableFromAST(query, database_name, database_data_path, context, has_force_restore_data_flag);
+                = createTableFromAST(query, database_name, database.getDataPath(), context, has_force_restore_data_flag);
             database.attachTable(table_name, table);
         }
         catch (const Exception & e)
@@ -115,13 +114,13 @@ namespace
 }
 
 
-DatabaseOrdinary::DatabaseOrdinary(String name_, const String & metadata_path_, const Context & context)
+DatabaseOrdinary::DatabaseOrdinary(String name_, const String & metadata_path_, const Context & context_)
     : DatabaseWithOwnTablesBase(std::move(name_))
     , metadata_path(metadata_path_)
-    , data_path(context.getPath() + "data/" + escapeForFileName(name) + "/")
+    , data_path("data/" + escapeForFileName(name) + "/")
     , log(&Logger::get("DatabaseOrdinary (" + name + ")"))
 {
-    Poco::File(getDataPath()).createDirectories();
+    Poco::File(context_.getPath() + getDataPath()).createDirectories();
 }
 
 
@@ -138,7 +137,7 @@ void DatabaseOrdinary::loadStoredObjects(
     FileNames file_names;
 
     size_t total_dictionaries = 0;
-    DatabaseOnDisk::iterateMetadataFiles(*this, log, [&file_names, &total_dictionaries, this](const String & file_name)
+    DatabaseOnDisk::iterateMetadataFiles(*this, log, context, [&file_names, &total_dictionaries, this](const String & file_name)
     {
         String full_path = metadata_path + "/" + file_name;
         try
@@ -176,7 +175,7 @@ void DatabaseOrdinary::loadStoredObjects(
         if (!create_query.is_dictionary)
             pool.scheduleOrThrowOnError([&]()
             {
-                tryAttachTable(context, create_query, *this, getDataPath(), getDatabaseName(), has_force_restore_data_flag);
+                tryAttachTable(context, create_query, *this, getDatabaseName(), has_force_restore_data_flag);
 
                 /// Messages, so that it's not boring to wait for the server to load for a long time.
                 logAboutProgress(log, ++tables_processed, total_tables, watch);
@@ -374,9 +373,9 @@ void DatabaseOrdinary::alterTable(
 }
 
 
-void DatabaseOrdinary::drop()
+void DatabaseOrdinary::drop(const Context & context)
 {
-    DatabaseOnDisk::drop(*this);
+    DatabaseOnDisk::drop(*this, context);
 }
 
 

diff --git a/dbms/src/Databases/DatabaseOrdinary.h b/dbms/src/Databases/DatabaseOrdinary.h
@@ -83,7 +83,7 @@ class DatabaseOrdinary : public DatabaseWithOwnTablesBase
     String getMetadataPath() const override;
     String getObjectMetadataPath(const String & table_name) const override;
 
-    void drop() override;
+    void drop(const Context & context) override;
 
 private:
     const String metadata_path;

diff --git a/dbms/src/Databases/DatabasesCommon.cpp b/dbms/src/Databases/DatabasesCommon.cpp
@@ -1,22 +1,13 @@
 #include <Databases/DatabasesCommon.h>
-
 #include <Interpreters/ExternalDictionariesLoader.h>
-#include <Interpreters/ExternalLoaderDatabaseConfigRepository.h>
 #include <Interpreters/Context.h>
 #include <Interpreters/InterpreterCreateQuery.h>
-#include <Parsers/ASTCreateQuery.h>
 #include <Parsers/ParserCreateQuery.h>
-#include <Parsers/ParserDictionary.h>
 #include <Parsers/formatAST.h>
-#include <Parsers/parseQuery.h>
-#include <Storages/IStorage.h>
 #include <Storages/StorageDictionary.h>
 #include <Storages/StorageFactory.h>
 #include <Common/typeid_cast.h>
 #include <TableFunctions/TableFunctionFactory.h>
-#include <Dictionaries/DictionaryFactory.h>
-
-#include <sstream>
 
 
 namespace DB

diff --git a/dbms/src/Databases/IDatabase.h b/dbms/src/Databases/IDatabase.h
@@ -237,7 +237,7 @@ class IDatabase : public std::enable_shared_from_this<IDatabase>
     virtual void shutdown() = 0;
 
     /// Delete data and metadata stored inside the database, if exists.
-    virtual void drop() {}
+    virtual void drop(const Context & /*context*/) {}
 
     virtual ~IDatabase() {}
 };

diff --git a/dbms/src/Interpreters/InterpreterCreateQuery.cpp b/dbms/src/Interpreters/InterpreterCreateQuery.cpp
@@ -632,7 +632,7 @@ bool InterpreterCreateQuery::doCreateTable(const ASTCreateQuery & create,
     else
     {
         res = StorageFactory::instance().get(create,
-            data_path,
+            data_path + escapeForFileName(table_name) + "/",
             table_name,
             create.database,
             context,

diff --git a/dbms/src/Interpreters/InterpreterDropQuery.cpp b/dbms/src/Interpreters/InterpreterDropQuery.cpp
@@ -137,7 +137,7 @@ BlockIO InterpreterDropQuery::executeToTable(
             /// If it is not virtual database like Dictionary then drop remaining data dir
             if (!database_data_path.empty())
             {
-                String table_data_path = database_data_path + "/" + escapeForFileName(database_and_table.second->getTableName());
+                String table_data_path = context.getPath() + database_data_path + "/" + escapeForFileName(table_name);
 
                 if (Poco::File(table_data_path).exists())
                     Poco::File(table_data_path).remove(true);
@@ -269,7 +269,7 @@ BlockIO InterpreterDropQuery::executeToDatabase(String & database_name, ASTDropQ
             database->shutdown();
 
             /// Delete the database.
-            database->drop();
+            database->drop(context);
 
             /// Old ClickHouse versions did not store database.sql files
             Poco::File database_metadata_file(context.getPath() + "metadata/" + escapeForFileName(database_name) + ".sql");

diff --git a/dbms/src/Interpreters/InterpreterSystemQuery.cpp b/dbms/src/Interpreters/InterpreterSystemQuery.cpp
@@ -5,6 +5,7 @@
 #include <Common/typeid_cast.h>
 #include <Common/getNumberOfPhysicalCPUCores.h>
 #include <Common/ThreadPool.h>
+#include <Common/escapeForFileName.h>
 #include <Interpreters/Context.h>
 #include <Interpreters/ExternalDictionariesLoader.h>
 #include <Interpreters/EmbeddedDictionaries.h>
@@ -294,7 +295,7 @@ StoragePtr InterpreterSystemQuery::tryRestartReplica(const String & database_nam
         auto constraints = InterpreterCreateQuery::getConstraintsDescription(create.columns_list->constraints);
 
         StoragePtr table = StorageFactory::instance().get(create,
-            data_path,
+            data_path + escapeForFileName(table_name) + "/",
             table_name,
             database_name,
             system_context,

diff --git a/dbms/src/Storages/IStorage.h b/dbms/src/Storages/IStorage.h
@@ -304,7 +304,7 @@ class IStorage : public std::enable_shared_from_this<IStorage>, public TypePromo
       * In this function, you need to rename the directory with the data, if any.
       * Called when the table structure is locked for write.
       */
-    virtual void rename(const String & /*new_path_to_db*/, const String & /*new_database_name*/, const String & /*new_table_name*/,
+    virtual void rename(const String & /*new_path_to_table_data*/, const String & /*new_database_name*/, const String & /*new_table_name*/,
                         TableStructureWriteLockHolder &)
     {
         throw Exception("Method rename is not supported by storage " + getName(), ErrorCodes::NOT_IMPLEMENTED);

diff --git a/dbms/src/Storages/MergeTree/MergeTreeData.cpp b/dbms/src/Storages/MergeTree/MergeTreeData.cpp
@@ -112,6 +112,7 @@ namespace
 MergeTreeData::MergeTreeData(
     const String & database_,
     const String & table_,
+    const String & relative_data_path_,
     const ColumnsDescription & columns_,
     const IndicesDescription & indices_,
     const ConstraintsDescription & constraints_,
@@ -134,6 +135,7 @@ MergeTreeData::MergeTreeData(
     , require_part_metadata(require_part_metadata_)
     , database_name(database_)
     , table_name(table_)
+    , relative_data_path(relative_data_path_)
     , broken_part_callback(broken_part_callback_)
     , log_name(database_name + "." + table_name)
     , log(&Logger::get(log_name))
@@ -1213,13 +1215,9 @@ void MergeTreeData::clearPartsFromFilesystem(const DataPartsVector & parts_to_re
 }
 
 void MergeTreeData::rename(
-    const String & /*new_path_to_db*/, const String & new_database_name,
+    const String & new_table_path, const String & new_database_name,
     const String & new_table_name, TableStructureWriteLockHolder &)
 {
-    auto old_table_path = "data/" + escapeForFileName(database_name) + '/' + escapeForFileName(table_name) + '/';
-    auto new_db_path = "data/" + escapeForFileName(new_database_name) + '/';
-    auto new_table_path = new_db_path + escapeForFileName(new_table_name) + '/';
-
     auto disks = storage_policy->getDisks();
 
     for (const auto & disk : disks)
@@ -1230,15 +1228,16 @@ void MergeTreeData::rename(
 
     for (const auto & disk : disks)
     {
-        disk->createDirectory(new_db_path);
-
-        disk->moveFile(old_table_path, new_table_path);
+        auto new_table_path_parent = Poco::Path(new_table_path).makeParent().toString();
+        disk->createDirectory(new_table_path_parent);
+        disk->moveFile(relative_data_path, new_table_path);
     }
 
     global_context.dropCaches();
 
-    database_name = new_database_name;
+    relative_data_path = new_table_path;
     table_name = new_table_name;
+    database_name = new_database_name;
 }
 
 void MergeTreeData::dropAllData()
@@ -3457,7 +3456,7 @@ MergeTreeData::MutableDataPartPtr MergeTreeData::cloneAndLoadDataPartOnSameDisk(
 
 String MergeTreeData::getFullPathOnDisk(const DiskPtr & disk) const
 {
-    return disk->getPath() + "data/" + escapeForFileName(database_name) + '/' + escapeForFileName(table_name) + '/';
+    return disk->getPath() + relative_data_path;
 }
 
 
@@ -3530,9 +3529,7 @@ void MergeTreeData::freezePartitionsByMatcher(MatcherFn matcher, const String &
 
         String part_absolute_path = Poco::Path(part->getFullPath()).absolute().toString();
         String backup_part_absolute_path = backup_path
-            + "data/"
-            + escapeForFileName(getDatabaseName()) + "/"
-            + escapeForFileName(getTableName()) + "/"
+            + relative_data_path
             + part->relative_path;
         localBackup(part_absolute_path, backup_part_absolute_path);
         part->is_frozen.store(true, std::memory_order_relaxed);