Improve catalog show tables query#94467
Conversation
|
Workflow [PR], commit [5d8e412] Summary: ❌
|
clickhouse-gh
bot
added
the
pr-improvement
There was a problem hiding this comment.
Pull request overview
This PR improves the performance of SHOW TABLES queries by optimizing how table metadata is fetched. Instead of loading full table objects, the code now returns lightweight table details (just table names) when only basic information is needed.
Changes:
- Modified
getLightweightTablesIterator()to return a vector ofLightWeightTableDetailsstructs instead of a full iterator with table objects - Added a new
showTablesQuery()method inTablesBlockSourcethat uses the lightweight iterator for queries that only need table names - Replaced all
getLightweightTablesIterator()calls withgetTablesIterator()except in the optimizedSHOW TABLESpath
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Databases/IDatabase.h | Changed getLightweightTablesIterator() signature to return a vector of lightweight table details instead of an iterator |
| src/Databases/DataLake/DatabaseDataLake.h | Updated method signature to match the new interface |
| src/Databases/DataLake/DatabaseDataLake.cpp | Simplified implementation to return only table names without loading full table objects or thread pool operations |
| src/Storages/System/StorageSystemTables.cpp | Added showTablesQuery() optimization and removed database-specific logic from needTable() |
| src/Storages/System/StorageSystemIcebergHistory.cpp | Changed to use getTablesIterator() since full table objects are needed |
| src/Storages/System/StorageSystemCompletions.cpp | Changed to use getTablesIterator() since full table objects are needed |
| src/Storages/System/StorageSystemColumns.cpp | Changed to use getTablesIterator() since full table objects are needed |
| src/Interpreters/InterpreterSystemQuery.cpp | Changed to use getTablesIterator() since full table objects are needed |
|
|
||
| if (!tables_it || !tables_it->isValid()) | ||
| tables_it = database->getLightweightTablesIterator(context, | ||
| tables_it = database->getTablesIterator(context, |
There was a problem hiding this comment.
So now if any exception is thrown from catalog it will be thrown here. Let's move try/catch logic into getTablesIterator as well.
There was a problem hiding this comment.
I think you mean for DatabaseDataLake, then yes try catch added in getTablesIterator
alesapin
left a comment
alesapin
left a comment
There was a problem hiding this comment.
We need to have a test which will show the perf improvement. Please introduce a fail point into tryGetTableImpl which will artificially slow down it (add sleep). Write an integration test with any catalog and check that SHOW TABLES is not affected by this sleep.
robot-clickhouse-ci-2
added
the
pr-synced-to-cloud
…how_tables Improve catalog show tables query
Changelog category (leave one):
Changelog entry (a user-readable short description of the changes that goes into CHANGELOG.md):
Improved processing 'show tables' query by fetching only names of tables and improved getLightweightTablesIterator to return structure containing only table names. resolves #93835
Documentation entry for user-facing changes