pvary commented on code in PR #13631:
URL: https://github.com/apache/iceberg/pull/13631#discussion_r2233840802
##########
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:
So we solved the technical riddle 😄
Let's go back to the original question:
- This solution seems fishy to me. We are mixing requirements for different
updates in a single method. This makes a bit hard to understand how we handle
them, because the code is scattered.
Wouldn't be better to allow wrapping/switching of the PendingUpdate? IIUC
that was the original requirement.
Maybe something like this:
```
protected <T extends PendingUpdate> <T> overrideUpdate(T original) {
return original;
}
```
And after the update, the method calls could remain update specific, and
they are automatically applied to the new update as well (notice the call to
the `overrideUpdate` method)
```
@Override
public RowDelta newRowDelta() {
checkLastOperationCommitted("RowDelta");
RowDelta delta = overrideUpdate(
new BaseRowDelta(tableName, transactionOps).reportWith(reporter));
delta.deleteWith(enqueueDelete);
updates.add(delta);
return delta;
}
```
Your override code could look like this:
```
@Override
protected <T extends PendingUpdate> <T> overrideUpdate(T original) {
return upfate(original);
}
private RowDelta update(RowDelta original) {
return new CustomRowDelta(tableName(), ((HasTableOperations)
table()).operations());
}
private PendingUpdate update(PendingUpdate original) {
return original;
}
```
This last one, I'm not sure. The generics code might cause the polymorphism
to misbehave again :smile:. In this case, the customization code might have to
resort to `instanceOf` checks, but that's only in custom code.
--
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]
