puchengy commented on issue #4583: URL: https://github.com/apache/iceberg/issues/4583#issuecomment-1219036911
@rdblue Hi I want to share some findings after trying to figure out the way to support this based on today's Iceberg community sync https://docs.google.com/document/d/1YuGhUdukLP5gGiqCbk0A5_Wifqe2CZWgOd3TbhY3UQg/edit# So to recap, we agreed that: - we want to deprecate `metadataFileLocation` method in `TableOperations` interface - we want to provide a new method in LocationProvider interface (maybe call `newMetadataLocation`) I did some investigations and it seems doesn't work because the implementations of `metadataFileLocation` function in catalogs' table operation are different, for example: - `BaseMetastoreTableOperations` & `RestTableOpeartions` use custom metadata location if set, otherwise fall back to `table_location/metadata/filename`. - However, (1) [`HadoopTableOperations`](https://github.com/apache/iceberg/blob/master/core/src/main/java/org/apache/iceberg/hadoop/HadoopTableOperations.java#L186) use its own way to define metadata location; (2) [`StaticTableOperations`](https://github.com/apache/iceberg/blob/master/core/src/main/java/org/apache/iceberg/StaticTableOperations.java#L78) on the other hand will throw exceptions. - Because of above, it seems not straightforward to me to add `newMetadataLocation` function in `LocationProvider` interface and deprecate `metadataFileLocation` function. Because we either need to change `HadoopTableOperations` and `StaticTableOperations` metadata location behavior or we need to somehow let the LocationProvider to be aware of different type of table operations so that we can return the location providers properly. Given the way we discussed doesn't seem working to me, I have this alternative option: add `newMetadataLocation` function in the `LocationProvider` interface and let it throw unimplemented exception by default. Then we still keep the `metadataFileLocation` function, so that we can first try to run `newMetadataLocation` and catch unimplemented exception to execute fallback logic. Could you share your thoughts if you have better way to achieve what we discussed this morning? or how do you think about my alternative solution, thanks. -- 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]
