diff --git ql/src/java/org/apache/hadoop/hive/ql/io/RecordIdentifier.java ql/src/java/org/apache/hadoop/hive/ql/io/RecordIdentifier.java index cdde3dc..6c660c8 100644 --- ql/src/java/org/apache/hadoop/hive/ql/io/RecordIdentifier.java +++ ql/src/java/org/apache/hadoop/hive/ql/io/RecordIdentifier.java @@ -31,6 +31,7 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.List; +import java.util.Objects; /** * Gives the Record identifier information for the current record. @@ -43,7 +44,7 @@ * Each field of RecordIdentifier which should be part of ROWID should be in this enum... which * really means that it should be part of VirtualColumn (so make a subclass for rowid). */ - public static enum Field { + public enum Field { //note the enum names match field names in the struct transactionId(TypeInfoFactory.longTypeInfo, PrimitiveObjectInspectorFactory.javaLongObjectInspector), @@ -197,7 +198,10 @@ public void readFields(DataInput dataInput) throws IOException { @Override public boolean equals(Object other) { - if (other == null || other.getClass() != getClass()) { + if(other == this) { + return true; + } + if (other.getClass() != getClass()) { return false; } RecordIdentifier oth = (RecordIdentifier) other; @@ -205,6 +209,14 @@ public boolean equals(Object other) { oth.bucketId == bucketId && oth.rowId == rowId; } + @Override + public int hashCode() { + int result = 17; + result = 31 * result + (int)(transactionId ^ (transactionId >>> 32)); + result = 31 * result + bucketId; + result = 31 * result + (int)(rowId ^ (rowId >>> 32)); + return result; + } @Override public String toString() { diff --git ql/src/java/org/apache/hadoop/hive/ql/io/orc/OrcRawRecordMerger.java ql/src/java/org/apache/hadoop/hive/ql/io/orc/OrcRawRecordMerger.java index dd53afa..95b8806 100644 --- ql/src/java/org/apache/hadoop/hive/ql/io/orc/OrcRawRecordMerger.java +++ ql/src/java/org/apache/hadoop/hive/ql/io/orc/OrcRawRecordMerger.java @@ -74,7 +74,8 @@ * reader is collapsing events to just the last update, just the first * instance of each record is required. */ - final static class ReaderKey extends RecordIdentifier{ + @VisibleForTesting + public final static class ReaderKey extends RecordIdentifier{ private long currentTransactionId; private int statementId;//sort on this descending, like currentTransactionId @@ -120,6 +121,14 @@ public boolean equals(Object other) { && statementId == ((ReaderKey) other).statementId//consistent with compareTo() ; } + @Override + public int hashCode() { + int result = super.hashCode(); + result = 31 * result + (int)(currentTransactionId ^ (currentTransactionId >>> 32)); + result = 31 * result + statementId; + return result; + } + @Override public int compareTo(RecordIdentifier other) { diff --git ql/src/test/org/apache/hadoop/hive/ql/io/TestRecordIdentifier.java ql/src/test/org/apache/hadoop/hive/ql/io/TestRecordIdentifier.java index 6d83f70..a2bd8da 100644 --- ql/src/test/org/apache/hadoop/hive/ql/io/TestRecordIdentifier.java +++ ql/src/test/org/apache/hadoop/hive/ql/io/TestRecordIdentifier.java @@ -17,8 +17,13 @@ */ package org.apache.hadoop.hive.ql.io; +import org.apache.hadoop.hive.ql.io.orc.OrcRawRecordMerger; import org.junit.Test; +import java.util.concurrent.ThreadLocalRandom; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotEquals; import static org.junit.Assert.assertTrue; public class TestRecordIdentifier { @@ -42,4 +47,27 @@ public void TestOrdering() throws Exception { assertTrue(left.compareTo(right) < 0); assertTrue(right.compareTo(left) > 0); } + + @Test + public void testHashEquals() throws Exception { + long origTxn = ThreadLocalRandom.current().nextLong(1, 10000000000L); + int bucketId = ThreadLocalRandom.current().nextInt(1, 512); + long rowId = ThreadLocalRandom.current().nextLong(1, 10000000000L); + long currTxn = origTxn + ThreadLocalRandom.current().nextLong(0, 10000000000L); + int stmtId = ThreadLocalRandom.current().nextInt(1, 512); + + RecordIdentifier left = new RecordIdentifier(origTxn, bucketId, rowId); + RecordIdentifier right = new RecordIdentifier(origTxn, bucketId, rowId); + OrcRawRecordMerger.ReaderKey rkLeft = new OrcRawRecordMerger.ReaderKey(origTxn, bucketId, rowId, currTxn, stmtId); + OrcRawRecordMerger.ReaderKey rkRight = new OrcRawRecordMerger.ReaderKey(origTxn, bucketId, rowId, currTxn, stmtId); + + assertEquals("RecordIdentifier.equals", left, right); + assertEquals("RecordIdentifier.hashCode", left.hashCode(), right.hashCode()); + + assertEquals("ReaderKey", rkLeft, rkLeft); + assertEquals("ReaderKey.hashCode", rkLeft.hashCode(), rkRight.hashCode()); + + //debatable if this is correct, but that's how it's implemented + assertNotEquals("RecordIdentifier <> ReaderKey", left, rkRight); + } }