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

Avoid superfluous dictionaries load #10164

Merged

Conversation

azat
Copy link
Collaborator

@azat azat commented Apr 9, 2020

Changelog category (leave one):

  • Improvement

Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):
Avoid superfluous dictionaries load (system.tables, DROP/SHOW CREATE TABLE)

Detailed description / Documentation draft:
This patch avoids loading dictionaries for:

  • SELECT * FROM system.tables (used by clickhouse-client for completion)
  • SHOW CREATE TABLE some_dict
  • DROP TABLE (user of DatabaseDictionary::tryGetTable()), and it is
    funny looks like the dictionary will be loaded before DROP

Refs: #7916

@blinkov blinkov added the pr-not-for-changelog This PR should not be mentioned in the changelog label Apr 9, 2020
@alexey-milovidov alexey-milovidov added pr-improvement Pull request with some product improvements and removed pr-not-for-changelog This PR should not be mentioned in the changelog labels Apr 9, 2020
@alexey-milovidov
Copy link
Member

This is not non-significant.

@blinkov blinkov added the pr-not-for-changelog This PR should not be mentioned in the changelog label Apr 9, 2020
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.

LGTM but needs review from @vitlibar

@azat azat force-pushed the system.tables-superfluous-dict-reload branch from 5aa3f81 to 79cfcb5 Compare April 10, 2020 07:38
@azat azat force-pushed the system.tables-superfluous-dict-reload branch from 79cfcb5 to 20e997e Compare April 10, 2020 07:38
@azat azat force-pushed the system.tables-superfluous-dict-reload branch from 20e997e to 5b1f40d Compare April 10, 2020 17:56
This patch avoids loading dictionaries for:
- SELECT * FROM system.tables (used by clickhouse-client for completion)
- SHOW CREATE TABLE some_dict

But the dictionary will still be loaded on:
- SHOW CREATE TABLE some_dict (from the database with Dictionary engine)
@azat azat force-pushed the system.tables-superfluous-dict-reload branch from 7619121 to 55a143d Compare April 11, 2020 10:27
@alexey-milovidov alexey-milovidov removed the pr-not-for-changelog This PR should not be mentioned in the changelog label Apr 11, 2020
@alexey-milovidov alexey-milovidov merged commit a4969d8 into ClickHouse:master Apr 11, 2020
@azat azat deleted the system.tables-superfluous-dict-reload branch April 11, 2020 20:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr-improvement Pull request with some product improvements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants