From 80018a6dafc2f33152731a21159e5c5253357a4f Mon Sep 17 00:00:00 2001
From: Jonathan Hsieh <jon@cloudera.com>
Date: Thu, 2 Jun 2011 20:05:13 -0700
Subject: [PATCH] SQOOP-48: Import bug when splitting over unsigned bigint column

---
 .../sqoop/mapreduce/db/IntegerSplitter.java        |   26 +++++++++++++++++--
 .../sqoop/mapreduce/db/TestIntegerSplitter.java    |   12 ++++++---
 2 files changed, 31 insertions(+), 7 deletions(-)

diff --git a/src/java/com/cloudera/sqoop/mapreduce/db/IntegerSplitter.java b/src/java/com/cloudera/sqoop/mapreduce/db/IntegerSplitter.java
index cd3b5ec..0905d9e 100644
--- a/src/java/com/cloudera/sqoop/mapreduce/db/IntegerSplitter.java
+++ b/src/java/com/cloudera/sqoop/mapreduce/db/IntegerSplitter.java
@@ -23,6 +23,8 @@ import java.sql.SQLException;
 import java.util.ArrayList;
 import java.util.List;
 
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
 import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.mapreduce.InputSplit;
 
@@ -32,6 +34,8 @@ import com.cloudera.sqoop.config.ConfigurationHelper;
  * Implement DBSplitter over integer values.
  */
 public class IntegerSplitter implements DBSplitter {
+  public static final Log LOG = LogFactory.getLog(IntegerSplitter.class.getName());
+
   public List<InputSplit> split(Configuration conf, ResultSet results,
       String colName) throws SQLException {
 
@@ -56,6 +60,13 @@ public class IntegerSplitter implements DBSplitter {
 
     // Get all the split points together.
     List<Long> splitPoints = split(numSplits, minVal, maxVal);
+    if (LOG.isDebugEnabled()) {
+      LOG.debug(String.format("Splits: [%,28d to %,28d] into %d parts",
+          minVal, maxVal, numSplits));
+      for (int i = 0; i < splitPoints.size(); i++) {
+        LOG.debug(String.format("%,28d", splitPoints.get(i)));
+      }
+    }
     List<InputSplit> splits = new ArrayList<InputSplit>();
 
     // Turn the split points into a set of intervals.
@@ -105,10 +116,19 @@ public class IntegerSplitter implements DBSplitter {
 
     List<Long> splits = new ArrayList<Long>();
 
-    // Use numSplits as a hint. May need an extra task if the size doesn't
-    // divide cleanly.
+    // We take the min-max interval and divide by the numSplits and then add
+    // one.  This will guarantee that the entire range of numbers is covered
+    // by numSplits groups.  numSplits-1 will have the same range size and
+    // the last split may have a smaller number.  For example let's say we
+    // have min 0 max 7 and want 4 groups.  splitsize=2 and we would end up
+    // with 4 groups: [0,2,4,6,7].
+
+    // If we just used min-max interval / numSplits, we may end up with many
+    // more splits than we intended.  For example, let's say we have min 0 max
+    // 7 and want 4 groups.  splitsize=1 and we  would end up with 7 groups!
+    // [0,1,2,3,4,5,6,7]
 
-    long splitSize = (maxVal - minVal) / numSplits;
+    long splitSize = ((maxVal - minVal) / numSplits) + 1;
     if (splitSize < 1) {
       splitSize = 1;
     }
diff --git a/src/test/com/cloudera/sqoop/mapreduce/db/TestIntegerSplitter.java b/src/test/com/cloudera/sqoop/mapreduce/db/TestIntegerSplitter.java
index 0a99900..4796df8 100644
--- a/src/test/com/cloudera/sqoop/mapreduce/db/TestIntegerSplitter.java
+++ b/src/test/com/cloudera/sqoop/mapreduce/db/TestIntegerSplitter.java
@@ -70,21 +70,20 @@ public class TestIntegerSplitter extends TestCase {
     if (actual.length > expected.length) {
       fail("Actual array has " + actual.length
           + " elements; expected " + expected.length
-          + ". ACtual array is " + formatLongArray(actual));
+          + ". Actual array is " + formatLongArray(actual));
     }
   }
 
   public void testEvenSplits() throws SQLException {
     List<Long> splits = new IntegerSplitter().split(10, 0, 100);
-    long [] expected = { 0, 10, 20, 30, 40, 50, 60, 70, 80, 90, 100 };
+    long [] expected = { 0, 11, 22, 33, 44, 55, 66, 77, 88, 99, 100, };
     assertLongArrayEquals(expected, toLongArray(splits));
   }
 
   public void testOddSplits() throws SQLException {
     List<Long> splits = new IntegerSplitter().split(10, 0, 95);
-    long [] expected = { 0, 9, 18, 27, 36, 45, 54, 63, 72, 81, 90, 95 };
+    long [] expected = { 0, 10, 20, 30, 40, 50, 60, 70, 80, 90, 95, };
     assertLongArrayEquals(expected, toLongArray(splits));
-
   }
 
   public void testSingletonSplit() throws SQLException {
@@ -105,5 +104,10 @@ public class TestIntegerSplitter extends TestCase {
     long [] expected = { 3, 4, 5 };
     assertLongArrayEquals(expected, toLongArray(splits));
   }
+
+  public void testBigIntSplits() throws SQLException {
+    List<Long> splits = new IntegerSplitter().split(4,14,7863696997872966707L);
+    assertEquals(splits.size(), 5);
+  }
 }
 
-- 
1.7.1

