eric-maynard commented on code in PR #1378:
URL: https://github.com/apache/polaris/pull/1378#discussion_r2051404974
##########
service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java:
##########
@@ -1328,6 +1344,25 @@ public void doCommit(TableMetadata base, TableMetadata
metadata) {
String newLocation = writeNewMetadataIfRequired(base == null, metadata);
String oldLocation = base == null ? null : base.metadataFileLocation();
+ // TODO: we should not need to do this hack, but there's no other way to
modify
+ // metadataFileLocation / currentMetadataLocation
+ if (updateMetadataOnCommit) {
+ try {
+ Field tableMetadataField =
TableMetadata.class.getDeclaredField("metadataFileLocation");
+ tableMetadataField.setAccessible(true);
+ tableMetadataField.set(metadata, newLocation);
Review Comment:
For some reason `IcebergCatalogHandler` is written to be semi-agnostic of
the underlying `Catalog` type and the code does not assume it's an
`IcebergCatalog` -- you can see this all over the place, where we do things
like:
```
this.viewCatalog = (baseCatalog instanceof ViewCatalog) ? (ViewCatalog)
baseCatalog : null;
```
We could add a case here where we check, similarly, if the underlying
catalog is an `IcebergCatalog` and then we can add a method for register/commit
here which goes through our own route. But as you note we'd have to write our
own `Table`, and very quickly this starts to look like the refactor/rewrite
discussed elsewhere. tbh I'm happy to take that work on, but I wanted to see if
we are OK with this smaller patch in the interim.
--
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...@polaris.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
