----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/8806/#review15183 -----------------------------------------------------------
Hi Nick, I think that you've done excellent job putting this together. I believe that we're almost there. What bothers me is that stored procedures are heavily dependent on target database. I'm hesitating to press the "Ship It" button when there are no third party tests for MySQL, PostgreSQL, Microsoft SQL Server nor Oracle. Looking at current Microsoft SQL Server tests (SQLServerManagerExportManualTest.java), I think that extending them to stored procedures should not be a big deal. The challenging part is to set up all third party databases so that you can actually run the tests somewhere. I can help with that if you want me to. ivy.xml <https://reviews.apache.org/r/8806/#comment32753> Nit: Trailing white space characters src/java/org/apache/sqoop/mapreduce/JdbcCallExportJob.java <https://reviews.apache.org/r/8806/#comment32754> I'm thinking whether it do make sense to escape the procedure name in all cases. We've recently find out that some third party connectors do have issues if we will automatically escape the table name (SQOOP-820), but that is probably not the case for stored procedures as this is completely new thing. Jarcec - Jarek Cecho On Jan. 7, 2013, 7:47 p.m., Nick White wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/8806/ > ----------------------------------------------------------- > > (Updated Jan. 7, 2013, 7:47 p.m.) > > > Review request for Sqoop and Jarek Cecho. > > > Description > ------- > > It'd be useful if you could use stored procedures (or functions) to insert > data - currently you can only use insert or update statements (or upsert / > merges, depending on the SqlManager you're using). This would help sqoop > adoption / migration into environments which have existing, SQL-based data > import workflows. > > The attached patch adds a --call argument to the export tool that can be used > in place of --table. > > Note that this patch depends on HSQLDB 2.x, whereas trunk currently depends > on 1.8, so it can't be committed until that situation's resolved (see > SQOOP-519 and linked tickets). > > > This addresses bug SQOOP-749. > https://issues.apache.org/jira/browse/SQOOP-749 > > > Diffs > ----- > > ivy.xml 1ee60df > ivy/libraries.properties 4c9e37d > src/docs/user/export-purpose.txt c26eaa7 > src/docs/user/export.txt 9f600fe > src/java/org/apache/sqoop/SqoopOptions.java 3e0ec3e > src/java/org/apache/sqoop/manager/ConnManager.java 21eea93 > src/java/org/apache/sqoop/manager/SqlManager.java 3a52c6d > src/java/org/apache/sqoop/mapreduce/ExportCallOutputFormat.java > PRE-CREATION > src/java/org/apache/sqoop/mapreduce/JdbcCallExportJob.java PRE-CREATION > src/java/org/apache/sqoop/mapreduce/JdbcExportJob.java 7c52110 > src/java/org/apache/sqoop/orm/ClassWriter.java b73711e > src/java/org/apache/sqoop/tool/BaseSqoopTool.java c0221c9 > src/java/org/apache/sqoop/tool/ExportTool.java acd296d > src/test/com/cloudera/sqoop/SmokeTests.java 76df6cf > src/test/com/cloudera/sqoop/TestConnFactory.java 893b388 > src/test/com/cloudera/sqoop/TestExport.java eba10aa > src/test/com/cloudera/sqoop/testutil/ExportJobTestCase.java 4f6fd37 > src/test/org/apache/sqoop/TestExportUsingProcedure.java PRE-CREATION > > Diff: https://reviews.apache.org/r/8806/diff/ > > > Testing > ------- > > I've added a subclass of TestExport to make sure all the usual export > functionality works. I've also used it on live data exporting to postgres. > > > Thanks, > > Nick White > >
