Refactoring: moved comment out of ASTStorage to ASTCreateQuery

src/Databases/DatabaseFactory.cpp

@@ -79,8 +79,8 @@ DatabasePtr DatabaseFactory::get(const ASTCreateQuery & create, const String & m
             context->getQueryContext()->addQueryFactoriesInfo(Context::QueryLogFactories::Database, impl->getEngineName());
 
         // Attach database metadata
-        if (impl && create.storage && create.storage->comment)
+        if (impl && create.comment)
-            impl->setDatabaseComment(create.storage->comment->as<ASTLiteral>()->value.safeGet<String>());
+            impl->setDatabaseComment(create.comment->as<ASTLiteral>()->value.safeGet<String>());
 
         return impl;
     }

src/Databases/DatabaseMemory.cpp

@@ -71,7 +71,7 @@ ASTPtr DatabaseMemory::getCreateDatabaseQuery() const
     create_query->storage->set(create_query->storage->engine, makeASTFunction(getEngineName()));
 
     if (const auto comment_value = getDatabaseComment(); !comment_value.empty())
-        create_query->storage->set(create_query->storage->comment, std::make_shared<ASTLiteral>(comment_value));
+        create_query->set(create_query->comment, std::make_shared<ASTLiteral>(comment_value));
 
     return create_query;
 }

src/Databases/DatabaseOnDisk.cpp

@@ -188,12 +188,12 @@ void applyMetadataChangesToCreateQuery(const ASTPtr & query, const StorageInMemo
             if (metadata.settings_changes)
                 storage_ast.set(storage_ast.settings, metadata.settings_changes);
         }
+    }
 
     if (metadata.comment.empty())
-            storage_ast.reset(storage_ast.comment);
+        ast_create_query.reset(ast_create_query.comment);
     else
-            storage_ast.set(storage_ast.comment, std::make_shared<ASTLiteral>(metadata.comment));
+        ast_create_query.set(ast_create_query.comment, std::make_shared<ASTLiteral>(metadata.comment));
-    }
 }
 
 
@@ -531,11 +531,7 @@ ASTPtr DatabaseOnDisk::getCreateDatabaseQuery() const
     if (const auto database_comment = getDatabaseComment(); !database_comment.empty())
     {
         auto & ast_create_query = ast->as<ASTCreateQuery &>();
-        // TODO(nemkov): this is a precaution and should never happen, remove if there are no failed tests on CI/CD.
+        ast_create_query.set(ast_create_query.comment, std::make_shared<ASTLiteral>(database_comment));
-        if (!ast_create_query.storage)
-            throw Exception(ErrorCodes::LOGICAL_ERROR, "ASTCreateQuery lacks engine clause, but a comment is present.");
-
-        ast_create_query.storage->set(ast_create_query.storage->comment, std::make_shared<ASTLiteral>(database_comment));
     }
 
     return ast;

src/Databases/MySQL/DatabaseMySQL.cpp

@@ -198,7 +198,7 @@ ASTPtr DatabaseMySQL::getCreateDatabaseQuery() const
     create_query->set(create_query->storage, database_engine_define);
 
     if (const auto comment_value = getDatabaseComment(); !comment_value.empty())
-        create_query->storage->set(create_query->storage->comment, std::make_shared<ASTLiteral>(comment_value));
+        create_query->set(create_query->comment, std::make_shared<ASTLiteral>(comment_value));
 
     return create_query;
 }

src/Databases/PostgreSQL/DatabasePostgreSQL.cpp

@@ -349,7 +349,7 @@ ASTPtr DatabasePostgreSQL::getCreateDatabaseQuery() const
     create_query->set(create_query->storage, database_engine_define);
 
     if (const auto comment_value = getDatabaseComment(); !comment_value.empty())
-        create_query->storage->set(create_query->storage->comment, std::make_shared<ASTLiteral>(comment_value));
+        create_query->set(create_query->comment, std::make_shared<ASTLiteral>(comment_value));
 
     return create_query;
 }

src/Databases/SQLite/DatabaseSQLite.cpp

@@ -164,7 +164,7 @@ ASTPtr DatabaseSQLite::getCreateDatabaseQuery() const
     create_query->set(create_query->storage, database_engine_define);
 
     if (const auto comment_value = getDatabaseComment(); !comment_value.empty())
-        create_query->storage->set(create_query->storage->comment, std::make_shared<ASTLiteral>(comment_value));
+        create_query->set(create_query->comment, std::make_shared<ASTLiteral>(comment_value));
 
     return create_query;
 }

src/Parsers/ASTCreateQuery.cpp

@@ -32,9 +32,6 @@ ASTPtr ASTStorage::clone() const
     if (settings)
         res->set(res->settings, settings->clone());
 
-    if (comment)
-        res->set(res->comment, comment->clone());
-
     return res;
 }
 
@@ -75,12 +72,6 @@ void ASTStorage::formatImpl(const FormatSettings & s, FormatState & state, Forma
         s.ostr << (s.hilite ? hilite_keyword : "") << s.nl_or_ws << "SETTINGS " << (s.hilite ? hilite_none : "");
         settings->formatImpl(s, state, frame);
     }
-    if (comment)
-    {
-        s.ostr << (s.hilite ? hilite_keyword : "") << s.nl_or_ws << "COMMENT " << (s.hilite ? hilite_none : "");
-        comment->formatImpl(s, state, frame);
-    }
-
 }
 
 
@@ -217,6 +208,9 @@ ASTPtr ASTCreateQuery::clone() const
         res->set(res->dictionary, dictionary->clone());
     }
 
+    if (comment)
+        res->set(res->comment, comment->clone());
+
     cloneOutputOptions(*res);
 
     return res;
@@ -245,6 +239,12 @@ void ASTCreateQuery::formatQueryImpl(const FormatSettings & settings, FormatStat
         if (storage)
             storage->formatImpl(settings, state, frame);
 
+        if (comment)
+        {
+            settings.ostr << (settings.hilite ? hilite_keyword : "") << settings.nl_or_ws << "COMMENT " << (settings.hilite ? hilite_none : "");
+            comment->formatImpl(settings, state, frame);
+        }
+
         return;
     }
 
@@ -405,6 +405,12 @@ void ASTCreateQuery::formatQueryImpl(const FormatSettings & settings, FormatStat
         settings.ostr << (settings.hilite ? hilite_keyword : "") << " WITH " << (settings.hilite ? hilite_none : "");
         tables->formatImpl(settings, state, frame);
     }
+
+    if (comment)
+    {
+        settings.ostr << (settings.hilite ? hilite_keyword : "") << settings.nl_or_ws << "COMMENT " << (settings.hilite ? hilite_none : "");
+        comment->formatImpl(settings, state, frame);
+    }
 }
 
 }

src/Parsers/ASTCreateQuery.h

@@ -23,7 +23,6 @@ public:
     IAST * order_by = nullptr;
     IAST * sample_by = nullptr;
     IAST * ttl_table = nullptr;
-    IAST * comment = nullptr;
     ASTSetQuery * settings = nullptr;
 
 
@@ -75,6 +74,7 @@ public:
     String as_table;
     ASTPtr as_table_function;
     ASTSelectWithUnionQuery * select = nullptr;
+    IAST * comment = nullptr;
 
     bool is_dictionary{false}; /// CREATE DICTIONARY
     ASTExpressionList * dictionary_attributes_list = nullptr; /// attributes of

src/Parsers/ParserCreateQuery.cpp

@@ -26,6 +26,20 @@ namespace ErrorCodes
     extern const int BAD_ARGUMENTS;
 }
 
+namespace
+{
+ASTPtr parseComment(IParser::Pos & pos, Expected & expected)
+{
+    ParserKeyword s_comment("COMMENT");
+    ParserStringLiteral string_literal_parser;
+    ASTPtr comment;
+
+    s_comment.ignore(pos, expected) && string_literal_parser.parse(pos, comment, expected);
+
+    return comment;
+}
+}
+
 bool ParserNestedTable::parseImpl(Pos & pos, ASTPtr & node, Expected & expected)
 {
     ParserToken open(TokenType::OpeningRoundBracket);
@@ -314,7 +328,6 @@ bool ParserStorage::parseImpl(Pos & pos, ASTPtr & node, Expected & expected)
     ParserKeyword s_sample_by("SAMPLE BY");
     ParserKeyword s_ttl("TTL");
     ParserKeyword s_settings("SETTINGS");
-    ParserKeyword s_comment("COMMENT");
 
     ParserIdentifierWithOptionalParameters ident_with_optional_params_p;
     ParserExpression expression_p;
@@ -329,7 +342,6 @@ bool ParserStorage::parseImpl(Pos & pos, ASTPtr & node, Expected & expected)
     ASTPtr sample_by;
     ASTPtr ttl_table;
     ASTPtr settings;
-    ASTPtr comment_expression;
 
     if (!s_engine.ignore(pos, expected))
         return false;
@@ -387,13 +399,6 @@ bool ParserStorage::parseImpl(Pos & pos, ASTPtr & node, Expected & expected)
                 return false;
         }
 
-        if (s_comment.ignore(pos, expected))
-        {
-            /// should be followed by a string literal
-            if (!string_literal_parser.parse(pos, comment_expression, expected))
-                return false;
-        }
-
         break;
     }
 
@@ -407,8 +412,6 @@ bool ParserStorage::parseImpl(Pos & pos, ASTPtr & node, Expected & expected)
 
     storage->set(storage->settings, settings);
 
-    storage->set(storage->comment, comment_expression);
-
     node = storage;
     return true;
 }
@@ -568,7 +571,7 @@ bool ParserCreateTableQuery::parseImpl(Pos & pos, ASTPtr & node, Expected & expe
             }
         }
     }
-
+    auto comment = parseComment(pos, expected);
 
     auto query = std::make_shared<ASTCreateQuery>();
     node = query;
@@ -590,6 +593,9 @@ bool ParserCreateTableQuery::parseImpl(Pos & pos, ASTPtr & node, Expected & expe
     query->set(query->columns_list, columns_list);
     query->set(query->storage, storage);
 
+    if (comment)
+        query->set(query->comment, comment);
+
     if (query->storage && query->columns_list && query->columns_list->primary_key)
     {
         if (query->storage->primary_key)
@@ -803,7 +809,7 @@ bool ParserCreateDatabaseQuery::parseImpl(Pos & pos, ASTPtr & node, Expected & e
     }
 
     storage_p.parse(pos, storage, expected);
-
+    auto comment = parseComment(pos, expected);
 
     auto query = std::make_shared<ASTCreateQuery>();
     node = query;
@@ -816,6 +822,8 @@ bool ParserCreateDatabaseQuery::parseImpl(Pos & pos, ASTPtr & node, Expected & e
     query->cluster = cluster_str;
 
     query->set(query->storage, storage);
+    if (comment)
+        query->set(query->comment, comment);
 
     return true;
 }
@@ -934,6 +942,7 @@ bool ParserCreateViewQuery::parseImpl(Pos & pos, ASTPtr & node, Expected & expec
     if (!select_p.parse(pos, select, expected))
         return false;
 
+    auto comment = parseComment(pos, expected);
 
     auto query = std::make_shared<ASTCreateQuery>();
     node = query;
@@ -958,6 +967,8 @@ bool ParserCreateViewQuery::parseImpl(Pos & pos, ASTPtr & node, Expected & expec
 
     query->set(query->columns_list, columns_list);
     query->set(query->storage, storage);
+    if (comment)
+        query->set(query->comment, comment);
 
     tryGetIdentifierNameInto(as_database, query->as_database);
     tryGetIdentifierNameInto(as_table, query->as_table);
@@ -1039,6 +1050,8 @@ bool ParserCreateDictionaryQuery::parseImpl(IParser::Pos & pos, ASTPtr & node, E
             return false;
     }
 
+    auto comment = parseComment(pos, expected);
+
     auto query = std::make_shared<ASTCreateQuery>();
     node = query;
     query->is_dictionary = true;
@@ -1056,6 +1069,9 @@ bool ParserCreateDictionaryQuery::parseImpl(IParser::Pos & pos, ASTPtr & node, E
     query->set(query->dictionary, dictionary);
     query->cluster = cluster_str;
 
+    if (comment)
+        query->set(query->comment, comment);
+
     return true;
 }
 

src/Storages/StorageFactory.cpp

@@ -146,8 +146,8 @@ StoragePtr StorageFactory::get(
                     throw Exception("Unknown table engine " + name, ErrorCodes::UNKNOWN_STORAGE);
             }
 
-            if (storage_def->comment)
+            if (query.comment)
-                comment = storage_def->comment->as<ASTLiteral &>().value.get<String>();
+                comment = query.comment->as<ASTLiteral &>().value.get<String>();
 
             auto check_feature = [&](String feature_description, FeatureMatcherFn feature_matcher_fn)
             {