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 32415: GenericJdbcConnector : ToJobConfiguration and Loader enhancements for Incremental write ( INSERT or UPDATE)
Date Tue, 24 Mar 2015 16:05:49 GMT

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32415/#review77562
-----------------------------------------------------------


Thank you for the patch Veena. I have couple of comments:


connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcToInitializer.java
<https://reviews.apache.org/r/32415/#comment125703>

    I think that we should expose ability for user to specify an updateColumn in case that
the table doesn't have a primary key. I know that it sounds unlikely, but I've seen a lot
of users that actually had such environments and need to export data into them.
    
    Also I think that we should add support for composite primary keys / update columns (e.g.
if we need to identify the update row based on multiple columns).



connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/configuration/IncrementalWriteConfig.java
<https://reviews.apache.org/r/32415/#comment125683>

    Missing header file.



connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/configuration/IncrementalWriteConfig.java
<https://reviews.apache.org/r/32415/#comment125682>

    I believe that USER_ONLY is the default value right? So we perhaps don't need to specify
it explicitly?



connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/configuration/IncrementalWriteMode.java
<https://reviews.apache.org/r/32415/#comment125684>

    Missing header file



connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/configuration/IncrementalWriteMode.java
<https://reviews.apache.org/r/32415/#comment125686>

    It seems that the UPSERT is not implemented yet, so I would suggest to not expose such
configuration option to the user and "comment it out" similarly as we have for the "DELETE"
example.
    
    (unless you will add it in subsequent patch as mentioned on the JIRA).


Jarcec

- Jarek Cecho


On March 23, 2015, 9:15 p.m., Veena Basavaraj wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32415/
> -----------------------------------------------------------
> 
> (Updated March 23, 2015, 9:15 p.m.)
> 
> 
> Review request for Sqoop and Jarek Cecho.
> 
> 
> Bugs: SQOOP-1810
>     https://issues.apache.org/jira/browse/SQOOP-1810
> 
> 
> Repository: sqoop-sqoop2
> 
> 
> Description
> -------
> 
> see JIRA for details on tests
> 
> 
> Diffs
> -----
> 
>   common/src/main/java/org/apache/sqoop/error/code/GenericJdbcConnectorError.java f18acbd

>   connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcExecutor.java
5af34a5 
>   connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcLoader.java
ab1ac86 
>   connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcToInitializer.java
400c0f2 
>   connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/configuration/IncrementalWriteConfig.java
PRE-CREATION 
>   connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/configuration/IncrementalWriteMode.java
PRE-CREATION 
>   connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/configuration/ToJobConfiguration.java
fd5d54b 
>   connector/connector-generic-jdbc/src/main/resources/generic-jdbc-connector-config.properties
52bf631 
> 
> Diff: https://reviews.apache.org/r/32415/diff/
> 
> 
> Testing
> -------
> 
> see JIRA for details on tests
> 
> 
> Thanks,
> 
> Veena Basavaraj
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message