HBASE-2378 Bulk insert with multiple reducers broken due to improper ImmutableBytesWritable comparator

git-svn-id: https://svn.apache.org/repos/asf/hadoop/hbase/trunk@928995 13f79535-47bb-0310-9956-ffa450edef68

CHANGES.txt

@@ -254,6 +254,8 @@ Release 0.21.0 - Unreleased
    HBASE-2358  Store doReconstructionLog will fail if oldlogfile.log is empty
                and won't load region (Cosmin Lehene via Stack)
    HBASE-2370  saveVersion.sh doesnt properly grab the git revision
+   HBASE-2378  Bulk insert with multiple reducers broken due to improper
+               ImmutableBytesWritable comparator (Todd Lipcon via Stack)
 
   IMPROVEMENTS
    HBASE-1760  Cleanup TODOs in HTable

core/src/main/java/org/apache/hadoop/hbase/io/ImmutableBytesWritable.java

@@ -178,10 +178,7 @@ implements WritableComparable<ImmutableBytesWritable> {
    *         negative if left is smaller than right.
    */
   public int compareTo(final byte [] that) {
-    int diff = this.length - that.length;
+    return WritableComparator.compareBytes(this.bytes, 0, this.length, that,
-    return (diff != 0)?
-      diff:
-      WritableComparator.compareBytes(this.bytes, 0, this.length, that,
         0, that.length);
   }
 

core/src/test/java/org/apache/hadoop/hbase/io/TestImmutableBytesWritable.java

@@ -0,0 +1,96 @@
+/**
+ * Copyright 2009 The Apache Software Foundation
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.hbase.io;
+
+import junit.framework.TestCase;
+
+import org.apache.hadoop.hbase.util.Bytes;
+import java.io.ByteArrayOutputStream;
+import java.io.DataOutputStream;
+import java.io.IOException;
+
+public class TestImmutableBytesWritable extends TestCase {
+  public void testComparison() throws Exception {
+    runTests("aa", "b", -1);
+    runTests("aa", "aa", 0);
+    runTests("aa", "ab", -1);
+    runTests("aa", "aaa", -1);
+    runTests("", "", 0);
+    runTests("", "a", -1);
+  }
+
+  private void runTests(String aStr, String bStr, int signum)
+    throws Exception {
+    ImmutableBytesWritable a = new ImmutableBytesWritable(
+      Bytes.toBytes(aStr));
+    ImmutableBytesWritable b = new ImmutableBytesWritable(
+      Bytes.toBytes(bStr));
+
+    doComparisonsOnObjects(a, b, signum);
+    doComparisonsOnRaw(a, b, signum);
+  }
+
+
+  private int signum(int i) {
+    if (i > 0) return 1;
+    if (i == 0) return 0;
+    return -1;
+  }
+
+  private void doComparisonsOnRaw(ImmutableBytesWritable a,
+                                  ImmutableBytesWritable b,
+                                  int expectedSignum)
+    throws IOException {
+    ImmutableBytesWritable.Comparator comparator =
+      new ImmutableBytesWritable.Comparator();
+
+    ByteArrayOutputStream baosA = new ByteArrayOutputStream();
+    ByteArrayOutputStream baosB = new ByteArrayOutputStream();
+
+    a.write(new DataOutputStream(baosA));
+    b.write(new DataOutputStream(baosB));
+
+    assertEquals(
+      "Comparing " + a + " and " + b + " as raw",
+      signum(comparator.compare(baosA.toByteArray(), 0, baosA.size(),
+                                baosB.toByteArray(), 0, baosB.size())),
+      expectedSignum);
+
+    assertEquals(
+      "Comparing " + a + " and " + b + " as raw (inverse)",
+      -signum(comparator.compare(baosB.toByteArray(), 0, baosB.size(),
+                                 baosA.toByteArray(), 0, baosA.size())),
+      expectedSignum);
+  }
+
+  private void doComparisonsOnObjects(ImmutableBytesWritable a,
+                                      ImmutableBytesWritable b,
+                                      int expectedSignum) {
+    ImmutableBytesWritable.Comparator comparator =
+      new ImmutableBytesWritable.Comparator();
+    assertEquals(
+      "Comparing " + a + " and " + b + " as objects",
+      signum(comparator.compare(a, b)), expectedSignum);
+    assertEquals(
+      "Comparing " + a + " and " + b + " as objects (inverse)",
+      -signum(comparator.compare(b, a)), expectedSignum);
+  }
+}