From 3e82a84ca8eaed3ecee9b7d1cf110921e6e8f02a Mon Sep 17 00:00:00 2001 From: stack Date: Mon, 21 Jul 2014 14:19:42 -0700 Subject: [PATCH] HBASE-11541 Wrong result when scaning meta with startRow (Liu Shaoqui) --- .../org/apache/hadoop/hbase/KeyValue.java | 53 +++++++--------- .../org/apache/hadoop/hbase/TestKeyValue.java | 62 ++++++++++++++----- 2 files changed, 69 insertions(+), 46 deletions(-) diff --git a/hbase-common/src/main/java/org/apache/hadoop/hbase/KeyValue.java b/hbase-common/src/main/java/org/apache/hadoop/hbase/KeyValue.java index a9c06c2a428..b9aa2f39aa2 100644 --- a/hbase-common/src/main/java/org/apache/hadoop/hbase/KeyValue.java +++ b/hbase-common/src/main/java/org/apache/hadoop/hbase/KeyValue.java @@ -1663,20 +1663,6 @@ public class KeyValue implements Cell, HeapSize, Cloneable { return Bytes.add(family, COLUMN_FAMILY_DELIM_ARRAY, qualifier); } - /** - * This function is only used in Meta key comparisons so its error message - * is specific for meta key errors. - */ - static int getRequiredDelimiterInReverse(final byte [] b, - final int offset, final int length, final int delimiter) { - int index = getDelimiterInReverse(b, offset, length, delimiter); - if (index < 0) { - throw new IllegalArgumentException("hbase:meta key must have two '" + (char)delimiter + "' " - + "delimiters and have the following format: ',,'"); - } - return index; - } - /** * @param b * @param delimiter @@ -1749,35 +1735,44 @@ public class KeyValue implements Cell, HeapSize, Cloneable { HConstants.DELIMITER); int rightDelimiter = getDelimiter(right, roffset, rlength, HConstants.DELIMITER); - if (leftDelimiter < 0 && rightDelimiter >= 0) { - // Nothing between hbase:meta and regionid. Its first key. - return -1; - } else if (rightDelimiter < 0 && leftDelimiter >= 0) { - return 1; - } else if (leftDelimiter < 0 && rightDelimiter < 0) { - return 0; - } // Compare up to the delimiter - int result = Bytes.compareTo(left, loffset, leftDelimiter - loffset, - right, roffset, rightDelimiter - roffset); + int lpart = (leftDelimiter < 0 ? llength :leftDelimiter - loffset); + int rpart = (rightDelimiter < 0 ? rlength :rightDelimiter - roffset); + int result = Bytes.compareTo(left, loffset, lpart, right, roffset, rpart); if (result != 0) { return result; + } else { + if (leftDelimiter < 0 && rightDelimiter >= 0) { + return -1; + } else if (rightDelimiter < 0 && leftDelimiter >= 0) { + return 1; + } else if (leftDelimiter < 0 && rightDelimiter < 0) { + return 0; + } } // Compare middle bit of the row. // Move past delimiter leftDelimiter++; rightDelimiter++; - int leftFarDelimiter = getRequiredDelimiterInReverse(left, leftDelimiter, + int leftFarDelimiter = getDelimiterInReverse(left, leftDelimiter, llength - (leftDelimiter - loffset), HConstants.DELIMITER); - int rightFarDelimiter = getRequiredDelimiterInReverse(right, + int rightFarDelimiter = getDelimiterInReverse(right, rightDelimiter, rlength - (rightDelimiter - roffset), HConstants.DELIMITER); // Now compare middlesection of row. - result = super.compareRows(left, leftDelimiter, - leftFarDelimiter - leftDelimiter, right, rightDelimiter, - rightFarDelimiter - rightDelimiter); + lpart = (leftFarDelimiter < 0 ? llength + loffset: leftFarDelimiter) - leftDelimiter; + rpart = (rightFarDelimiter < 0 ? rlength + roffset: rightFarDelimiter)- rightDelimiter; + result = super.compareRows(left, leftDelimiter, lpart, right, rightDelimiter, rpart); if (result != 0) { return result; + } else { + if (leftDelimiter < 0 && rightDelimiter >= 0) { + return -1; + } else if (rightDelimiter < 0 && leftDelimiter >= 0) { + return 1; + } else if (leftDelimiter < 0 && rightDelimiter < 0) { + return 0; + } } // Compare last part of row, the rowid. leftFarDelimiter++; diff --git a/hbase-common/src/test/java/org/apache/hadoop/hbase/TestKeyValue.java b/hbase-common/src/test/java/org/apache/hadoop/hbase/TestKeyValue.java index b9c82e2d6d9..467c7fe0247 100644 --- a/hbase-common/src/test/java/org/apache/hadoop/hbase/TestKeyValue.java +++ b/hbase-common/src/test/java/org/apache/hadoop/hbase/TestKeyValue.java @@ -150,23 +150,6 @@ public class TestKeyValue extends TestCase { metacomparisons(new KeyValue.MetaComparator()); } - public void testBadMetaCompareSingleDelim() { - MetaComparator c = new KeyValue.MetaComparator(); - long now = System.currentTimeMillis(); - // meta keys values are not quite right. A users can enter illegal values - // from shell when scanning meta. - KeyValue a = new KeyValue(Bytes.toBytes("table,a1"), now); - KeyValue b = new KeyValue(Bytes.toBytes("table,a2"), now); - try { - c.compare(a, b); - } catch (IllegalArgumentException iae) { - assertEquals("hbase:meta key must have two ',' delimiters and have the following" + - " format: '
,,'", iae.getMessage()); - return; - } - fail("Expected IllegalArgumentException"); - } - public void testMetaComparatorTableKeysWithCommaOk() { MetaComparator c = new KeyValue.MetaComparator(); long now = System.currentTimeMillis(); @@ -589,4 +572,49 @@ public class TestKeyValue extends TestCase { Bytes.equals(next.getValue(), metaValue2); assertFalse(tagItr.hasNext()); } + + public void testMetaKeyComparator() { + MetaComparator c = new KeyValue.MetaComparator(); + long now = System.currentTimeMillis(); + + KeyValue a = new KeyValue(Bytes.toBytes("table1"), now); + KeyValue b = new KeyValue(Bytes.toBytes("table2"), now); + assertTrue(c.compare(a, b) < 0); + + a = new KeyValue(Bytes.toBytes("table1,111"), now); + b = new KeyValue(Bytes.toBytes("table2"), now); + assertTrue(c.compare(a, b) < 0); + + a = new KeyValue(Bytes.toBytes("table1"), now); + b = new KeyValue(Bytes.toBytes("table2,111"), now); + assertTrue(c.compare(a, b) < 0); + + a = new KeyValue(Bytes.toBytes("table,111"), now); + b = new KeyValue(Bytes.toBytes("table,2222"), now); + assertTrue(c.compare(a, b) < 0); + + a = new KeyValue(Bytes.toBytes("table,111,aaaa"), now); + b = new KeyValue(Bytes.toBytes("table,2222"), now); + assertTrue(c.compare(a, b) < 0); + + a = new KeyValue(Bytes.toBytes("table,111"), now); + b = new KeyValue(Bytes.toBytes("table,2222.bbb"), now); + assertTrue(c.compare(a, b) < 0); + + a = new KeyValue(Bytes.toBytes("table,,aaaa"), now); + b = new KeyValue(Bytes.toBytes("table,111,bbb"), now); + assertTrue(c.compare(a, b) < 0); + + a = new KeyValue(Bytes.toBytes("table,111,aaaa"), now); + b = new KeyValue(Bytes.toBytes("table,111,bbb"), now); + assertTrue(c.compare(a, b) < 0); + + a = new KeyValue(Bytes.toBytes("table,111,xxxx"), now); + b = new KeyValue(Bytes.toBytes("table,111,222,bbb"), now); + assertTrue(c.compare(a, b) < 0); + + a = new KeyValue(Bytes.toBytes("table,111,11,xxx"), now); + b = new KeyValue(Bytes.toBytes("table,111,222,bbb"), now); + assertTrue(c.compare(a, b) < 0); + } }