Skip to content

Allow Atomic database inside MaterializeMySQL#14849

Merged
tavplubix merged 18 commits intomasterfrom
allow_atomic_database_inside_materialize_mysql
Dec 14, 2020
Merged

Allow Atomic database inside MaterializeMySQL#14849
tavplubix merged 18 commits intomasterfrom
allow_atomic_database_inside_materialize_mysql

Conversation

@tavplubix
Copy link
Copy Markdown
Member

I hereby agree to the terms of the CLA available at: https://yandex.ru/legal/cla/?lang=en

Changelog category (leave one):

  • Improvement

Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):
Allow using Atomic engine for nested database of MaterializeMySQL engine.

cc: @zhang2014 @bohutang

@robot-clickhouse robot-clickhouse added the pr-improvement Pull request with some product improvements label Sep 15, 2020
Copy link
Copy Markdown
Contributor

@abyss7 abyss7 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please, merge with master.

else if (query.kind == ASTDropQuery::Kind::Detach)
{
if (auto * atomic = typeid_cast<DatabaseAtomic *>(database.get()))
if (auto * atomic = dynamic_cast<DatabaseAtomic *>(database.get()))
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In other places it's a check for UUID to detect Atomic DB. Why dynamic_cast here?

{
if (!MaterializeMySQLSyncThread::isMySQLSyncThread())
db_and_table.second = std::make_shared<StorageMaterializeMySQL>(std::move(db_and_table.second), db_and_table.first.get());
return db_and_table;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Extra return ...

@tavplubix tavplubix merged commit dd2ae69 into master Dec 14, 2020
@tavplubix tavplubix deleted the allow_atomic_database_inside_materialize_mysql branch December 14, 2020 13:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no-docs-needed pr-improvement Pull request with some product improvements

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants