From 45b84d491e8528b1bacad29165306dec48de824b Mon Sep 17 00:00:00 2001
From: Gleb Novikov <glebnovikov98@gmail.com>
Date: Sat, 16 May 2020 23:31:17 +0300
Subject: [PATCH] small fixes of review remarks

---
 src/Disks/IDisk.h                             |  2 +-
 src/Disks/IVolume.cpp                         | 23 -------------
 src/Disks/IVolume.h                           | 32 +++----------------
 src/Disks/createVolume.cpp                    |  2 +-
 src/Disks/createVolume.h                      |  2 +-
 src/Storages/MergeTree/IMergeTreeDataPart.h   |  2 +-
 .../MergeTree/IMergeTreeDataPartWriter.cpp    |  5 +--
 7 files changed, 12 insertions(+), 56 deletions(-)

diff --git a/src/Disks/IDisk.h b/src/Disks/IDisk.h
index b89128e7ded..011c75402f4 100644
--- a/src/Disks/IDisk.h
+++ b/src/Disks/IDisk.h
@@ -24,7 +24,7 @@ class IDiskDirectoryIterator;
 using DiskDirectoryIteratorPtr = std::unique_ptr<IDiskDirectoryIterator>;
 
 class IReservation;
-using ReservationPtr = std::shared_ptr<IReservation>;
+using ReservationPtr = std::unique_ptr<IReservation>;
 
 class ReadBufferFromFileBase;
 class WriteBufferFromFileBase;
diff --git a/src/Disks/IVolume.cpp b/src/Disks/IVolume.cpp
index 4da9aec1ab9..a71afa4a840 100644
--- a/src/Disks/IVolume.cpp
+++ b/src/Disks/IVolume.cpp
@@ -13,29 +13,6 @@ namespace ErrorCodes
     extern const int UNKNOWN_VOLUME_TYPE;
 }
 
-void VolumeType::fromString(const String & str)
-{
-    if (str == "JBOD")
-        value = JBOD;
-    else if (str == "SINGLE_DISK")
-        value = SINGLE_DISK;
-    else
-        throw DB::Exception("Unexpected string for volume type: " + str, ErrorCodes::UNKNOWN_VOLUME_TYPE);
-}
-
-String VolumeType::toString() const
-{
-    switch (value)
-    {
-        case JBOD:
-            return "JBOD";
-        case SINGLE_DISK:
-            return "SINGLE_DISK";
-        default:
-            return "Unknown";
-    }
-}
-
 IVolume::IVolume(
     String name_, const Poco::Util::AbstractConfiguration & config, const String & config_prefix, DiskSelectorPtr disk_selector)
     : name(std::move(name_))
diff --git a/src/Disks/IVolume.h b/src/Disks/IVolume.h
index 0b1c0a20ad9..a762958a33f 100644
--- a/src/Disks/IVolume.h
+++ b/src/Disks/IVolume.h
@@ -8,33 +8,11 @@
 namespace DB
 {
 
-class VolumeType
+enum class VolumeType
 {
-public:
-    enum Value
-    {
-        JBOD,
-        SINGLE_DISK,
-        UNKNOWN
-    };
-    VolumeType() : value(UNKNOWN) {}
-    VolumeType(Value value_) : value(value_) {}
-
-    bool operator==(const VolumeType & other) const
-    {
-        return value == other.value;
-    }
-
-    bool operator!=(const VolumeType & other) const
-    {
-        return !(*this == other);
-    }
-
-    void fromString(const String & str);
-    String toString() const;
-
-private:
-    Value value;
+    JBOD,
+    SINGLE_DISK,
+    UNKNOWN
 };
 
 class IVolume;
@@ -55,7 +33,7 @@ using Volumes = std::vector<VolumePtr>;
 class IVolume : public Space
 {
 public:
-    IVolume(const String & name_, Disks disks_): disks(std::move(disks_)), name(name_)
+    IVolume(String name_, Disks disks_): disks(std::move(disks_)), name(name_)
     {
     }
 
diff --git a/src/Disks/createVolume.cpp b/src/Disks/createVolume.cpp
index 3faca365bd4..d7aa7d3c9ad 100644
--- a/src/Disks/createVolume.cpp
+++ b/src/Disks/createVolume.cpp
@@ -3,7 +3,7 @@
 namespace DB
 {
 
-VolumePtr createVolumeFromReservation(ReservationPtr reservation, VolumePtr other_volume)
+VolumePtr createVolumeFromReservation(const ReservationPtr & reservation, VolumePtr other_volume)
 {
     if (other_volume->getType() == VolumeType::JBOD || other_volume->getType() == VolumeType::SINGLE_DISK)
     {
diff --git a/src/Disks/createVolume.h b/src/Disks/createVolume.h
index 9180d23ea73..52085ec16bc 100644
--- a/src/Disks/createVolume.h
+++ b/src/Disks/createVolume.h
@@ -7,6 +7,6 @@
 namespace DB
 {
 
-VolumePtr createVolumeFromReservation(ReservationPtr reservation, VolumePtr other_volume);
+VolumePtr createVolumeFromReservation(const ReservationPtr & reservation, VolumePtr other_volume);
 
 }
diff --git a/src/Storages/MergeTree/IMergeTreeDataPart.h b/src/Storages/MergeTree/IMergeTreeDataPart.h
index a2fe9661832..e1b276a4dd7 100644
--- a/src/Storages/MergeTree/IMergeTreeDataPart.h
+++ b/src/Storages/MergeTree/IMergeTreeDataPart.h
@@ -29,7 +29,7 @@ struct ColumnSize;
 class MergeTreeData;
 struct FutureMergedMutatedPart;
 class IReservation;
-using ReservationPtr = std::shared_ptr<IReservation>;
+using ReservationPtr = std::unique_ptr<IReservation>;
 
 class IVolume;
 using VolumePtr = std::shared_ptr<IVolume>;
diff --git a/src/Storages/MergeTree/IMergeTreeDataPartWriter.cpp b/src/Storages/MergeTree/IMergeTreeDataPartWriter.cpp
index 239ccba1d89..763f4fff879 100644
--- a/src/Storages/MergeTree/IMergeTreeDataPartWriter.cpp
+++ b/src/Storages/MergeTree/IMergeTreeDataPartWriter.cpp
@@ -85,8 +85,9 @@ IMergeTreeDataPartWriter::IMergeTreeDataPartWriter(
     if (settings.blocks_are_granules_size && !index_granularity.empty())
         throw Exception("Can't take information about index granularity from blocks, when non empty index_granularity array specified", ErrorCodes::LOGICAL_ERROR);
 
-    if (!data_part->volume->getDisk()->exists(part_path))
-        data_part->volume->getDisk()->createDirectories(part_path);
+    auto disk = data_part->volume->getDisk();
+    if (!disk->exists(part_path))
+        disk->createDirectories(part_path);
 }
 
 IMergeTreeDataPartWriter::~IMergeTreeDataPartWriter() = default;