You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sqoop.apache.org by Bilung Lee <bl...@gmail.com> on 2011/08/23 01:20:55 UTC

Review Request: SQOOP-321 Support date/time columns for "--incremental append" option

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

Review request for Sqoop, Ahmed Radwan and Arvind Prabhakar.


Summary
-------

Currently, when "--incremental append" option is used, only numeric columns are supported for check column.


This addresses bug SQOOP-321.
    https://issues.apache.org/jira/browse/SQOOP-321


Diffs
-----

  src/java/com/cloudera/sqoop/manager/ConnManager.java cea0a73 
  src/java/com/cloudera/sqoop/manager/OracleManager.java bad45dc 
  src/java/com/cloudera/sqoop/tool/ImportTool.java 66e60bd 
  src/test/com/cloudera/sqoop/TestIncrementalImport.java 96d4309 

Diff: https://reviews.apache.org/r/1623/diff


Testing
-------


Thanks,

Bilung


Re: Review Request: SQOOP-321 Support date/time columns for "--incremental append" option

Posted by Bilung Lee <bl...@gmail.com>.

> On 2011-08-24 03:29:53, Arvind Prabhakar wrote:
> > src/java/com/cloudera/sqoop/manager/ConnManager.java, lines 300-301
> > <https://reviews.apache.org/r/1623/diff/1/?file=34386#file34386line300>
> >
> >     This may be a compatibility issue for third party connectors. It is best to leave this method as is and introduce a new method that gets called when the check column type is not numeric.

Existing method is kept in new patch. Thanks!


- Bilung


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


On 2011-08-25 01:57:36, Bilung Lee wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/1623/
> -----------------------------------------------------------
> 
> (Updated 2011-08-25 01:57:36)
> 
> 
> Review request for Sqoop, Ahmed Radwan and Arvind Prabhakar.
> 
> 
> Summary
> -------
> 
> Currently, when "--incremental append" option is used, only numeric columns are supported for check column.
> 
> 
> This addresses bug SQOOP-321.
>     https://issues.apache.org/jira/browse/SQOOP-321
> 
> 
> Diffs
> -----
> 
>   src/java/com/cloudera/sqoop/manager/ConnManager.java cea0a73 
>   src/java/com/cloudera/sqoop/manager/OracleManager.java bad45dc 
>   src/java/com/cloudera/sqoop/tool/ImportTool.java 66e60bd 
>   src/test/com/cloudera/sqoop/TestIncrementalImport.java 96d4309 
> 
> Diff: https://reviews.apache.org/r/1623/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Bilung
> 
>


Re: Review Request: SQOOP-321 Support date/time columns for "--incremental append" option

Posted by Arvind Prabhakar <ar...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/1623/#review1610
-----------------------------------------------------------


Changes look good Bilung. I do have a backward compatibility concern noted below. Other than that, it will be good to give some examples of use of this feature in the user guide. The more specific examples you can give, the better it would be for the users.


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

    This may be a compatibility issue for third party connectors. It is best to leave this method as is and introduce a new method that gets called when the check column type is not numeric.


- Arvind


On 2011-08-22 23:20:55, Bilung Lee wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/1623/
> -----------------------------------------------------------
> 
> (Updated 2011-08-22 23:20:55)
> 
> 
> Review request for Sqoop, Ahmed Radwan and Arvind Prabhakar.
> 
> 
> Summary
> -------
> 
> Currently, when "--incremental append" option is used, only numeric columns are supported for check column.
> 
> 
> This addresses bug SQOOP-321.
>     https://issues.apache.org/jira/browse/SQOOP-321
> 
> 
> Diffs
> -----
> 
>   src/java/com/cloudera/sqoop/manager/ConnManager.java cea0a73 
>   src/java/com/cloudera/sqoop/manager/OracleManager.java bad45dc 
>   src/java/com/cloudera/sqoop/tool/ImportTool.java 66e60bd 
>   src/test/com/cloudera/sqoop/TestIncrementalImport.java 96d4309 
> 
> Diff: https://reviews.apache.org/r/1623/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Bilung
> 
>


Re: Review Request: SQOOP-321 Support date/time columns for "--incremental append" option

Posted by Arvind Prabhakar <ar...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/1623/#review1633
-----------------------------------------------------------

Ship it!


+1

- Arvind


On 2011-08-25 01:57:36, Bilung Lee wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/1623/
> -----------------------------------------------------------
> 
> (Updated 2011-08-25 01:57:36)
> 
> 
> Review request for Sqoop, Ahmed Radwan and Arvind Prabhakar.
> 
> 
> Summary
> -------
> 
> Currently, when "--incremental append" option is used, only numeric columns are supported for check column.
> 
> 
> This addresses bug SQOOP-321.
>     https://issues.apache.org/jira/browse/SQOOP-321
> 
> 
> Diffs
> -----
> 
>   src/java/com/cloudera/sqoop/manager/ConnManager.java cea0a73 
>   src/java/com/cloudera/sqoop/manager/OracleManager.java bad45dc 
>   src/java/com/cloudera/sqoop/tool/ImportTool.java 66e60bd 
>   src/test/com/cloudera/sqoop/TestIncrementalImport.java 96d4309 
> 
> Diff: https://reviews.apache.org/r/1623/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Bilung
> 
>


Re: Review Request: SQOOP-321 Support date/time columns for "--incremental append" option

Posted by Arvind Prabhakar <ar...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/1623/#review1634
-----------------------------------------------------------

Ship it!


+1

- Arvind


On 2011-08-25 01:57:36, Bilung Lee wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/1623/
> -----------------------------------------------------------
> 
> (Updated 2011-08-25 01:57:36)
> 
> 
> Review request for Sqoop, Ahmed Radwan and Arvind Prabhakar.
> 
> 
> Summary
> -------
> 
> Currently, when "--incremental append" option is used, only numeric columns are supported for check column.
> 
> 
> This addresses bug SQOOP-321.
>     https://issues.apache.org/jira/browse/SQOOP-321
> 
> 
> Diffs
> -----
> 
>   src/java/com/cloudera/sqoop/manager/ConnManager.java cea0a73 
>   src/java/com/cloudera/sqoop/manager/OracleManager.java bad45dc 
>   src/java/com/cloudera/sqoop/tool/ImportTool.java 66e60bd 
>   src/test/com/cloudera/sqoop/TestIncrementalImport.java 96d4309 
> 
> Diff: https://reviews.apache.org/r/1623/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Bilung
> 
>


Re: Review Request: SQOOP-321 Support date/time columns for "--incremental append" option

Posted by Bilung Lee <bl...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/1623/
-----------------------------------------------------------

(Updated 2011-08-25 01:57:36.357578)


Review request for Sqoop, Ahmed Radwan and Arvind Prabhakar.


Summary
-------

Currently, when "--incremental append" option is used, only numeric columns are supported for check column.


This addresses bug SQOOP-321.
    https://issues.apache.org/jira/browse/SQOOP-321


Diffs (updated)
-----

  src/java/com/cloudera/sqoop/manager/ConnManager.java cea0a73 
  src/java/com/cloudera/sqoop/manager/OracleManager.java bad45dc 
  src/java/com/cloudera/sqoop/tool/ImportTool.java 66e60bd 
  src/test/com/cloudera/sqoop/TestIncrementalImport.java 96d4309 

Diff: https://reviews.apache.org/r/1623/diff


Testing
-------


Thanks,

Bilung