From 3d7c1db763a6b25f4f2ec954851080f0c3df066f Mon Sep 17 00:00:00 2001 From: Pablo Marcos Date: Sun, 31 Mar 2024 12:56:44 +0200 Subject: [PATCH] Copy comment when using CREATE AS statement --- src/Backups/DDLAdjustingForBackupVisitor.cpp | 2 +- src/Databases/DatabaseFilesystem.cpp | 2 +- src/Databases/DatabaseMemory.cpp | 2 +- src/Databases/DatabaseOnDisk.cpp | 5 ++--- src/Databases/DatabasesCommon.cpp | 4 ++-- src/Interpreters/InterpreterCreateQuery.cpp | 3 +++ src/Interpreters/SystemLog.cpp | 2 +- src/Parsers/ASTCreateQuery.cpp | 2 +- src/Parsers/ASTCreateQuery.h | 2 +- src/Parsers/ParserCreateQuery.cpp | 12 ++++++------ .../03033_create_as_copies_comment.reference | 3 +++ .../0_stateless/03033_create_as_copies_comment.sql | 11 +++++++++++ 12 files changed, 33 insertions(+), 17 deletions(-) create mode 100644 tests/queries/0_stateless/03033_create_as_copies_comment.reference create mode 100644 tests/queries/0_stateless/03033_create_as_copies_comment.sql diff --git a/src/Backups/DDLAdjustingForBackupVisitor.cpp b/src/Backups/DDLAdjustingForBackupVisitor.cpp index 5ea91094b75..89c24ac17b7 100644 --- a/src/Backups/DDLAdjustingForBackupVisitor.cpp +++ b/src/Backups/DDLAdjustingForBackupVisitor.cpp @@ -20,7 +20,7 @@ namespace /// If this is a definition of a system table we'll remove columns and comment because they're redundant for backups. auto & create = data.create_query->as(); create.reset(create.columns_list); - create.reset(create.comment); + create.comment.reset(); } void visitStorageReplicatedTableEngine(ASTStorage & storage, const DDLAdjustingForBackupVisitor::Data & data) diff --git a/src/Databases/DatabaseFilesystem.cpp b/src/Databases/DatabaseFilesystem.cpp index 05af0acf978..5d12c442700 100644 --- a/src/Databases/DatabaseFilesystem.cpp +++ b/src/Databases/DatabaseFilesystem.cpp @@ -192,7 +192,7 @@ ASTPtr DatabaseFilesystem::getCreateDatabaseQuery() const if (const auto database_comment = getDatabaseComment(); !database_comment.empty()) { auto & ast_create_query = ast->as(); - ast_create_query.set(ast_create_query.comment, std::make_shared(database_comment)); + ast_create_query.comment = std::make_shared(database_comment); } return ast; diff --git a/src/Databases/DatabaseMemory.cpp b/src/Databases/DatabaseMemory.cpp index 4ff7b3c7f2b..5b4e0fc3daf 100644 --- a/src/Databases/DatabaseMemory.cpp +++ b/src/Databases/DatabaseMemory.cpp @@ -107,7 +107,7 @@ ASTPtr DatabaseMemory::getCreateDatabaseQuery() const create_query->storage->set(create_query->storage->engine, engine); if (const auto comment_value = getDatabaseComment(); !comment_value.empty()) - create_query->set(create_query->comment, std::make_shared(comment_value)); + create_query->comment = std::make_shared(comment_value); return create_query; } diff --git a/src/Databases/DatabaseOnDisk.cpp b/src/Databases/DatabaseOnDisk.cpp index 550f1a756cb..953143f6c1c 100644 --- a/src/Databases/DatabaseOnDisk.cpp +++ b/src/Databases/DatabaseOnDisk.cpp @@ -535,7 +535,7 @@ ASTPtr DatabaseOnDisk::getCreateDatabaseQuery() const if (const auto database_comment = getDatabaseComment(); !database_comment.empty()) { auto & ast_create_query = ast->as(); - ast_create_query.set(ast_create_query.comment, std::make_shared(database_comment)); + ast_create_query.comment = std::make_shared(database_comment); } return ast; @@ -784,8 +784,7 @@ ASTPtr DatabaseOnDisk::getCreateQueryFromStorage(const String & table_name, cons static_cast(settings.max_parser_backtracks), throw_on_error); - create_table_query->set(create_table_query->as()->comment, - std::make_shared("SYSTEM TABLE is built on the fly.")); + create_table_query->as()->comment = std::make_shared("SYSTEM TABLE is built on the fly."); return create_table_query; } diff --git a/src/Databases/DatabasesCommon.cpp b/src/Databases/DatabasesCommon.cpp index 4dffb16e486..924b785d8b8 100644 --- a/src/Databases/DatabasesCommon.cpp +++ b/src/Databases/DatabasesCommon.cpp @@ -114,9 +114,9 @@ void applyMetadataChangesToCreateQuery(const ASTPtr & query, const StorageInMemo } if (metadata.comment.empty()) - ast_create_query.reset(ast_create_query.comment); + ast_create_query.comment.reset(); else - ast_create_query.set(ast_create_query.comment, std::make_shared(metadata.comment)); + ast_create_query.comment = std::make_shared(metadata.comment); } diff --git a/src/Interpreters/InterpreterCreateQuery.cpp b/src/Interpreters/InterpreterCreateQuery.cpp index 7c3bed7388c..75208a501ca 100644 --- a/src/Interpreters/InterpreterCreateQuery.cpp +++ b/src/Interpreters/InterpreterCreateQuery.cpp @@ -785,6 +785,9 @@ InterpreterCreateQuery::TableProperties InterpreterCreateQuery::getTableProperti auto as_storage_metadata = as_storage->getInMemoryMetadataPtr(); properties.columns = as_storage_metadata->getColumns(); + if (!create.comment) + create.comment = std::make_shared(Field(as_storage_metadata->comment)); + /// Secondary indices and projections make sense only for MergeTree family of storage engines. /// We should not copy them for other storages. if (create.storage && endsWith(create.storage->engine->name, "MergeTree")) diff --git a/src/Interpreters/SystemLog.cpp b/src/Interpreters/SystemLog.cpp index db73fe038c0..b0b18924010 100644 --- a/src/Interpreters/SystemLog.cpp +++ b/src/Interpreters/SystemLog.cpp @@ -656,7 +656,7 @@ ASTPtr SystemLog::getCreateTableQuery() StorageWithComment & storage_with_comment = storage_with_comment_ast->as(); create->set(create->storage, storage_with_comment.storage); - create->set(create->comment, storage_with_comment.comment); + create->comment = storage_with_comment.comment; /// Write additional (default) settings for MergeTree engine to make it make it possible to compare ASTs /// and recreate tables on settings changes. diff --git a/src/Parsers/ASTCreateQuery.cpp b/src/Parsers/ASTCreateQuery.cpp index 0403dc33164..b977c441de0 100644 --- a/src/Parsers/ASTCreateQuery.cpp +++ b/src/Parsers/ASTCreateQuery.cpp @@ -255,7 +255,7 @@ ASTPtr ASTCreateQuery::clone() const if (as_table_function) res->set(res->as_table_function, as_table_function->clone()); if (comment) - res->set(res->comment, comment->clone()); + res->comment = comment->clone(); cloneOutputOptions(*res); cloneTableOptions(*res); diff --git a/src/Parsers/ASTCreateQuery.h b/src/Parsers/ASTCreateQuery.h index 64e6bc8ce48..40e5fda300d 100644 --- a/src/Parsers/ASTCreateQuery.h +++ b/src/Parsers/ASTCreateQuery.h @@ -112,7 +112,7 @@ public: String as_table; IAST * as_table_function = nullptr; ASTSelectWithUnionQuery * select = nullptr; - IAST * comment = nullptr; + ASTPtr comment = nullptr; ASTPtr sql_security = nullptr; ASTTableOverrideList * table_overrides = nullptr; /// For CREATE DATABASE with engines that automatically create tables diff --git a/src/Parsers/ParserCreateQuery.cpp b/src/Parsers/ParserCreateQuery.cpp index f2c09e9b050..ab8bfcfbff9 100644 --- a/src/Parsers/ParserCreateQuery.cpp +++ b/src/Parsers/ParserCreateQuery.cpp @@ -861,7 +861,7 @@ bool ParserCreateTableQuery::parseImpl(Pos & pos, ASTPtr & node, Expected & expe query->set(query->as_table_function, as_table_function); if (comment) - query->set(query->comment, comment); + query->comment = comment; if (query->columns_list && query->columns_list->primary_key) { @@ -1012,8 +1012,7 @@ bool ParserCreateLiveViewQuery::parseImpl(Pos & pos, ASTPtr & node, Expected & e query->set(query->select, select); if (comment) - query->set(query->comment, comment); - + query->comment = comment; if (sql_security) query->sql_security = typeid_cast>(sql_security); @@ -1414,7 +1413,8 @@ bool ParserCreateDatabaseQuery::parseImpl(Pos & pos, ASTPtr & node, Expected & e query->set(query->storage, storage); if (comment) - query->set(query->comment, comment); + query->comment = comment; + if (table_overrides && !table_overrides->children.empty()) query->set(query->table_overrides, table_overrides); @@ -1617,7 +1617,7 @@ bool ParserCreateViewQuery::parseImpl(Pos & pos, ASTPtr & node, Expected & expec if (refresh_strategy) query->set(query->refresh_strategy, refresh_strategy); if (comment) - query->set(query->comment, comment); + query->comment = comment; if (sql_security) query->sql_security = typeid_cast>(sql_security); @@ -1794,7 +1794,7 @@ bool ParserCreateDictionaryQuery::parseImpl(IParser::Pos & pos, ASTPtr & node, E query->cluster = cluster_str; if (comment) - query->set(query->comment, comment); + query->comment = comment; return true; } diff --git a/tests/queries/0_stateless/03033_create_as_copies_comment.reference b/tests/queries/0_stateless/03033_create_as_copies_comment.reference new file mode 100644 index 00000000000..98efe03b172 --- /dev/null +++ b/tests/queries/0_stateless/03033_create_as_copies_comment.reference @@ -0,0 +1,3 @@ +original comment +original comment +new comment diff --git a/tests/queries/0_stateless/03033_create_as_copies_comment.sql b/tests/queries/0_stateless/03033_create_as_copies_comment.sql new file mode 100644 index 00000000000..0583b6ed130 --- /dev/null +++ b/tests/queries/0_stateless/03033_create_as_copies_comment.sql @@ -0,0 +1,11 @@ +DROP TABLE IF EXISTS base; +DROP TABLE IF EXISTS copy_without_comment; +DROP TABLE IF EXISTS copy_with_comment; + +CREATE TABLE base (a Int32) ENGINE = MergeTree ORDER BY a COMMENT 'original comment'; +CREATE TABLE copy_without_comment as base; +CREATE TABLE copy_with_comment as base COMMENT 'new comment'; + +SELECT comment FROM system.tables WHERE name = 'base'; +SELECT comment FROM system.tables WHERE name = 'copy_without_comment'; +SELECT comment FROM system.tables WHERE name = 'copy_with_comment'; \ No newline at end of file