[GitHub] [iceberg] Fokko commented on a change in pull request #2011: Add equals and hashcode to BaseSnapshot

[GitBox <gi...@apache.org>]

Fokko commented on a change in pull request #2011:
URL: https://github.com/apache/iceberg/pull/2011#discussion_r550764983



##########
File path: core/src/main/java/org/apache/iceberg/BaseSnapshot.java
##########
@@ -211,6 +211,32 @@ private void cacheChanges() {
     this.cachedDeletes = deletes.build();
   }
 
+  @Override
+  public boolean equals(Object o) {
+    if (this == o) {
+      return true;
+    }
+    if (o instanceof BaseSnapshot) {
+      BaseSnapshot other = (BaseSnapshot) o;
+      return this.snapshotId == other.snapshotId() &&
+          this.parentId.equals(other.parentId()) &&
+          this.sequenceNumber == other.sequenceNumber() &&
+          this.timestampMillis == other.timestampMillis();
+
+    }
+    return false;

Review comment:
       Thanks for letting me know, I've updated the style.

##########
File path: core/src/main/java/org/apache/iceberg/BaseSnapshot.java
##########
@@ -211,6 +211,32 @@ private void cacheChanges() {
     this.cachedDeletes = deletes.build();
   }
 
+  @Override
+  public boolean equals(Object o) {
+    if (this == o) {
+      return true;
+    }
+    if (o instanceof BaseSnapshot) {
+      BaseSnapshot other = (BaseSnapshot) o;
+      return this.snapshotId == other.snapshotId() &&
+          this.parentId.equals(other.parentId()) &&
+          this.sequenceNumber == other.sequenceNumber() &&
+          this.timestampMillis == other.timestampMillis();
+
+    }
+    return false;
+  }
+
+  @Override
+  public int hashCode() {
+    int hash = 7;
+    hash = 31 * hash + (int) this.snapshotId;
+    hash = 31 * hash + (this.parentId == null ? 0 : this.parentId.hashCode());
+    hash = 31 * hash + (int) sequenceNumber;
+    hash = 31 * hash + (int) timestampMillis;
+    return hash;

Review comment:
       Sure!

##########
File path: core/src/main/java/org/apache/iceberg/BaseSnapshot.java
##########
@@ -211,6 +211,32 @@ private void cacheChanges() {
     this.cachedDeletes = deletes.build();
   }
 
+  @Override
+  public boolean equals(Object o) {
+    if (this == o) {
+      return true;
+    }
+    if (o instanceof BaseSnapshot) {
+      BaseSnapshot other = (BaseSnapshot) o;
+      return this.snapshotId == other.snapshotId() &&

Review comment:
       I had the same thought. For the Beam use-case, this is sufficient. If you want to split out a single stream into multiple tables, then you would do a key-by on the destination table. The comparisons will be made within the table, therefore the sequenceNumber will make this unique. I'm happy to also add the table that was loaded, but this isn't in the class right now.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org