----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64371/#review193133 -----------------------------------------------------------
src/java/org/apache/sqoop/mapreduce/hcat/SqoopHCatImportHelper.java Line 403 (original), 403 (patched) <https://reviews.apache.org/r/64371/#comment271639> Nice catch! I think we can make this statement more readable if we extract the condition in the inner parenthesis to another method. That might prevent some future bugs and could eliminate some code duplication as well. src/java/org/apache/sqoop/mapreduce/hcat/SqoopHCatImportHelper.java Lines 413 (patched) <https://reviews.apache.org/r/64371/#comment271640> nit: I know this was not originally your code, but can we rename the one letter variable? - Szabolcs Vasas On Dec. 6, 2017, 2:13 p.m., Fero Szabo wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/64371/ > ----------------------------------------------------------- > > (Updated Dec. 6, 2017, 2:13 p.m.) > > > Review request for Sqoop, Boglarka Egyed and Szabolcs Vasas. > > > Bugs: SQOOP-3233 > https://issues.apache.org/jira/browse/SQOOP-3233 > > > Repository: sqoop-trunk > > > Description > ------- > > - Corrected the type from VARCHAR to CHAR in > SqoopHCatImportHelper.convertNumberTypes, that the ticket description > mentions. > - Found another bug in the convertNumberTypes function: missing parentheses > in the if clause that checks whether the type of val is BigDecimal. Corrected > it. > - Slight refactoring: extracted the BigDecimal and Number related conversion > code into two different methods, for better readability. > > > Diffs > ----- > > src/java/org/apache/sqoop/mapreduce/hcat/SqoopHCatImportHelper.java > 1c1ed1e58c1e1c9a0364e61627ac1ef725bea8f9 > src/test/org/apache/sqoop/hcat/HCatalogImportTest.java > 4686493c79498d9ca45db227904049736b5ea493 > > > Diff: https://reviews.apache.org/r/64371/diff/2/ > > > Testing > ------- > > Ran unit tests (ant clean test), and third party tests as well successfully. > > > Thanks, > > Fero Szabo > >
