Deprecate PinotHelixResourceManager#getAllTables() in favour of getAllTables(String databaseName) #12782
Conversation
| _helixResourceManager.getDatabaseNames().stream() | ||
| .map(_helixResourceManager::getAllTables) | ||
| .flatMap(List::stream) | ||
| .forEach(tableNameWithType -> { |
There was a problem hiding this comment.
looped code is not changed at all.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #12782 +/- ##
============================================
+ Coverage 61.75% 61.99% +0.24%
+ Complexity 207 198 -9
============================================
Files 2436 2461 +25
Lines 133233 134737 +1504
Branches 20636 20818 +182
============================================
+ Hits 82274 83533 +1259
- Misses 44911 45055 +144
- Partials 6048 6149 +101
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
| if (existSchemaName == null || existSchemaName.equals(rawTableName)) { | ||
| // Although the table config is valid, we still need to ensure the schema exists | ||
| if (!schemaExists) { | ||
| LOGGER.warn("Failed to find schema for table: {}", tableNameWithType); |
There was a problem hiding this comment.
minor: change all the log lines to reflect the database name as well along with table name
There was a problem hiding this comment.
Ok, seems like all table names already contain the dbName so this can be ignored.
There was a problem hiding this comment.
Yeah that's correct.
|
This is not efficient, and there are other places not handled correctly. Please take a look at #12804 |
Description
PinotHelixResourceManager#getAllTables()in favour ofPinotHelixResourceManager#getAllTables(String databaseName)asgetAllTables()only works fordefaultdatabase.labels
release-notesrelease-notes
Deprecating
PinotHelixResourceManager#getAllTables()in favour ofPinotHelixResourceManager#getAllTables(String databaseName)