-
Notifications
You must be signed in to change notification settings - Fork 286
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
list table names with snapshot #566
Conversation
for (String tableName : polarisStoreService.getTables(name.getDatabaseName(), tableFilter)) { | ||
for (String tableName : polarisStoreService.getTables(name.getDatabaseName(), | ||
tableFilter, | ||
1000) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could this constant in the code be made into a constant variable
final List<String> tableNames = polarisStoreService.getTables( | ||
name.getDatabaseName(), | ||
"", | ||
1000); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since names are smaller than the objects themselves, it might be more efficient to make the constant larger for names, maybe we can try 5k or 10k
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like this one was missed, can you update it to read from config as well
List<PolarisTableEntity> findAllTablesByDbNameAndTablePrefix( | ||
String dbName, String tableNamePrefix, int pageSize); | ||
List<?> findAllTablesByDbNameAndTablePrefix( | ||
String dbName, String tableNamePrefix, int pageSize, boolean selectAll); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The selectAll
name could be improved since the method name already has findAll
so it could seem like it might not return the whole list, this could be renamed instead to selectNames
or something similar
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for addressing the comments
Also makes list table names using the AS OF SYSTEM TIME semantics.