Merge pull request #35852 from ClickHouse/bigo-sg-add_hints

Refactoring of hints for column descriptor

src/Common/NamePrompter.cpp

@@ -0,0 +1,15 @@
+#include <IO/WriteHelpers.h>
+#include <Common/NamePrompter.h>
+
+namespace DB::detail
+{
+void appendHintsMessageImpl(String & message, const std::vector<String> & hints)
+{
+    if (hints.empty())
+    {
+        return;
+    }
+
+    message += ". Maybe you meant: " + toString(hints);
+}
+}

src/Common/NamePrompter.h

@@ -90,6 +90,10 @@ private:
     }
 };
 
+namespace detail
+{
+void appendHintsMessageImpl(String & message, const std::vector<String> & hints);
+}
 
 template <size_t MaxNumHints, typename Self>
 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<MaxNumHints> prompter;
 };
-
 }

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",
+                String exception_message = fmt::format("Wrong column name. Cannot find column {} to drop", backQuote(command.column_name));
-                    ErrorCodes::NOT_FOUND_COLUMN_IN_BLOCK);
+                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;
             }

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<String> ColumnsDescription::getAllRegisteredNames() const
+{
+    std::vector<String> 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)

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>(f)))
@@ -196,6 +200,8 @@ public:
         return columns.empty();
     }
 
+    std::vector<String> getAllRegisteredNames() const override;
+
     /// Keep the sequence of columns and allow to lookup by name.
     using ColumnsContainer = boost::multi_index_container<
         ColumnDescription,

src/Storages/IndicesDescription.h

@@ -74,7 +74,6 @@ struct IndicesDescription : public std::vector<IndexDescription>, IHints<1, Indi
     /// Return common expression for all stored indices
     ExpressionActionsPtr getSingleExpressionForIndices(const ColumnsDescription & columns, ContextPtr context) const;
 
-public:
     Names getAllRegisteredNames() const override;
 };
 

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<String> ProjectionsDescription::getAllRegisteredNames() const
+{
+    std::vector<String> 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
 {

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<String> getAllRegisteredNames() const override;
+
 private:
     /// Keep the sequence of columns and allow to lookup by name.
     using Container = std::list<ProjectionDescription>;

tests/queries/0_stateless/02250_hints_for_columns.reference

@@ -0,0 +1,3 @@
+OK
+OK
+OK

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"

tests/queries/0_stateless/02250_hints_for_projections.reference

@@ -0,0 +1 @@
+OK

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"