> On Sept. 17, 2018, 7:08 a.m., Szabolcs Vasas wrote: > > Hi Dae Myung, > > > > Thank you for submitting this patch! > > I think it is a very good start but there are a few improvements we should > > make before we can commit it. > > > > - The JIRA says that this is an improvement for the MySQL direct connector > > but the change suggests that it affects the plain MySQL connector only. Can > > you please clarify this? > > - Since the option you introduce only affects the MySQL connector I think > > it should be added as an extra argument to > > org.apache.sqoop.manager.MySQLManager. You can see a good example in > > org.apache.sqoop.manager.PostgresqlManager#parseExtraArgs. > > - Apart from manual testing you should add automated tests too. I think > > creating a test file with UTF-8 encoding and special characters and > > exporting it into MySQL would be a great test case. You can see a few > > examples in org.apache.sqoop.manager.mysql.DirectMySQLExportTest and > > org.apache.sqoop.manager.mysql.JdbcMySQLExportTest. > > > > If you need any help with the above I am happy to help. > > > > Regards, > > Szabolcs
Thanks Szabolcs, you make sense, I will apply your reviews. - Dae Myung ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68717/#review208672 ----------------------------------------------------------- On Sept. 14, 2018, 4:47 p.m., Dae Myung Kang wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/68717/ > ----------------------------------------------------------- > > (Updated Sept. 14, 2018, 4:47 p.m.) > > > Review request for Sqoop. > > > Bugs: sqoop-2639 > https://issues.apache.org/jira/browse/sqoop-2639 > > > Repository: sqoop-trunk > > > Description > ------- > > sqmanually test with testing senario. > > It will just change mysqlCharset by commandline option > > when not given --mysql-charset > > select query returned > > ``` > FX???? > ``` > > given --mysql-charset=UTF-8 > ``` > FX???? <-- I think ReviewBoard can't support utf8 now. even I posted valid > utf-8 string > ``` > > > Diffs > ----- > > src/java/org/apache/sqoop/SqoopOptions.java f06872f9 > src/java/org/apache/sqoop/manager/MySQLUtils.java b005c793 > src/java/org/apache/sqoop/mapreduce/MySQLExportJob.java e17f3df8 > src/java/org/apache/sqoop/mapreduce/MySQLExportMapper.java bb751ee6 > src/java/org/apache/sqoop/tool/BaseSqoopTool.java 955d3a65 > src/java/org/apache/sqoop/tool/ExportTool.java 060f2c07 > > > Diff: https://reviews.apache.org/r/68717/diff/2/ > > > Testing > ------- > > manually test, this is only change commandline cli > > > Thanks, > > Dae Myung Kang > >
