dramaticlly commented on code in PR #13631:
URL: https://github.com/apache/iceberg/pull/13631#discussion_r2229139846
##########
core/src/main/java/org/apache/iceberg/BaseTransaction.java:
##########
@@ -121,180 +121,143 @@ public TableOperations underlyingOps() {
return ops;
}
- private void checkLastOperationCommitted(String operation) {
+ protected <T extends PendingUpdate> T appendUpdates(T update) {
+ if (!(update instanceof ManageSnapshots)) {
Review Comment:
thank you @pvary for the suggestion!
I did give it a try but running into some problems about our class hierarchy
and I find it's a bit hard to track down the problem compare to instanceof
check.
Let me share an concrete example for
([FastAppend](https://github.com/apache/iceberg/blob/main/core/src/main/java/org/apache/iceberg/FastAppend.java#L36))
, which extends SnapshotProducer and implements
[AppendFiles](https://github.com/apache/iceberg/blob/main/api/src/main/java/org/apache/iceberg/AppendFiles.java#L30)
(which itself is an interface that extends SnapshotUpdate)
Assume we have both both protected method defined here
```java
protected <T extends SnapshotProducer> T appendUpdates(T update) {
checkLastOperationCommitted(update.getClass().getSimpleName());
update.deleteWith(enqueueDelete);
update.reportWith(reporter);
updates.add(update);
return update;
}
protected <T extends SnapshotUpdate> T appendUpdates(T update) {
checkLastOperationCommitted(update.getClass().getSimpleName());
update.deleteWith(enqueueDelete);
updates.add(update);
return update;
}
```
and if our newFastAppend is defined as follow
```java
@Override
public AppendFiles newFastAppend() {
AppendFiles append = new FastAppend(tableName, transactionOps);
// for some reason, at static binding it will use the SnapshotUpdate
instead of SnapshotProducer
return appendUpdates(append);
}
```
so it will incorrectly not report with custom MetricsReporter and fail the
`testCatalogWithCustomMetricsReporter` in CatalogTests.java
--
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]
