----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55079/#review160324 -----------------------------------------------------------
Hi Frédéric, I have added some review to the specific lines, most of them related to indentation problems. On the top of them I would have two more things: 1. Could you please recursively replace every usage of the DBConfiguration's getInputTableName() in Netezza related classes, e.g. in NetezzaDataDrivenDBInputFormat too? 2. Could you please add some unit test cases for this new command line option? Thanks, Bogi src/java/org/apache/sqoop/manager/DirectNetezzaManager.java (lines 299 - 301) <https://reviews.apache.org/r/55079/#comment231473> These lines have an incorrect indentation, please correct it. src/java/org/apache/sqoop/manager/DirectNetezzaManager.java (lines 311 - 312) <https://reviews.apache.org/r/55079/#comment231474> Is this included into the log intentionally or was just because of debugging reasons? If it's intentional, could you please provide a more verbose description instead of just writing "extraArgs"? src/java/org/apache/sqoop/manager/DirectNetezzaManager.java (lines 331 - 336) <https://reviews.apache.org/r/55079/#comment231475> These lines have an incorrect indentation, please correct them. src/java/org/apache/sqoop/manager/DirectNetezzaManager.java (lines 370 - 383) <https://reviews.apache.org/r/55079/#comment231477> These lines have an incorrect indentation, please correct them. src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableImportMapper.java (line 81) <https://reviews.apache.org/r/55079/#comment231480> In escapeTableName method of the DirectNetezzaManager class you checked got schema value against null and isEmpty too. Is there any specific reason for checking it here only against null value? - Boglarka Egyed On Jan. 1, 2017, 8: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, 8: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 > >
