From caed1898b09a48b14ccab4c73365b97e9e4b8c0e Mon Sep 17 00:00:00 2001 From: taiyang-li <654010905@qq.com> Date: Sun, 6 Feb 2022 11:22:05 +0800 Subject: [PATCH 1/2] add options for clickhouse-format --- programs/format/Format.cpp | 8 ++++++-- src/Core/Settings.cpp | 26 +++++++++++++++++++------- src/Core/Settings.h | 10 ++++++++++ 3 files changed, 35 insertions(+), 9 deletions(-) diff --git a/programs/format/Format.cpp b/programs/format/Format.cpp index 4b0e8ad1ca1..9c756a27915 100644 --- a/programs/format/Format.cpp +++ b/programs/format/Format.cpp @@ -57,8 +57,12 @@ int mainEntryClickHouseFormat(int argc, char ** argv) ("seed", po::value(), "seed (arbitrary string) that determines the result of obfuscation") ; + Settings cmd_settings; + cmd_settings.addFormatOptions(desc); + boost::program_options::variables_map options; boost::program_options::store(boost::program_options::parse_command_line(argc, argv, desc), options); + po::notify(options); if (options.count("help")) { @@ -149,7 +153,8 @@ int mainEntryClickHouseFormat(int argc, char ** argv) ParserQuery parser(end); do { - ASTPtr res = parseQueryAndMovePosition(parser, pos, end, "query", multiple, 0, DBMS_DEFAULT_MAX_PARSER_DEPTH); + ASTPtr res = parseQueryAndMovePosition( + parser, pos, end, "query", multiple, cmd_settings.max_query_size, cmd_settings.max_parser_depth); /// For insert query with data(INSERT INTO ... VALUES ...), will lead to format fail, /// should throw exception early and make exception message more readable. if (const auto * insert_query = res->as(); insert_query && insert_query->data) @@ -222,6 +227,5 @@ int mainEntryClickHouseFormat(int argc, char ** argv) std::cerr << getCurrentExceptionMessage(true) << '\n'; return getCurrentExceptionCode(); } - return 0; } diff --git a/src/Core/Settings.cpp b/src/Core/Settings.cpp index 8daf39d9928..772cadab3fc 100644 --- a/src/Core/Settings.cpp +++ b/src/Core/Settings.cpp @@ -85,16 +85,28 @@ void Settings::addProgramOptions(boost::program_options::options_description & o { for (const auto & field : all()) { - const std::string_view name = field.getName(); - auto on_program_option - = boost::function1([this, name](const std::string & value) { set(name, value); }); - options.add(boost::shared_ptr(new boost::program_options::option_description( - name.data(), - boost::program_options::value()->composing()->notifier(on_program_option), - field.getDescription()))); + addProgramOption(options, field); } } +void Settings::addFormatOptions(boost::program_options::options_description & options) +{ + for (const auto & field : all()) + { + const auto & name = field.getName(); + if (formatSettingNames.count(name)) + addProgramOption(options, field); + } +} + +void Settings::addProgramOption(boost::program_options::options_description & options, const SettingFieldRef & field) +{ + const std::string_view name = field.getName(); + auto on_program_option = boost::function1([this, name](const std::string & value) { set(name, value); }); + options.add(boost::shared_ptr(new boost::program_options::option_description( + name.data(), boost::program_options::value()->composing()->notifier(on_program_option), field.getDescription()))); +} + void Settings::checkNoSettingNamesAtTopLevel(const Poco::Util::AbstractConfiguration & config, const String & config_path) { if (config.getBool("skip_check_for_incorrect_settings", false)) diff --git a/src/Core/Settings.h b/src/Core/Settings.h index 48dd637a943..f46066a426f 100644 --- a/src/Core/Settings.h +++ b/src/Core/Settings.h @@ -712,11 +712,21 @@ struct Settings : public BaseSettings, public IHints<2, Settings /// (Don't forget to call notify() on the `variables_map` after parsing it!) void addProgramOptions(boost::program_options::options_description & options); + /// Adds program options for clickhouse-format to set the settings from a command line. + /// (Don't forget to call notify() on the `variables_map` after parsing it!) + void addFormatOptions(boost::program_options::options_description & options); + /// Check that there is no user-level settings at the top level in config. /// This is a common source of mistake (user don't know where to write user-level setting). static void checkNoSettingNamesAtTopLevel(const Poco::Util::AbstractConfiguration & config, const String & config_path); std::vector getAllRegisteredNames() const override; + +private: + void addProgramOption(boost::program_options::options_description & options, const SettingFieldRef & field); + + inline static const std::unordered_set formatSettingNames + = {"max_parser_depth", "max_query_size"}; }; /* From 26f2a0ef5147fbb5b54b63cc91f035ce359be61e Mon Sep 17 00:00:00 2001 From: taiyang-li <654010905@qq.com> Date: Mon, 7 Feb 2022 13:34:13 +0800 Subject: [PATCH 2/2] move clickhouse-format code from settings to format.cpp --- programs/format/Format.cpp | 6 +++++- src/Core/Settings.cpp | 10 ---------- src/Core/Settings.h | 8 -------- 3 files changed, 5 insertions(+), 19 deletions(-) diff --git a/programs/format/Format.cpp b/programs/format/Format.cpp index 9c756a27915..835afcdb2ed 100644 --- a/programs/format/Format.cpp +++ b/programs/format/Format.cpp @@ -58,7 +58,11 @@ int mainEntryClickHouseFormat(int argc, char ** argv) ; Settings cmd_settings; - cmd_settings.addFormatOptions(desc); + for (const auto & field : cmd_settings.all()) + { + if (field.getName() == "max_parser_depth" || field.getName() == "max_query_size") + cmd_settings.addProgramOption(desc, field); + } boost::program_options::variables_map options; boost::program_options::store(boost::program_options::parse_command_line(argc, argv, desc), options); diff --git a/src/Core/Settings.cpp b/src/Core/Settings.cpp index 772cadab3fc..87d7eee0daa 100644 --- a/src/Core/Settings.cpp +++ b/src/Core/Settings.cpp @@ -89,16 +89,6 @@ void Settings::addProgramOptions(boost::program_options::options_description & o } } -void Settings::addFormatOptions(boost::program_options::options_description & options) -{ - for (const auto & field : all()) - { - const auto & name = field.getName(); - if (formatSettingNames.count(name)) - addProgramOption(options, field); - } -} - void Settings::addProgramOption(boost::program_options::options_description & options, const SettingFieldRef & field) { const std::string_view name = field.getName(); diff --git a/src/Core/Settings.h b/src/Core/Settings.h index f46066a426f..430c7a194eb 100644 --- a/src/Core/Settings.h +++ b/src/Core/Settings.h @@ -712,21 +712,13 @@ struct Settings : public BaseSettings, public IHints<2, Settings /// (Don't forget to call notify() on the `variables_map` after parsing it!) void addProgramOptions(boost::program_options::options_description & options); - /// Adds program options for clickhouse-format to set the settings from a command line. - /// (Don't forget to call notify() on the `variables_map` after parsing it!) - void addFormatOptions(boost::program_options::options_description & options); - /// Check that there is no user-level settings at the top level in config. /// This is a common source of mistake (user don't know where to write user-level setting). static void checkNoSettingNamesAtTopLevel(const Poco::Util::AbstractConfiguration & config, const String & config_path); std::vector getAllRegisteredNames() const override; -private: void addProgramOption(boost::program_options::options_description & options, const SettingFieldRef & field); - - inline static const std::unordered_set formatSettingNames - = {"max_parser_depth", "max_query_size"}; }; /*