Fix SYSTEM UNFREEZE for ordinary database

src/Parsers/ASTSystemQuery.cpp

@@ -201,6 +201,10 @@ void ASTSystemQuery::formatImpl(const FormatSettings & settings, FormatState &,
         if (!filesystem_cache_path.empty())
             settings.ostr << (settings.hilite ? hilite_none : "") << " " << filesystem_cache_path;
     }
+    else if (type == Type::UNFREEZE)
+    {
+        settings.ostr << (settings.hilite ? hilite_identifier : "") << backQuoteIfNeed(backup_name);
+    }
 }
 
 

src/Storages/Freeze.cpp

@@ -9,8 +9,6 @@ namespace DB
 {
 void FreezeMetaData::fill(const StorageReplicatedMergeTree & storage)
 {
-    is_replicated = storage.supportsReplication();
-    is_remote = storage.isRemote();
     replica_name = storage.getReplicaName();
     zookeeper_name = storage.getZooKeeperName();
     table_shared_id = storage.getTableSharedID();
@@ -26,11 +24,16 @@ void FreezeMetaData::save(DiskPtr data_disk, const String & path) const
 
     writeIntText(version, buffer);
     buffer.write("\n", 1);
-    writeBoolText(is_replicated, buffer);
-    buffer.write("\n", 1);
-    writeBoolText(is_remote, buffer);
-    buffer.write("\n", 1);
-    writeString(replica_name, buffer);
+    if (version == 1) {
+        /// is_replicated and is_remote are not used
+        bool is_replicated = true;
+        writeBoolText(is_replicated, buffer);
+        buffer.write("\n", 1);
+        bool is_remote = true;
+        writeBoolText(is_remote, buffer);
+        buffer.write("\n", 1);
+    }
+    writeString(escapeForFileName(replica_name), buffer);
     buffer.write("\n", 1);
     writeString(zookeeper_name, buffer);
     buffer.write("\n", 1);
@@ -51,17 +54,23 @@ bool FreezeMetaData::load(DiskPtr data_disk, const String & path)
     auto metadata_str = metadata_storage->readFileToString(file_path);
     ReadBufferFromString buffer(metadata_str);
     readIntText(version, buffer);
-    if (version != 1)
+    if (version < 1 or version > 2)
     {
         LOG_ERROR(&Poco::Logger::get("FreezeMetaData"), "Unknown freezed metadata version: {}", version);
         return false;
     }
     DB::assertChar('\n', buffer);
-    readBoolText(is_replicated, buffer);
-    DB::assertChar('\n', buffer);
-    readBoolText(is_remote, buffer);
-    DB::assertChar('\n', buffer);
-    readString(replica_name, buffer);
+    if (version == 1) {
+        /// is_replicated and is_remote are not used
+        bool is_replicated;
+        readBoolText(is_replicated, buffer);
+        DB::assertChar('\n', buffer);
+        bool is_remote;
+        readBoolText(is_remote, buffer);
+        DB::assertChar('\n', buffer);
+    }
+    auto unescaped_replica_name = unescapeForFileName(replica_name);
+    readString(unescaped_replica_name, buffer);
     DB::assertChar('\n', buffer);
     readString(zookeeper_name, buffer);
     DB::assertChar('\n', buffer);
@@ -89,41 +98,46 @@ String FreezeMetaData::getFileName(const String & path)
 
 BlockIO Unfreezer::unfreeze(const String & backup_name, ContextPtr local_context)
 {
-    LOG_DEBUG(log, "Unfreezing backup {}", backup_name);
+    LOG_DEBUG(log, "Unfreezing backup {}", escapeForFileName(backup_name));
     auto disks_map = local_context->getDisksMap();
     Disks disks;
     for (auto & [name, disk]: disks_map)
     {
         disks.push_back(disk);
     }
     auto backup_path = fs::path(backup_directory_prefix) / escapeForFileName(backup_name);
-    auto store_path = backup_path / "store";
+    auto store_paths = {backup_path / "store", backup_path / "data"};
 
     PartitionCommandsResultInfo result_info;
 
     for (const auto & disk: disks)
     {
-        if (!disk->exists(store_path))
-            continue;
-        for (auto prefix_it = disk->iterateDirectory(store_path); prefix_it->isValid(); prefix_it->next())
+        for (auto store_path: store_paths)
         {
-            auto prefix_directory = store_path / prefix_it->name();
-            for (auto table_it = disk->iterateDirectory(prefix_directory); table_it->isValid(); table_it->next())
+            if (!disk->exists(store_path))
+                continue;
+            for (auto prefix_it = disk->iterateDirectory(store_path); prefix_it->isValid(); prefix_it->next())
             {
-                auto table_directory = prefix_directory / table_it->name();
-                auto current_result_info = unfreezePartitionsFromTableDirectory([] (const String &) { return true; }, backup_name, {disk}, table_directory, local_context);
-                for (auto & command_result : current_result_info)
+                auto prefix_directory = store_path / prefix_it->name();
+                for (auto table_it = disk->iterateDirectory(prefix_directory); table_it->isValid(); table_it->next())
                 {
-                    command_result.command_type = "SYSTEM UNFREEZE";
+                    auto table_directory = prefix_directory / table_it->name();
+                    auto current_result_info = unfreezePartitionsFromTableDirectory(
+                        [](const String &) { return true; }, backup_name, {disk}, table_directory, local_context);
+                    for (auto & command_result : current_result_info)
+                    {
+                        command_result.command_type = "SYSTEM UNFREEZE";
+                    }
+                    result_info.insert(
+                        result_info.end(),
+                        std::make_move_iterator(current_result_info.begin()),
+                        std::make_move_iterator(current_result_info.end()));
                 }
-                result_info.insert(
-                                result_info.end(),
-                                std::make_move_iterator(current_result_info.begin()),
-                                std::make_move_iterator(current_result_info.end()));
             }
         }
         if (disk->exists(backup_path))
         {
+            /// After unfreezing we need to clear revision.txt file and empty directories
             disk->removeRecursive(backup_path);
         }
     }
@@ -143,11 +157,8 @@ bool Unfreezer::removeFreezedPart(DiskPtr disk, const String & path, const Strin
         FreezeMetaData meta;
         if (meta.load(disk, path))
         {
-            if (meta.is_replicated)
-            {
-                FreezeMetaData::clean(disk, path);
-                return StorageReplicatedMergeTree::removeSharedDetachedPart(disk, path, part_name, meta.table_shared_id, meta.zookeeper_name, meta.replica_name, "", local_context);
-            }
+            FreezeMetaData::clean(disk, path);
+            return StorageReplicatedMergeTree::removeSharedDetachedPart(disk, path, part_name, meta.table_shared_id, meta.zookeeper_name, meta.replica_name, "", local_context);
         }
     }
 

src/Storages/Freeze.h

@@ -23,9 +23,7 @@ struct FreezeMetaData
     static String getFileName(const String & path);
 
 public:
-    int version = 1;
-    bool is_replicated{false};
-    bool is_remote{false};
+    int version = 2;
     String replica_name;
     String zookeeper_name;
     String table_shared_id;

src/Storages/MergeTree/MergeTreeData.cpp

@@ -1890,7 +1890,7 @@ size_t MergeTreeData::clearOldBrokenPartsFromDetachedDirecory()
 
     for (const auto & [old_name, new_name, disk] : renamed_parts.old_and_new_names)
     {
-        removeDetachedPart(disk, fs::path(relative_data_path) / "detached" / new_name / "", old_name, false);
+        removeDetachedPart(disk, fs::path(relative_data_path) / "detached" / new_name / "", old_name);
         LOG_DEBUG(log, "Removed broken detached part {} due to a timeout for broken detached parts", old_name);
     }
 
@@ -4575,7 +4575,7 @@ void MergeTreeData::dropDetached(const ASTPtr & partition, bool part, ContextPtr
 
     for (auto & [old_name, new_name, disk] : renamed_parts.old_and_new_names)
     {
-        bool keep_shared = removeDetachedPart(disk, fs::path(relative_data_path) / "detached" / new_name / "", old_name, false);
+        bool keep_shared = removeDetachedPart(disk, fs::path(relative_data_path) / "detached" / new_name / "", old_name);
         LOG_DEBUG(log, "Dropped detached part {}, keep shared data: {}", old_name, keep_shared);
         old_name.clear();
     }
@@ -6172,7 +6172,7 @@ PartitionCommandsResultInfo MergeTreeData::unfreezeAll(
     return unfreezePartitionsByMatcher([] (const String &) { return true; }, backup_name, local_context);
 }
 
-bool MergeTreeData::removeDetachedPart(DiskPtr disk, const String & path, const String &, bool)
+bool MergeTreeData::removeDetachedPart(DiskPtr disk, const String & path, const String &)
 {
     disk->removeRecursive(path);
 

src/Storages/MergeTree/MergeTreeData.h

@@ -980,7 +980,7 @@ class MergeTreeData : public IStorage, public WithMutableContext
 
     /// Check shared data usage on other replicas for detached/freezed part
     /// Remove local files and remote files if needed
-    virtual bool removeDetachedPart(DiskPtr disk, const String & path, const String & part_name, bool is_freezed);
+    virtual bool removeDetachedPart(DiskPtr disk, const String & path, const String & part_name);
 
     virtual String getTableSharedID() const { return ""; }
 

src/Storages/StorageReplicatedMergeTree.cpp

@@ -8185,25 +8185,12 @@ void StorageReplicatedMergeTree::createZeroCopyLockNode(
     }
 }
 
-bool StorageReplicatedMergeTree::removeDetachedPart(DiskPtr disk, const String & path, const String & part_name, bool is_freezed)
+bool StorageReplicatedMergeTree::removeDetachedPart(DiskPtr disk, const String & path, const String & part_name)
 {
     if (disk->supportZeroCopyReplication())
     {
-        if (is_freezed)
-        {
-            FreezeMetaData meta;
-            if (meta.load(disk, path))
-            {
-                FreezeMetaData::clean(disk, path);
-                return removeSharedDetachedPart(disk, path, part_name, meta.table_shared_id, meta.zookeeper_name, meta.replica_name, "", getContext());
-            }
-        }
-        else
-        {
-            String table_id = getTableSharedID();
-
-            return removeSharedDetachedPart(disk, path, part_name, table_id, zookeeper_name, replica_name, zookeeper_path, getContext());
-        }
+        String table_id = getTableSharedID();
+        return removeSharedDetachedPart(disk, path, part_name, table_id, zookeeper_name, replica_name, zookeeper_path, getContext());
     }
 
     disk->removeRecursive(path);

src/Storages/StorageReplicatedMergeTree.h

@@ -813,7 +813,7 @@ class StorageReplicatedMergeTree final : public MergeTreeData
         int32_t mode = zkutil::CreateMode::Persistent, bool replace_existing_lock = false,
         const String & path_to_set_hardlinked_files = "", const NameSet & hardlinked_files = {});
 
-    bool removeDetachedPart(DiskPtr disk, const String & path, const String & part_name, bool is_freezed) override;
+    bool removeDetachedPart(DiskPtr disk, const String & path, const String & part_name) override;
 
     /// Create freeze metadata for table and save in zookeeper. Required only if zero-copy replication enabled.
     void createAndStoreFreezeMetadata(DiskPtr disk, DataPartPtr part, String backup_part_path) const override;

tests/queries/0_stateless/01417_freeze_partition_verbose.sh

@@ -1,5 +1,5 @@
 #!/usr/bin/env bash
-# Tags: no-replicated-database, no-parallel, no-ordinary-database
+# Tags: no-replicated-database, no-parallel
 # Tag no-replicated-database: Unsupported type of ALTER query
 
 CURDIR=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)