Fix bugs

programs/disks/CMakeLists.txt

@@ -1,18 +1,18 @@
 set (CLICKHOUSE_DISKS_SOURCES
-    ICommand.cpp
-    DisksClient.cpp
     DisksApp.cpp
-    CommandCopy.cpp
-    CommandListDisks.cpp
-    CommandList.cpp
-    CommandLink.cpp
+    DisksClient.cpp
+    ICommand.cpp
     CommandChangeDirectory.cpp
+    CommandCopy.cpp
+    CommandLink.cpp
+    CommandList.cpp
+    CommandListDisks.cpp
     CommandMkDir.cpp
     CommandMove.cpp
     CommandRead.cpp
     CommandRemove.cpp
-    CommandWrite.cpp
-    CommandSwitchDisk.cpp)
+    CommandSwitchDisk.cpp
+    CommandWrite.cpp)
 
 if (CLICKHOUSE_CLOUD)
   set (CLICKHOUSE_DISKS_SOURCES ${CLICKHOUSE_DISKS_SOURCES} CommandPackedIO.cpp)

programs/disks/CommandChangeDirectory.cpp

@@ -14,7 +14,7 @@ public:
     {
         command_name = "cd";
         description = "Change directory";
-        options_description.add_options()("path", po::value<String>(), "the path of listing  (mandatory, positional)")(
+        options_description.add_options()("path", po::value<String>(), "the path we want to get to (mandatory, positional)")(
             "disk", po::value<String>(), "A disk where the path is changed");
         positional_options_description.add("path", 1);
     }

programs/disks/CommandLink.cpp

@@ -12,8 +12,8 @@ public:
         command_name = "link";
         description = "Create hardlink from `from_path` to `to_path`";
         options_description.add_options()(
-            "path-to", po::value<String>(), "the path from which a hard link will be created (mandatory, positional)")(
-            "path-from", po::value<String>(), "the path where a hard link will be created (mandatory, positional)");
+            "path-from", po::value<String>(), "the path where a hard link will be created (mandatory, positional)")(
+            "path-to", po::value<String>(), "the path from which a hard link will be created (mandatory, positional)");
         positional_options_description.add("path-from", 1);
         positional_options_description.add("path-to", 1);
     }

programs/disks/CommandMove.cpp

@@ -25,9 +25,9 @@ public:
         String path_to = disk.getRelativeFromRoot(getValueFromCommandLineOptionsThrow<String>(options, "path-to"));
 
         if (disk.getDisk()->isFile(path_from))
-            disk.getDisk()->moveFile(path_from, path_from);
+            disk.getDisk()->moveFile(path_from, path_to);
         else
-            disk.getDisk()->moveDirectory(path_from, path_from);
+            disk.getDisk()->moveDirectory(path_from, path_to);
     }
 };
 

programs/disks/CommandRead.cpp

@@ -2,6 +2,7 @@
 #include <IO/WriteBufferFromFile.h>
 #include <IO/copyData.h>
 #include <Interpreters/Context.h>
+#include "Common/Exception.h"
 #include <Common/TerminalSize.h>
 #include "ICommand.h"
 
@@ -15,9 +16,8 @@ public:
     {
         command_name = "read";
         description = "Read a file from `FROM_PATH` to `TO_PATH`";
-        options_description.add_options()(
-            "path-from", po::value<String>(), "file from which we are reading, defaults to `stdin` (mandatory, positional)")(
-            "path-to", po::value<String>(), "file to which we are writing");
+        options_description.add_options()("path-from", po::value<String>(), "file from which we are reading (mandatory, positional)")(
+            "path-to", po::value<String>(), "file to which we are writing, , defaults to `stdout`");
         positional_options_description.add("path-from", 1);
     }
 
@@ -25,26 +25,24 @@ public:
     {
         auto disk = client.getCurrentDiskWithPath();
         String path_from = disk.getRelativeFromRoot(getValueFromCommandLineOptionsThrow<String>(options, "path-from"));
+        std::cerr << path_from << std::endl;
         std::optional<String> path_to = getValueFromCommandLineOptionsWithOptional<String>(options, "path-to");
-        if (path_to.has_value())
-        {
-            path_to = std::optional{disk.getRelativeFromRoot(path_to.value())};
-        }
 
         auto in = disk.getDisk()->readFile(path_from);
+        std::unique_ptr<WriteBufferFromFileBase> out = {};
         if (path_to.has_value())
         {
             String relative_path_to = disk.getRelativeFromRoot(path_to.value());
-
-            auto out = disk.getDisk()->writeFile(relative_path_to);
+            out = disk.getDisk()->writeFile(relative_path_to);
             copyData(*in, *out);
-            out->finalize();
         }
         else
         {
-            std::unique_ptr<WriteBufferFromFileBase> out = std::make_unique<WriteBufferFromFileDescriptor>(STDOUT_FILENO);
+            out = std::make_unique<WriteBufferFromFileDescriptor>(STDOUT_FILENO);
             copyData(*in, *out);
+            out->write('\n');
         }
+        out->finalize();
     }
 };
 

programs/disks/CommandSwitchDisk.cpp

@@ -30,11 +30,7 @@ public:
         String disk = getValueFromCommandLineOptions<String>(options, "disk");
         std::optional<String> path = getValueFromCommandLineOptionsWithOptional<String>(options, "path");
 
-        if (!client.switchToDisk(disk, path))
-        {
-            throw Exception(
-                ErrorCodes::BAD_ARGUMENTS, "Unable to switch to disk: {}, path: {}", disk, path.has_value() ? path.value() : "NO PATH");
-        }
+        client.switchToDisk(disk, path);
     }
 };
 

programs/disks/CommandWrite.cpp

@@ -21,15 +21,12 @@ public:
         positional_options_description.add("path-to", 1);
     }
 
+
     void executeImpl(const CommandLineOptions & options, DisksClient & client) override
     {
         auto disk = client.getCurrentDiskWithPath();
 
         std::optional<String> path_from = getValueFromCommandLineOptionsWithOptional<String>(options, "path-from");
-        if (path_from.has_value())
-        {
-            path_from = std::optional{disk.getRelativeFromRoot(path_from.value())};
-        }
 
         String path_to = disk.getRelativeFromRoot(getValueFromCommandLineOptionsThrow<String>(options, "path-to"));
 

programs/disks/DisksClient.h

@@ -252,7 +252,7 @@ public:
 
     DiskPtr getDisk(const String & disk) const { return getDiskWithPath(disk).getDisk(); }
 
-    bool switchToDisk(const String & disk_, const std::optional<String> & path_)
+    void switchToDisk(const String & disk_, const std::optional<String> & path_)
     {
         if (disks.contains(disk_))
         {
@@ -261,7 +261,6 @@ public:
                 disks.at(disk_).setPath(path_.value());
             }
             current_disk = disk_;
-            return true;
         }
         else
         {

programs/disks/ICommand.h

@@ -58,7 +58,7 @@ protected:
         }
         catch (...)
         {
-            throw DB::Exception(ErrorCodes::BAD_ARGUMENTS, "Argument {} has wrong type and can't be parsed", name);
+            throw DB::Exception(ErrorCodes::BAD_ARGUMENTS, "Argument '{}' has wrong type and can't be parsed", name);
         }
     }
 
@@ -71,7 +71,7 @@ protected:
         }
         else
         {
-            throw DB::Exception(ErrorCodes::BAD_ARGUMENTS, "Mandatory argument {} is missing", name);
+            throw DB::Exception(ErrorCodes::BAD_ARGUMENTS, "Mandatory argument '{}' is missing", name);
         }
     }