Make comments for system tables also available in local mode and enforce comments not to be empty (#59493)

Co-authored-by: Alexey Milovidov <milovidov@clickhouse.com>
This commit is contained in:
Nikita Mikhaylov 2024-02-05 11:17:58 +01:00 committed by GitHub
parent e64466c676
commit 6ba35b5459
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 31 additions and 16 deletions

View File

@ -211,16 +211,17 @@ std::shared_ptr<TSystemLog> createSystemLog(
if (!settings.empty())
log_settings.engine += (storage_policy.empty() ? " " : ", ") + settings;
}
/// Add comment to AST. So it will be saved when the table will be renamed.
log_settings.engine += fmt::format(" COMMENT {} ", quoteString(comment));
}
/// Validate engine definition syntax to prevent some configuration errors.
ParserStorageWithComment storage_parser;
parseQuery(storage_parser, log_settings.engine.data(), log_settings.engine.data() + log_settings.engine.size(),
auto storage_ast = parseQuery(storage_parser, log_settings.engine.data(), log_settings.engine.data() + log_settings.engine.size(),
"Storage to create table for " + config_prefix, 0, DBMS_DEFAULT_MAX_PARSER_DEPTH);
auto & storage_with_comment = storage_ast->as<StorageWithComment &>();
/// Add comment to AST. So it will be saved when the table will be renamed.
if (!storage_with_comment.comment || storage_with_comment.comment->as<ASTLiteral &>().value.safeGet<String>().empty())
log_settings.engine += fmt::format(" COMMENT {} ", quoteString(comment));
log_settings.queue_settings.flush_interval_milliseconds = config.getUInt64(config_prefix + ".flush_interval_milliseconds",
TSystemLog::getDefaultFlushIntervalMilliseconds());

View File

@ -7,14 +7,20 @@
namespace DB
{
template<typename StorageT, typename... StorageArgs>
void attach(ContextPtr context, IDatabase & system_database, const String & table_name, const String & comment, StorageArgs && ... args)
template <int Length>
using StringLiteral = const char(&)[Length];
template<typename StorageT, int CommentSize, typename... StorageArgs>
void attach(ContextPtr context, IDatabase & system_database, const String & table_name, StringLiteral<CommentSize> comment, StorageArgs && ... args)
{
static_assert(CommentSize > 15, "The comment for a system table is too short or empty");
assert(system_database.getDatabaseName() == DatabaseCatalog::SYSTEM_DATABASE);
auto table_id = StorageID::createEmpty();
if (system_database.getUUID() == UUIDHelpers::Nil)
{
/// Attach to Ordinary database.
auto table_id = StorageID(DatabaseCatalog::SYSTEM_DATABASE, table_name);
table_id = StorageID(DatabaseCatalog::SYSTEM_DATABASE, table_name);
system_database.attachTable(context, table_name, std::make_shared<StorageT>(table_id, std::forward<StorageArgs>(args)...));
}
else
@ -22,18 +28,18 @@ void attach(ContextPtr context, IDatabase & system_database, const String & tabl
/// Attach to Atomic database.
/// NOTE: UUIDs are not persistent, but it's ok since no data are stored on disk for these storages
/// and path is actually not used
auto table_id = StorageID(DatabaseCatalog::SYSTEM_DATABASE, table_name, UUIDHelpers::generateV4());
table_id = StorageID(DatabaseCatalog::SYSTEM_DATABASE, table_name, UUIDHelpers::generateV4());
DatabaseCatalog::instance().addUUIDMapping(table_id.uuid);
String path = "store/" + DatabaseCatalog::getPathForUUID(table_id.uuid);
system_database.attachTable(context, table_name, std::make_shared<StorageT>(table_id, std::forward<StorageArgs>(args)...), path);
/// Set the comment
auto table = DatabaseCatalog::instance().getTable(table_id, context);
assert(table);
auto metadata = table->getInMemoryMetadata();
metadata.comment = comment;
table->setInMemoryMetadata(metadata);
}
/// Set the comment
auto table = DatabaseCatalog::instance().getTable(table_id, context);
assert(table);
auto metadata = table->getInMemoryMetadata();
metadata.comment = comment;
table->setInMemoryMetadata(metadata);
}
}

View File

@ -0,0 +1,8 @@
#!/usr/bin/env bash
CUR_DIR=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)
# shellcheck source=../shell_config.sh
. "$CUR_DIR"/../shell_config.sh
${CLICKHOUSE_LOCAL} --query "SELECT 'Table ' || database || '.' || name || ' doesnt have a comment' FROM system.tables WHERE name NOT LIKE '%\_log\_%' AND database='system' AND comment==''"
${CLICKHOUSE_CLIENT} --query "SELECT 'Table ' || database || '.' || name || ' doesnt have a comment' FROM system.tables WHERE name NOT LIKE '%\_log\_%' AND database='system' AND comment==''"