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