sqoop-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jarek Cecho <jar...@apache.org>
Subject Re: Review Request 18452: Add high performance Oracle connector into Sqoop
Date Thu, 27 Feb 2014 15:15:27 GMT
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:noreply@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
> > 
> >
> 

Mime
View raw message