Hi David, thank you very much for your quick response! > I agree we should get rid of the OraOopManagerFactory long term - I was > thinking if we merge the missing functionality from the current Oracle > connector into OraOop (it's mainly to do with importing views, index > organised tables etc) then we'd only need one Oracle manager again.
Yup that seems as the best solution, so I'm +1 on the idea. Jarcec On Thu, Feb 27, 2014 at 02:34:24AM +0000, David Robson wrote: > Hi Jarcec, > > I agree we should get rid of the OraOopManagerFactory long term - I was > thinking if we merge the missing functionality from the current Oracle > connector into OraOop (it's mainly to do with importing views, index > organised tables etc) then we'd only need one Oracle manager again. > > The whitespace won't be a problem - I'll do that as part of cleaning up the > checkstyle issues - I haven't checked yet but I'm guessing there'll be a fair > few violations! > > As for the Oracle types - I'll have to experiment with using the generic > types instead. Otherwise I guess we'll have to do reflection like you > suggested. > > David > > -----Original Message----- > From: Jarek Cecho [mailto:nore...@reviews.apache.org] On Behalf Of Jarek Cecho > Sent: Thursday, 27 February 2014 5:38 AM > To: David Robson; Sqoop; Jarek Cecho > Subject: Re: Review Request 18452: Add high performance Oracle connector into > Sqoop > > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/18452/#review35545 > ----------------------------------------------------------- > > > Hi David, > thank you very much for publishing the first draft! Overall it looks good to > me, I have couple of high level notes: > > * Can we please remove all the trailing whitespaces? Most of the IDEs have > ability to do that automatically. > > * I see that we are using a lot of Oracle JDBC driver classes from packages > oracle.sql.* or oracle.jdbc.* thus making the Oracle JDBC driver as a compile > time dependency. As the Oracle JDBC driver do not have open license, I'm not > sure this will fly. Is there an option to use the Java JDBC generic > structures? Or perhaps a reflection? > > > src/java/org/apache/sqoop/ConnFactory.java > <https://reviews.apache.org/r/18452/#comment66144> > > I would like to eventually see the OraOopManagerFactory merged into the > DefaultManagerFactory. I'm fine with doing it in follow up JIRA though. > > > Jarcec > > - Jarek Cecho > > > On Feb. 25, 2014, 3:24 a.m., David Robson wrote: > > > > ----------------------------------------------------------- > > This is an automatically generated e-mail. To reply, visit: > > https://reviews.apache.org/r/18452/ > > ----------------------------------------------------------- > > > > (Updated Feb. 25, 2014, 3:24 a.m.) > > > > > > Review request for Sqoop. > > > > > > Bugs: SQOOP-1287 > > https://issues.apache.org/jira/browse/SQOOP-1287 > > > > > > Repository: sqoop-trunk > > > > > > Description > > ------- > > > > Dell Software is contributing an Oracle connector for the Sqoop project. > > This is an initial patch to get early feedback - it is not finished. At the > > moment it is just the code itself - no tests or documentation. > > There is still more work to do in the code - checkstyle and findbugs has > > not been resolved as yet. > > > > > > Diffs > > ----- > > > > src/java/org/apache/sqoop/ConnFactory.java 61d3307 > > src/java/org/apache/sqoop/manager/oracle/OraOopConnManager.java > > PRE-CREATION > > src/java/org/apache/sqoop/manager/oracle/OraOopConstants.java > > PRE-CREATION > > src/java/org/apache/sqoop/manager/oracle/OraOopDBInputSplit.java > > PRE-CREATION > > src/java/org/apache/sqoop/manager/oracle/OraOopDBRecordReader.java > > PRE-CREATION > > > > src/java/org/apache/sqoop/manager/oracle/OraOopDataDrivenDBInputFormat.java > > PRE-CREATION > > src/java/org/apache/sqoop/manager/oracle/OraOopGenerics.java PRE-CREATION > > src/java/org/apache/sqoop/manager/oracle/OraOopJdbcUrl.java PRE-CREATION > > src/java/org/apache/sqoop/manager/oracle/OraOopLog.java PRE-CREATION > > src/java/org/apache/sqoop/manager/oracle/OraOopLogFactory.java > > PRE-CREATION > > src/java/org/apache/sqoop/manager/oracle/OraOopLogMessage.java > > PRE-CREATION > > src/java/org/apache/sqoop/manager/oracle/OraOopManagerFactory.java > > PRE-CREATION > > src/java/org/apache/sqoop/manager/oracle/OraOopOracleDataChunk.java > > PRE-CREATION > > src/java/org/apache/sqoop/manager/oracle/OraOopOracleDataChunkExtent.java > > PRE-CREATION > > > > src/java/org/apache/sqoop/manager/oracle/OraOopOracleDataChunkPartition.java > > PRE-CREATION > > src/java/org/apache/sqoop/manager/oracle/OraOopOracleQueries.java > > PRE-CREATION > > src/java/org/apache/sqoop/manager/oracle/OraOopOutputFormatBase.java > > PRE-CREATION > > src/java/org/apache/sqoop/manager/oracle/OraOopOutputFormatInsert.java > > PRE-CREATION > > src/java/org/apache/sqoop/manager/oracle/OraOopOutputFormatUpdate.java > > PRE-CREATION > > src/java/org/apache/sqoop/manager/oracle/OraOopUtilities.java > > PRE-CREATION > > src/java/org/apache/sqoop/manager/oracle/OracleActiveInstance.java > > PRE-CREATION > > src/java/org/apache/sqoop/manager/oracle/OracleConnectionFactory.java > > PRE-CREATION > > src/java/org/apache/sqoop/manager/oracle/OracleTable.java PRE-CREATION > > src/java/org/apache/sqoop/manager/oracle/OracleTableColumn.java > > PRE-CREATION > > src/java/org/apache/sqoop/manager/oracle/OracleTableColumns.java > > PRE-CREATION > > src/java/org/apache/sqoop/manager/oracle/OracleTablePartition.java > > PRE-CREATION > > src/java/org/apache/sqoop/manager/oracle/OracleTablePartitions.java > > PRE-CREATION > > src/java/org/apache/sqoop/manager/oracle/OracleVersion.java PRE-CREATION > > > > Diff: https://reviews.apache.org/r/18452/diff/ > > > > > > Testing > > ------- > > > > > > Thanks, > > > > David Robson > > > > >
signature.asc
Description: Digital signature
