Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix wrong data path in DatabaseMemory #8753

Merged
merged 2 commits into from
Jan 21, 2020

Conversation

tavplubix
Copy link
Member

@tavplubix tavplubix commented Jan 20, 2020

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

Changelog category (leave one):

  • Bug Fix

Changelog entry (up to few sentences, required except for Non-significant/Documentation categories):
In 20.1.1 Memory databases use empty data path, so tables are created in path directory (e.g. /var/lib/clickhouse/), not in data directory of database (e.g. /var/lib/clickhouse/db_name).

@tavplubix tavplubix added the pr-bugfix Pull request with bugfix, not backported by default label Jan 20, 2020
@@ -132,6 +132,12 @@ StoragePtr StorageFactory::get(
}
}

if (relative_data_path.empty())
{
if (endsWith(name, "MergeTree") || endsWith(name, "Log") || name == "Join" || name == "Set")
Copy link
Member

Choose a reason for hiding this comment

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

If it means "all table engines that store data on disk", I would add Distributed,
but better to just remove that if,
because if relative path is empty, it is logical error anyway?

Copy link
Member Author

Choose a reason for hiding this comment

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

Distributed and File can be created with empty path

Copy link
Member

Choose a reason for hiding this comment

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

Let's add a virtual method to IStorage?

Copy link
Member

Choose a reason for hiding this comment

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

Or, better - check for nonempty path in storages constructor?

Copy link
Member

@alexey-milovidov alexey-milovidov left a comment

Choose a reason for hiding this comment

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

Ok and one question remains.

@alesapin alesapin merged commit bef1eae into master Jan 21, 2020
tavplubix pushed a commit that referenced this pull request Jan 21, 2020
…ta_path

Fix wrong data path in DatabaseMemory
@tavplubix tavplubix deleted the fix_database_memory_wrong_data_path branch March 2, 2020 15:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr-bugfix Pull request with bugfix, not backported by default
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants