sqoop-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From jmhs...@apache.org
Subject Re: Review Request: SQOOP-327 Mixed update/insert export support for OracleManager
Date Tue, 06 Sep 2011 07:45:14 GMT

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


I'm -0 about the flags/user interface.  Note, I did not carefully review implementation. 



src/docs/user/export.txt
<https://reviews.apache.org/r/1717/#comment4004>

    which mode is default?  
    
    maybe make it --export-mode with options 'update', 'insert', or 'upsert'?  Or maybe just
have --update, --insert and --upsert?



src/docs/user/export.txt
<https://reviews.apache.org/r/1717/#comment4001>

    this is confusing.  reword to be positive?
    
    I think 'mixed' mode means "insert or update" while "updateonly" means update only.  maybe
change 'mixed' to 'allowinsert'?



src/java/com/cloudera/sqoop/manager/ConnManager.java
<https://reviews.apache.org/r/1717/#comment4002>

    maybe you should have a '--update' and '--upsert' options instead of introducing modality?



src/test/com/cloudera/sqoop/manager/OracleExportTest.java
<https://reviews.apache.org/r/1717/#comment4003>

    Are you upserting the same values? would it make more sense to put new values in and verify
them?
    
    Also, would it make sense to have some overlapping range to prove that some are updated
and some are inserted?


- jmhsieh


On 2011-09-06 00:02:42, Bilung Lee wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/1717/
> -----------------------------------------------------------
> 
> (Updated 2011-09-06 00:02:42)
> 
> 
> Review request for Sqoop, Ahmed Radwan and Arvind Prabhakar.
> 
> 
> Summary
> -------
> 
> A new option is introduced to allow update records if they exist in table already or
to insert records if they do not exist yet.
> 
> 
> This addresses bug SQOOP-327.
>     https://issues.apache.org/jira/browse/SQOOP-327
> 
> 
> Diffs
> -----
> 
>   src/docs/man/sqoop-export.txt 50052cc 
>   src/docs/user/export.txt 4401c26 
>   src/java/com/cloudera/sqoop/SqoopOptions.java d07aecc 
>   src/java/com/cloudera/sqoop/manager/ConnManager.java f5f5a4b 
>   src/java/com/cloudera/sqoop/manager/OracleManager.java 1d08c4d 
>   src/java/com/cloudera/sqoop/mapreduce/JdbcUpsertExportJob.java PRE-CREATION 
>   src/java/com/cloudera/sqoop/mapreduce/OracleUpsertOutputFormat.java PRE-CREATION 
>   src/java/com/cloudera/sqoop/mapreduce/UpdateOutputFormat.java d5339d9 
>   src/java/com/cloudera/sqoop/tool/BaseSqoopTool.java 879c7c8 
>   src/java/com/cloudera/sqoop/tool/ExportTool.java d156eeb 
>   src/test/com/cloudera/sqoop/manager/OracleExportTest.java 12858d7 
> 
> Diff: https://reviews.apache.org/r/1717/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Bilung
> 
>


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