----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57551/#review169364 -----------------------------------------------------------
Hi Eric, Thanks for your contribution! I had some minor findings related to your change, please find them below. On the top of them, could you please add a Hive import test case too for your use case for example in TestHiveImport class? Many thanks, Bogi src/java/org/apache/sqoop/hive/TableDefWriter.java Line 23 (original), 23 (patched) <https://reviews.apache.org/r/57551/#comment241723> java.sql.Types shown as an unused import for me when I'm applying your patch. src/java/org/apache/sqoop/hive/TableDefWriter.java Line 24 (original), 24 (patched) <https://reviews.apache.org/r/57551/#comment241722> Please avoid using wildcard imports based on our common guidelines and add java.util.ArrayList, java.util.Date and java.util.Properties imports back. src/test/com/cloudera/sqoop/hive/TestTableDefWriter.java Lines 38 (patched) <https://reviews.apache.org/r/57551/#comment241724> Please avoid using wildcard imports based on our common guidelines. src/test/com/cloudera/sqoop/hive/TestTableDefWriter.java Lines 256 (patched) <https://reviews.apache.org/r/57551/#comment241727> This test case looks great, however, I would split it into two smaller test case to make it easier to maintain in the future: one for info1 and one for info2 as these test two different behavior. - Boglarka Egyed On March 13, 2017, 10:02 a.m., Eric Lin wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/57551/ > ----------------------------------------------------------- > > (Updated March 13, 2017, 10:02 a.m.) > > > Review request for Sqoop, Attila Szabo and Szabolcs Vasas. > > > Repository: sqoop-trunk > > > Description > ------- > > Currently Sqoop converts DECIMAL from RDMS into DOUBLE in Hive, which is not > correct as user will lose precisions. Since Hive supports DECIMAL long ago, > we should support DECIMAL to DECIMAL conversion from Sqoop to Hive. > > > Diffs > ----- > > src/java/org/apache/sqoop/hive/HiveTypes.java ad00535 > src/java/org/apache/sqoop/hive/TableDefWriter.java 32fcca3 > src/test/com/cloudera/sqoop/hive/TestTableDefWriter.java 4db629f > > > Diff: https://reviews.apache.org/r/57551/diff/1/ > > > Testing > ------- > > Test case + maunaul test > > > Thanks, > > Eric Lin > >
