From 901294d35206d52c29f876dda7658a4d8252d191 Mon Sep 17 00:00:00 2001 From: Maksim Kita Date: Wed, 25 Oct 2023 21:41:26 +0300 Subject: [PATCH] Fixed code review issues --- src/Common/escapeString.cpp | 16 ++++++++++++++++ src/Common/escapeString.h | 10 ++++++++++ src/Interpreters/InterpreterShowColumnsQuery.cpp | 9 +++------ src/Interpreters/InterpreterShowIndexesQuery.cpp | 12 +++--------- src/Interpreters/InterpreterShowSettingQuery.cpp | 7 ++----- src/Parsers/ParserShowSettingQuery.cpp | 3 --- src/Parsers/ParserShowSettingQuery.h | 6 +++--- .../02905_show_setting_query.reference | 1 + .../0_stateless/02905_show_setting_query.sql | 5 +++++ 9 files changed, 43 insertions(+), 26 deletions(-) create mode 100644 src/Common/escapeString.cpp create mode 100644 src/Common/escapeString.h diff --git a/src/Common/escapeString.cpp b/src/Common/escapeString.cpp new file mode 100644 index 00000000000..621726d38ac --- /dev/null +++ b/src/Common/escapeString.cpp @@ -0,0 +1,16 @@ +#include + +#include +#include + +namespace DB +{ + +String escapeString(std::string_view value) +{ + WriteBufferFromOwnString buf; + writeEscapedString(value, buf); + return buf.str(); +} + +} diff --git a/src/Common/escapeString.h b/src/Common/escapeString.h new file mode 100644 index 00000000000..0018296889c --- /dev/null +++ b/src/Common/escapeString.h @@ -0,0 +1,10 @@ +#pragma once + +#include + +namespace DB +{ + +String escapeString(std::string_view value); + +} diff --git a/src/Interpreters/InterpreterShowColumnsQuery.cpp b/src/Interpreters/InterpreterShowColumnsQuery.cpp index d14a36ef7e1..c8fb64e37f2 100644 --- a/src/Interpreters/InterpreterShowColumnsQuery.cpp +++ b/src/Interpreters/InterpreterShowColumnsQuery.cpp @@ -1,6 +1,7 @@ #include #include +#include #include #include #include @@ -31,12 +32,8 @@ String InterpreterShowColumnsQuery::getRewrittenQuery() WriteBufferFromOwnString buf_database; String resolved_database = getContext()->resolveDatabase(query.database); - writeEscapedString(resolved_database, buf_database); - String database = buf_database.str(); - - WriteBufferFromOwnString buf_table; - writeEscapedString(query.table, buf_table); - String table = buf_table.str(); + String database = escapeString(resolved_database); + String table = escapeString(query.table); String rewritten_query; if (use_mysql_types) diff --git a/src/Interpreters/InterpreterShowIndexesQuery.cpp b/src/Interpreters/InterpreterShowIndexesQuery.cpp index 9b36f1496e7..63cda814683 100644 --- a/src/Interpreters/InterpreterShowIndexesQuery.cpp +++ b/src/Interpreters/InterpreterShowIndexesQuery.cpp @@ -1,6 +1,7 @@ #include #include +#include #include #include #include @@ -23,16 +24,9 @@ InterpreterShowIndexesQuery::InterpreterShowIndexesQuery(const ASTPtr & query_pt String InterpreterShowIndexesQuery::getRewrittenQuery() { const auto & query = query_ptr->as(); - - WriteBufferFromOwnString buf_table; - writeEscapedString(query.table, buf_table); - String table = buf_table.str(); - - WriteBufferFromOwnString buf_database; + String table = escapeString(query.table); String resolved_database = getContext()->resolveDatabase(query.database); - writeEscapedString(resolved_database, buf_database); - String database = buf_database.str(); - + String database = escapeString(resolved_database); String where_expression = query.where_expression ? fmt::format("WHERE ({})", query.where_expression) : ""; String rewritten_query = fmt::format(R"( diff --git a/src/Interpreters/InterpreterShowSettingQuery.cpp b/src/Interpreters/InterpreterShowSettingQuery.cpp index aa3556a8837..7567e77d28f 100644 --- a/src/Interpreters/InterpreterShowSettingQuery.cpp +++ b/src/Interpreters/InterpreterShowSettingQuery.cpp @@ -1,10 +1,7 @@ #include -#include -#include -#include +#include #include -#include #include #include @@ -23,7 +20,7 @@ InterpreterShowSettingQuery::InterpreterShowSettingQuery(const ASTPtr & query_pt String InterpreterShowSettingQuery::getRewrittenQuery() { const auto & query = query_ptr->as(); - return fmt::format(R"(SELECT value FROM system.settings WHERE name = '{0}')", query.getSettingName()); + return fmt::format(R"(SELECT value FROM system.settings WHERE name = '{0}')", escapeString(query.getSettingName())); } diff --git a/src/Parsers/ParserShowSettingQuery.cpp b/src/Parsers/ParserShowSettingQuery.cpp index 92bed51b688..2586cbdfb43 100644 --- a/src/Parsers/ParserShowSettingQuery.cpp +++ b/src/Parsers/ParserShowSettingQuery.cpp @@ -1,13 +1,10 @@ #include #include -#include #include #include #include -#include -#include namespace DB { diff --git a/src/Parsers/ParserShowSettingQuery.h b/src/Parsers/ParserShowSettingQuery.h index 56c835eb349..ef166133d09 100644 --- a/src/Parsers/ParserShowSettingQuery.h +++ b/src/Parsers/ParserShowSettingQuery.h @@ -5,9 +5,9 @@ namespace DB { -/** Parses queries of the form - * SHOW SETTING setting_name - */ +/** Parses queries of the form: + * SHOW SETTING [setting_name] + */ class ParserShowSettingQuery : public IParserBase { protected: diff --git a/tests/queries/0_stateless/02905_show_setting_query.reference b/tests/queries/0_stateless/02905_show_setting_query.reference index d00491fd7e5..1191247b6d9 100644 --- a/tests/queries/0_stateless/02905_show_setting_query.reference +++ b/tests/queries/0_stateless/02905_show_setting_query.reference @@ -1 +1,2 @@ 1 +2 diff --git a/tests/queries/0_stateless/02905_show_setting_query.sql b/tests/queries/0_stateless/02905_show_setting_query.sql index ee7c9379146..bbbb1a7e237 100644 --- a/tests/queries/0_stateless/02905_show_setting_query.sql +++ b/tests/queries/0_stateless/02905_show_setting_query.sql @@ -1,2 +1,7 @@ SET max_threads = 1; SHOW SETTING max_threads; + +SET max_threads = 2; +SHOW SETTING max_threads; + +SHOW SETTING `max_threads' OR name = 'max_memory_usage`;