add is_mutable method to AlterCommand. change comment column test

dbms/src/Parsers/ParserCreateQuery.h

@@ -128,17 +128,26 @@ bool IParserColumnDeclaration<NameParser>::parseImpl(Pos & pos, ASTPtr & node, E
     const auto fallback_pos = pos;
     if (!s_default.check(pos, expected) &&
         !s_materialized.check(pos, expected) &&
-        !s_alias.check(pos, expected))
+        !s_alias.check(pos, expected) &&
+        !s_comment.check(pos, expected))
     {
         type_parser.parse(pos, type, expected);
     }
     else
         pos = fallback_pos;
 
-    /// parse {DEFAULT, MATERIALIZED, ALIAS}
+    /// parse {DEFAULT, MATERIALIZED, ALIAS, COMMENT}
     String default_specifier;
     ASTPtr default_expression;
+    ASTPtr comment_expression;
     Pos pos_before_specifier = pos;
+    if (!s_default.ignore(pos, expected) &&
+        !s_materialized.ignore(pos, expected) &&
+        !s_alias.ignore(pos, expected) &&
+        !s_comment.ignore(pos, expected) &&
+        !type)
+        return false;  /// reject sole column name without type
+
     if (s_default.ignore(pos, expected) ||
         s_materialized.ignore(pos, expected) ||
         s_alias.ignore(pos, expected))
@@ -149,14 +158,12 @@ bool IParserColumnDeclaration<NameParser>::parseImpl(Pos & pos, ASTPtr & node, E
         if (!expr_parser.parse(pos, default_expression, expected))
             return false;
     }
-    else if (!type)
-        return false; /// reject sole column name without type
-
-    ASTPtr comment_expression;
-    if (s_comment.ignore(pos, expected))
+    else if (s_comment.ignore(pos, expected))
     {
         string_literal_parser.parse(pos, comment_expression, expected);
     }
+    else if (!type) // TODO: тут надо очень хорошо подумать. есть проблема с тем, что для modify column имя колонки и коммент ок, а для создания таблицы не ок.
+        return false; /// reject sole column name without type
 
     const auto column_declaration = std::make_shared<ASTColumnDeclaration>();
     node = column_declaration;

dbms/src/Storages/AlterCommands.cpp

@@ -188,6 +188,20 @@ void AlterCommand::apply(ColumnsDescription & columns_description) const
     }
     else if (type == MODIFY_COLUMN)
     {
+        if (!is_mutable())
+        {
+            auto & comments = columns_description.comments;
+            if (comment.empty())
+            {
+                if (auto it = comments.find(column_name); it != comments.end())
+                    comments.erase(it);
+            }
+            else
+                columns_description.comments[column_name] = comment;
+
+            return;
+        }
+
         const auto default_it = columns_description.defaults.find(column_name);
         const auto had_default_expr = default_it != std::end(columns_description.defaults);
         const auto old_default_kind = had_default_expr ? default_it->second.kind : ColumnDefaultKind{};
@@ -256,6 +270,15 @@ void AlterCommand::apply(ColumnsDescription & columns_description) const
         throw Exception("Wrong parameter type in ALTER query", ErrorCodes::LOGICAL_ERROR);
 }
 
+bool AlterCommand::is_mutable() const
+{
+    if (type == COMMENT_COLUMN)
+        return false;
+    if (type == MODIFY_COLUMN)
+        return data_type.get() || default_expression;
+
+    return true;
+}
 
 void AlterCommands::apply(ColumnsDescription & columns_description) const
 {

dbms/src/Storages/AlterCommands.h

@@ -56,6 +56,8 @@ struct AlterCommand
 
     void apply(ColumnsDescription & columns_description) const;
 
+    /// Checks that not only metadata touched by that command
+    bool is_mutable() const;
 };
 
 class IStorage;

dbms/src/Storages/IStorage.h

@@ -239,8 +239,8 @@ public:
     {
         for (const auto & param : params)
         {
-            if (param.type != AlterCommand::Type::COMMENT_COLUMN)
-                throw Exception("Method alter only supports change comment of column for storage " + getName(), ErrorCodes::NOT_IMPLEMENTED);
+            if (param.is_mutable())
+                throw Exception("Method alter supports only change comment of column for storage " + getName(), ErrorCodes::NOT_IMPLEMENTED);
         }
 
         auto lock = lockStructureForAlter(__PRETTY_FUNCTION__);

dbms/tests/queries/0_stateless/00725_comment_columns.sql

@@ -60,4 +60,18 @@ FROM system.columns
 WHERE table = 'check_query_comment_column' and database = 'test'
 FORMAT PrettyCompactNoEscapes;
 
-DROP table test.check_query_comment_column;
+DROP TABLE IF test.check_query_comment_column;
+
+CREATE TABLE test.check_query_comment_column
+  (
+    first_column UInt8 COMMENT 'first comment'
+  ) ENGINE = TinyLog;
+
+ALTER TABLE test.check_query_comment_column MODIFY COLUMN first_column COMMENT 'another comment';
+
+SELECT table, name, comment
+FROM system.columns
+WHERE table = 'check_query_comment_column' and database = 'test'
+FORMAT PrettyCompactNoEscapes;
+
+DROP TABLE IF EXISTS test.check_query_comment_column;