rdblue commented on PR #7880: URL: https://github.com/apache/iceberg/pull/7880#issuecomment-1705735777
@nastra, I opened https://github.com/nastra/iceberg/pull/136 with an alternative idea. The implementation added to `BaseMetastoreCatalog` here seems strange to me because it has so many implementations that throw `UnsupportedOperationException`. I don't think that adding `ViewCatalog` to that base class is a good idea because it implements so little of the interface -- I'd rather use a util class of helpers or some alternative. That PR is one idea. I don't think I like it either, but it at least demonstrates one way to accomplish the same goal. It also shows to how fix tests without needing `CatalogWithViews`. I think that the problem this was trying to solve is to provide a base implementation to catalogs already inheriting from `BaseMetastoreCatalog`. In that case, I think the cleanest solution is to add a subclass of `BaseMetastoreCatalog` that adds the shared view methods. That can be an abstract class that can leave out all of the unsupported methods. Then to use the new base class, you'd just switch from `extends BaseMetastoreCatalog` to `extends `BaseMetastoreViewCatalog`. -- 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
