openinx commented on a change in pull request #2506:
URL: https://github.com/apache/iceberg/pull/2506#discussion_r620794698
##########
File path: core/src/main/java/org/apache/iceberg/TableOperations.java
##########
@@ -117,7 +117,7 @@ default long newSnapshotId() {
UUID uuid = UUID.randomUUID();
long mostSignificantBits = uuid.getMostSignificantBits();
long leastSignificantBits = uuid.getLeastSignificantBits();
- return Math.abs(mostSignificantBits ^ leastSignificantBits);
+ return (mostSignificantBits ^ leastSignificantBits) & Long.MAX_VALUE;
Review comment:
Okay, after read the original PR
https://github.com/apache/iceberg/pull/57, although there's small probability
to produce the same snapshot id , it won't be able to commit the iceberg table
metadata because we will encounter a non-recoverable IllegalArgumentException
when building the [immutable `snapshotsById` map
](https://github.com/apache/iceberg/blob/90225d6c9413016d611e2ce5eff37db1bc1b4fc5/core/src/main/java/org/apache/iceberg/TableMetadata.java#L888).
So this should not be a bug.
( I also did a small test to validate the conflicts in `ImmutableMap`:
```java
public static void main(String[] args) {
ImmutableMap.Builder<Long, String> builder = ImmutableMap.builder();
builder.put(1L, "hello");
builder.put(1L, "world");
Map<Long, String> map = builder.build();
System.out.println(map);
}
```
It will throw a `IllegalArgumentException`:
```
Exception in thread "main" java.lang.IllegalArgumentException: Multiple
entries with same key: 1=world and 1=hello
at
org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap.conflictException(ImmutableMap.java:214)
at
org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap.checkNoConflict(ImmutableMap.java:208)
at
org.apache.iceberg.relocated.com.google.common.collect.RegularImmutableMap.checkNoConflictInKeyBucket(RegularImmutableMap.java:146)
at
org.apache.iceberg.relocated.com.google.common.collect.RegularImmutableMap.fromEntryArray(RegularImmutableMap.java:109)
at
org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap$Builder.build(ImmutableMap.java:392)
at org.apache.iceberg.TestImmutableMap.main(TestImmutableMap.java:33)
```
)
--
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:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
