make modify column comment work

dbms/src/Parsers/IParser.h

@@ -86,6 +86,14 @@ public:
             return true;
     }
 
+    /* The same, but never move the position and do not write the result to node.
+     */
+    bool check_without_moving(Pos pos, Expected & expected)
+    {
+        ASTPtr node;
+        return parse(pos, node, expected);
+    }
+
     virtual ~IParser() {}
 };
 

dbms/src/Parsers/ParserAlterQuery.cpp

@@ -47,6 +47,7 @@ bool ParserAlterCommand::parseImpl(Pos & pos, ASTPtr & node, Expected & expected
     ParserCompoundIdentifier parser_name;
     ParserStringLiteral parser_string_literal;
     ParserCompoundColumnDeclaration parser_col_decl;
+    ParserCompoundColumnDeclaration parser_modify_col_decl(false);
     ParserPartition parser_partition;
     ParserExpression parser_exp_elem;
     ParserList parser_assignment_list(
@@ -181,7 +182,7 @@ bool ParserAlterCommand::parseImpl(Pos & pos, ASTPtr & node, Expected & expected
     }
     else if (s_modify_column.ignore(pos, expected))
     {
-        if (!parser_col_decl.parse(pos, command->col_decl, expected))
+        if (!parser_modify_col_decl.parse(pos, command->col_decl, expected))
             return false;
 
         command->type = ASTAlterCommand::MODIFY_COLUMN;

dbms/src/Parsers/ParserCreateQuery.h

@@ -96,9 +96,16 @@ protected:
 template <typename NameParser>
 class IParserColumnDeclaration : public IParserBase
 {
+public:
+    explicit IParserColumnDeclaration(bool require_type_ = true) : require_type(require_type_)
+    {
+    }
+
 protected:
     const char * getName() const { return "column declaration"; }
     bool parseImpl(Pos & pos, ASTPtr & node, Expected & expected);
+
+    bool require_type = true;
 };
 
 using ParserColumnDeclaration = IParserColumnDeclaration<ParserIdentifier>;
@@ -122,35 +129,24 @@ bool IParserColumnDeclaration<NameParser>::parseImpl(Pos & pos, ASTPtr & node, E
         return false;
 
     /** column name should be followed by type name if it
-      *    is not immediately followed by {DEFAULT, MATERIALIZED, ALIAS}
+      *    is not immediately followed by {DEFAULT, MATERIALIZED, ALIAS, COMMENT}
       */
     ASTPtr type;
-    const auto fallback_pos = pos;
-    if (!s_default.check(pos, expected) &&
-        !s_materialized.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, 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))
+    if (!s_default.check_without_moving(pos, expected) &&
+        !s_materialized.check_without_moving(pos, expected) &&
+        !s_alias.check_without_moving(pos, expected) &&
+        !s_comment.check_without_moving(pos, expected))
+    {
+        if (!type_parser.parse(pos, type, expected))
+            return false;
+    }
+
+    Pos pos_before_specifier = pos;
+    if (s_default.ignore(pos, expected) || s_materialized.ignore(pos, expected) || s_alias.ignore(pos, expected))
     {
         default_specifier = Poco::toUpper(std::string{pos_before_specifier->begin, pos_before_specifier->end});
 
@@ -158,12 +154,17 @@ bool IParserColumnDeclaration<NameParser>::parseImpl(Pos & pos, ASTPtr & node, E
         if (!expr_parser.parse(pos, default_expression, expected))
             return false;
     }
-    else if (s_comment.ignore(pos, expected))
+
+    if (require_type && !type && !default_expression)
+        return false; /// reject column name without type
+
+
+    if (s_comment.ignore(pos, expected))
     {
-        string_literal_parser.parse(pos, comment_expression, expected);
+        /// should be followed by a string literal
+        if (!string_literal_parser.parse(pos, comment_expression, expected))
+            return false;
     }
-    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

@@ -81,6 +81,12 @@ std::optional<AlterCommand> AlterCommand::parse(const ASTAlterCommand * command_
             command.default_expression = ast_col_decl.default_expression;
         }
 
+        if (ast_col_decl.comment)
+        {
+            const auto & ast_comment = typeid_cast<ASTLiteral &>(*ast_col_decl.comment);
+            command.comment = ast_comment.value.get<String>();
+        }
+
         return command;
     }
     else if (command_ast->type == ASTAlterCommand::MODIFY_PRIMARY_KEY)
@@ -276,7 +282,7 @@ bool AlterCommand::is_mutable() const
         return false;
     if (type == MODIFY_COLUMN)
         return data_type.get() || default_expression;
-
+    // TODO: возможно, здесь нужно дополнить
     return true;
 }
 

dbms/src/Storages/AlterCommands.h

@@ -22,9 +22,10 @@ struct AlterCommand
         MODIFY_COLUMN,
         MODIFY_PRIMARY_KEY,
         COMMENT_COLUMN,
+        UKNOWN_TYPE,
     };
 
-    Type type;
+    Type type = UKNOWN_TYPE;
 
     String column_name;
 
@@ -47,7 +48,7 @@ struct AlterCommand
     AlterCommand() = default;
     AlterCommand(const Type type, const String & column_name, const DataTypePtr & data_type,
                  const ColumnDefaultKind default_kind, const ASTPtr & default_expression,
-                 const String & after_column = String{}, const String & comment = "")
+                 const String & after_column = String{}, const String & comment = "") // TODO: разобраться здесь с параметром по умолчанию
         : type{type}, column_name{column_name}, data_type{data_type}, default_kind{default_kind},
         default_expression{default_expression}, comment(comment), after_column{after_column}
     {}