liurenjie1024 commented on issue #488: URL: https://github.com/apache/iceberg-rust/issues/488#issuecomment-2256011677
> Thanks for starting this discussion first. > > My reason for not including a `default_table_root_location` (or `default_warehouse_location`) at the catalog implementation level is that it might confuse our users about the actual location they are using. > > My initial idea was to move `root` to the `FileIO` level for greater consistency and clarity. However, as @liurenjie1024 and @fqaiser94 pointed out, this property does not exist. > > So I visit the pyiceberg's logic and found that it has a `_get_default_warehouse_location`: > > https://github.com/apache/iceberg-python/blob/be5c42649914e71e8366c22558f8234ce062b145/pyiceberg/catalog/__init__.py#L885-L895 > > ```python > def _get_default_warehouse_location(self, database_name: str, table_name: str) -> str: > database_properties = self.load_namespace_properties(database_name) > if database_location := database_properties.get(LOCATION): > database_location = database_location.rstrip("/") > return f"{database_location}/{table_name}" > > if warehouse_path := self.properties.get(WAREHOUSE_LOCATION): > warehouse_path = warehouse_path.rstrip("/") > return f"{warehouse_path}/{database_name}.db/{table_name}" > > raise ValueError("No default path is set, please specify a location when creating a table") > ``` > > I believe it makes more sense for us to share the implementation of the default warehouse location logic. > > * load database location if `database_properties` has `LOCATION`, and use `{database_location}/{table_name}` > * Otherwise, try `WAREHOUSE_LOCATION` and use `{warehouse_path}/{database_name}.db/{table_name}` > * And finally returns error. > > Maybe we can align with this behavior? Sounds reasonable to me. A more complete method is here: https://github.com/apache/iceberg-python/blob/055938d36d46efb94849b1d861cd8a8a111f6ae5/pyiceberg/catalog/__init__.py#L880 1. Use user passed table location first. 2. Fallback to database location. 3. Fallback to warehouse location, the default_warehouse config could be treated as warehouse location property. -- 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
