sqoop-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Szabolcs Vasas <vasas.szabo...@gmail.com>
Subject Re: Review Request 68717: [sqoop-2639] supporting mysql export with specific encoding with --mysql-charset option
Date Mon, 17 Sep 2018 07:08:01 GMT

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68717/#review208672
-----------------------------------------------------------



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


src/java/org/apache/sqoop/tool/BaseSqoopTool.java
Lines 1872 (patched)
<https://reviews.apache.org/r/68717/#comment292791>

    Do we need this empty method?


- Szabolcs Vasas


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
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message