----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55079/#review160323 -----------------------------------------------------------
Hi Fred, I've got some comments regarding formatting and one question about escaping. src/java/org/apache/sqoop/manager/DirectNetezzaManager.java (lines 300 - 301) <https://reviews.apache.org/r/55079/#comment231476> "Please use two space indents, and space instead of tabs." (https://cwiki.apache.org/confluence/display/SQOOP/How+to+Contribute/#HowtoContribute-ProvidingPatches) src/java/org/apache/sqoop/manager/DirectNetezzaManager.java (line 311) <https://reviews.apache.org/r/55079/#comment231471> Please make sure you remove unnecessary white spaces. You can easily spot them here in the Review Board with checking the "View Diff" view where they show up with red highlighting. src/java/org/apache/sqoop/manager/DirectNetezzaManager.java (line 330) <https://reviews.apache.org/r/55079/#comment231472> I know this whitespace is not your doing, but so close to your change it would be nice if you could include removing it. src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableImportMapper.java (line 83) <https://reviews.apache.org/r/55079/#comment231479> You have implemented a new escaping mechanism escapeTableName. Are you sure escaping is not needed here too? I'm also curious if you've considered adding schema support at a higher level, e.g. Import instead of Direct Import or even just for Netezza in general, as schemas can be used for exports as well? Have you considered adding some tests? I understand that we currently don't have tests for Netezza direct import, but there is a DirectNetezzaExportManualTest for direct export and also NetezzaImportManualTest for testing import. We should always strive for increasing testing coverage. Thank you, Liz - Erzsebet Szilagyi On Jan. 1, 2017, 9:33 p.m., Frédéric Escandell wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/55079/ > ----------------------------------------------------------- > > (Updated Jan. 1, 2017, 9:33 p.m.) > > > Review request for Sqoop. > > > Repository: sqoop-trunk > > > Description > ------- > > SQOOP-3096 > > > Diffs > ----- > > src/java/org/apache/sqoop/manager/DirectNetezzaManager.java af15824 > > src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableImportMapper.java > 2efea53 > > Diff: https://reviews.apache.org/r/55079/diff/ > > > Testing > ------- > > Tested with a schema included in Netezza database (adding -- --schema <schema > name>) to the sqoop import command line > > > Thanks, > > Frédéric Escandell > >
