> On Aug. 8, 2015, 7:04 p.m., Jarek Cecho wrote:
> > Thanks for the patch Colin! I've just preliminary skimmed through the patch 
> > and I have few comments:
> > 
> > 1) Please clean up the trailing whitespaces.
> > 
> > 2) I would suggest to use SQL_MODE to solve the problem with escaping 
> > object names with double quotes [1]
> > 
> > Links:
> > 1: https://dev.mysql.com/doc/refman/5.0/en/sql-mode.html

Thanks for the comments, the new patch is based on SQL_MODE = ANSI_QUOTES.


> On Aug. 8, 2015, 7:04 p.m., Jarek Cecho wrote:
> > repository/repository-mysql/pom.xml, lines 48-51
> > <https://reviews.apache.org/r/37121/diff/1/?file=1032305#file1032305line48>
> >
> >     I think that we need to mark this as a provided dependency right? 
> > Otherwise we will package the JDBC driver which we can't do due to 
> > incompatible licenses.

This dependency is for the integration test, and I add <scope>test<scope> in 
the new patch, thanks for the comment.


> On Aug. 8, 2015, 7:04 p.m., Jarek Cecho wrote:
> > repository/repository-mysql/src/main/java/org/apache/sqoop/repository/mysql/MysqlRepositoryInsertUpdateDeleteSelectQuery.java,
> >  lines 20-79
> > <https://reviews.apache.org/r/37121/diff/1/?file=1032310#file1032310line20>
> >
> >     Let's do * import here.

In the new patch, 11 constants are imported, so I don't use the * here, and 
list the import one by one.
If you think * is more suitable in the new patch, I'll update it.


- Colin


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


On Aug. 10, 2015, 8:35 a.m., Colin Ma wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37121/
> -----------------------------------------------------------
> 
> (Updated Aug. 10, 2015, 8:35 a.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Repository: sqoop-sqoop2
> 
> 
> Description
> -------
> 
> The sqoop-repository-mysql should be implemented with the 
> sqoop-repository-comm.
> 
> 
> Diffs
> -----
> 
>   
> common-test/src/main/java/org/apache/sqoop/common/test/db/DatabaseProvider.java
>  dd4e546 
>   
> common-test/src/main/java/org/apache/sqoop/common/test/db/MySQLProvider.java 
> 3083ee6 
>   common/src/main/java/org/apache/sqoop/error/code/MySqlRepoError.java 
> PRE-CREATION 
>   pom.xml 7bcb212 
>   repository/pom.xml c63595c 
>   
> repository/repository-common/src/main/java/org/apache/sqoop/repository/common/CommonRepoUtils.java
>  73293c0 
>   repository/repository-mysql/pom.xml PRE-CREATION 
>   
> repository/repository-mysql/src/main/java/org/apache/sqoop/repository/mysql/MySqlRepoConstants.java
>  PRE-CREATION 
>   
> repository/repository-mysql/src/main/java/org/apache/sqoop/repository/mysql/MySqlRepositoryHandler.java
>  PRE-CREATION 
>   
> repository/repository-mysql/src/main/java/org/apache/sqoop/repository/mysql/MySqlSchemaCreateQuery.java
>  PRE-CREATION 
>   
> repository/repository-mysql/src/main/java/org/apache/sqoop/repository/mysql/MySqlSchemaQuery.java
>  PRE-CREATION 
>   
> repository/repository-mysql/src/main/java/org/apache/sqoop/repository/mysql/MysqlRepositoryInsertUpdateDeleteSelectQuery.java
>  PRE-CREATION 
>   
> repository/repository-mysql/src/test/java/org/apache/sqoop/integration/repository/mysql/MySqlTestCase.java
>  PRE-CREATION 
>   
> repository/repository-mysql/src/test/java/org/apache/sqoop/integration/repository/mysql/MySqlTestUtils.java
>  PRE-CREATION 
>   
> repository/repository-mysql/src/test/java/org/apache/sqoop/integration/repository/mysql/TestConnectorHandling.java
>  PRE-CREATION 
>   
> repository/repository-mysql/src/test/java/org/apache/sqoop/integration/repository/mysql/TestDriverHandling.java
>  PRE-CREATION 
>   
> repository/repository-mysql/src/test/java/org/apache/sqoop/integration/repository/mysql/TestHandler.java
>  PRE-CREATION 
>   
> repository/repository-mysql/src/test/java/org/apache/sqoop/integration/repository/mysql/TestJobHandling.java
>  PRE-CREATION 
>   
> repository/repository-mysql/src/test/java/org/apache/sqoop/integration/repository/mysql/TestLinkHandling.java
>  PRE-CREATION 
>   
> repository/repository-mysql/src/test/java/org/apache/sqoop/integration/repository/mysql/TestStructure.java
>  PRE-CREATION 
>   
> repository/repository-mysql/src/test/java/org/apache/sqoop/integration/repository/mysql/TestSubmissionHandling.java
>  PRE-CREATION 
>   server/pom.xml aabefc0 
> 
> Diff: https://reviews.apache.org/r/37121/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Colin Ma
> 
>

Reply via email to