eric-maynard commented on code in PR #1378:
URL: https://github.com/apache/polaris/pull/1378#discussion_r2048450757
##########
service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java:
##########
@@ -1328,6 +1383,21 @@ 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
+ // currentMetadata / currentMetadataLocation
+ if (updateMetadataOnCommit) {
+ try {
+ tableMetadataField.set(metadata, newLocation);
+ unsafe.putObject(metadata, changesFieldOffset, new
ArrayList<MetadataUpdate>());
Review Comment:
In general I agree that we we are hitting the limits of
`BaseMetastoreTableOperations`; I actually hit an issue similar to this on
[another open PR of
mine](https://github.com/apache/polaris/pull/433/files#diff-e060b07656c0ba07c3719293c7137a8a98bc2cc8ef801a2b7d8ecc5708d71944R1456).
I further agree that the use of reflection here is unfortunate and I hope that
we can fix this upstream and rip this logic out whether or not we do rip out
`BaseMetastoreTableOperations`.
However I think that rewriting this whole stack is a nontrivial task that'll
be pretty hard to test in our current setup. In this case the performance
benefit of an immediate fix is so dramatic that I feel it may outweigh the
hackiness.
--
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]
