[GitHub] [hbase] saintstack commented on a change in pull request #3496: HBASE-24734 Wrong comparator opening Region when 'split-to-WAL' enabled.

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

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



##########
File path: hbase-common/src/main/java/org/apache/hadoop/hbase/CellComparatorImpl.java
##########
@@ -677,6 +677,18 @@ public int compareRows(Cell left, byte[] right, int roffset, int rlength) {
         roffset, rlength);
   }
 
+  /**
+   * Compares two row bytes
+   * @param leftRow the byte array of the left row
+   * @param rightRow the byte array of the right row
+   * @return greater than 0 if leftRow is bigger, less than 0 if rightRow is bigger, 0 if both
+   *         rows are equal
+   */
+  @Override
+  public int compareRows(byte[] leftRow, byte[] rightRow) {

Review comment:
       This could be the default impl.

##########
File path: hbase-common/src/main/java/org/apache/hadoop/hbase/CellComparator.java
##########
@@ -82,6 +82,15 @@ static CellComparator getInstance() {
    */
   int compareRows(Cell cell, byte[] bytes, int offset, int length);
 
+  /**
+   * Compares two row bytes
+   * @param leftRow the byte array of the left row
+   * @param rightRow the byte array of the right row
+   * @return greater than 0 if leftRow is bigger, less than 0 if rightRow is bigger, 0 if both
+   *         rows are equal
+   */
+  int compareRows(byte[] leftRow, byte[] rightRow);

Review comment:
       I think you should provide a default implementation to be safe compatibility-wise. See how the   ```default int compareRows(ByteBuffer row, Cell cell) {``` does it in this class.




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org