From 4547ed370a4bbe20260ccdd6cd020b4c5d8ba55a Mon Sep 17 00:00:00 2001 From: taiyang-li <654010905@qq.com> Date: Wed, 30 Mar 2022 20:54:33 +0800 Subject: [PATCH 01/10] add hints for column description --- src/Common/NamePrompter.h | 7 +++++++ src/Storages/ColumnsDescription.cpp | 21 ++++++++++++++++++--- src/Storages/ColumnsDescription.h | 7 +++++-- src/Storages/IndicesDescription.h | 1 - 4 files changed, 30 insertions(+), 6 deletions(-) diff --git a/src/Common/NamePrompter.h b/src/Common/NamePrompter.h index a88d4bdea8e..8e301dec8b7 100644 --- a/src/Common/NamePrompter.h +++ b/src/Common/NamePrompter.h @@ -2,6 +2,7 @@ #include #include +#include #include #include @@ -102,6 +103,12 @@ public: return prompter.getHints(name, getAllRegisteredNames()); } + String getHintsString(const String & name) const + { + const auto hints = getHints(name); + return !hints.empty() ? ", may be you meant: " + toString(hints) : ""; + } + IHints() = default; IHints(const IHints &) = default; diff --git a/src/Storages/ColumnsDescription.cpp b/src/Storages/ColumnsDescription.cpp index 69ca6002c22..a694405665b 100644 --- a/src/Storages/ColumnsDescription.cpp +++ b/src/Storages/ColumnsDescription.cpp @@ -230,8 +230,8 @@ void ColumnsDescription::remove(const String & column_name) { auto range = getNameRange(columns, column_name); if (range.first == range.second) - throw Exception("There is no column " + column_name + " in table.", - ErrorCodes::NO_SUCH_COLUMN_IN_TABLE); + throw Exception( + "There is no column " + column_name + " in table" + getHintsString(column_name), ErrorCodes::NO_SUCH_COLUMN_IN_TABLE); for (auto list_it = range.first; list_it != range.second;) { @@ -244,7 +244,10 @@ void ColumnsDescription::rename(const String & column_from, const String & colum { auto it = columns.get<1>().find(column_from); if (it == columns.get<1>().end()) - throw Exception("Cannot find column " + column_from + " in ColumnsDescription", ErrorCodes::LOGICAL_ERROR); + { + throw Exception( + "Cannot find column " + column_from + " in ColumnsDescription" + getHintsString(column_from), ErrorCodes::LOGICAL_ERROR); + } columns.get<1>().modify_key(it, [&column_to] (String & old_name) { @@ -745,6 +748,18 @@ void ColumnsDescription::removeSubcolumns(const String & name_in_storage) subcolumns.get<1>().erase(range.first, range.second); } +std::vector ColumnsDescription::getAllRegisteredNames() const +{ + std::vector names; + names.reserve(columns.size()); + for (const auto & column : columns) + { + if (column.name.find('.') == std::string::npos) + names.push_back(column.name); + } + return names; +} + Block validateColumnsDefaultsAndGetSampleBlock(ASTPtr default_expr_list, const NamesAndTypesList & all_columns, ContextPtr context) { for (const auto & child : default_expr_list->children) diff --git a/src/Storages/ColumnsDescription.h b/src/Storages/ColumnsDescription.h index 4ae1dcfc2cd..affe2ef5a56 100644 --- a/src/Storages/ColumnsDescription.h +++ b/src/Storages/ColumnsDescription.h @@ -91,7 +91,7 @@ struct ColumnDescription /// Description of multiple table columns (in CREATE TABLE for example). -class ColumnsDescription +class ColumnsDescription : public IHints<2, ColumnsDescription> { public: ColumnsDescription() = default; @@ -149,7 +149,8 @@ public: { auto it = columns.get<1>().find(column_name); if (it == columns.get<1>().end()) - throw Exception("Cannot find column " + column_name + " in ColumnsDescription", ErrorCodes::LOGICAL_ERROR); + throw Exception( + "Cannot find column " + column_name + " in ColumnsDescription" + getHintsString(column_name), ErrorCodes::LOGICAL_ERROR); removeSubcolumns(it->name); if (!columns.get<1>().modify(it, std::forward(f))) @@ -196,6 +197,8 @@ public: return columns.empty(); } + std::vector getAllRegisteredNames() const override; + /// Keep the sequence of columns and allow to lookup by name. using ColumnsContainer = boost::multi_index_container< ColumnDescription, diff --git a/src/Storages/IndicesDescription.h b/src/Storages/IndicesDescription.h index 72e0748778f..862df6fe23c 100644 --- a/src/Storages/IndicesDescription.h +++ b/src/Storages/IndicesDescription.h @@ -74,7 +74,6 @@ struct IndicesDescription : public std::vector, IHints<1, Indi /// Return common expression for all stored indices ExpressionActionsPtr getSingleExpressionForIndices(const ColumnsDescription & columns, ContextPtr context) const; -public: Names getAllRegisteredNames() const override; }; From fd9a10ef5300ac4ad20eca03b7f213ba5b571e98 Mon Sep 17 00:00:00 2001 From: taiyang-li <654010905@qq.com> Date: Wed, 30 Mar 2022 21:33:23 +0800 Subject: [PATCH 02/10] add hints for projections --- src/Storages/ColumnsDescription.h | 2 +- src/Storages/ProjectionsDescription.cpp | 17 +++++++++++++++-- src/Storages/ProjectionsDescription.h | 4 +++- .../02250_hints_for_columns.reference | 3 +++ .../0_stateless/02250_hints_for_columns.sh | 17 +++++++++++++++++ .../02250_hints_for_projections.reference | 1 + .../0_stateless/02250_hints_for_projections.sh | 13 +++++++++++++ 7 files changed, 53 insertions(+), 4 deletions(-) create mode 100644 tests/queries/0_stateless/02250_hints_for_columns.reference create mode 100644 tests/queries/0_stateless/02250_hints_for_columns.sh create mode 100644 tests/queries/0_stateless/02250_hints_for_projections.reference create mode 100644 tests/queries/0_stateless/02250_hints_for_projections.sh diff --git a/src/Storages/ColumnsDescription.h b/src/Storages/ColumnsDescription.h index affe2ef5a56..81cb475a1f6 100644 --- a/src/Storages/ColumnsDescription.h +++ b/src/Storages/ColumnsDescription.h @@ -91,7 +91,7 @@ struct ColumnDescription /// Description of multiple table columns (in CREATE TABLE for example). -class ColumnsDescription : public IHints<2, ColumnsDescription> +class ColumnsDescription : public IHints<1, ColumnsDescription> { public: ColumnsDescription() = default; diff --git a/src/Storages/ProjectionsDescription.cpp b/src/Storages/ProjectionsDescription.cpp index 7c340cda739..70e312931cc 100644 --- a/src/Storages/ProjectionsDescription.cpp +++ b/src/Storages/ProjectionsDescription.cpp @@ -335,7 +335,9 @@ const ProjectionDescription & ProjectionsDescription::get(const String & project { auto it = map.find(projection_name); if (it == map.end()) - throw Exception("There is no projection " + projection_name + " in table", ErrorCodes::NO_SUCH_PROJECTION_IN_TABLE); + throw Exception( + "There is no projection " + projection_name + " in table" + getHintsString(projection_name), + ErrorCodes::NO_SUCH_PROJECTION_IN_TABLE); return *(it->second); } @@ -376,13 +378,24 @@ void ProjectionsDescription::remove(const String & projection_name, bool if_exis { if (if_exists) return; - throw Exception("There is no projection " + projection_name + " in table.", ErrorCodes::NO_SUCH_PROJECTION_IN_TABLE); + throw Exception( + "There is no projection " + projection_name + " in table" + getHintsString(projection_name), + ErrorCodes::NO_SUCH_PROJECTION_IN_TABLE); } projections.erase(it->second); map.erase(it); } +std::vector ProjectionsDescription::getAllRegisteredNames() const +{ + std::vector names; + names.reserve(map.size()); + for (const auto & pair : map) + names.push_back(pair.first); + return names; +} + ExpressionActionsPtr ProjectionsDescription::getSingleExpressionForProjections(const ColumnsDescription & columns, ContextPtr query_context) const { diff --git a/src/Storages/ProjectionsDescription.h b/src/Storages/ProjectionsDescription.h index 3e8d5e1a4f1..c48942eb0ec 100644 --- a/src/Storages/ProjectionsDescription.h +++ b/src/Storages/ProjectionsDescription.h @@ -106,7 +106,7 @@ struct ProjectionDescription using ProjectionDescriptionRawPtr = const ProjectionDescription *; /// All projections in storage -struct ProjectionsDescription +struct ProjectionsDescription : public IHints<1, ProjectionsDescription> { ProjectionsDescription() = default; ProjectionsDescription(ProjectionsDescription && other) = default; @@ -138,6 +138,8 @@ struct ProjectionsDescription add(ProjectionDescription && projection, const String & after_projection = String(), bool first = false, bool if_not_exists = false); void remove(const String & projection_name, bool if_exists); + std::vector getAllRegisteredNames() const override; + private: /// Keep the sequence of columns and allow to lookup by name. using Container = std::list; diff --git a/tests/queries/0_stateless/02250_hints_for_columns.reference b/tests/queries/0_stateless/02250_hints_for_columns.reference new file mode 100644 index 00000000000..0eabe367130 --- /dev/null +++ b/tests/queries/0_stateless/02250_hints_for_columns.reference @@ -0,0 +1,3 @@ +OK +OK +OK diff --git a/tests/queries/0_stateless/02250_hints_for_columns.sh b/tests/queries/0_stateless/02250_hints_for_columns.sh new file mode 100644 index 00000000000..e8fe1a7a160 --- /dev/null +++ b/tests/queries/0_stateless/02250_hints_for_columns.sh @@ -0,0 +1,17 @@ +#!/usr/bin/env bash + +CURDIR=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd) +# shellcheck source=../shell_config.sh +. "$CURDIR"/../shell_config.sh + +$CLICKHOUSE_CLIENT --query="DROP TABLE IF EXISTS t" + +$CLICKHOUSE_CLIENT --query="CREATE TABLE t (CounterID UInt32, StartDate Date, UserID UInt32, VisitID UInt32, NestedColumn Nested(A UInt8, S String), ToDrop UInt32) ENGINE = MergeTree(StartDate, intHash32(UserID), (CounterID, StartDate, intHash32(UserID), VisitID), 8192)" + +$CLICKHOUSE_CLIENT --query="ALTER TABLE t DROP COLUMN ToDro" 2>&1 | grep -q "may be you meant: \['ToDrop'\]" && echo 'OK' || echo 'FAIL' + +$CLICKHOUSE_CLIENT --query="ALTER TABLE t MODIFY COLUMN ToDro UInt64" 2>&1 | grep -q "may be you meant: \['ToDrop'\]" && echo 'OK' || echo 'FAIL' + +$CLICKHOUSE_CLIENT --query="ALTER TABLE t RENAME COLUMN ToDro to ToDropp" 2>&1 | grep -q "may be you meant: \['ToDrop'\]" && echo 'OK' || echo 'FAIL' + +$CLICKHOUSE_CLIENT --query="DROP TABLE t" \ No newline at end of file diff --git a/tests/queries/0_stateless/02250_hints_for_projections.reference b/tests/queries/0_stateless/02250_hints_for_projections.reference new file mode 100644 index 00000000000..d86bac9de59 --- /dev/null +++ b/tests/queries/0_stateless/02250_hints_for_projections.reference @@ -0,0 +1 @@ +OK diff --git a/tests/queries/0_stateless/02250_hints_for_projections.sh b/tests/queries/0_stateless/02250_hints_for_projections.sh new file mode 100644 index 00000000000..57123b88bde --- /dev/null +++ b/tests/queries/0_stateless/02250_hints_for_projections.sh @@ -0,0 +1,13 @@ +#!/usr/bin/env bash + +CURDIR=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd) +# shellcheck source=../shell_config.sh +. "$CURDIR"/../shell_config.sh + +$CLICKHOUSE_CLIENT --query="DROP TABLE IF EXISTS t" + +$CLICKHOUSE_CLIENT --query="create table t (x Int32, y Int32, projection pToDrop (select x, y order by x)) engine = MergeTree order by y;" + +$CLICKHOUSE_CLIENT --query="ALTER TABLE t DROP PROJECTION pToDro" 2>&1 | grep -q "may be you meant: \['pToDrop'\]" && echo 'OK' || echo 'FAIL' + +$CLICKHOUSE_CLIENT --query="DROP TABLE t" \ No newline at end of file From eda299b48b744d321d013885ecddcff0eb08fc0d Mon Sep 17 00:00:00 2001 From: taiyang-li <654010905@qq.com> Date: Thu, 31 Mar 2022 12:14:28 +0800 Subject: [PATCH 03/10] fix building --- src/Common/NamePrompter.h | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/Common/NamePrompter.h b/src/Common/NamePrompter.h index 8e301dec8b7..9663427ef12 100644 --- a/src/Common/NamePrompter.h +++ b/src/Common/NamePrompter.h @@ -1,8 +1,8 @@ #pragma once #include +#include #include -#include #include #include @@ -105,8 +105,12 @@ public: String getHintsString(const String & name) const { - const auto hints = getHints(name); - return !hints.empty() ? ", may be you meant: " + toString(hints) : ""; + auto hints = getHints(name); + + /// Note: we don't use toString because it will cause writeCString naming conflict in src/Dictionaries/MongoDBDictionarySource.cpp + for (auto & hint : hints) + hint = "'" + hint + "'"; + return !hints.empty() ? ", may be you meant: " + boost::algorithm::join(hints, ",") : ""; } IHints() = default; From d6247338de5ba64f4180ef99dd7e003416ee04d7 Mon Sep 17 00:00:00 2001 From: taiyang-li <654010905@qq.com> Date: Thu, 31 Mar 2022 13:33:20 +0800 Subject: [PATCH 04/10] fix failed stateless tests --- src/Common/NamePrompter.h | 2 +- src/Storages/AlterCommands.cpp | 21 ++++++++++++++------- 2 files changed, 15 insertions(+), 8 deletions(-) diff --git a/src/Common/NamePrompter.h b/src/Common/NamePrompter.h index 9663427ef12..25206cbd25f 100644 --- a/src/Common/NamePrompter.h +++ b/src/Common/NamePrompter.h @@ -110,7 +110,7 @@ public: /// Note: we don't use toString because it will cause writeCString naming conflict in src/Dictionaries/MongoDBDictionarySource.cpp for (auto & hint : hints) hint = "'" + hint + "'"; - return !hints.empty() ? ", may be you meant: " + boost::algorithm::join(hints, ",") : ""; + return !hints.empty() ? ", may be you meant: [" + boost::algorithm::join(hints, ",") + "]" : ""; } IHints() = default; diff --git a/src/Storages/AlterCommands.cpp b/src/Storages/AlterCommands.cpp index 44f208adacc..3ddeec4fa47 100644 --- a/src/Storages/AlterCommands.cpp +++ b/src/Storages/AlterCommands.cpp @@ -1046,8 +1046,10 @@ void AlterCommands::validate(const StorageInMemoryMetadata & metadata, ContextPt if (!all_columns.has(column_name)) { if (!command.if_exists) - throw Exception{"Wrong column name. Cannot find column " + backQuote(column_name) + " to modify", - ErrorCodes::NOT_FOUND_COLUMN_IN_BLOCK}; + throw Exception{ + "Wrong column name. Cannot find column " + backQuote(column_name) + " to modify" + + all_columns.getHintsString(column_name), + ErrorCodes::NOT_FOUND_COLUMN_IN_BLOCK}; else continue; } @@ -1153,7 +1155,8 @@ void AlterCommands::validate(const StorageInMemoryMetadata & metadata, ContextPt } else if (!command.if_exists) throw Exception( - "Wrong column name. Cannot find column " + backQuote(command.column_name) + " to drop", + "Wrong column name. Cannot find column " + backQuote(command.column_name) + " to drop" + + all_columns.getHintsString(command.column_name), ErrorCodes::NOT_FOUND_COLUMN_IN_BLOCK); } else if (command.type == AlterCommand::COMMENT_COLUMN) @@ -1161,8 +1164,10 @@ void AlterCommands::validate(const StorageInMemoryMetadata & metadata, ContextPt if (!all_columns.has(command.column_name)) { if (!command.if_exists) - throw Exception{"Wrong column name. Cannot find column " + backQuote(command.column_name) + " to comment", - ErrorCodes::NOT_FOUND_COLUMN_IN_BLOCK}; + throw Exception{ + "Wrong column name. Cannot find column " + backQuote(command.column_name) + " to comment" + + all_columns.getHintsString(command.column_name), + ErrorCodes::NOT_FOUND_COLUMN_IN_BLOCK}; } } else if (command.type == AlterCommand::MODIFY_SETTING || command.type == AlterCommand::RESET_SETTING) @@ -1196,8 +1201,10 @@ void AlterCommands::validate(const StorageInMemoryMetadata & metadata, ContextPt if (!all_columns.has(command.column_name)) { if (!command.if_exists) - throw Exception{"Wrong column name. Cannot find column " + backQuote(command.column_name) + " to rename", - ErrorCodes::NOT_FOUND_COLUMN_IN_BLOCK}; + throw Exception{ + "Wrong column name. Cannot find column " + backQuote(command.column_name) + " to rename" + + all_columns.getHintsString(command.column_name), + ErrorCodes::NOT_FOUND_COLUMN_IN_BLOCK}; else continue; } From 6bc1786047e0adee4c3d5c121fa9d9b3b0626c1a Mon Sep 17 00:00:00 2001 From: taiyang-li <654010905@qq.com> Date: Thu, 31 Mar 2022 16:43:23 +0800 Subject: [PATCH 05/10] fix style --- src/Storages/AlterCommands.cpp | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/Storages/AlterCommands.cpp b/src/Storages/AlterCommands.cpp index 3ddeec4fa47..5b44a4676c6 100644 --- a/src/Storages/AlterCommands.cpp +++ b/src/Storages/AlterCommands.cpp @@ -1046,8 +1046,7 @@ void AlterCommands::validate(const StorageInMemoryMetadata & metadata, ContextPt if (!all_columns.has(column_name)) { if (!command.if_exists) - throw Exception{ - "Wrong column name. Cannot find column " + backQuote(column_name) + " to modify" + throw Exception{"Wrong column name. Cannot find column " + backQuote(column_name) + " to modify" + all_columns.getHintsString(column_name), ErrorCodes::NOT_FOUND_COLUMN_IN_BLOCK}; else @@ -1164,8 +1163,7 @@ void AlterCommands::validate(const StorageInMemoryMetadata & metadata, ContextPt if (!all_columns.has(command.column_name)) { if (!command.if_exists) - throw Exception{ - "Wrong column name. Cannot find column " + backQuote(command.column_name) + " to comment" + throw Exception{"Wrong column name. Cannot find column " + backQuote(command.column_name) + " to comment" + all_columns.getHintsString(command.column_name), ErrorCodes::NOT_FOUND_COLUMN_IN_BLOCK}; } @@ -1201,8 +1199,7 @@ void AlterCommands::validate(const StorageInMemoryMetadata & metadata, ContextPt if (!all_columns.has(command.column_name)) { if (!command.if_exists) - throw Exception{ - "Wrong column name. Cannot find column " + backQuote(command.column_name) + " to rename" + throw Exception{"Wrong column name. Cannot find column " + backQuote(command.column_name) + " to rename" + all_columns.getHintsString(command.column_name), ErrorCodes::NOT_FOUND_COLUMN_IN_BLOCK}; else From 9dd1a76fd85e3ed677d084a01c1a550255e19c9e Mon Sep 17 00:00:00 2001 From: taiyang-li <654010905@qq.com> Date: Thu, 31 Mar 2022 16:45:36 +0800 Subject: [PATCH 06/10] fix stateless tests --- tests/queries/0_stateless/02250_hints_for_columns.reference | 2 -- tests/queries/0_stateless/02250_hints_for_projections.reference | 1 - 2 files changed, 3 deletions(-) diff --git a/tests/queries/0_stateless/02250_hints_for_columns.reference b/tests/queries/0_stateless/02250_hints_for_columns.reference index 0eabe367130..d86bac9de59 100644 --- a/tests/queries/0_stateless/02250_hints_for_columns.reference +++ b/tests/queries/0_stateless/02250_hints_for_columns.reference @@ -1,3 +1 @@ OK -OK -OK diff --git a/tests/queries/0_stateless/02250_hints_for_projections.reference b/tests/queries/0_stateless/02250_hints_for_projections.reference index d86bac9de59..e69de29bb2d 100644 --- a/tests/queries/0_stateless/02250_hints_for_projections.reference +++ b/tests/queries/0_stateless/02250_hints_for_projections.reference @@ -1 +0,0 @@ -OK From 10bbb965127f6d3f1ae15ff2a6b0cfbbdee68a18 Mon Sep 17 00:00:00 2001 From: taiyang-li <654010905@qq.com> Date: Fri, 1 Apr 2022 11:56:43 +0800 Subject: [PATCH 07/10] fix stateless test --- tests/queries/0_stateless/02250_hints_for_columns.reference | 2 ++ tests/queries/0_stateless/02250_hints_for_projections.reference | 1 + 2 files changed, 3 insertions(+) diff --git a/tests/queries/0_stateless/02250_hints_for_columns.reference b/tests/queries/0_stateless/02250_hints_for_columns.reference index d86bac9de59..0eabe367130 100644 --- a/tests/queries/0_stateless/02250_hints_for_columns.reference +++ b/tests/queries/0_stateless/02250_hints_for_columns.reference @@ -1 +1,3 @@ OK +OK +OK diff --git a/tests/queries/0_stateless/02250_hints_for_projections.reference b/tests/queries/0_stateless/02250_hints_for_projections.reference index e69de29bb2d..d86bac9de59 100644 --- a/tests/queries/0_stateless/02250_hints_for_projections.reference +++ b/tests/queries/0_stateless/02250_hints_for_projections.reference @@ -0,0 +1 @@ +OK From f4772d3b8fe416355324cac849ab925ae6bdfbe3 Mon Sep 17 00:00:00 2001 From: taiyang-li <654010905@qq.com> Date: Fri, 1 Apr 2022 14:45:20 +0800 Subject: [PATCH 08/10] chmod a+x 02250_hints_for_columns/02250_hints_for_projections --- tests/queries/0_stateless/02250_hints_for_columns.sh | 0 tests/queries/0_stateless/02250_hints_for_projections.sh | 0 2 files changed, 0 insertions(+), 0 deletions(-) mode change 100644 => 100755 tests/queries/0_stateless/02250_hints_for_columns.sh mode change 100644 => 100755 tests/queries/0_stateless/02250_hints_for_projections.sh diff --git a/tests/queries/0_stateless/02250_hints_for_columns.sh b/tests/queries/0_stateless/02250_hints_for_columns.sh old mode 100644 new mode 100755 diff --git a/tests/queries/0_stateless/02250_hints_for_projections.sh b/tests/queries/0_stateless/02250_hints_for_projections.sh old mode 100644 new mode 100755 From d96b682a5562132186da9f3aaea9af2647877b5b Mon Sep 17 00:00:00 2001 From: Antonio Andelic Date: Fri, 1 Apr 2022 13:12:54 +0000 Subject: [PATCH 09/10] Refactor --- src/Common/NamePrompter.cpp | 15 +++++++++ src/Common/NamePrompter.h | 15 +++++---- src/Storages/AlterCommands.cpp | 32 ++++++++++++------- src/Storages/ColumnsDescription.cpp | 12 ++++--- src/Storages/ColumnsDescription.h | 7 ++-- src/Storages/ProjectionsDescription.cpp | 15 +++++---- .../0_stateless/02250_hints_for_columns.sh | 8 ++--- .../02250_hints_for_projections.sh | 4 +-- 8 files changed, 71 insertions(+), 37 deletions(-) create mode 100644 src/Common/NamePrompter.cpp diff --git a/src/Common/NamePrompter.cpp b/src/Common/NamePrompter.cpp new file mode 100644 index 00000000000..c5a2224dcb4 --- /dev/null +++ b/src/Common/NamePrompter.cpp @@ -0,0 +1,15 @@ +#include +#include + +namespace DB::detail +{ +void appendHintsMessageImpl(String & message, const std::vector & hints) +{ + if (hints.empty()) + { + return; + } + + message += ". Maybe you meant: " + toString(hints); +} +} diff --git a/src/Common/NamePrompter.h b/src/Common/NamePrompter.h index 25206cbd25f..b3eb271c0f0 100644 --- a/src/Common/NamePrompter.h +++ b/src/Common/NamePrompter.h @@ -1,7 +1,6 @@ #pragma once #include -#include #include #include @@ -91,6 +90,10 @@ private: } }; +namespace detail +{ +void appendHintsMessageImpl(String & message, const std::vector & hints); +} template class IHints @@ -103,14 +106,10 @@ public: return prompter.getHints(name, getAllRegisteredNames()); } - String getHintsString(const String & name) const + void appendHintsMessage(String & message, const String & name) const { auto hints = getHints(name); - - /// Note: we don't use toString because it will cause writeCString naming conflict in src/Dictionaries/MongoDBDictionarySource.cpp - for (auto & hint : hints) - hint = "'" + hint + "'"; - return !hints.empty() ? ", may be you meant: [" + boost::algorithm::join(hints, ",") + "]" : ""; + detail::appendHintsMessageImpl(message, hints); } IHints() = default; @@ -126,4 +125,6 @@ private: NamePrompter prompter; }; +void appendHintsString(String & message, const std::vector & hints, const String & name); + } diff --git a/src/Storages/AlterCommands.cpp b/src/Storages/AlterCommands.cpp index 5b44a4676c6..2870dc42af7 100644 --- a/src/Storages/AlterCommands.cpp +++ b/src/Storages/AlterCommands.cpp @@ -1046,9 +1046,12 @@ void AlterCommands::validate(const StorageInMemoryMetadata & metadata, ContextPt if (!all_columns.has(column_name)) { if (!command.if_exists) - throw Exception{"Wrong column name. Cannot find column " + backQuote(column_name) + " to modify" - + all_columns.getHintsString(column_name), + { + String exception_message = fmt::format("Wrong column. Cannot find colum {} to modify", backQuote(column_name)); + all_columns.appendHintsMessage(exception_message, column_name); + throw Exception{exception_message, ErrorCodes::NOT_FOUND_COLUMN_IN_BLOCK}; + } else continue; } @@ -1153,19 +1156,22 @@ void AlterCommands::validate(const StorageInMemoryMetadata & metadata, ContextPt all_columns.remove(command.column_name); } else if (!command.if_exists) - throw Exception( - "Wrong column name. Cannot find column " + backQuote(command.column_name) + " to drop" - + all_columns.getHintsString(command.column_name), - ErrorCodes::NOT_FOUND_COLUMN_IN_BLOCK); + { + String exception_message = fmt::format("Wrong column name. Cannot find column {} to drop", backQuote(command.column_name)); + all_columns.appendHintsMessage(exception_message, command.column_name); + throw Exception(exception_message, ErrorCodes::NOT_FOUND_COLUMN_IN_BLOCK); + } } else if (command.type == AlterCommand::COMMENT_COLUMN) { if (!all_columns.has(command.column_name)) { if (!command.if_exists) - throw Exception{"Wrong column name. Cannot find column " + backQuote(command.column_name) + " to comment" - + all_columns.getHintsString(command.column_name), - ErrorCodes::NOT_FOUND_COLUMN_IN_BLOCK}; + { + String exception_message = fmt::format("Wrong column name. Cannot find column {} to comment", backQuote(command.column_name)); + all_columns.appendHintsMessage(exception_message, command.column_name); + throw Exception(exception_message, ErrorCodes::NOT_FOUND_COLUMN_IN_BLOCK); + } } } else if (command.type == AlterCommand::MODIFY_SETTING || command.type == AlterCommand::RESET_SETTING) @@ -1199,9 +1205,11 @@ void AlterCommands::validate(const StorageInMemoryMetadata & metadata, ContextPt if (!all_columns.has(command.column_name)) { if (!command.if_exists) - throw Exception{"Wrong column name. Cannot find column " + backQuote(command.column_name) + " to rename" - + all_columns.getHintsString(command.column_name), - ErrorCodes::NOT_FOUND_COLUMN_IN_BLOCK}; + { + String exception_message = fmt::format("Wrong column name. Cannot find column {} to rename", backQuote(command.column_name)); + all_columns.appendHintsMessage(exception_message, command.column_name); + throw Exception(exception_message, ErrorCodes::NOT_FOUND_COLUMN_IN_BLOCK); + } else continue; } diff --git a/src/Storages/ColumnsDescription.cpp b/src/Storages/ColumnsDescription.cpp index a694405665b..f3a939614c1 100644 --- a/src/Storages/ColumnsDescription.cpp +++ b/src/Storages/ColumnsDescription.cpp @@ -230,8 +230,11 @@ void ColumnsDescription::remove(const String & column_name) { auto range = getNameRange(columns, column_name); if (range.first == range.second) - throw Exception( - "There is no column " + column_name + " in table" + getHintsString(column_name), ErrorCodes::NO_SUCH_COLUMN_IN_TABLE); + { + String exception_message = fmt::format("There is no column {} in table", column_name); + appendHintsMessage(exception_message, column_name); + throw Exception(exception_message, ErrorCodes::NO_SUCH_COLUMN_IN_TABLE); + } for (auto list_it = range.first; list_it != range.second;) { @@ -245,8 +248,9 @@ void ColumnsDescription::rename(const String & column_from, const String & colum auto it = columns.get<1>().find(column_from); if (it == columns.get<1>().end()) { - throw Exception( - "Cannot find column " + column_from + " in ColumnsDescription" + getHintsString(column_from), ErrorCodes::LOGICAL_ERROR); + String exception_message = fmt::format("Cannot find column {} in ColumnsDescription", column_from); + appendHintsMessage(exception_message, column_from); + throw Exception(exception_message, ErrorCodes::LOGICAL_ERROR); } columns.get<1>().modify_key(it, [&column_to] (String & old_name) diff --git a/src/Storages/ColumnsDescription.h b/src/Storages/ColumnsDescription.h index 81cb475a1f6..d3d6f7f2ff5 100644 --- a/src/Storages/ColumnsDescription.h +++ b/src/Storages/ColumnsDescription.h @@ -149,8 +149,11 @@ public: { auto it = columns.get<1>().find(column_name); if (it == columns.get<1>().end()) - throw Exception( - "Cannot find column " + column_name + " in ColumnsDescription" + getHintsString(column_name), ErrorCodes::LOGICAL_ERROR); + { + String exception_message = fmt::format("Cannot find column {} in ColumnsDescription", column_name); + appendHintsMessage(exception_message, column_name); + throw Exception(exception_message, ErrorCodes::LOGICAL_ERROR); + } removeSubcolumns(it->name); if (!columns.get<1>().modify(it, std::forward(f))) diff --git a/src/Storages/ProjectionsDescription.cpp b/src/Storages/ProjectionsDescription.cpp index 70e312931cc..69d7c5f8ed6 100644 --- a/src/Storages/ProjectionsDescription.cpp +++ b/src/Storages/ProjectionsDescription.cpp @@ -335,9 +335,11 @@ const ProjectionDescription & ProjectionsDescription::get(const String & project { auto it = map.find(projection_name); if (it == map.end()) - throw Exception( - "There is no projection " + projection_name + " in table" + getHintsString(projection_name), - ErrorCodes::NO_SUCH_PROJECTION_IN_TABLE); + { + String exception_message = fmt::format("There is no projection {} in table", projection_name); + appendHintsMessage(exception_message, projection_name); + throw Exception(exception_message, ErrorCodes::NO_SUCH_PROJECTION_IN_TABLE); + } return *(it->second); } @@ -378,9 +380,10 @@ void ProjectionsDescription::remove(const String & projection_name, bool if_exis { if (if_exists) return; - throw Exception( - "There is no projection " + projection_name + " in table" + getHintsString(projection_name), - ErrorCodes::NO_SUCH_PROJECTION_IN_TABLE); + + String exception_message = fmt::format("There is no projection {} in table", projection_name); + appendHintsMessage(exception_message, projection_name); + throw Exception(exception_message, ErrorCodes::NO_SUCH_PROJECTION_IN_TABLE); } projections.erase(it->second); diff --git a/tests/queries/0_stateless/02250_hints_for_columns.sh b/tests/queries/0_stateless/02250_hints_for_columns.sh index e8fe1a7a160..45fd2f238b1 100755 --- a/tests/queries/0_stateless/02250_hints_for_columns.sh +++ b/tests/queries/0_stateless/02250_hints_for_columns.sh @@ -8,10 +8,10 @@ $CLICKHOUSE_CLIENT --query="DROP TABLE IF EXISTS t" $CLICKHOUSE_CLIENT --query="CREATE TABLE t (CounterID UInt32, StartDate Date, UserID UInt32, VisitID UInt32, NestedColumn Nested(A UInt8, S String), ToDrop UInt32) ENGINE = MergeTree(StartDate, intHash32(UserID), (CounterID, StartDate, intHash32(UserID), VisitID), 8192)" -$CLICKHOUSE_CLIENT --query="ALTER TABLE t DROP COLUMN ToDro" 2>&1 | grep -q "may be you meant: \['ToDrop'\]" && echo 'OK' || echo 'FAIL' +$CLICKHOUSE_CLIENT --query="ALTER TABLE t DROP COLUMN ToDro" 2>&1 | grep -q "Maybe you meant: \['ToDrop'\]" && echo 'OK' || echo 'FAIL' -$CLICKHOUSE_CLIENT --query="ALTER TABLE t MODIFY COLUMN ToDro UInt64" 2>&1 | grep -q "may be you meant: \['ToDrop'\]" && echo 'OK' || echo 'FAIL' +$CLICKHOUSE_CLIENT --query="ALTER TABLE t MODIFY COLUMN ToDro UInt64" 2>&1 | grep -q "Maybe you meant: \['ToDrop'\]" && echo 'OK' || echo 'FAIL' -$CLICKHOUSE_CLIENT --query="ALTER TABLE t RENAME COLUMN ToDro to ToDropp" 2>&1 | grep -q "may be you meant: \['ToDrop'\]" && echo 'OK' || echo 'FAIL' +$CLICKHOUSE_CLIENT --query="ALTER TABLE t RENAME COLUMN ToDro to ToDropp" 2>&1 | grep -q "Maybe you meant: \['ToDrop'\]" && echo 'OK' || echo 'FAIL' -$CLICKHOUSE_CLIENT --query="DROP TABLE t" \ No newline at end of file +$CLICKHOUSE_CLIENT --query="DROP TABLE t" diff --git a/tests/queries/0_stateless/02250_hints_for_projections.sh b/tests/queries/0_stateless/02250_hints_for_projections.sh index 57123b88bde..7db8b243ae4 100755 --- a/tests/queries/0_stateless/02250_hints_for_projections.sh +++ b/tests/queries/0_stateless/02250_hints_for_projections.sh @@ -8,6 +8,6 @@ $CLICKHOUSE_CLIENT --query="DROP TABLE IF EXISTS t" $CLICKHOUSE_CLIENT --query="create table t (x Int32, y Int32, projection pToDrop (select x, y order by x)) engine = MergeTree order by y;" -$CLICKHOUSE_CLIENT --query="ALTER TABLE t DROP PROJECTION pToDro" 2>&1 | grep -q "may be you meant: \['pToDrop'\]" && echo 'OK' || echo 'FAIL' +$CLICKHOUSE_CLIENT --query="ALTER TABLE t DROP PROJECTION pToDro" 2>&1 | grep -q "Maybe you meant: \['pToDrop'\]" && echo 'OK' || echo 'FAIL' -$CLICKHOUSE_CLIENT --query="DROP TABLE t" \ No newline at end of file +$CLICKHOUSE_CLIENT --query="DROP TABLE t" From a926bc19eabc3d739b5ed9bef0c324ac6d49ca62 Mon Sep 17 00:00:00 2001 From: Antonio Andelic Date: Mon, 4 Apr 2022 07:24:10 +0000 Subject: [PATCH 10/10] Address PR comments --- src/Common/NamePrompter.h | 3 --- src/Storages/AlterCommands.cpp | 2 +- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/src/Common/NamePrompter.h b/src/Common/NamePrompter.h index b3eb271c0f0..962a89a8e76 100644 --- a/src/Common/NamePrompter.h +++ b/src/Common/NamePrompter.h @@ -124,7 +124,4 @@ public: private: NamePrompter prompter; }; - -void appendHintsString(String & message, const std::vector & hints, const String & name); - } diff --git a/src/Storages/AlterCommands.cpp b/src/Storages/AlterCommands.cpp index 2870dc42af7..76df6316fed 100644 --- a/src/Storages/AlterCommands.cpp +++ b/src/Storages/AlterCommands.cpp @@ -1047,7 +1047,7 @@ void AlterCommands::validate(const StorageInMemoryMetadata & metadata, ContextPt { if (!command.if_exists) { - String exception_message = fmt::format("Wrong column. Cannot find colum {} to modify", backQuote(column_name)); + String exception_message = fmt::format("Wrong column. Cannot find column {} to modify", backQuote(column_name)); all_columns.appendHintsMessage(exception_message, column_name); throw Exception{exception_message, ErrorCodes::NOT_FOUND_COLUMN_IN_BLOCK};