nastra commented on PR #7880: URL: https://github.com/apache/iceberg/pull/7880#issuecomment-1714020768
> @nastra, I opened [nastra#136](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`. Funny enough, that's exactly the version I had at the very beginning (can be seen in https://github.com/apache/iceberg/commit/582d3673f35dc04147fb959a30e371e1b5e375bd#diff-9c8b69f86aec86aea82cd6675c1c24267d9a9d77d7715c2c792934cb954b328b). There was some feedback from @jackye1995 in https://github.com/apache/iceberg/pull/7880#discussion_r1246848836, so I switched things up a bit and landed at the version we're currently seeing here. I'm ok either way and both approaches have their ups/downs. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
