From 5539faa54e183619ed305268b74656e04db2006c Mon Sep 17 00:00:00 2001 From: Michael Kolupaev Date: Fri, 7 Jun 2024 02:07:06 +0000 Subject: [PATCH 1/2] Disallow creating refreshable MV on Linux < 3.15 --- src/Common/atomicRename.cpp | 28 ++++++++++++++------- src/Common/atomicRename.h | 2 +- src/Databases/DatabaseAtomic.cpp | 5 ++-- src/Interpreters/InterpreterCreateQuery.cpp | 5 ++++ 4 files changed, 28 insertions(+), 12 deletions(-) diff --git a/src/Common/atomicRename.cpp b/src/Common/atomicRename.cpp index 4acdff5f66c..1ce1c6b0cf5 100644 --- a/src/Common/atomicRename.cpp +++ b/src/Common/atomicRename.cpp @@ -57,11 +57,15 @@ namespace ErrorCodes namespace DB { -static bool supportsAtomicRenameImpl() +static bool supportsAtomicRenameImpl(std::string * out_message) { VersionNumber renameat2_minimal_version(3, 15, 0); VersionNumber linux_version(Poco::Environment::osVersion()); - return linux_version >= renameat2_minimal_version; + if (linux_version >= renameat2_minimal_version) + return true; + if (out_message) + *out_message = fmt::format("Linux kernel 3.15+ is required, got {}", linux_version.toString()); + return false; } static bool renameat2(const std::string & old_path, const std::string & new_path, int flags) @@ -97,9 +101,9 @@ static bool renameat2(const std::string & old_path, const std::string & new_path ErrnoException::throwFromPath(ErrorCodes::SYSTEM_ERROR, new_path, "Cannot rename {} to {}", old_path, new_path); } -bool supportsAtomicRename() +bool supportsAtomicRename(std::string * out_message) { - static bool supports = supportsAtomicRenameImpl(); + static bool supports = supportsAtomicRenameImpl(out_message); return supports; } @@ -152,15 +156,19 @@ static bool renameat2(const std::string & old_path, const std::string & new_path } -static bool supportsAtomicRenameImpl() +static bool supportsAtomicRenameImpl(std::string * out_message) { auto fun = dlsym(RTLD_DEFAULT, "renamex_np"); - return fun != nullptr; + if (fun != nullptr) + return true; + if (out_message) + *out_message = "macOS 10.12 or later is required"; + return false; } -bool supportsAtomicRename() +bool supportsAtomicRename(std::string * out_message) { - static bool supports = supportsAtomicRenameImpl(); + static bool supports = supportsAtomicRenameImpl(out_message); return supports; } @@ -179,8 +187,10 @@ static bool renameat2(const std::string &, const std::string &, int) return false; } -bool supportsAtomicRename() +bool supportsAtomicRename(std::string * out_message) { + if (out_message) + *out_message = "only Linux and macOS are supported"; return false; } diff --git a/src/Common/atomicRename.h b/src/Common/atomicRename.h index 6da8a8f623b..96d0d6d1e5a 100644 --- a/src/Common/atomicRename.h +++ b/src/Common/atomicRename.h @@ -6,7 +6,7 @@ namespace DB { /// Returns true, if the following functions supported by the system -bool supportsAtomicRename(); +bool supportsAtomicRename(std::string * out_message = nullptr); /// Atomically rename old_path to new_path. If new_path exists, do not overwrite it and throw exception void renameNoReplace(const std::string & old_path, const std::string & new_path); diff --git a/src/Databases/DatabaseAtomic.cpp b/src/Databases/DatabaseAtomic.cpp index d86e29ca915..e2e2414b1ca 100644 --- a/src/Databases/DatabaseAtomic.cpp +++ b/src/Databases/DatabaseAtomic.cpp @@ -197,8 +197,9 @@ void DatabaseAtomic::renameTable(ContextPtr local_context, const String & table_ throw Exception(ErrorCodes::NOT_IMPLEMENTED, "Moving tables between databases of different engines is not supported"); } - if (exchange && !supportsAtomicRename()) - throw Exception(ErrorCodes::NOT_IMPLEMENTED, "RENAME EXCHANGE is not supported"); + std::string message; + if (exchange && !supportsAtomicRename(&message)) + throw Exception(ErrorCodes::NOT_IMPLEMENTED, "RENAME EXCHANGE is not supported because exchanging files is not supported by the OS ({})", message); waitDatabaseStarted(); diff --git a/src/Interpreters/InterpreterCreateQuery.cpp b/src/Interpreters/InterpreterCreateQuery.cpp index c1f9b4637f8..6b79552ef0b 100644 --- a/src/Interpreters/InterpreterCreateQuery.cpp +++ b/src/Interpreters/InterpreterCreateQuery.cpp @@ -968,6 +968,11 @@ void InterpreterCreateQuery::validateMaterializedViewColumnsAndEngine(const ASTC if (database && database->getEngineName() != "Atomic") throw Exception(ErrorCodes::INCORRECT_QUERY, "Refreshable materialized views (except with APPEND) only support Atomic database engine, but database {} has engine {}", create.getDatabase(), database->getEngineName()); + + std::string message; + if (!supportsAtomicRename(&message)) + throw Exception(ErrorCodes::NOT_IMPLEMENTED, + "Can't create refreshable materialized view because exchanging files is not supported by the OS ({})", message); } Block input_block; From 263cd9e1c38f74c1e9fe7351fd15c69d6e7b5da9 Mon Sep 17 00:00:00 2001 From: Michael Kolupaev Date: Fri, 7 Jun 2024 20:09:07 +0000 Subject: [PATCH 2/2] Fix error message --- src/Common/atomicRename.cpp | 32 ++++++++++++++++++-------------- 1 file changed, 18 insertions(+), 14 deletions(-) diff --git a/src/Common/atomicRename.cpp b/src/Common/atomicRename.cpp index 1ce1c6b0cf5..7d181d72154 100644 --- a/src/Common/atomicRename.cpp +++ b/src/Common/atomicRename.cpp @@ -57,15 +57,13 @@ namespace ErrorCodes namespace DB { -static bool supportsAtomicRenameImpl(std::string * out_message) +static std::optional supportsAtomicRenameImpl() { VersionNumber renameat2_minimal_version(3, 15, 0); VersionNumber linux_version(Poco::Environment::osVersion()); if (linux_version >= renameat2_minimal_version) - return true; - if (out_message) - *out_message = fmt::format("Linux kernel 3.15+ is required, got {}", linux_version.toString()); - return false; + return std::nullopt; + return fmt::format("Linux kernel 3.15+ is required, got {}", linux_version.toString()); } static bool renameat2(const std::string & old_path, const std::string & new_path, int flags) @@ -103,8 +101,12 @@ static bool renameat2(const std::string & old_path, const std::string & new_path bool supportsAtomicRename(std::string * out_message) { - static bool supports = supportsAtomicRenameImpl(out_message); - return supports; + static auto error = supportsAtomicRenameImpl(); + if (!error.has_value()) + return true; + if (out_message) + *out_message = error.value(); + return false; } } @@ -156,20 +158,22 @@ static bool renameat2(const std::string & old_path, const std::string & new_path } -static bool supportsAtomicRenameImpl(std::string * out_message) +static std::optional supportsAtomicRenameImpl() { auto fun = dlsym(RTLD_DEFAULT, "renamex_np"); if (fun != nullptr) - return true; - if (out_message) - *out_message = "macOS 10.12 or later is required"; - return false; + return std::nullopt; + return "macOS 10.12 or later is required"; } bool supportsAtomicRename(std::string * out_message) { - static bool supports = supportsAtomicRenameImpl(out_message); - return supports; + static auto error = supportsAtomicRenameImpl(); + if (!error.has_value()) + return true; + if (out_message) + *out_message = error.value(); + return false; } }