You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sqoop.apache.org by Venkat Ranganathan <n....@live.com> on 2013/07/09 00:16:08 UTC
Review Request 12342: Fix for SQOOP-1097
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12342/
-----------------------------------------------------------
Review request for Sqoop.
Bugs: SQOOP-1097
https://issues.apache.org/jira/browse/SQOOP-1097
Repository: sqoop-trunk
Description
-------
Fix for mysql export using procedure. While fixing this, found that Oracle export also has additional issues when using DB specific types that are not handled. Fixed both the issues and provided a generic implementation for future db tests.
Diffs
-----
src/java/org/apache/sqoop/manager/ConnManager.java c9e05da
src/java/org/apache/sqoop/manager/MySQLManager.java 2090b1a
src/java/org/apache/sqoop/manager/OracleManager.java edc888e
src/java/org/apache/sqoop/manager/SqlManager.java e96368b
src/test/org/apache/sqoop/TestExportUsingProcedure.java 6414ef7
src/test/org/apache/sqoop/manager/mysql/MySqlCallExportTest.java PRE-CREATION
src/test/org/apache/sqoop/manager/oracle/OracleCallExportTest.java PRE-CREATION
Diff: https://reviews.apache.org/r/12342/diff/
Testing
-------
All unit tests pass. Created a mysql and oracle specific test for testing both the functionality of doing procedure based export and handling db specific types
Thanks,
Venkat Ranganathan
Re: Review Request 12342: Fix for SQOOP-1097
Posted by Jarek Cecho <ja...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12342/#review22947
-----------------------------------------------------------
Ship it!
Ship It!
- Jarek Cecho
On July 10, 2013, 3:07 a.m., Venkat Ranganathan wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/12342/
> -----------------------------------------------------------
>
> (Updated July 10, 2013, 3:07 a.m.)
>
>
> Review request for Sqoop.
>
>
> Bugs: SQOOP-1097
> https://issues.apache.org/jira/browse/SQOOP-1097
>
>
> Repository: sqoop-trunk
>
>
> Description
> -------
>
> Fix for mysql export using procedure. While fixing this, found that Oracle export also has additional issues when using DB specific types that are not handled. Fixed both the issues and provided a generic implementation for future db tests.
>
>
> Diffs
> -----
>
> src/java/org/apache/sqoop/manager/ConnManager.java c9e05da
> src/java/org/apache/sqoop/manager/MySQLManager.java 2090b1a
> src/java/org/apache/sqoop/manager/OracleManager.java edc888e
> src/java/org/apache/sqoop/manager/SqlManager.java e96368b
> src/test/com/cloudera/sqoop/ThirdPartyTests.java 7fae052
> src/test/org/apache/sqoop/TestExportUsingProcedure.java 6414ef7
> src/test/org/apache/sqoop/manager/mysql/MySqlCallExportTest.java PRE-CREATION
> src/test/org/apache/sqoop/manager/oracle/OracleCallExportTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/12342/diff/
>
>
> Testing
> -------
>
> All unit tests pass. Created a mysql and oracle specific test for testing both the functionality of doing procedure based export and handling db specific types
>
>
> Thanks,
>
> Venkat Ranganathan
>
>
Re: Review Request 12342: Fix for SQOOP-1097
Posted by Venkat Ranganathan <n....@live.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12342/
-----------------------------------------------------------
(Updated July 10, 2013, 3:07 a.m.)
Review request for Sqoop.
Changes
-------
Fixed the trailing space. Sorry about that. Uploading it to the JIRA as well.
Thanks
Venkat
Bugs: SQOOP-1097
https://issues.apache.org/jira/browse/SQOOP-1097
Repository: sqoop-trunk
Description
-------
Fix for mysql export using procedure. While fixing this, found that Oracle export also has additional issues when using DB specific types that are not handled. Fixed both the issues and provided a generic implementation for future db tests.
Diffs (updated)
-----
src/java/org/apache/sqoop/manager/ConnManager.java c9e05da
src/java/org/apache/sqoop/manager/MySQLManager.java 2090b1a
src/java/org/apache/sqoop/manager/OracleManager.java edc888e
src/java/org/apache/sqoop/manager/SqlManager.java e96368b
src/test/com/cloudera/sqoop/ThirdPartyTests.java 7fae052
src/test/org/apache/sqoop/TestExportUsingProcedure.java 6414ef7
src/test/org/apache/sqoop/manager/mysql/MySqlCallExportTest.java PRE-CREATION
src/test/org/apache/sqoop/manager/oracle/OracleCallExportTest.java PRE-CREATION
Diff: https://reviews.apache.org/r/12342/diff/
Testing
-------
All unit tests pass. Created a mysql and oracle specific test for testing both the functionality of doing procedure based export and handling db specific types
Thanks,
Venkat Ranganathan
Re: Review Request 12342: Fix for SQOOP-1097
Posted by Jarek Cecho <ja...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12342/#review22944
-----------------------------------------------------------
Ship it!
Hi Venkat,
please upload the patch (preferably removing the following nit) to the JIRA and I'll commit it!
src/java/org/apache/sqoop/manager/ConnManager.java
<https://reviews.apache.org/r/12342/#comment46730>
Nit: Trailing white space.
Jarcec
- Jarek Cecho
On July 9, 2013, 7:52 p.m., Venkat Ranganathan wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/12342/
> -----------------------------------------------------------
>
> (Updated July 9, 2013, 7:52 p.m.)
>
>
> Review request for Sqoop.
>
>
> Bugs: SQOOP-1097
> https://issues.apache.org/jira/browse/SQOOP-1097
>
>
> Repository: sqoop-trunk
>
>
> Description
> -------
>
> Fix for mysql export using procedure. While fixing this, found that Oracle export also has additional issues when using DB specific types that are not handled. Fixed both the issues and provided a generic implementation for future db tests.
>
>
> Diffs
> -----
>
> src/java/org/apache/sqoop/manager/ConnManager.java c9e05da
> src/java/org/apache/sqoop/manager/MySQLManager.java 2090b1a
> src/java/org/apache/sqoop/manager/OracleManager.java edc888e
> src/java/org/apache/sqoop/manager/SqlManager.java e96368b
> src/test/com/cloudera/sqoop/ThirdPartyTests.java 7fae052
> src/test/org/apache/sqoop/TestExportUsingProcedure.java 6414ef7
> src/test/org/apache/sqoop/manager/mysql/MySqlCallExportTest.java PRE-CREATION
> src/test/org/apache/sqoop/manager/oracle/OracleCallExportTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/12342/diff/
>
>
> Testing
> -------
>
> All unit tests pass. Created a mysql and oracle specific test for testing both the functionality of doing procedure based export and handling db specific types
>
>
> Thanks,
>
> Venkat Ranganathan
>
>
Re: Review Request 12342: Fix for SQOOP-1097
Posted by Venkat Ranganathan <n....@live.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12342/
-----------------------------------------------------------
(Updated July 9, 2013, 7:52 p.m.)
Review request for Sqoop.
Changes
-------
Thanks [~jarcec] for the reviews.
Updated with review comments
Bugs: SQOOP-1097
https://issues.apache.org/jira/browse/SQOOP-1097
Repository: sqoop-trunk
Description
-------
Fix for mysql export using procedure. While fixing this, found that Oracle export also has additional issues when using DB specific types that are not handled. Fixed both the issues and provided a generic implementation for future db tests.
Diffs (updated)
-----
src/java/org/apache/sqoop/manager/ConnManager.java c9e05da
src/java/org/apache/sqoop/manager/MySQLManager.java 2090b1a
src/java/org/apache/sqoop/manager/OracleManager.java edc888e
src/java/org/apache/sqoop/manager/SqlManager.java e96368b
src/test/com/cloudera/sqoop/ThirdPartyTests.java 7fae052
src/test/org/apache/sqoop/TestExportUsingProcedure.java 6414ef7
src/test/org/apache/sqoop/manager/mysql/MySqlCallExportTest.java PRE-CREATION
src/test/org/apache/sqoop/manager/oracle/OracleCallExportTest.java PRE-CREATION
Diff: https://reviews.apache.org/r/12342/diff/
Testing
-------
All unit tests pass. Created a mysql and oracle specific test for testing both the functionality of doing procedure based export and handling db specific types
Thanks,
Venkat Ranganathan
Re: Review Request 12342: Fix for SQOOP-1097
Posted by Jarek Cecho <ja...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12342/#review22914
-----------------------------------------------------------
Hi Venkat,
thank you very much for the refactoring in the patch, I like your changes!
src/java/org/apache/sqoop/manager/ConnManager.java
<https://reviews.apache.org/r/12342/#comment46657>
Nit: s/table/stored procedure/
src/java/org/apache/sqoop/manager/ConnManager.java
<https://reviews.apache.org/r/12342/#comment46658>
I'm afraid that this is backward incompatible change that might cause issues with third party connectors.
Maybe we can add method similar to the following?
public Map<String, String> getColumnTypesNames(String tableName, String sqlQuery) {
return getColumnTypeNames(tableName, null, sqlQuery);
}
src/test/org/apache/sqoop/manager/mysql/MySqlCallExportTest.java
<https://reviews.apache.org/r/12342/#comment46660>
Nit: please put this file into ThirdPartyTest suite:
https://github.com/apache/sqoop/blob/trunk/src/test/com/cloudera/sqoop/ThirdPartyTests.java
src/test/org/apache/sqoop/manager/oracle/OracleCallExportTest.java
<https://reviews.apache.org/r/12342/#comment46661>
Nit: please put this file into ThirdPartyTest suite:
https://github.com/apache/sqoop/blob/trunk/src/test/com/cloudera/sqoop/ThirdPartyTests.java
Jarcec
- Jarek Cecho
On July 8, 2013, 10:16 p.m., Venkat Ranganathan wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/12342/
> -----------------------------------------------------------
>
> (Updated July 8, 2013, 10:16 p.m.)
>
>
> Review request for Sqoop.
>
>
> Bugs: SQOOP-1097
> https://issues.apache.org/jira/browse/SQOOP-1097
>
>
> Repository: sqoop-trunk
>
>
> Description
> -------
>
> Fix for mysql export using procedure. While fixing this, found that Oracle export also has additional issues when using DB specific types that are not handled. Fixed both the issues and provided a generic implementation for future db tests.
>
>
> Diffs
> -----
>
> src/java/org/apache/sqoop/manager/ConnManager.java c9e05da
> src/java/org/apache/sqoop/manager/MySQLManager.java 2090b1a
> src/java/org/apache/sqoop/manager/OracleManager.java edc888e
> src/java/org/apache/sqoop/manager/SqlManager.java e96368b
> src/test/org/apache/sqoop/TestExportUsingProcedure.java 6414ef7
> src/test/org/apache/sqoop/manager/mysql/MySqlCallExportTest.java PRE-CREATION
> src/test/org/apache/sqoop/manager/oracle/OracleCallExportTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/12342/diff/
>
>
> Testing
> -------
>
> All unit tests pass. Created a mysql and oracle specific test for testing both the functionality of doing procedure based export and handling db specific types
>
>
> Thanks,
>
> Venkat Ranganathan
>
>