saintstack commented on a change in pull request #2776:
URL: https://github.com/apache/hbase/pull/2776#discussion_r546259482



##########
File path: 
hbase-common/src/main/java/org/apache/hadoop/hbase/CellComparatorImpl.java
##########
@@ -57,29 +57,287 @@ public final int compare(final Cell a, final Cell b) {
   }
 
   @Override
-  public int compare(final Cell a, final Cell b, boolean ignoreSequenceid) {
-
+  public int compare(final Cell l, final Cell r, boolean ignoreSequenceid) {
     int diff = 0;
     // "Peel off" the most common path.
-    if (a instanceof ByteBufferKeyValue && b instanceof ByteBufferKeyValue) {
-      diff = BBKVComparator.compare((ByteBufferKeyValue)a, 
(ByteBufferKeyValue)b, ignoreSequenceid);
+    if (l instanceof KeyValue && r instanceof KeyValue) {
+      diff = compareKeyValues((KeyValue) l, (KeyValue) r);
+      if (diff != 0) {
+        return diff;
+      }
+    } else if (l instanceof KeyValue && r instanceof ByteBufferKeyValue) {
+      diff = compareKVVsBBKV((KeyValue) l, (ByteBufferKeyValue) r);
+      if (diff != 0) {
+        return diff;
+      }
+    } else if (l instanceof ByteBufferKeyValue && r instanceof KeyValue) {
+      diff = compareKVVsBBKV((KeyValue) r, (ByteBufferKeyValue) l);
+      if (diff != 0) {
+        // negate- Findbugs will complain?
+        return -diff;
+      }
+      // if ignoreSequenceid then the diff will be 0.
+      return ignoreSequenceid ? diff : Long.compare(r.getSequenceId(), 
l.getSequenceId());
+    } else if (l instanceof ByteBufferKeyValue && r instanceof 
ByteBufferKeyValue) {
+      diff = compareBBKV((ByteBufferKeyValue) l, (ByteBufferKeyValue) r);
       if (diff != 0) {
         return diff;
       }
     } else {
-      diff = compareRows(a, b);
+      int leftRowLength = l.getRowLength();
+      int rightRowLength = r.getRowLength();
+      diff = compareRows(l, leftRowLength, r, rightRowLength);
       if (diff != 0) {
         return diff;
       }
 
-      diff = compareWithoutRow(a, b);
+      diff = compareWithoutRow(l, r);
       if (diff != 0) {
         return diff;
       }
     }
-
     // Negate following comparisons so later edits show up first mvccVersion: 
later sorts first
-    return ignoreSequenceid? diff: Long.compare(b.getSequenceId(), 
a.getSequenceId());
+    return ignoreSequenceid ? diff : Long.compare(r.getSequenceId(), 
l.getSequenceId());
+  }
+
+  private static int compareBBKV(final ByteBufferKeyValue left, final 
ByteBufferKeyValue right) {
+    int diff;
+    // Compare Rows. Cache row length.
+    int leftRowLength = left.getRowLength();
+    int rightRowLength = right.getRowLength();
+    diff = ByteBufferUtils.compareTo(left.getRowByteBuffer(), 
left.getRowPosition(),
+      leftRowLength, right.getRowByteBuffer(), right.getRowPosition(), 
rightRowLength);
+    if (diff != 0) {
+      return diff;
+    }
+
+    // If the column is not specified, the "minimum" key type appears as 
latest in the sorted
+    // order, regardless of the timestamp. This is used for specifying the 
last key/value in a
+    // given row, because there is no "lexicographically last column" (it 
would be infinitely
+    // long).
+    // The "maximum" key type does not need this behavior. Copied from 
KeyValue. This is bad in
+    // that
+    // we can't do memcmp w/ special rules like this.
+    // TODO: Is there a test for this behavior?
+    int leftFamilyLengthPosition = left.getFamilyLengthPosition(leftRowLength);
+    int leftFamilyLength = left.getFamilyLength(leftFamilyLengthPosition);
+    int leftKeyLength = left.getKeyLength();
+    int leftQualifierLength =
+        left.getQualifierLength(leftKeyLength, leftRowLength, 
leftFamilyLength);
+
+    // No need of left row length below here.
+
+    byte leftType = left.getTypeByte(leftKeyLength);
+    if (leftFamilyLength + leftQualifierLength == 0
+        && leftType == KeyValue.Type.Minimum.getCode()) {
+      // left is "bigger", i.e. it appears later in the sorted order
+      return 1;
+    }
+
+    int rightFamilyLengthPosition = 
right.getFamilyLengthPosition(rightRowLength);
+    int rightFamilyLength = right.getFamilyLength(rightFamilyLengthPosition);
+    int rightKeyLength = right.getKeyLength();
+    int rightQualifierLength =
+        right.getQualifierLength(rightKeyLength, rightRowLength, 
rightFamilyLength);
+
+    // No need of right row length below here.
+
+    byte rightType = right.getTypeByte(rightKeyLength);
+    if (rightFamilyLength + rightQualifierLength == 0
+        && rightType == KeyValue.Type.Minimum.getCode()) {
+      return -1;
+    }
+
+    // Compare families.
+    int leftFamilyPosition = left.getFamilyPosition(leftFamilyLengthPosition);
+    int rightFamilyPosition = 
right.getFamilyPosition(rightFamilyLengthPosition);
+    diff = ByteBufferUtils.compareTo(left.getFamilyByteBuffer(), 
leftFamilyPosition,
+      leftFamilyLength, right.getFamilyByteBuffer(), rightFamilyPosition, 
rightFamilyLength);
+    if (diff != 0) {
+      return diff;
+    }
+
+    // Compare qualifiers
+    diff = ByteBufferUtils.compareTo(left.getQualifierByteBuffer(),
+      left.getQualifierPosition(leftFamilyPosition, leftFamilyLength), 
leftQualifierLength,
+      right.getQualifierByteBuffer(),
+      right.getQualifierPosition(rightFamilyPosition, rightFamilyLength), 
rightQualifierLength);
+    if (diff != 0) {
+      return diff;
+    }
+
+    // Timestamps.
+    // Swap order we pass into compare so we get DESCENDING order.
+    diff = Long.compare(right.getTimestamp(rightKeyLength), 
left.getTimestamp(leftKeyLength));

Review comment:
       Nice idea. Wonder if we can do this elsewhere?




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


Reply via email to