Small style corrections

This commit is contained in:
kssenii 2023-06-17 01:54:38 +02:00
parent 55135b6711
commit b902f5d045
6 changed files with 114 additions and 109 deletions

View File

@ -31,21 +31,22 @@ namespace ErrorCodes
DatabaseFilesystem::DatabaseFilesystem(const String & name_, const String & path_, ContextPtr context_)
: IDatabase(name_), WithContext(context_->getGlobalContext()), path(path_), log(&Poco::Logger::get("DatabaseFileSystem(" + name_ + ")"))
{
fs::path user_files_path;
const auto & application_type = context_->getApplicationType();
if (application_type != Context::ApplicationType::LOCAL)
user_files_path = fs::canonical(fs::path(getContext()->getUserFilesPath()));
bool is_local = context_->getApplicationType() == Context::ApplicationType::LOCAL;
fs::path user_files_path = is_local ? "" : fs::canonical(getContext()->getUserFilesPath());
if (fs::path(path).is_relative())
{
path = user_files_path / path;
else if (application_type != Context::ApplicationType::LOCAL && !pathStartsWith(fs::path(path), user_files_path))
throw Exception(ErrorCodes::BAD_ARGUMENTS, "Path must be inside user-files path ({})", user_files_path.string());
path = fs::absolute(path).lexically_normal().string();
}
else if (!is_local && !pathStartsWith(fs::path(path), user_files_path))
{
throw Exception(ErrorCodes::BAD_ARGUMENTS,
"Path must be inside user-files path: {}", user_files_path.string());
}
path = fs::absolute(path).lexically_normal();
if (!fs::exists(path))
throw Exception(ErrorCodes::BAD_ARGUMENTS, "Path does not exist ({})", path);
throw Exception(ErrorCodes::BAD_ARGUMENTS, "Path does not exist: {}", path);
}
std::string DatabaseFilesystem::getTablePath(const std::string & table_name) const
@ -62,19 +63,17 @@ void DatabaseFilesystem::addTable(const std::string & table_name, StoragePtr tab
throw Exception(
ErrorCodes::LOGICAL_ERROR,
"Table with name `{}` already exists in database `{}` (engine {})",
table_name,
getDatabaseName(),
getEngineName());
table_name, getDatabaseName(), getEngineName());
}
bool DatabaseFilesystem::checkTableFilePath(const std::string & table_path, ContextPtr context_, bool throw_on_error) const
{
// If run in Local mode, no need for path checking.
bool need_check_path = context_->getApplicationType() != Context::ApplicationType::LOCAL;
std::string user_files_path = fs::canonical(fs::path(context_->getUserFilesPath())).string();
/// If run in Local mode, no need for path checking.
bool check_path = context_->getApplicationType() != Context::ApplicationType::LOCAL;
const auto & user_files_path = context_->getUserFilesPath();
// Check access for file before checking its existence
if (need_check_path && !fileOrSymlinkPathStartsWith(table_path, user_files_path))
/// Check access for file before checking its existence.
if (check_path && !fileOrSymlinkPathStartsWith(table_path, user_files_path))
{
if (throw_on_error)
throw Exception(ErrorCodes::DATABASE_ACCESS_DENIED, "File is not inside {}", user_files_path);
@ -82,11 +81,20 @@ bool DatabaseFilesystem::checkTableFilePath(const std::string & table_path, Cont
return false;
}
// Check if the corresponding file exists
if (!fs::exists(table_path) || !fs::is_regular_file(table_path))
/// Check if the corresponding file exists.
if (!fs::exists(table_path))
{
if (throw_on_error)
throw Exception(ErrorCodes::FILE_DOESNT_EXIST, "File does not exist ({})", table_path);
throw Exception(ErrorCodes::FILE_DOESNT_EXIST, "File does not exist: {}", table_path);
else
return false;
}
if (!fs::is_regular_file(table_path))
{
if (throw_on_error)
throw Exception(ErrorCodes::FILE_DOESNT_EXIST,
"File is directory, but expected a file: {}", table_path);
else
return false;
}
@ -104,7 +112,7 @@ StoragePtr DatabaseFilesystem::tryGetTableFromCache(const std::string & name) co
table = it->second;
}
// invalidate cache if file no longer exists
/// Invalidate cache if file no longer exists.
if (table && !fs::exists(getTablePath(name)))
{
std::lock_guard lock(mutex);
@ -120,29 +128,26 @@ bool DatabaseFilesystem::isTableExist(const String & name, ContextPtr context_)
if (tryGetTableFromCache(name))
return true;
fs::path table_file_path(getTablePath(name));
return checkTableFilePath(table_file_path, context_, false);
return checkTableFilePath(getTablePath(name), context_, /* throw_on_error */false);
}
StoragePtr DatabaseFilesystem::getTableImpl(const String & name, ContextPtr context_) const
{
// Check if table exists in loaded tables map
/// Check if table exists in loaded tables map.
if (auto table = tryGetTableFromCache(name))
return table;
auto table_path = getTablePath(name);
checkTableFilePath(table_path, context_, /* throw_on_error */true);
checkTableFilePath(table_path, context_, true);
// If the file exists, create a new table using TableFunctionFile and return it.
/// If the file exists, create a new table using TableFunctionFile and return it.
auto args = makeASTFunction("file", std::make_shared<ASTLiteral>(table_path));
auto table_function = TableFunctionFactory::instance().get(args, context_);
if (!table_function)
return nullptr;
// TableFunctionFile throws exceptions, if table cannot be created
/// TableFunctionFile throws exceptions, if table cannot be created.
auto table_storage = table_function->execute(args, context_, name);
if (table_storage)
addTable(name, table_storage);
@ -152,10 +157,12 @@ StoragePtr DatabaseFilesystem::getTableImpl(const String & name, ContextPtr cont
StoragePtr DatabaseFilesystem::getTable(const String & name, ContextPtr context_) const
{
// rethrow all exceptions from TableFunctionFile to show correct error to user
/// getTableImpl can throw exceptions, do not catch them to show correct error to user.
if (auto storage = getTableImpl(name, context_))
return storage;
throw Exception(ErrorCodes::UNKNOWN_TABLE, "Table {}.{} doesn't exist", backQuoteIfNeed(getDatabaseName()), backQuoteIfNeed(name));
throw Exception(ErrorCodes::UNKNOWN_TABLE, "Table {}.{} doesn't exist",
backQuoteIfNeed(getDatabaseName()), backQuoteIfNeed(name));
}
StoragePtr DatabaseFilesystem::tryGetTable(const String & name, ContextPtr context_) const
@ -166,15 +173,14 @@ StoragePtr DatabaseFilesystem::tryGetTable(const String & name, ContextPtr conte
}
catch (const Exception & e)
{
// Ignore exceptions thrown by TableFunctionFile, which indicate that there is no table
// see tests/02722_database_filesystem.sh for more details
if (e.code() == ErrorCodes::BAD_ARGUMENTS)
/// Ignore exceptions thrown by TableFunctionFile, which indicate that there is no table
/// see tests/02722_database_filesystem.sh for more details.
if (e.code() == ErrorCodes::BAD_ARGUMENTS
|| e.code() == ErrorCodes::DATABASE_ACCESS_DENIED
|| e.code() == ErrorCodes::FILE_DOESNT_EXIST)
{
return nullptr;
if (e.code() == ErrorCodes::DATABASE_ACCESS_DENIED)
return nullptr;
if (e.code() == ErrorCodes::FILE_DOESNT_EXIST)
return nullptr;
}
throw;
}
}
@ -187,10 +193,10 @@ bool DatabaseFilesystem::empty() const
ASTPtr DatabaseFilesystem::getCreateDatabaseQuery() const
{
auto settings = getContext()->getSettingsRef();
ParserCreateQuery parser;
const auto & settings = getContext()->getSettingsRef();
const String query = fmt::format("CREATE DATABASE {} ENGINE = Filesystem('{}')", backQuoteIfNeed(getDatabaseName()), path);
ParserCreateQuery parser;
ASTPtr ast = parseQuery(parser, query.data(), query.data() + query.size(), "", 0, settings.max_parser_depth);
if (const auto database_comment = getDatabaseComment(); !database_comment.empty())
@ -238,4 +244,4 @@ DatabaseTablesIteratorPtr DatabaseFilesystem::getTablesIterator(ContextPtr, cons
return std::make_unique<DatabaseTablesSnapshotIterator>(Tables{}, getDatabaseName());
}
} // DB
}

View File

@ -12,11 +12,13 @@ namespace DB
class Context;
/**
* DatabaseFilesystem allows to interact with files stored on the file system
* Uses TableFunctionFile to implicitly load file when a user requests the table, and provides read-only access to the data in the file
* DatabaseFilesystem allows to interact with files stored on the local filesystem.
* Uses TableFunctionFile to implicitly load file when a user requests the table,
* and provides a read-only access to the data in the file.
* Tables are cached inside the database for quick access
*
* Used in clickhouse-local to access local files
* Used in clickhouse-local to access local files.
* For clickhouse-server requires allows to access file only fron user_files directory.
*/
class DatabaseFilesystem : public IDatabase, protected WithContext
{
@ -31,8 +33,7 @@ public:
StoragePtr tryGetTable(const String & name, ContextPtr context) const override;
// Contains only temporary tables
bool shouldBeEmptyOnDetach() const override { return false; }
bool shouldBeEmptyOnDetach() const override { return false; } /// Contains only temporary tables.
bool empty() const override;
@ -43,6 +44,7 @@ public:
void shutdown() override;
std::vector<std::pair<ASTPtr, StoragePtr>> getTablesForBackup(const FilterByNameFunction &, const ContextPtr &) const override;
DatabaseTablesIteratorPtr getTablesIterator(ContextPtr, const FilterByNameFunction &) const override;
protected:
@ -62,4 +64,4 @@ private:
Poco::Logger * log;
};
} // DB
}

View File

@ -49,7 +49,9 @@ DatabaseHDFS::DatabaseHDFS(const String & name_, const String & source_url, Cont
if (!source.empty())
{
if (!re2::RE2::FullMatch(source, std::string(HDFS_HOST_REGEXP)))
throw Exception(ErrorCodes::BAD_ARGUMENTS, "Bad hdfs host: {}. It should have structure 'hdfs://<host_name>:<port>'", source);
throw Exception(ErrorCodes::BAD_ARGUMENTS, "Bad hdfs host: {}. "
"It should have structure 'hdfs://<host_name>:<port>'", source);
context_->getGlobalContext()->getRemoteHostFilter().checkURL(Poco::URI(source));
}
}
@ -62,18 +64,19 @@ void DatabaseHDFS::addTable(const std::string & table_name, StoragePtr table_sto
throw Exception(
ErrorCodes::LOGICAL_ERROR,
"Table with name `{}` already exists in database `{}` (engine {})",
table_name,
getDatabaseName(),
getEngineName());
table_name, getDatabaseName(), getEngineName());
}
std::string DatabaseHDFS::getTablePath(const std::string & table_name) const
{
if (table_name.starts_with("hdfs://"))
return table_name;
if (source.empty())
throw Exception(ErrorCodes::BAD_ARGUMENTS, "Bad hdfs url: {}. It should have structure 'hdfs://<host_name>:<port>/path'", table_name);
return (fs::path(source) / table_name).string();
throw Exception(ErrorCodes::BAD_ARGUMENTS, "Bad hdfs url: {}. "
"It should have structure 'hdfs://<host_name>:<port>/path'", table_name);
return fs::path(source) / table_name;
}
bool DatabaseHDFS::checkUrl(const std::string & url, ContextPtr context_, bool throw_on_error) const
@ -104,7 +107,7 @@ bool DatabaseHDFS::isTableExist(const String & name, ContextPtr context_) const
StoragePtr DatabaseHDFS::getTableImpl(const String & name, ContextPtr context_) const
{
// Check if the table exists in the loaded tables map
/// Check if the table exists in the loaded tables map.
{
std::lock_guard lock(mutex);
auto it = loaded_tables.find(name);
@ -116,14 +119,13 @@ StoragePtr DatabaseHDFS::getTableImpl(const String & name, ContextPtr context_)
checkUrl(url, context_, true);
// call TableFunctionHDFS
auto args = makeASTFunction("hdfs", std::make_shared<ASTLiteral>(url));
auto table_function = TableFunctionFactory::instance().get(args, context_);
if (!table_function)
return nullptr;
// TableFunctionHDFS throws exceptions, if table cannot be created
/// TableFunctionHDFS throws exceptions, if table cannot be created.
auto table_storage = table_function->execute(args, context_, name);
if (table_storage)
addTable(name, table_storage);
@ -133,10 +135,12 @@ StoragePtr DatabaseHDFS::getTableImpl(const String & name, ContextPtr context_)
StoragePtr DatabaseHDFS::getTable(const String & name, ContextPtr context_) const
{
// rethrow all exceptions from TableFunctionHDFS to show correct error to user
/// Rethrow all exceptions from TableFunctionHDFS to show correct error to user.
if (auto storage = getTableImpl(name, context_))
return storage;
throw Exception(ErrorCodes::UNKNOWN_TABLE, "Table {}.{} doesn't exist", backQuoteIfNeed(getDatabaseName()), backQuoteIfNeed(name));
throw Exception(ErrorCodes::UNKNOWN_TABLE, "Table {}.{} doesn't exist",
backQuoteIfNeed(getDatabaseName()), backQuoteIfNeed(name));
}
StoragePtr DatabaseHDFS::tryGetTable(const String & name, ContextPtr context_) const
@ -148,20 +152,16 @@ StoragePtr DatabaseHDFS::tryGetTable(const String & name, ContextPtr context_) c
catch (const Exception & e)
{
// Ignore exceptions thrown by TableFunctionHDFS, which indicate that there is no table
if (e.code() == ErrorCodes::BAD_ARGUMENTS)
return nullptr;
if (e.code() == ErrorCodes::ACCESS_DENIED)
return nullptr;
if (e.code() == ErrorCodes::DATABASE_ACCESS_DENIED)
return nullptr;
if (e.code() == ErrorCodes::FILE_DOESNT_EXIST)
return nullptr;
if (e.code() == ErrorCodes::UNACCEPTABLE_URL)
return nullptr;
if (e.code() == ErrorCodes::HDFS_ERROR)
return nullptr;
if (e.code() == ErrorCodes::CANNOT_EXTRACT_TABLE_STRUCTURE)
if (e.code() == ErrorCodes::BAD_ARGUMENTS
|| e.code() == ErrorCodes::ACCESS_DENIED
|| e.code() == ErrorCodes::DATABASE_ACCESS_DENIED
|| e.code() == ErrorCodes::FILE_DOESNT_EXIST
|| e.code() == ErrorCodes::UNACCEPTABLE_URL
|| e.code() == ErrorCodes::HDFS_ERROR
|| e.code() == ErrorCodes::CANNOT_EXTRACT_TABLE_STRUCTURE)
{
return nullptr;
}
throw;
}
catch (const Poco::URISyntaxException &)
@ -178,7 +178,7 @@ bool DatabaseHDFS::empty() const
ASTPtr DatabaseHDFS::getCreateDatabaseQuery() const
{
auto settings = getContext()->getSettingsRef();
const auto & settings = getContext()->getSettingsRef();
ParserCreateQuery parser;
const String query = fmt::format("CREATE DATABASE {} ENGINE = HDFS('{}')", backQuoteIfNeed(getDatabaseName()), source);

View File

@ -16,9 +16,10 @@ namespace DB
class Context;
/**
* DatabaseHDFS allows to interact with files stored on the file system
* Uses TableFunctionHDFS to implicitly load file when a user requests the table, and provides read-only access to the data in the file
* Tables are cached inside the database for quick access
* DatabaseHDFS allows to interact with files stored on the file system.
* Uses TableFunctionHDFS to implicitly load file when a user requests the table,
* and provides read-only access to the data in the file.
* Tables are cached inside the database for quick access.
*/
class DatabaseHDFS : public IDatabase, protected WithContext
{
@ -33,8 +34,7 @@ public:
StoragePtr tryGetTable(const String & name, ContextPtr context) const override;
// Contains only temporary tables
bool shouldBeEmptyOnDetach() const override { return false; }
bool shouldBeEmptyOnDetach() const override { return false; } /// Contains only temporary tables.
bool empty() const override;
@ -63,6 +63,6 @@ private:
Poco::Logger * log;
};
} // DB
}
#endif

View File

@ -60,15 +60,13 @@ void DatabaseS3::addTable(const std::string & table_name, StoragePtr table_stora
throw Exception(
ErrorCodes::LOGICAL_ERROR,
"Table with name `{}` already exists in database `{}` (engine {})",
table_name,
getDatabaseName(),
getEngineName());
table_name, getDatabaseName(), getEngineName());
}
std::string DatabaseS3::getFullUrl(const std::string & name) const
{
if (!config.url_prefix.empty())
return (fs::path(config.url_prefix) / name).string();
return fs::path(config.url_prefix) / name;
return name;
}
@ -100,7 +98,7 @@ bool DatabaseS3::isTableExist(const String & name, ContextPtr context_) const
StoragePtr DatabaseS3::getTableImpl(const String & name, ContextPtr context_) const
{
// Check if the table exists in the loaded tables map
/// Check if the table exists in the loaded tables map.
{
std::lock_guard lock(mutex);
auto it = loaded_tables.find(name);
@ -109,12 +107,9 @@ StoragePtr DatabaseS3::getTableImpl(const String & name, ContextPtr context_) co
}
auto url = getFullUrl(name);
checkUrl(url, context_, /* throw_on_error */true);
checkUrl(url, context_, true);
// call TableFunctionS3
auto function = std::make_shared<ASTFunction>();
function->name = "s3";
function->arguments = std::make_shared<ASTExpressionList>();
function->children.push_back(function->arguments);
@ -134,7 +129,7 @@ StoragePtr DatabaseS3::getTableImpl(const String & name, ContextPtr context_) co
if (!table_function)
return nullptr;
// TableFunctionS3 throws exceptions, if table cannot be created
/// TableFunctionS3 throws exceptions, if table cannot be created.
auto table_storage = table_function->execute(function, context_, name);
if (table_storage)
addTable(name, table_storage);
@ -144,10 +139,12 @@ StoragePtr DatabaseS3::getTableImpl(const String & name, ContextPtr context_) co
StoragePtr DatabaseS3::getTable(const String & name, ContextPtr context_) const
{
// rethrow all exceptions from TableFunctionS3 to show correct error to user
/// Rethrow all exceptions from TableFunctionS3 to show correct error to user.
if (auto storage = getTableImpl(name, context_))
return storage;
throw Exception(ErrorCodes::UNKNOWN_TABLE, "Table {}.{} doesn't exist", backQuoteIfNeed(getDatabaseName()), backQuoteIfNeed(name));
throw Exception(ErrorCodes::UNKNOWN_TABLE, "Table {}.{} doesn't exist",
backQuoteIfNeed(getDatabaseName()), backQuoteIfNeed(name));
}
StoragePtr DatabaseS3::tryGetTable(const String & name, ContextPtr context_) const
@ -158,15 +155,14 @@ StoragePtr DatabaseS3::tryGetTable(const String & name, ContextPtr context_) con
}
catch (const Exception & e)
{
// Ignore exceptions thrown by TableFunctionS3, which indicate that there is no table
if (e.code() == ErrorCodes::BAD_ARGUMENTS)
return nullptr;
if (e.code() == ErrorCodes::S3_ERROR)
return nullptr;
if (e.code() == ErrorCodes::FILE_DOESNT_EXIST)
return nullptr;
if (e.code() == ErrorCodes::UNACCEPTABLE_URL)
/// Ignore exceptions thrown by TableFunctionS3, which indicate that there is no table.
if (e.code() == ErrorCodes::BAD_ARGUMENTS
|| e.code() == ErrorCodes::S3_ERROR
|| e.code() == ErrorCodes::FILE_DOESNT_EXIST
|| e.code() == ErrorCodes::UNACCEPTABLE_URL)
{
return nullptr;
}
throw;
}
catch (const Poco::URISyntaxException &)
@ -183,7 +179,7 @@ bool DatabaseS3::empty() const
ASTPtr DatabaseS3::getCreateDatabaseQuery() const
{
auto settings = getContext()->getSettingsRef();
const auto & settings = getContext()->getSettingsRef();
ParserCreateQuery parser;
std::string creation_args;
@ -236,8 +232,8 @@ DatabaseS3::Configuration DatabaseS3::parseArguments(ASTs engine_args, ContextPt
result.url_prefix = collection.getOrDefault<String>("url", "");
result.no_sign_request = collection.getOrDefault<bool>("no_sign_request", false);
auto key_id = collection.getOrDefault<std::string>("access_key_id", "");
auto secret_key = collection.getOrDefault<std::string>("secret_access_key", "");
auto key_id = collection.getOrDefault<String>("access_key_id", "");
auto secret_key = collection.getOrDefault<String>("secret_access_key", "");
if (!key_id.empty())
result.access_key_id = key_id;
@ -311,6 +307,6 @@ DatabaseTablesIteratorPtr DatabaseS3::getTablesIterator(ContextPtr, const Filter
return std::make_unique<DatabaseTablesSnapshotIterator>(Tables{}, getDatabaseName());
}
} // DB
}
#endif

View File

@ -16,9 +16,10 @@ namespace DB
class Context;
/**
* DatabaseS3 provides access to data stored in S3
* Uses TableFunctionS3 to implicitly load file when a user requests the table, and provides read-only access to the data in the file
* Tables are cached inside the database for quick access
* DatabaseS3 provides access to data stored in S3.
* Uses TableFunctionS3 to implicitly load file when a user requests the table,
* and provides read-only access to the data in the file.
* Tables are cached inside the database for quick access.
*/
class DatabaseS3 : public IDatabase, protected WithContext
{
@ -75,6 +76,6 @@ private:
Poco::Logger * log;
};
} // DB
}
#endif