mirror of
https://github.com/ClickHouse/ClickHouse.git
synced 2026-10-28 16:18:23 +08:00
Groene AI
3fbb02f2bc
`clickhouse-disks --disk <D> move <dir> <newname>` always failed with `DIRECTORY_ALREADY_EXISTS` on a disk with `metadata_type = plain_rewritable`, and left a stray empty destination directory behind. `CommandMove::executeImpl` pre-created the destination and then renamed onto it. `DiskObjectStorage::createDirectory` and `DiskObjectStorage::moveDirectory` each open and commit their own transaction, so the pre-created directory was durably visible to the rename, whose `plain_rewritable` implementation rejects an existing `path_to`. `DiskLocal::moveDirectory` is `fs::rename`, which tolerates an existing empty target, so the local path masked this and the existing `move` coverage (local disk only) never saw it. Dropping the `createDirectory` call makes the directory move a plain rename, which is what `moveDirectory` already promises on every backend. The pre-creation was also strictly more restrictive than the rename it preceded: non-recursive `createDirectory` requires the target's parent to exist, while the rename materializes missing intermediate nodes itself. So `move src aa/bb/cc` with `aa/bb` absent now succeeds on `plain_rewritable` instead of failing; on a local disk it still fails, with `in rename` rather than `in create_directory` in the message. The sibling `createDirectory` call in `CommandCopy` is intentionally left alone: `copyDirectoryContent` copies into an existing target directory, so that caller does need it. The new stateless test runs the same scenarios against a `local` and a `plain_rewritable` metadata disk and asserts the observable disk state. `clickhouse-disks` exits 0 even for this failure, so an exit-code oracle would be vacuous.
78 lines
2.9 KiB
C++
78 lines
2.9 KiB
C++
#include <Interpreters/Context.h>
|
|
#include <ICommand.h>
|
|
#include <Common/logger_useful.h>
|
|
|
|
namespace DB
|
|
{
|
|
|
|
namespace ErrorCodes
|
|
{
|
|
extern const int BAD_ARGUMENTS;
|
|
}
|
|
|
|
|
|
class CommandMove final : public ICommand
|
|
{
|
|
public:
|
|
CommandMove() : ICommand("CommandMove")
|
|
{
|
|
command_name = "move";
|
|
description = "Move file or directory from `from_path` to `to_path`";
|
|
options_description.add_options()("path-from", po::value<String>(), "path from which we copy (mandatory, positional)")(
|
|
"path-to", po::value<String>(), "path to which we copy (mandatory, positional)");
|
|
positional_options_description.add("path-from", 1);
|
|
positional_options_description.add("path-to", 1);
|
|
}
|
|
|
|
void executeImpl(const CommandLineOptions & options, DisksClient & client) override
|
|
{
|
|
auto disk = client.getCurrentDiskWithPath();
|
|
|
|
String path_from = disk.getRelativeFromRoot(getValueFromCommandLineOptionsThrow<String>(options, "path-from"));
|
|
String path_to = disk.getRelativeFromRoot(getValueFromCommandLineOptionsThrow<String>(options, "path-to"));
|
|
|
|
if (disk.getDisk()->existsFile(path_from))
|
|
{
|
|
LOG_INFO(log, "Moving file from '{}' to '{}' at disk '{}'", path_from, path_to, disk.getDisk()->getName());
|
|
disk.getDisk()->moveFile(path_from, path_to);
|
|
}
|
|
else if (disk.getDisk()->existsDirectory(path_from))
|
|
{
|
|
auto target_location = getTargetLocation(path_from, disk, path_to);
|
|
if (!disk.getDisk()->existsDirectory(target_location))
|
|
{
|
|
LOG_INFO(log, "Moving directory from '{}' to '{}' at disk '{}'", path_from, target_location, disk.getDisk()->getName());
|
|
disk.getDisk()->moveDirectory(path_from, target_location);
|
|
}
|
|
else
|
|
{
|
|
if (disk.getDisk()->existsFile(target_location))
|
|
{
|
|
throw Exception(
|
|
ErrorCodes::BAD_ARGUMENTS, "cannot overwrite non-directory '{}' with directory '{}'", target_location, path_from);
|
|
}
|
|
if (!disk.getDisk()->isDirectoryEmpty(target_location))
|
|
{
|
|
throw Exception(ErrorCodes::BAD_ARGUMENTS, "cannot move '{}' to '{}': Directory not empty", path_from, target_location);
|
|
}
|
|
LOG_INFO(log, "Moving directory from '{}' to '{}' at disk '{}'", path_from, target_location, disk.getDisk()->getName());
|
|
disk.getDisk()->moveDirectory(path_from, target_location);
|
|
}
|
|
}
|
|
else
|
|
{
|
|
throw Exception(
|
|
ErrorCodes::BAD_ARGUMENTS,
|
|
"cannot stat '{}' on disk: '{}': No such file or directory",
|
|
path_from,
|
|
disk.getDisk()->getName());
|
|
}
|
|
}
|
|
};
|
|
|
|
CommandPtr makeCommandMove()
|
|
{
|
|
return std::make_shared<DB::CommandMove>();
|
|
}
|
|
}
|