From 9bbeb6359a24d1bc037838d96f215c8edf97b49e Mon Sep 17 00:00:00 2001 From: Alexander Tokmakov Date: Wed, 30 Oct 2019 17:17:55 +0300 Subject: [PATCH] refactor StorageFile construction --- dbms/src/Storages/StorageFile.cpp | 157 +++++++++--------- dbms/src/Storages/StorageFile.h | 35 ++-- dbms/src/TableFunctions/TableFunctionFile.cpp | 12 +- 3 files changed, 97 insertions(+), 107 deletions(-) diff --git a/dbms/src/Storages/StorageFile.cpp b/dbms/src/Storages/StorageFile.cpp index c27fb3b9b9a..63742535fab 100644 --- a/dbms/src/Storages/StorageFile.cpp +++ b/dbms/src/Storages/StorageFile.cpp @@ -26,7 +26,6 @@ #include #include -#include #include namespace fs = std::filesystem; @@ -118,63 +117,52 @@ static void checkCreationIsAllowed(Context & context_global, const std::string & } } -StorageFile::StorageFile( - const std::string & table_path_, - int table_fd_, - const std::string & relative_table_dir_path, - const std::string & database_name_, - const std::string & table_name_, - const std::string & format_name_, - const ColumnsDescription & columns_, - const ConstraintsDescription & constraints_, - Context & context_) - : - table_name(table_name_), database_name(database_name_), format_name(format_name_), context_global(context_), table_fd(table_fd_) +StorageFile::StorageFile(int table_fd_, CommonArguments args) + : StorageFile(args) { - setColumns(columns_); - setConstraints(constraints_); + if (context_global.getApplicationType() == Context::ApplicationType::SERVER) + throw Exception("Using file descriptor as source of storage isn't allowed for server daemons", ErrorCodes::DATABASE_ACCESS_DENIED); - if (table_fd < 0) /// Will use file - { - String table_dir_path = context_global.getPath() + relative_table_dir_path + "/"; - use_table_fd = false; + is_db_table = false; + use_table_fd = true; + table_fd = table_fd_; - if (!table_path_.empty()) /// Is user's file - { - Poco::Path poco_path = Poco::Path(table_path_); - if (poco_path.isRelative()) - poco_path = Poco::Path(table_dir_path, poco_path); - - const std::string path = poco_path.absolute().toString(); - paths = listFilesWithRegexpMatching("/", path); - for (const auto & cur_path : paths) - checkCreationIsAllowed(context_global, table_dir_path, cur_path); - is_db_table = false; - } - else /// Is DB's file - { - if (relative_table_dir_path.empty()) - throw Exception("Storage " + getName() + " requires data path", ErrorCodes::INCORRECT_FILE_NAME); - - paths = {getTablePath(table_dir_path, format_name)}; - is_db_table = true; - Poco::File(Poco::Path(paths.back()).parent()).createDirectories(); - } - } - else /// Will use FD - { - if (context_global.getApplicationType() == Context::ApplicationType::SERVER) - throw Exception("Using file descriptor as source of storage isn't allowed for server daemons", ErrorCodes::DATABASE_ACCESS_DENIED); - - is_db_table = false; - use_table_fd = true; - - /// Save initial offset, it will be used for repeating SELECTs - /// If FD isn't seekable (lseek returns -1), then the second and subsequent SELECTs will fail. - table_fd_init_offset = lseek(table_fd, 0, SEEK_CUR); - } + /// Save initial offset, it will be used for repeating SELECTs + /// If FD isn't seekable (lseek returns -1), then the second and subsequent SELECTs will fail. + table_fd_init_offset = lseek(table_fd, 0, SEEK_CUR); } +StorageFile::StorageFile(const std::string & table_path_, const std::string & user_files_absolute_path, CommonArguments args) + : StorageFile(args) +{ + is_db_table = false; + Poco::Path poco_path = Poco::Path(table_path_); + if (poco_path.isRelative()) + poco_path = Poco::Path(user_files_absolute_path, poco_path); + + const std::string path = poco_path.absolute().toString(); + paths = listFilesWithRegexpMatching("/", path); + for (const auto & cur_path : paths) + checkCreationIsAllowed(context_global, user_files_absolute_path, cur_path); +} + +StorageFile::StorageFile(const std::string & relative_table_dir_path, CommonArguments args) + : StorageFile(args) +{ + if (relative_table_dir_path.empty()) + throw Exception("Storage " + getName() + " requires data path", ErrorCodes::INCORRECT_FILE_NAME); + + String table_dir_path = context_global.getPath() + relative_table_dir_path + "/"; + Poco::File(table_dir_path).createDirectories(); + paths = {getTablePath(table_dir_path, format_name)}; +} + +StorageFile::StorageFile(CommonArguments args) + : table_name(args.table_name), database_name(args.database_name), format_name(args.format_name), context_global(args.context) +{ + setColumns(args.columns); + setConstraints(args.constraints); +} class StorageFileBlockInputStream : public IBlockInputStream { @@ -369,41 +357,44 @@ void registerStorageFile(StorageFactory & factory) engine_args[0] = evaluateConstantExpressionOrIdentifierAsLiteral(engine_args[0], args.local_context); String format_name = engine_args[0]->as().value.safeGet(); + StorageFile::CommonArguments common_args{args.database_name, args.table_name, format_name, + args.columns, args.constraints,args.context}; + + if (engine_args.size() == 1) /// Table in database + return StorageFile::create(args.relative_data_path, common_args); + + /// Will use FD if engine_args[1] is int literal or identifier with std* name int source_fd = -1; String source_path; - if (engine_args.size() >= 2) + if (auto opt_name = tryGetIdentifierName(engine_args[1])) { - /// Will use FD if engine_args[1] is int literal or identifier with std* name - - if (auto opt_name = tryGetIdentifierName(engine_args[1])) - { - if (*opt_name == "stdin") - source_fd = STDIN_FILENO; - else if (*opt_name == "stdout") - source_fd = STDOUT_FILENO; - else if (*opt_name == "stderr") - source_fd = STDERR_FILENO; - else - throw Exception("Unknown identifier '" + *opt_name + "' in second arg of File storage constructor", - ErrorCodes::UNKNOWN_IDENTIFIER); - } - else if (const auto * literal = engine_args[1]->as()) - { - auto type = literal->value.getType(); - if (type == Field::Types::Int64) - source_fd = static_cast(literal->value.get()); - else if (type == Field::Types::UInt64) - source_fd = static_cast(literal->value.get()); - else if (type == Field::Types::String) - source_path = literal->value.get(); - } + if (*opt_name == "stdin") + source_fd = STDIN_FILENO; + else if (*opt_name == "stdout") + source_fd = STDOUT_FILENO; + else if (*opt_name == "stderr") + source_fd = STDERR_FILENO; + else + throw Exception("Unknown identifier '" + *opt_name + "' in second arg of File storage constructor", + ErrorCodes::UNKNOWN_IDENTIFIER); + } + else if (const auto * literal = engine_args[1]->as()) + { + auto type = literal->value.getType(); + if (type == Field::Types::Int64) + source_fd = static_cast(literal->value.get()); + else if (type == Field::Types::UInt64) + source_fd = static_cast(literal->value.get()); + else if (type == Field::Types::String) + source_path = literal->value.get(); + else + throw Exception("Second argument must be path or file descriptor", ErrorCodes::BAD_ARGUMENTS); } - return StorageFile::create( - source_path, source_fd, - args.relative_data_path, - args.database_name, args.table_name, format_name, args.columns, args.constraints, - args.context); + if (0 <= source_fd) /// File descriptor + return StorageFile::create(source_fd, common_args); + else /// User's file + return StorageFile::create(source_path, args.context.getUserFilesPath(), common_args); }); } diff --git a/dbms/src/Storages/StorageFile.h b/dbms/src/Storages/StorageFile.h index 019fb96c75b..e5fd8a40094 100644 --- a/dbms/src/Storages/StorageFile.h +++ b/dbms/src/Storages/StorageFile.h @@ -42,27 +42,32 @@ public: Strings getDataPaths() const override; + struct CommonArguments + { + const std::string & database_name; + const std::string & table_name; + const std::string & format_name; + const ColumnsDescription & columns; + const ConstraintsDescription & constraints; + Context & context; + }; + protected: friend class StorageFileBlockInputStream; friend class StorageFileBlockOutputStream; - /** there are three options (ordered by priority): - - use specified file descriptor if (fd >= 0) - - use specified table_path if it isn't empty - - create own table inside data/db/table/ - */ - StorageFile( - const std::string & table_path_, - int table_fd_, - const std::string & relative_table_dir_path, - const std::string & database_name_, - const std::string & table_name_, - const std::string & format_name_, - const ColumnsDescription & columns_, - const ConstraintsDescription & constraints_, - Context & context_); + /// From file descriptor + StorageFile(int table_fd_, CommonArguments args); + + /// From user's file + StorageFile(const std::string & table_path_, const std::string & user_files_absolute_path, CommonArguments args); + + /// From table in database + StorageFile(const std::string & relative_table_dir_path, CommonArguments args); private: + explicit StorageFile(CommonArguments args); + std::string table_name; std::string database_name; std::string format_name; diff --git a/dbms/src/TableFunctions/TableFunctionFile.cpp b/dbms/src/TableFunctions/TableFunctionFile.cpp index 7cf2c500f1e..eaa2d8f27cb 100644 --- a/dbms/src/TableFunctions/TableFunctionFile.cpp +++ b/dbms/src/TableFunctions/TableFunctionFile.cpp @@ -8,15 +8,9 @@ namespace DB StoragePtr TableFunctionFile::getStorage( const String & source, const String & format, const ColumnsDescription & columns, Context & global_context, const std::string & table_name) const { - return StorageFile::create(source, - -1, - global_context.getUserFilesPath(), - getDatabaseName(), - table_name, - format, - columns, - ConstraintsDescription{}, - global_context); + StorageFile::CommonArguments args{getDatabaseName(), table_name, format, columns,ConstraintsDescription{},global_context}; + + return StorageFile::create(source, global_context.getUserFilesPath(), args); } void registerTableFunctionFile(TableFunctionFactory & factory)