Review fixes

src/Common/Config/ConfigProcessor.cpp

@@ -416,16 +416,15 @@ ConfigProcessor::Files ConfigProcessor::getConfigMergeFiles(const std::string &
     std::set<std::string> merge_dirs;
 
     /// Add path_to_config/config_name.d dir
-    merge_dir_path = merge_dir_path.parent_path() / (merge_dir_path.stem().string() + ".d");
+    merge_dir_path.replace_extension("d");
     merge_dirs.insert(merge_dir_path);
     /// Add path_to_config/conf.d dir
-    merge_dir_path = merge_dir_path.parent_path() / "conf.d";
+    merge_dir_path.replace_filename("conf.d");
     merge_dirs.insert(merge_dir_path);
 
     for (const std::string & merge_dir_name : merge_dirs)
     {
-        fs::path merge_dir(merge_dir_name);
+        if (!fs::exists(merge_dir_name) || !fs::is_directory(merge_dir_name))
-        if (!fs::exists(merge_dir) || !is_directory(merge_dir))
             continue;
 
         for (fs::directory_iterator it(merge_dir_name); it != fs::directory_iterator(); ++it)
@@ -635,7 +634,7 @@ void ConfigProcessor::savePreprocessedConfig(const LoadedConfig & loaded_config,
                     fs::path parent_path = fs::path(loaded_config.config_path).parent_path();
                     preprocessed_dir = parent_path.string();
                     fs::path fs_new_path(new_path);
-                    fs_new_path = fs_new_path.parent_path() / (fs_new_path.stem().string() + PREPROCESSED_SUFFIX + fs_new_path.extension().string());
+                    fs_new_path.replace_filename(fs_new_path.stem().string() + PREPROCESSED_SUFFIX + fs_new_path.extension().string());
                     new_path = fs_new_path.string();
                 }
                 else
@@ -652,7 +651,7 @@ void ConfigProcessor::savePreprocessedConfig(const LoadedConfig & loaded_config,
 
             preprocessed_path = (fs::path(preprocessed_dir) / fs::path(new_path)).string();
             auto preprocessed_path_parent = fs::path(preprocessed_path).parent_path();
-            if (!preprocessed_path_parent.string().empty())
+            if (!preprocessed_path_parent.empty())
                 fs::create_directories(preprocessed_path_parent);
         }
         DOMWriter().writeNode(preprocessed_path, loaded_config.preprocessed_xml);

src/Common/ErrorCodes.cpp

@@ -553,7 +553,6 @@
     M(583, ILLEGAL_PROJECTION) \
     M(584, PROJECTION_NOT_USED) \
     \
-    M(996, OPERATION_NOT_PERMITTED) \
     M(997, CANNOT_CREATE_FILE) \
     M(998, POSTGRESQL_CONNECTION_FAILURE) \
     M(999, KEEPER_EXCEPTION) \

src/Common/createFile.cpp

@@ -11,7 +11,7 @@ namespace DB
 namespace ErrorCodes
 {
 extern const int FILE_ALREADY_EXISTS;
-extern const int OPERATION_NOT_PERMITTED;
+extern const int PATH_ACCESS_DENIED;
 extern const int NOT_ENOUGH_SPACE;
 extern const int CANNOT_CREATE_FILE;
 }
@@ -26,7 +26,7 @@ namespace FS
         case EEXIST:
             throw DB::Exception(DB::ErrorCodes::FILE_ALREADY_EXISTS, "File {} already exist", path);
         case EPERM:
-            throw DB::Exception(DB::ErrorCodes::OPERATION_NOT_PERMITTED, "Not enough permissions to create file {}", path);
+            throw DB::Exception(DB::ErrorCodes::PATH_ACCESS_DENIED, "Not enough permissions to create file {}", path);
         case ENOSPC:
             throw DB::Exception(DB::ErrorCodes::NOT_ENOUGH_SPACE, "Not enough space to create file {}", path);
         case ENAMETOOLONG:

src/Databases/DatabaseOnDisk.cpp

@@ -19,8 +19,6 @@
 #include <Databases/DatabaseAtomic.h>
 #include <Common/assert_cast.h>
 #include <filesystem>
-#include <Poco/File.h>
-#include <Poco/Path.h>
 #include <Common/createFile.h>
 
 namespace fs = std::filesystem;

src/Interpreters/DatabaseCatalog.cpp

@@ -356,7 +356,7 @@ DatabasePtr DatabaseCatalog::detachDatabase(const String & database_name, bool d
         /// Old ClickHouse versions did not store database.sql files
         fs::path database_metadata_file = fs::path(getContext()->getPath()) / "metadata" / (escapeForFileName(database_name) + ".sql");
         if (fs::exists(database_metadata_file))
-            fs::remove_all(database_metadata_file);
+            fs::remove(database_metadata_file);
     }
 
     return db;

src/Interpreters/loadMetadata.cpp

@@ -112,15 +112,15 @@ void loadMetadata(ContextPtr context, const String & default_database_name)
         if (!it->is_directory())
         {
             /// TODO: DETACH DATABASE PERMANENTLY ?
-            if (endsWith(current_file, ".sql"))
+            if (fs::path(current_file).extension() == ".sql")
             {
                 String db_name = current_file.substr(0, current_file.size() - 4);
                 if (db_name != DatabaseCatalog::SYSTEM_DATABASE)
-                    databases.emplace(unescapeForFileName(db_name), path + "/" + db_name);
+                    databases.emplace(unescapeForFileName(db_name), fs::path(path) / db_name);
             }
 
             /// Temporary fails may be left from previous server runs.
-            if (endsWith(current_file, ".tmp"))
+            if (fs::path(current_file).extension() == ".tmp")
             {
                 LOG_WARNING(log, "Removing temporary file {}", it->path().string());
                 try

src/Storages/StorageReplicatedMergeTree.cpp

@@ -6532,7 +6532,7 @@ void StorageReplicatedMergeTree::movePartitionToTable(const StoragePtr & dest_ta
         ops.emplace_back(zkutil::makeCheckRequest(alter_partition_version_path, alter_partition_version_stat.version));
         ops.emplace_back(zkutil::makeSetRequest(alter_partition_version_path, "", -1));
         /// Just update version, because merges assignment relies on it
-        ops.emplace_back(zkutil::makeSetRequest(fs:path(dest_table_storage->zookeeper_path) / "log", "", -1));
+        ops.emplace_back(zkutil::makeSetRequest(fs::path(dest_table_storage->zookeeper_path) / "log", "", -1));
         ops.emplace_back(zkutil::makeCreateRequest(fs::path(dest_table_storage->zookeeper_path) / "log/log-",
                                                    entry.toString(), zkutil::CreateMode::PersistentSequential));