----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57551/#review169389 -----------------------------------------------------------
Hi Eric, Thanks for the corrections. I ran 'ant clean test' with your patch and TestHiveImport failed for me so please review your changes there. Besides that, I had some minor findings related to naming conventions. Thanks, Bogi src/test/com/cloudera/sqoop/hive/TestTableDefWriter.java Lines 256 (patched) <https://reviews.apache.org/r/57551/#comment241739> Thanks for splitting up the test case, however, could you please use a self-explanatory naming? testGetCreateTableStmtDecimal1 and testGetCreateTableStmtDecimal2 don't bring a clear message when some runs the test and these fail. src/test/com/cloudera/sqoop/hive/TestTableDefWriter.java Lines 278 (patched) <https://reviews.apache.org/r/57551/#comment241740> Is it still needed to use decimal_column1 name instead of decimal_column? It could be misleading in my opinion. This applies to the other test case too. - Boglarka Egyed On March 20, 2017, 8:13 a.m., Eric Lin wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/57551/ > ----------------------------------------------------------- > > (Updated March 20, 2017, 8:13 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/TestHiveImport.java 1d67a2d > src/test/com/cloudera/sqoop/hive/TestTableDefWriter.java 4db629f > testdata/hive/scripts/decimalImport.q PRE-CREATION > > > Diff: https://reviews.apache.org/r/57551/diff/3/ > > > Testing > ------- > > Test case + maunaul test > > > Thanks, > > Eric Lin > >
