Github user srowen commented on a diff in the pull request:

    https://github.com/apache/spark/pull/19180#discussion_r138454402
  
    --- Diff: 
common/unsafe/src/main/java/org/apache/spark/unsafe/types/UTF8String.java ---
    @@ -1097,8 +1101,21 @@ public UTF8String copy() {
       @Override
       public int compareTo(@Nonnull final UTF8String other) {
         int len = Math.min(numBytes, other.numBytes);
    -    // TODO: compare 8 bytes as unsigned long
    -    for (int i = 0; i < len; i ++) {
    +    int words = len / Longs.BYTES;
    +    long roffset = other.getBaseOffset();
    +    Object rbase = other.getBaseObject();
    +    for (int i = 0; i < words * Longs.BYTES; i += Longs.BYTES) {
    --- End diff --
    
    I don't object to `Longs.BYTES` so much, but it's not used elsewhere. I 
think just writing "8" is clear and consistent.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

Reply via email to