[GitHub] [iceberg] RussellSpitzer commented on a change in pull request #2506: Ensure snapshot ID is positive

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

RussellSpitzer commented on a change in pull request #2506:
URL: https://github.com/apache/iceberg/pull/2506#discussion_r620391167



##########
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:
       There definitely are :), 
   
   There are at least 2 (same is with the ^ Long.MaxValue)
   We have 64 random bits ^ 64 Random bits and we put it into 63 bits (Remember the first bit is sign so we only have 63 bits of values after abs) BUT because of the overflow we get 1 extra value in our output domain, so we actually get 63 Bits (All positive longs) + 1 (Long.minValue)
   
   With  the 
   ^ & method
   We actually do the same thing
   64 random bits & 64 random bits ^ 63 bits (the first bit is 0 because Long.MaxValue is positive) so we still can only get 63 bit's worth of values in there. (No Extra 1 for long.min value)
   
   I think the only difference is that because of the 0 point you end up with 3 values which end up matching Long.maxValue: Long.maxValue, Long.minValue, -Long.maxValue
   
   Now again this basically isn't an issue any more than using a UUID is for repeated values since the chance of collision is really small so no big worry.




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