[GitHub] [iceberg] RussellSpitzer commented on pull request #5200: Core: Fix ErrorProne Warnings

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

RussellSpitzer commented on PR #5200:
URL: https://github.com/apache/iceberg/pull/5200#issuecomment-1176226163

   > The only warning that is left is this one from
   > 
   > https://github.com/apache/iceberg/blob/2f8a09a8f81b27b8ba4025efaa328244ae0fc2a6/core/src/main/java/org/apache/iceberg/util/ZOrderByteUtils.java#L182-L183
   > 
   > 
   > which seems legit as we're doing a lossy cast that would fail compilation if I rewrite it (https://errorprone.info/bugpattern/NarrowingCompoundAssignment):
   > ```
   > > Task :iceberg-core:compileJava
   > /home/nastra/Development/workspace/iceberg/core/src/main/java/org/apache/iceberg/util/ZOrderByteUtils.java:182: warning: [NarrowingCompoundAssignment] Compound assignments from int to byte hide lossy casts
   >       interleavedBytes[interleaveByte] |=
   >                                        ^
   >     (see https://errorprone.info/bugpattern/NarrowingCompoundAssignment)
   >   Did you mean 'interleavedBytes[interleaveByte] = (byte) (interleavedBytes[interleaveByte] | (columnsBinary[sourceColumn][sourceByte] & 1 << sourceBit) >>> sourceBit << interleaveBit);'?
   > ```
   
   So this is as intended 
   
   The code is a bit complicated but a description would be
   ``` java
    interleavedBytes[interleaveByte] |=  //  if the target bit isn't set it
            (columnsBinary[sourceColumn][sourceByte] & 1 << sourceBit) >>> sourceBit  // Get the bit we are checking and move it all the way into the 1's place
   << interleaveBit;  // Move the bit back into our target position
   ```
   
   So 
   ```java
   (columnsBinary[sourceColumn][sourceByte] & 1 << sourceBit) 
   ```
   Can only produce values between 0 and 128 (00000000), (00000001), (00000010), .... (10000000)
   
   Then
   
   ```
   >>> sourceBit 
   ```
   Can only produce values between 0 and 1 
   
   then
   
   ```
   << interleaveBit
   ```
   
   Can only produce values again between 0 and 128
   
   Now if interleave bit or source bit were greater than 7 we would be in trouble but those are locked down
   


-- 
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-unsubscribe@iceberg.apache.org

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