pvary commented on PR #8907:
URL: https://github.com/apache/iceberg/pull/8907#issuecomment-1798279159
@nk1506: I have a few concerns here:
1. `listAllTables` - This configuration has been explicitly created to
prevent reading all of the actual table data for the given database
(namespace). If we keep this, then we should be sure that we do not need
specific table data (even by batches). The code for generating
`TableIdentifier`s for every table is very different than the filtered solution
- we might want to keep the code separated (different method, or a clearly
different code path inside of a single method). Also we can decide that we do
not want to support this feature, but that is a different discussion/PR etc.
2. If we see that there are code duplications
(HiveTableOperations/HiveViewOperations) then I prefer one of the following 2
methods:
- Do the refactor in a 1st PR, to extract the code which will be reused,
and in a 2nd PR add the new feature. It is fine to do the WIP PR to decide
which will be the common parts, but we should not commit code which is not
production ready.
- Do the refactor and the new feature in the same PR. It is viable only
for small refactors.
3. We should decide if we want to have the same error messages for the
`HiveCatalog` that are used by the other Catalogs. If we think so, then it
would be great if the new error messages (for the Views) should be the same.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org
