From 9a4ea44d1ee46f6d0883dd10f173810bc6685c7d Mon Sep 17 00:00:00 2001 From: tedyu Date: Fri, 1 May 2015 18:31:16 -0700 Subject: [PATCH] HBASE-12413 Mismatch in the equals and hashcode methods of KeyValue (Jingcheng Du and Gariel Reid) Conflicts: hbase-common/src/main/java/org/apache/hadoop/hbase/CellComparator.java hbase-common/src/test/java/org/apache/hadoop/hbase/TestKeyValue.java --- .../apache/hadoop/hbase/CellComparator.java | 6 ++---- .../org/apache/hadoop/hbase/KeyValue.java | 11 ++++------ .../org/apache/hadoop/hbase/TestKeyValue.java | 20 +++++++++++++++++++ 3 files changed, 26 insertions(+), 11 deletions(-) diff --git a/hbase-common/src/main/java/org/apache/hadoop/hbase/CellComparator.java b/hbase-common/src/main/java/org/apache/hadoop/hbase/CellComparator.java index 46c0eafa6f4..2c1cc090a92 100644 --- a/hbase-common/src/main/java/org/apache/hadoop/hbase/CellComparator.java +++ b/hbase-common/src/main/java/org/apache/hadoop/hbase/CellComparator.java @@ -244,7 +244,6 @@ public class CellComparator implements Comparator, Serializable { /** * Returns a hash code that is always the same for two Cells having a matching equals(..) result. - * Currently does not guard against nulls, but it could if necessary. */ public static int hashCode(Cell cell){ if (cell == null) {// return 0 for empty Cell @@ -258,9 +257,8 @@ public class CellComparator implements Comparator, Serializable { /** * Returns a hash code that is always the same for two Cells having a matching - * equals(..) result. Currently does not guard against nulls, but it could if - * necessary. Note : Ignore mvcc while calculating the hashcode - * + * equals(..) result. Note : Ignore mvcc while calculating the hashcode + * * @param cell * @return hashCode */ 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 de66b890b37..bae5fb480a3 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 @@ -1123,15 +1123,12 @@ public class KeyValue implements Cell, HeapSize, Cloneable, SettableSequenceId, return CellComparator.equals(this, (Cell)other); } + /** + * In line with {@link #equals(Object)}, only uses the key portion, not the value. + */ @Override public int hashCode() { - byte[] b = getBuffer(); - int start = getOffset(), end = getOffset() + getLength(); - int h = b[start++]; - for (int i = start; i < end; i++) { - h = (h * 13) ^ b[i]; - } - return h; + return CellComparator.hashCodeIgnoreMvcc(this); } //--------------------------------------------------------------------------- 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 cd1a77432aa..3baf729dedd 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 @@ -39,6 +39,8 @@ import org.apache.hadoop.hbase.KeyValue.MetaComparator; import org.apache.hadoop.hbase.KeyValue.Type; import org.apache.hadoop.hbase.util.Bytes; +import static org.junit.Assert.assertNotEquals; + public class TestKeyValue extends TestCase { private static final Log LOG = LogFactory.getLog(TestKeyValue.class); @@ -829,4 +831,22 @@ public class TestKeyValue extends TestCase { return this.kv.getTagsArray(); } } + + public void testEqualsAndHashCode() throws Exception { + KeyValue kvA1 = new KeyValue(Bytes.toBytes("key"), Bytes.toBytes("cf"), + Bytes.toBytes("qualA"), Bytes.toBytes("1")); + KeyValue kvA2 = new KeyValue(Bytes.toBytes("key"), Bytes.toBytes("cf"), + Bytes.toBytes("qualA"), Bytes.toBytes("2")); + // We set a different sequence id on kvA2 to demonstrate that the equals and hashCode also + // don't take this into account. + kvA2.setSequenceId(2); + KeyValue kvB = new KeyValue(Bytes.toBytes("key"), Bytes.toBytes("cf"), + Bytes.toBytes("qualB"), Bytes.toBytes("1")); + + assertEquals(kvA1, kvA2); + assertNotEquals(kvA1, kvB); + assertEquals(kvA1.hashCode(), kvA2.hashCode()); + assertNotEquals(kvA1.hashCode(), kvB.hashCode()); + } + }