SQOOP-2027: Sqoop2: SqoopIDFUtils handling of decimal need to be fixed

(Veena Basavaraj via Abraham Elmahrek)

common/src/main/java/org/apache/sqoop/json/util/SchemaSerialization.java

@@ -243,9 +243,9 @@ private static Column restoreColumn(JSONObject obj) {
       output = new DateTime(name, hasFraction, hasTimezone);
       break;
     case DECIMAL:
-      Long precision = (Long) obj.get(PRECISION);
+      Integer precision = obj.get(PRECISION) != null ? ((Long) obj.get(PRECISION)).intValue() : null;
-      Long scale = (Long) obj.get(SCALE);
+      Integer scale = obj.get(SCALE) != null ? ((Long) obj.get(SCALE)).intValue() : null;
-      output = new Decimal(name).setPrecision(precision).setScale(scale);
+      output = new Decimal(name, precision, scale);
       break;
     case ENUM:
       output = new Enum(name, options);

common/src/main/java/org/apache/sqoop/schema/type/Decimal.java

@@ -30,49 +30,36 @@
 public class Decimal extends AbstractNumber {
 
   /**
-   * Number of valid numbers.
+   * Numeric precision refers to the maximum number of digits that are present in the number.
+   * ie 1234567.89 has a precision of 9
+   * Numeric scale refers to the maximum number of decimal places
+   * ie 123456.789 has a scale of 3
+   * Thus the maximum allowed value for decimal(5,2) is 999.99
    */
-  private Long precision;
+  private Integer precision;
 
-  /**
+  private Integer scale;
-   * Number of decimal places.
-   */
-  private Long scale;
 
-  public Decimal(String name) {
+  public Decimal(String name, Integer precision, Integer scale) {
-    super(name);
-  }
-
-  public Decimal(String name, Long precision, Long scale) {
     super(name);
     this.precision = precision;
     this.scale = scale;
   }
 
-  public Decimal(String name, Boolean nullable, Long precision, Long scale) {
+  public Decimal(String name, Boolean nullable, Integer precision, Integer scale) {
     super(name, nullable);
     this.precision = precision;
     this.scale = scale;
   }
 
-  public Long getPrecision() {
+  public Integer getPrecision() {
     return precision;
   }
 
-  public Decimal setPrecision(Long precision) {
+  public Integer getScale() {
-    this.precision = precision;
-    return this;
-  }
-
-  public Long getScale() {
     return scale;
   }
 
-  public Decimal setScale(Long scale) {
-    this.scale = scale;
-    return this;
-  }
-
   @Override
   public ColumnType getType() {
     return ColumnType.DECIMAL;

common/src/test/java/org/apache/sqoop/json/TestSubmissionBean.java

@@ -449,7 +449,7 @@ public void testTransferToSchema() {
   private Schema getSchema() {
     return new Schema("schema")
       .addColumn(new Text("col1"))
-      .addColumn(new Decimal("col2"))
+      .addColumn(new Decimal("col2", 5, 2))
     ;
   }
 

common/src/test/java/org/apache/sqoop/json/util/TestSchemaSerialization.java

@@ -95,7 +95,7 @@ public void testByteArraySchemaObject() {
   @Test
   public void testArray() {
     // create an array type containing decimals
-    Schema array = new Schema("array").addColumn(new Array("a", new Decimal("a1")).setSize(1L));
+    Schema array = new Schema("array").addColumn(new Array("a", new Decimal("a1", 5, 2)).setSize(1L));
     transferAndAssert(array);
   }
 
@@ -125,7 +125,7 @@ public void testDateTime() {
 
   @Test
   public void testDecimal() {
-    Schema decimal = new Schema("d").addColumn(new Decimal("d", 12L, 15L));
+    Schema decimal = new Schema("d").addColumn(new Decimal("d", 5, 2));
     transferAndAssert(decimal);
   }
 
@@ -150,7 +150,7 @@ public void testFloatingPoint() {
 
   @Test
   public void testMap() {
-    Schema m = new Schema("m").addColumn(new Map("m", new Text("m1"), new Decimal("m2")));
+    Schema m = new Schema("m").addColumn(new Map("m", new Text("m1"), new Decimal("m2", 5, 2)));
     transferAndAssert(m);
   }
 
@@ -191,7 +191,7 @@ public void testAllTypes() {
       .addColumn(new Bit("c"))
       .addColumn(new Date("d"))
       .addColumn(new DateTime("e", true, true))
-      .addColumn(new Decimal("f"))
+      .addColumn(new Decimal("f", 5, 2))
       .addColumn(new Enum("g", Collections.unmodifiableSet(new HashSet<String>(Arrays.asList(new String[] { "X", "Y" })))))
       .addColumn(new FixedPoint("h", 2L, false))
       .addColumn(new FloatingPoint("i", 4L))

connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcFromInitializer.java

@@ -92,7 +92,7 @@ public Schema getSchema(InitializerContext context, LinkConfiguration linkConfig
             columnName = "Column " + i;
           }
         }
-        Column column = SqlTypesUtils.sqlTypeToSchemaType(rsmt.getColumnType(i), columnName);
+        Column column = SqlTypesUtils.sqlTypeToSchemaType(rsmt.getColumnType(i), columnName, rsmt.getPrecision(i), rsmt.getScale(i));
         schema.addColumn(column);
       }
 

connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcToInitializer.java

@@ -92,7 +92,7 @@ public Schema getSchema(InitializerContext context, LinkConfiguration linkConfig
           }
         }
 
-        Column column = SqlTypesUtils.sqlTypeToSchemaType(rsmt.getColumnType(i), columnName);
+        Column column = SqlTypesUtils.sqlTypeToSchemaType(rsmt.getColumnType(i), columnName, rsmt.getPrecision(i), rsmt.getScale(i));
         schema.addColumn(column);
       }
 

connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/util/SqlTypesUtils.java

@@ -44,7 +44,7 @@ public class SqlTypesUtils {
    *
    * @return Concrete Column implementation
    */
-  public static Column sqlTypeToSchemaType(int sqlType, String columnName) {
+  public static Column sqlTypeToSchemaType(int sqlType, String columnName, int precision, int scale) {
     switch (sqlType) {
       case Types.SMALLINT:
       case Types.TINYINT:
@@ -80,10 +80,9 @@ public static Column sqlTypeToSchemaType(int sqlType, String columnName) {
       case Types.DOUBLE:
         return new FloatingPoint(columnName, 8L);
 
-      //TODO:SQOOP-2027 The following mapping needs to be revisited
       case Types.NUMERIC:
       case Types.DECIMAL:
-        return new Decimal(columnName);
+        return new Decimal(columnName, precision, scale);
 
       case Types.BIT:
       case Types.BOOLEAN:

connector/connector-generic-jdbc/src/test/java/org/apache/sqoop/connector/jdbc/TestExtractor.java

@@ -103,7 +103,7 @@ public void testQuery() throws Exception {
     Schema schema = new Schema("TestExtractor");
     // dummy columns added, all we need is the column count to match to the
     // result set
-    schema.addColumn(new FixedPoint("c1",2L, true)).addColumn(new Decimal("c2")).addColumn(new Text("c3")).addColumn(new Date("c4"));
+    schema.addColumn(new FixedPoint("c1",2L, true)).addColumn(new Decimal("c2", 5, 2)).addColumn(new Text("c3")).addColumn(new Date("c4"));
 
     ExtractorContext extractorContext = new ExtractorContext(context, writer, schema);
 
@@ -216,7 +216,7 @@ public void testNullValueExtracted() throws Exception {
     Extractor extractor = new GenericJdbcExtractor();
     DummyNullDataWriter writer = new DummyNullDataWriter();
     Schema schema = new Schema("TestExtractor");
-    schema.addColumn(new FixedPoint("c1",2L, true)).addColumn(new Decimal("c2")).addColumn(new Text("c3")).addColumn(new Date("c4"));
+    schema.addColumn(new FixedPoint("c1",2L, true)).addColumn(new Decimal("c2", 5, 2)).addColumn(new Text("c3")).addColumn(new Date("c4"));
 
     ExtractorContext extractorContext = new ExtractorContext(context, writer, schema);
 

connector/connector-sdk/src/main/java/org/apache/sqoop/connector/common/SqoopIDFUtils.java

@@ -37,6 +37,8 @@
 
 import java.io.UnsupportedEncodingException;
 import java.math.BigDecimal;
+import java.math.MathContext;
+import java.math.RoundingMode;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.HashMap;
@@ -130,7 +132,7 @@ public static Object toFixedPoint(String csvString, Column column) {
 
   public static String toCSVFloatingPoint(Object obj, Column column) {
     Long byteSize = ((FloatingPoint) column).getByteSize();
-    if (byteSize != null && byteSize <= (Float.SIZE/Byte.SIZE)) {
+    if (byteSize != null && byteSize <= (Float.SIZE / Byte.SIZE)) {
       return ((Float) obj).toString();
     } else {
       return ((Double) obj).toString();
@@ -140,7 +142,7 @@ public static String toCSVFloatingPoint(Object obj, Column column) {
   public static Object toFloatingPoint(String csvString, Column column) {
     Object returnValue;
     Long byteSize = ((FloatingPoint) column).getByteSize();
-    if (byteSize != null && byteSize <= (Float.SIZE/Byte.SIZE)) {
+    if (byteSize != null && byteSize <= (Float.SIZE / Byte.SIZE)) {
       returnValue = Float.valueOf(csvString);
     } else {
       returnValue = Double.valueOf(csvString);
@@ -153,7 +155,22 @@ public static String toCSVDecimal(Object obj) {
   }
 
   public static Object toDecimal(String csvString, Column column) {
-    return new BigDecimal(csvString);
+    Integer precision = ((org.apache.sqoop.schema.type.Decimal) column).getPrecision();
+    Integer scale = ((org.apache.sqoop.schema.type.Decimal) column).getScale();
+    BigDecimal bd = null;
+    if (precision != null) {
+      MathContext mc = new MathContext(precision);
+      bd = new BigDecimal(csvString, mc);
+    } else {
+      bd = new BigDecimal(csvString);
+    }
+    if (scale != null) {
+      // we have decided to use the default MathContext DEFAULT_ROUNDINGMODE
+      // which is RoundingMode.HALF_UP,
+      // we are aware that there may be some loss
+      bd.setScale(scale, RoundingMode.HALF_UP);
+    }
+    return bd;
   }
 
   // ********** BIT Column Type utils******************

connector/connector-sdk/src/test/java/org/apache/sqoop/connector/common/TestSqoopIDFUtils.java

@@ -24,12 +24,14 @@
 import org.apache.sqoop.schema.type.AbstractComplexListType;
 import org.apache.sqoop.schema.type.Array;
 import org.apache.sqoop.schema.type.Column;
+import org.apache.sqoop.schema.type.Decimal;
 import org.apache.sqoop.schema.type.FixedPoint;
 import org.apache.sqoop.schema.type.FloatingPoint;
 import org.apache.sqoop.schema.type.Text;
 import org.testng.annotations.Test;
 
 import java.io.UnsupportedEncodingException;
+import java.math.BigDecimal;
 import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.List;
@@ -273,8 +275,8 @@ public void testEncodeMapToCSVString() {
     list.add("B");
     Map<Object, Object> map = new HashMap<Object, Object>();
     map.put("A", list);
-    org.apache.sqoop.schema.type.Map mapCol = new org.apache.sqoop.schema.type.Map("a", new Text(
+    org.apache.sqoop.schema.type.Map mapCol = new org.apache.sqoop.schema.type.Map("a", new Text("t"), new Array("r", new Text(
-        "t"), new Array("r", new Text("tr")));
+        "tr")));
     String encodedText = toCSVMap(map, mapCol);
     assertEquals(encodedText, "'{\"A\":[\"A\",\"B\"]}'");
   }
@@ -291,4 +293,31 @@ public void testParseCSVString() {
 
   }
 
+  @Test
+  public void testToDecimaPointReturnsDecimal() {
+    String text = "23.44444444";
+    Decimal col = new Decimal("dd", 4, 2);
+    assertTrue(toDecimal(text, col) instanceof BigDecimal);
+    BigDecimal bd = (BigDecimal) toDecimal(text, col);
+    assertEquals("23.44", toCSVDecimal(bd));
+  }
+
+  @Test
+  public void testToDecimaPoint2ReturnsDecimal() {
+    String text = "23.44444444";
+    Decimal col = new Decimal("dd", 8, 2);
+    assertTrue(toDecimal(text, col) instanceof BigDecimal);
+    BigDecimal bd = (BigDecimal) toDecimal(text, col);
+    assertEquals("23.444444", toCSVDecimal(bd));
+  }
+
+  @Test
+  public void testToDecimaPointNoScaleNoPrecisionReturnsDecimal() {
+    String text = "23.44444444";
+    Decimal col = new Decimal("dd", null, null);
+    assertTrue(toDecimal(text, col) instanceof BigDecimal);
+    BigDecimal bd = (BigDecimal) toDecimal(text, col);
+    assertEquals("23.44444444", toCSVDecimal(bd));
+  }
+
 }

connector/connector-sdk/src/test/java/org/apache/sqoop/connector/idf/TestCSVIntermediateDataFormat.java

@@ -89,7 +89,7 @@ public void testEmptyInputAsCSVTextInObjectArrayOut() {
   @Test
   public void testNullValueAsObjectArrayInAndCSVTextOut() {
     Schema schema = new Schema("test");
-    schema.addColumn(new FixedPoint("1", 2L, false)).addColumn(new Decimal("2")).addColumn(new Text("3"))
+    schema.addColumn(new FixedPoint("1", 2L, false)).addColumn(new Decimal("2", 5, 2)).addColumn(new Text("3"))
         .addColumn(new Array("4", new Text("t"))).addColumn(new Binary("5"))
         .addColumn(new org.apache.sqoop.schema.type.Map("6", new Text("t1"), new Text("t2"))).addColumn(new Bit("7"))
         .addColumn(new org.apache.sqoop.schema.type.DateTime("8", false, false))
@@ -113,7 +113,7 @@ public void testNullValueAsObjectArrayInAndCSVTextOut() {
   @Test
   public void testNullValueAsObjectArrayInAndObjectArrayOut() {
     Schema schema = new Schema("test");
-    schema.addColumn(new FixedPoint("1", 2L, true)).addColumn(new Decimal("2")).addColumn(new Text("3"))
+    schema.addColumn(new FixedPoint("1", 2L, true)).addColumn(new Decimal("2", 5, 2)).addColumn(new Text("3"))
         .addColumn(new Array("4", new Text("t"))).addColumn(new Binary("5"))
         .addColumn(new org.apache.sqoop.schema.type.Map("6", new Text("t1"), new Text("t2"))).addColumn(new Bit("7"))
         .addColumn(new org.apache.sqoop.schema.type.DateTime("8", false, false))
@@ -136,7 +136,7 @@ public void testNullValueAsObjectArrayInAndObjectArrayOut() {
   @Test
   public void testNullValueAsCSVTextInAndObjectArrayOut() {
     Schema schema = new Schema("test");
-    schema.addColumn(new FixedPoint("1", 2L, true)).addColumn(new Decimal("2")).addColumn(new Text("3"))
+    schema.addColumn(new FixedPoint("1", 2L, true)).addColumn(new Decimal("2", 5, 2)).addColumn(new Text("3"))
         .addColumn(new Array("4", new Text("t"))).addColumn(new Binary("5"))
         .addColumn(new org.apache.sqoop.schema.type.Map("6", new Text("t1"), new Text("t2"))).addColumn(new Bit("7"))
         .addColumn(new org.apache.sqoop.schema.type.DateTime("8", false, false))
@@ -159,7 +159,7 @@ public void testNullValueAsCSVTextInAndObjectArrayOut() {
   @Test
   public void testNullValueAsCSVTextInAndCSVTextOut() {
     Schema schema = new Schema("test");
-    schema.addColumn(new FixedPoint("1", 2L, true)).addColumn(new Decimal("2")).addColumn(new Text("3"))
+    schema.addColumn(new FixedPoint("1", 2L, true)).addColumn(new Decimal("2", 5, 2)).addColumn(new Text("3"))
         .addColumn(new Array("4", new Text("t"))).addColumn(new Binary("5"))
         .addColumn(new org.apache.sqoop.schema.type.Map("6", new Text("t1"), new Text("t2"))).addColumn(new Bit("7"))
         .addColumn(new org.apache.sqoop.schema.type.DateTime("8", false, false))
@@ -859,7 +859,7 @@ public void testSetOfIntegers() {
   public void testArrayOfDecimals() {
     Schema schema = new Schema("test");
     schema.addColumn(new org.apache.sqoop.schema.type.Array("1",
-        new org.apache.sqoop.schema.type.Decimal("deci")));
+        new org.apache.sqoop.schema.type.Decimal("deci", 5, 2)));
     schema.addColumn(new org.apache.sqoop.schema.type.Text("2"));
     dataFormat = new CSVIntermediateDataFormat(schema);
     Object[] givenArray = { 1.22, 2.444 };