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 a88d4bdea8e..962a89a8e76 100644 --- a/src/Common/NamePrompter.h +++ b/src/Common/NamePrompter.h @@ -90,6 +90,10 @@ private: } }; +namespace detail +{ +void appendHintsMessageImpl(String & message, const std::vector & hints); +} template class IHints @@ -102,6 +106,12 @@ public: return prompter.getHints(name, getAllRegisteredNames()); } + void appendHintsMessage(String & message, const String & name) const + { + auto hints = getHints(name); + detail::appendHintsMessageImpl(message, hints); + } + IHints() = default; IHints(const IHints &) = default; @@ -114,5 +124,4 @@ public: private: NamePrompter prompter; }; - } diff --git a/src/Storages/AlterCommands.cpp b/src/Storages/AlterCommands.cpp index 44f208adacc..76df6316fed 100644 --- a/src/Storages/AlterCommands.cpp +++ b/src/Storages/AlterCommands.cpp @@ -1046,8 +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", - ErrorCodes::NOT_FOUND_COLUMN_IN_BLOCK}; + { + 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}; + } else continue; } @@ -1152,17 +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", - 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", - 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) @@ -1196,8 +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", - 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 69ca6002c22..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.", - 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;) { @@ -244,7 +247,11 @@ 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); + { + 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) { @@ -745,6 +752,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..d3d6f7f2ff5 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<1, ColumnsDescription> { public: ColumnsDescription() = default; @@ -149,7 +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", 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))) @@ -196,6 +200,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; }; diff --git a/src/Storages/ProjectionsDescription.cpp b/src/Storages/ProjectionsDescription.cpp index 7c340cda739..69d7c5f8ed6 100644 --- a/src/Storages/ProjectionsDescription.cpp +++ b/src/Storages/ProjectionsDescription.cpp @@ -335,7 +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", 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); } @@ -376,13 +380,25 @@ 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); + + 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); 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 100755 index 00000000000..45fd2f238b1 --- /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 "Maybe 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 "Maybe you meant: \['ToDrop'\]" && echo 'OK' || echo 'FAIL' + +$CLICKHOUSE_CLIENT --query="DROP TABLE t" 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 100755 index 00000000000..7db8b243ae4 --- /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 "Maybe you meant: \['pToDrop'\]" && echo 'OK' || echo 'FAIL' + +$CLICKHOUSE_CLIENT --query="DROP TABLE t"