From c8dbec4813ab83e0050258297dfc7c2d3cfa5d96 Mon Sep 17 00:00:00 2001 From: Abraham Elmahrek Date: Fri, 23 Jan 2015 11:58:59 -0800 Subject: [PATCH] SQOOP-2027: Sqoop2: SqoopIDFUtils handling of decimal need to be fixed (Veena Basavaraj via Abraham Elmahrek) --- .../sqoop/json/util/SchemaSerialization.java | 6 ++-- .../org/apache/sqoop/schema/type/Decimal.java | 35 ++++++------------- .../apache/sqoop/json/TestSubmissionBean.java | 2 +- .../json/util/TestSchemaSerialization.java | 8 ++--- .../jdbc/GenericJdbcFromInitializer.java | 2 +- .../jdbc/GenericJdbcToInitializer.java | 2 +- .../connector/jdbc/util/SqlTypesUtils.java | 5 ++- .../sqoop/connector/jdbc/TestExtractor.java | 4 +-- .../sqoop/connector/common/SqoopIDFUtils.java | 23 ++++++++++-- .../connector/common/TestSqoopIDFUtils.java | 33 +++++++++++++++-- .../idf/TestCSVIntermediateDataFormat.java | 10 +++--- 11 files changed, 81 insertions(+), 49 deletions(-) diff --git a/common/src/main/java/org/apache/sqoop/json/util/SchemaSerialization.java b/common/src/main/java/org/apache/sqoop/json/util/SchemaSerialization.java index 4ec2f654..21fad35c 100644 --- a/common/src/main/java/org/apache/sqoop/json/util/SchemaSerialization.java +++ b/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); - Long scale = (Long) obj.get(SCALE); - output = new Decimal(name).setPrecision(precision).setScale(scale); + Integer precision = obj.get(PRECISION) != null ? ((Long) obj.get(PRECISION)).intValue() : null; + Integer scale = obj.get(SCALE) != null ? ((Long) obj.get(SCALE)).intValue() : null; + output = new Decimal(name, precision, scale); break; case ENUM: output = new Enum(name, options); diff --git a/common/src/main/java/org/apache/sqoop/schema/type/Decimal.java b/common/src/main/java/org/apache/sqoop/schema/type/Decimal.java index b7572cdd..f455402e 100644 --- a/common/src/main/java/org/apache/sqoop/schema/type/Decimal.java +++ b/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; - /** - * Number of decimal places. - */ - private Long scale; + private Integer scale; - public Decimal(String name) { - super(name); - } - - public Decimal(String name, Long precision, Long scale) { + public Decimal(String name, Integer precision, Integer 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) { - this.precision = precision; - return this; - } - - public Long getScale() { + public Integer getScale() { return scale; } - public Decimal setScale(Long scale) { - this.scale = scale; - return this; - } - @Override public ColumnType getType() { return ColumnType.DECIMAL; diff --git a/common/src/test/java/org/apache/sqoop/json/TestSubmissionBean.java b/common/src/test/java/org/apache/sqoop/json/TestSubmissionBean.java index abbfbb41..74081ae0 100644 --- a/common/src/test/java/org/apache/sqoop/json/TestSubmissionBean.java +++ b/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)) ; } diff --git a/common/src/test/java/org/apache/sqoop/json/util/TestSchemaSerialization.java b/common/src/test/java/org/apache/sqoop/json/util/TestSchemaSerialization.java index b7741ec8..45b34a9d 100644 --- a/common/src/test/java/org/apache/sqoop/json/util/TestSchemaSerialization.java +++ b/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(Arrays.asList(new String[] { "X", "Y" }))))) .addColumn(new FixedPoint("h", 2L, false)) .addColumn(new FloatingPoint("i", 4L)) diff --git a/connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcFromInitializer.java b/connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcFromInitializer.java index 425b2cc0..1b30fb4a 100644 --- a/connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcFromInitializer.java +++ b/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); } diff --git a/connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcToInitializer.java b/connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcToInitializer.java index bc720dab..5e6f983a 100644 --- a/connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcToInitializer.java +++ b/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); } diff --git a/connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/util/SqlTypesUtils.java b/connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/util/SqlTypesUtils.java index d84498e1..a6ffa7ce 100644 --- a/connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/util/SqlTypesUtils.java +++ b/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: diff --git a/connector/connector-generic-jdbc/src/test/java/org/apache/sqoop/connector/jdbc/TestExtractor.java b/connector/connector-generic-jdbc/src/test/java/org/apache/sqoop/connector/jdbc/TestExtractor.java index c2b4b301..803d37b8 100644 --- a/connector/connector-generic-jdbc/src/test/java/org/apache/sqoop/connector/jdbc/TestExtractor.java +++ b/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); diff --git a/connector/connector-sdk/src/main/java/org/apache/sqoop/connector/common/SqoopIDFUtils.java b/connector/connector-sdk/src/main/java/org/apache/sqoop/connector/common/SqoopIDFUtils.java index 32f6a869..c5eb6a38 100644 --- a/connector/connector-sdk/src/main/java/org/apache/sqoop/connector/common/SqoopIDFUtils.java +++ b/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****************** diff --git a/connector/connector-sdk/src/test/java/org/apache/sqoop/connector/common/TestSqoopIDFUtils.java b/connector/connector-sdk/src/test/java/org/apache/sqoop/connector/common/TestSqoopIDFUtils.java index 63c95357..588ce291 100644 --- a/connector/connector-sdk/src/test/java/org/apache/sqoop/connector/common/TestSqoopIDFUtils.java +++ b/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 map = new HashMap(); map.put("A", list); - org.apache.sqoop.schema.type.Map mapCol = new org.apache.sqoop.schema.type.Map("a", new Text( - "t"), new Array("r", new Text("tr"))); + org.apache.sqoop.schema.type.Map mapCol = new org.apache.sqoop.schema.type.Map("a", new Text("t"), new Array("r", new Text( + "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)); + } + } diff --git a/connector/connector-sdk/src/test/java/org/apache/sqoop/connector/idf/TestCSVIntermediateDataFormat.java b/connector/connector-sdk/src/test/java/org/apache/sqoop/connector/idf/TestCSVIntermediateDataFormat.java index cbbbd44d..2630a9d8 100644 --- a/connector/connector-sdk/src/test/java/org/apache/sqoop/connector/idf/TestCSVIntermediateDataFormat.java +++ b/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 };