snazy commented on code in PR #6789:
URL: https://github.com/apache/iceberg/pull/6789#discussion_r1138293161
##########
nessie/src/main/java/org/apache/iceberg/nessie/NessieIcebergClient.java:
##########
@@ -307,12 +320,18 @@ namespace, getRef().getName()),
public void renameTable(TableIdentifier from, TableIdentifier to) {
getRef().checkMutable();
+ try {
+ getRef().refresh(api);
+ } catch (NessieNotFoundException e) {
+ throw new RuntimeException(e);
+ }
- IcebergTable existingFromTable = table(from);
+ UpdateableReference updateableReference = getRef();
+ IcebergTable existingFromTable = table(from,
updateableReference.getReference());
Review Comment:
What is the point of allowing an explicit hash here and below? Shouldn't it
fail, if an explicit hash is provided?
##########
nessie/src/main/java/org/apache/iceberg/nessie/UpdateableReference.java:
##########
@@ -60,13 +60,6 @@ public String getHash() {
return reference.getHash();
}
- public Branch getAsBranch() {
Review Comment:
> I deprecated it instead of removing it because it is a public method.
This method a safe cast - you replaced it (not clear for which reason) with
an unconditional cast (and BTW left another then unused method).
##########
nessie/src/main/java/org/apache/iceberg/nessie/NessieIcebergClient.java:
##########
@@ -491,6 +519,15 @@ private boolean isSnapshotOperation(TableMetadata base,
TableMetadata metadata)
|| snapshot.snapshotId() != base.currentSnapshot().snapshotId());
}
+ private void addReference(OnReferenceBuilder<?> builder) {
Review Comment:
Call sites would be be simpler, if this would return the properly typed
builder back.
##########
nessie/src/main/java/org/apache/iceberg/nessie/NessieIcebergClient.java:
##########
@@ -374,8 +393,15 @@ public void renameTable(TableIdentifier from,
TableIdentifier to) {
public boolean dropTable(TableIdentifier identifier, boolean purge) {
getRef().checkMutable();
+ try {
+ getRef().refresh(api);
+ } catch (NessieNotFoundException e) {
+ throw new RuntimeException(e);
+ }
+
+ UpdateableReference updateableReference = getRef();
- IcebergTable existingTable = table(identifier);
+ IcebergTable existingTable = table(identifier,
updateableReference.getReference());
Review Comment:
Same comments as for renameTable()
##########
nessie/src/main/java/org/apache/iceberg/nessie/NessieIcebergClient.java:
##########
@@ -165,10 +166,10 @@ private TableIdentifier
toIdentifier(EntriesResponse.Entry entry) {
return TableIdentifier.of(elements.toArray(new String[elements.size()]));
}
- public IcebergTable table(TableIdentifier tableIdentifier) {
+ public IcebergTable table(TableIdentifier tableIdentifier, Reference ref) {
Review Comment:
What's the real world scenario then wrt user interaction?
--
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
