From d67e06191584b312cc281138fa0c86f9dd3a2785 Mon Sep 17 00:00:00 2001 From: HeenaBansal2009 Date: Tue, 19 Jul 2022 21:38:36 -0700 Subject: [PATCH 1/8] Clickhouse-local fixes --- programs/local/LocalServer.cpp | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/programs/local/LocalServer.cpp b/programs/local/LocalServer.cpp index 4f3b92bbcf0..376fa1c2bc5 100644 --- a/programs/local/LocalServer.cpp +++ b/programs/local/LocalServer.cpp @@ -325,7 +325,20 @@ void LocalServer::setupUsers() access_control.setPlaintextPasswordAllowed(config().getBool("allow_plaintext_password", true)); if (config().has("users_config") || config().has("config-file") || fs::exists("config.xml")) { - const auto users_config_path = config().getString("users_config", config().getString("config-file", "config.xml")); + String config_path = config().getString("config-file",""); + bool has_user_directories = config().has("user_directories"); + const auto config_dir = std::filesystem::path{config_path}.remove_filename().string(); + String users_config_path = config().getString("users_config",""); + if (users_config_path.empty()) + { + if (!has_user_directories) + users_config_path = config_path; + } + if (has_user_directories) + users_config_path = config().getString("user_directories.users_xml.path",""); + + if (std::filesystem::path{users_config_path}.is_relative() && std::filesystem::exists(config_dir + users_config_path)) + users_config_path = config_dir + users_config_path; ConfigProcessor config_processor(users_config_path); const auto loaded_config = config_processor.loadConfig(); users_config = loaded_config.configuration; @@ -575,7 +588,8 @@ void LocalServer::processConfig() * Otherwise, metadata of temporary File(format, EXPLICIT_PATH) tables will pollute metadata/ directory; * if such tables will not be dropped, clickhouse-server will not be able to load them due to security reasons. */ - std::string default_database = config().getString("default_database", "_local"); + std::string default_database = config().getString("default_database", ""); + default_database = default_database + "_local"; DatabaseCatalog::instance().attachDatabase(default_database, std::make_shared(default_database, global_context)); global_context->setCurrentDatabase(default_database); applyCmdOptions(global_context); From 06e8b78efa20f4b6998d475b1582e41150033455 Mon Sep 17 00:00:00 2001 From: HeenaBansal2009 Date: Thu, 21 Jul 2022 19:46:36 -0700 Subject: [PATCH 2/8] Added FT testcase --- programs/local/LocalServer.cpp | 29 +++++----- ...0_clickhouse_local_config-option.reference | 1 + .../02360_clickhouse_local_config-option.sh | 54 +++++++++++++++++++ 3 files changed, 72 insertions(+), 12 deletions(-) create mode 100644 tests/queries/0_stateless/02360_clickhouse_local_config-option.reference create mode 100755 tests/queries/0_stateless/02360_clickhouse_local_config-option.sh diff --git a/programs/local/LocalServer.cpp b/programs/local/LocalServer.cpp index 376fa1c2bc5..de1f08dd5f5 100644 --- a/programs/local/LocalServer.cpp +++ b/programs/local/LocalServer.cpp @@ -323,25 +323,30 @@ void LocalServer::setupUsers() auto & access_control = global_context->getAccessControl(); access_control.setNoPasswordAllowed(config().getBool("allow_no_password", true)); access_control.setPlaintextPasswordAllowed(config().getBool("allow_plaintext_password", true)); - if (config().has("users_config") || config().has("config-file") || fs::exists("config.xml")) + if (config().has("config-file") || fs::exists("config.xml")) { String config_path = config().getString("config-file",""); bool has_user_directories = config().has("user_directories"); const auto config_dir = std::filesystem::path{config_path}.remove_filename().string(); String users_config_path = config().getString("users_config",""); - if (users_config_path.empty()) + if (users_config_path.empty() && !has_user_directories) + users_config = getConfigurationFromXMLString(minimal_default_user_xml); + else { - if (!has_user_directories) - users_config_path = config_path; - } - if (has_user_directories) - users_config_path = config().getString("user_directories.users_xml.path",""); + if (users_config_path.empty()) + { + if (!has_user_directories) + users_config_path = config_path; + } + if (has_user_directories) + users_config_path = config().getString("user_directories.users_xml.path",""); - if (std::filesystem::path{users_config_path}.is_relative() && std::filesystem::exists(config_dir + users_config_path)) - users_config_path = config_dir + users_config_path; - ConfigProcessor config_processor(users_config_path); - const auto loaded_config = config_processor.loadConfig(); - users_config = loaded_config.configuration; + if (std::filesystem::path{users_config_path}.is_relative() && std::filesystem::exists(config_dir + users_config_path)) + users_config_path = config_dir + users_config_path; + ConfigProcessor config_processor(users_config_path); + const auto loaded_config = config_processor.loadConfig(); + users_config = loaded_config.configuration; + } } else users_config = getConfigurationFromXMLString(minimal_default_user_xml); diff --git a/tests/queries/0_stateless/02360_clickhouse_local_config-option.reference b/tests/queries/0_stateless/02360_clickhouse_local_config-option.reference new file mode 100644 index 00000000000..d00491fd7e5 --- /dev/null +++ b/tests/queries/0_stateless/02360_clickhouse_local_config-option.reference @@ -0,0 +1 @@ +1 diff --git a/tests/queries/0_stateless/02360_clickhouse_local_config-option.sh b/tests/queries/0_stateless/02360_clickhouse_local_config-option.sh new file mode 100755 index 00000000000..8eaa87689c1 --- /dev/null +++ b/tests/queries/0_stateless/02360_clickhouse_local_config-option.sh @@ -0,0 +1,54 @@ +#!/usr/bin/env bash +set -e + +CUR_DIR=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd) +# shellcheck source=../shell_config.sh +. "$CUR_DIR"/../shell_config.sh + +echo " + + + trace + true + + + 9000 + + ./ + + 0 + + + + users.xml + + +" > $CUR_DIR/config.xml + +echo " + + + + + + + + ::/0 + + default + default + + + + + + " > $CUR_DIR/users.xml + +local_opts=( + "--config-file=$CUR_DIR/config.xml" + "--send_logs_level=none") + +${CLICKHOUSE_LOCAL} "${local_opts[@]}" --query 'Select 1' | grep -v -e 'Processing configuration file' + +rm -rf $CUR_DIR/users.xml +rm -rf $CUR_DIR/config.xml From c82059cb9b0071d507f16ead96596a0efb21c542 Mon Sep 17 00:00:00 2001 From: HeenaBansal2009 Date: Thu, 21 Jul 2022 20:05:35 -0700 Subject: [PATCH 3/8] Fixed FT testcase --- .../0_stateless/02360_clickhouse_local_config-option.sh | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/queries/0_stateless/02360_clickhouse_local_config-option.sh b/tests/queries/0_stateless/02360_clickhouse_local_config-option.sh index 8eaa87689c1..cbce333a493 100755 --- a/tests/queries/0_stateless/02360_clickhouse_local_config-option.sh +++ b/tests/queries/0_stateless/02360_clickhouse_local_config-option.sh @@ -5,8 +5,7 @@ CUR_DIR=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd) # shellcheck source=../shell_config.sh . "$CUR_DIR"/../shell_config.sh -echo " - +echo " trace true From 4e0b840cdc322828e1495a58ff47c03c9e4bec74 Mon Sep 17 00:00:00 2001 From: HeenaBansal2009 Date: Thu, 21 Jul 2022 20:27:35 -0700 Subject: [PATCH 4/8] Fixed FT testcase --- .../queries/0_stateless/02360_clickhouse_local_config-option.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/queries/0_stateless/02360_clickhouse_local_config-option.sh b/tests/queries/0_stateless/02360_clickhouse_local_config-option.sh index cbce333a493..ed92b66ec87 100755 --- a/tests/queries/0_stateless/02360_clickhouse_local_config-option.sh +++ b/tests/queries/0_stateless/02360_clickhouse_local_config-option.sh @@ -47,7 +47,7 @@ local_opts=( "--config-file=$CUR_DIR/config.xml" "--send_logs_level=none") -${CLICKHOUSE_LOCAL} "${local_opts[@]}" --query 'Select 1' | grep -v -e 'Processing configuration file' +${CLICKHOUSE_LOCAL} "${local_opts[@]}" --query 'Select 1' |& grep -v -e 'Processing configuration file' rm -rf $CUR_DIR/users.xml rm -rf $CUR_DIR/config.xml From f4f05d33b1a38bf75aab0b54cca3198e23aadcd7 Mon Sep 17 00:00:00 2001 From: HeenaBansal2009 Date: Thu, 21 Jul 2022 22:23:54 -0700 Subject: [PATCH 5/8] Fixed FT testcase --- .../queries/0_stateless/02360_clickhouse_local_config-option.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/queries/0_stateless/02360_clickhouse_local_config-option.sh b/tests/queries/0_stateless/02360_clickhouse_local_config-option.sh index ed92b66ec87..df0bdf38b4d 100755 --- a/tests/queries/0_stateless/02360_clickhouse_local_config-option.sh +++ b/tests/queries/0_stateless/02360_clickhouse_local_config-option.sh @@ -1,4 +1,5 @@ #!/usr/bin/env bash +# Tags: no-parallel set -e CUR_DIR=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd) From a30dbed6b8dba4c08f9a27b786d2cad59f66becf Mon Sep 17 00:00:00 2001 From: Alexey Milovidov Date: Sun, 31 Jul 2022 03:02:20 +0300 Subject: [PATCH 6/8] Update programs/local/LocalServer.cpp Co-authored-by: Kseniia Sumarokova <54203879+kssenii@users.noreply.github.com> --- programs/local/LocalServer.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/programs/local/LocalServer.cpp b/programs/local/LocalServer.cpp index de1f08dd5f5..75cc9d0ceb4 100644 --- a/programs/local/LocalServer.cpp +++ b/programs/local/LocalServer.cpp @@ -325,7 +325,7 @@ void LocalServer::setupUsers() access_control.setPlaintextPasswordAllowed(config().getBool("allow_plaintext_password", true)); if (config().has("config-file") || fs::exists("config.xml")) { - String config_path = config().getString("config-file",""); + String config_path = config().getString("config-file", ""); bool has_user_directories = config().has("user_directories"); const auto config_dir = std::filesystem::path{config_path}.remove_filename().string(); String users_config_path = config().getString("users_config",""); From fa9c3dcc4899227156d877cf9b676629103a537f Mon Sep 17 00:00:00 2001 From: Alexey Milovidov Date: Sun, 31 Jul 2022 03:02:27 +0300 Subject: [PATCH 7/8] Update programs/local/LocalServer.cpp Co-authored-by: Kseniia Sumarokova <54203879+kssenii@users.noreply.github.com> --- programs/local/LocalServer.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/programs/local/LocalServer.cpp b/programs/local/LocalServer.cpp index 75cc9d0ceb4..c0c8aa64b20 100644 --- a/programs/local/LocalServer.cpp +++ b/programs/local/LocalServer.cpp @@ -328,7 +328,7 @@ void LocalServer::setupUsers() String config_path = config().getString("config-file", ""); bool has_user_directories = config().has("user_directories"); const auto config_dir = std::filesystem::path{config_path}.remove_filename().string(); - String users_config_path = config().getString("users_config",""); + String users_config_path = config().getString("users_config", ""); if (users_config_path.empty() && !has_user_directories) users_config = getConfigurationFromXMLString(minimal_default_user_xml); else From 800ed546bef57d0ae6d7c7c2809c645aec32f487 Mon Sep 17 00:00:00 2001 From: HeenaBansal2009 Date: Mon, 1 Aug 2022 07:03:36 -0700 Subject: [PATCH 8/8] Updated as per comments --- programs/local/LocalServer.cpp | 31 ++++++++++++++----------------- 1 file changed, 14 insertions(+), 17 deletions(-) diff --git a/programs/local/LocalServer.cpp b/programs/local/LocalServer.cpp index de1f08dd5f5..ed5a5c15cfb 100644 --- a/programs/local/LocalServer.cpp +++ b/programs/local/LocalServer.cpp @@ -325,24 +325,23 @@ void LocalServer::setupUsers() access_control.setPlaintextPasswordAllowed(config().getBool("allow_plaintext_password", true)); if (config().has("config-file") || fs::exists("config.xml")) { - String config_path = config().getString("config-file",""); + String config_path = config().getString("config-file", ""); bool has_user_directories = config().has("user_directories"); - const auto config_dir = std::filesystem::path{config_path}.remove_filename().string(); - String users_config_path = config().getString("users_config",""); - if (users_config_path.empty() && !has_user_directories) + const auto config_dir = fs::path{config_path}.remove_filename().string(); + String users_config_path = config().getString("users_config", ""); + + if (users_config_path.empty() && has_user_directories) + { + users_config_path = config().getString("user_directories.users_xml.path"); + if (fs::path(users_config_path).is_relative() + && fs::exists(fs::path(config_dir) / users_config_path)) + users_config_path = fs::path(config_dir) / users_config_path; + } + + if (users_config_path.empty()) users_config = getConfigurationFromXMLString(minimal_default_user_xml); else { - if (users_config_path.empty()) - { - if (!has_user_directories) - users_config_path = config_path; - } - if (has_user_directories) - users_config_path = config().getString("user_directories.users_xml.path",""); - - if (std::filesystem::path{users_config_path}.is_relative() && std::filesystem::exists(config_dir + users_config_path)) - users_config_path = config_dir + users_config_path; ConfigProcessor config_processor(users_config_path); const auto loaded_config = config_processor.loadConfig(); users_config = loaded_config.configuration; @@ -356,7 +355,6 @@ void LocalServer::setupUsers() throw Exception("Can't load config for users", ErrorCodes::CANNOT_LOAD_CONFIG); } - void LocalServer::connect() { connection_parameters = ConnectionParameters(config()); @@ -593,8 +591,7 @@ void LocalServer::processConfig() * Otherwise, metadata of temporary File(format, EXPLICIT_PATH) tables will pollute metadata/ directory; * if such tables will not be dropped, clickhouse-server will not be able to load them due to security reasons. */ - std::string default_database = config().getString("default_database", ""); - default_database = default_database + "_local"; + std::string default_database = config().getString("default_database", "_local"); DatabaseCatalog::instance().attachDatabase(default_database, std::make_shared(default_database, global_context)); global_context->setCurrentDatabase(default_database); applyCmdOptions(global_context);