You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sqoop.apache.org by Raghav Gautam <ra...@gmail.com> on 2013/06/26 15:21:24 UTC
Review Request 12032: Incremental import using lastmodified mode always
assumes column type to be timestamp, this patch fixes that
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12032/
-----------------------------------------------------------
Review request for Sqoop.
Bugs: SQOOP-906
https://issues.apache.org/jira/browse/SQOOP-906
Repository: sqoop-trunk
Description
-------
Incremental import using lastmodified mode always assumes column type to be TIMESTAMP which is causing issues with Oracle Connector. This patch fixes that.
Diffs
-----
src/java/org/apache/sqoop/manager/ConnManager.java c9e05da
src/java/org/apache/sqoop/manager/OracleManager.java edc888e
src/java/org/apache/sqoop/tool/ImportTool.java cb800b6
Diff: https://reviews.apache.org/r/12032/diff/
Testing
-------
Unit tests & oracle third party tests are passing. Also tested manually.
Thanks,
Raghav Gautam
Re: Review Request 12032: Incremental import using lastmodified mode always
assumes column type to be timestamp, this patch fixes that
Posted by Jarek Cecho <ja...@apache.org>.
> On June 26, 2013, 7 p.m., Jarek Cecho wrote:
> > Hi Raghav,
> > thank you very much for taking up this JIRA, appreciated! I'm wondering what is the reasoning for introducing a new query in the Oracle manager to retrive the check column type when the type has been already retrieved and seems to be stored in checkColumnType variable [1]? It seems to me that the real problem here is that the code in the ImportTool is simply overriding this type to Timestamp on [2] before passing it to the datetimeToQueryString [3]. Did you by any chance experiment with removing this line?
> >
> > It would be also great if you could include a new test for this, so that we can be sure that we won't break this in the future again.
> >
> > Jarcec
> >
> > Links:
> > 1: https://github.com/apache/sqoop/blob/trunk/src/java/org/apache/sqoop/tool/ImportTool.java#L70
> > 2: https://github.com/apache/sqoop/blob/trunk/src/java/org/apache/sqoop/tool/ImportTool.java#L286
> > 3: https://github.com/apache/sqoop/blob/trunk/src/java/org/apache/sqoop/manager/OracleManager.java#L578
>
> Raghav Gautam wrote:
> Hi Jarek,
> checkColumnType is a private variable which gets assigned in method getMaxColumnId.
> https://github.com/apache/sqoop/blob/trunk/src/java/org/apache/sqoop/tool/ImportTool.java#L165
> Since, this method does not get called in case incremental mode is lastmodified. My changes preserve existing behavior for the most part - changing it only for Oracle. Hence, I have added a new method to the OracleManager.
>
> I have added a test as per your comment.
>
> Thanks,
> Raghav.
Thank you Raghav for the explanation, it make complete sense to me! I feel that introducing new method into the ConnManager class for retrieving one column type is a bit of overhaul when we can do simply manager.getColumnTypes(table, query).get(options.getIncrementalTestColumn()) [1] to achieve the same with existing code. Is there a particular reason why you've choose implementing a new method instead?
Links:
1: https://github.com/apache/sqoop/blob/trunk/src/java/org/apache/sqoop/manager/ConnManager.java#L342
- Jarek
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12032/#review22413
-----------------------------------------------------------
On June 30, 2013, 2:18 p.m., Raghav Gautam wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/12032/
> -----------------------------------------------------------
>
> (Updated June 30, 2013, 2:18 p.m.)
>
>
> Review request for Sqoop.
>
>
> Bugs: SQOOP-906
> https://issues.apache.org/jira/browse/SQOOP-906
>
>
> Repository: sqoop-trunk
>
>
> Description
> -------
>
> Incremental import using lastmodified mode always assumes column type to be TIMESTAMP which is causing issues with Oracle Connector. This patch fixes that.
>
>
> Diffs
> -----
>
> src/java/org/apache/sqoop/manager/ConnManager.java c9e05da
> src/java/org/apache/sqoop/manager/OracleManager.java edc888e
> src/java/org/apache/sqoop/tool/ImportTool.java cb800b6
> src/test/com/cloudera/sqoop/manager/OracleIncrementalImportTest.java PRE-CREATION
> src/test/com/cloudera/sqoop/testutil/BaseSqoopTestCase.java 877d7f8
>
> Diff: https://reviews.apache.org/r/12032/diff/
>
>
> Testing
> -------
>
> Unit tests & oracle third party tests are passing. Also tested manually.
>
>
> Thanks,
>
> Raghav Gautam
>
>
Re: Review Request 12032: Incremental import using lastmodified mode always
assumes column type to be timestamp, this patch fixes that
Posted by Raghav Gautam <ra...@gmail.com>.
> On June 26, 2013, 7 p.m., Jarek Cecho wrote:
> > Hi Raghav,
> > thank you very much for taking up this JIRA, appreciated! I'm wondering what is the reasoning for introducing a new query in the Oracle manager to retrive the check column type when the type has been already retrieved and seems to be stored in checkColumnType variable [1]? It seems to me that the real problem here is that the code in the ImportTool is simply overriding this type to Timestamp on [2] before passing it to the datetimeToQueryString [3]. Did you by any chance experiment with removing this line?
> >
> > It would be also great if you could include a new test for this, so that we can be sure that we won't break this in the future again.
> >
> > Jarcec
> >
> > Links:
> > 1: https://github.com/apache/sqoop/blob/trunk/src/java/org/apache/sqoop/tool/ImportTool.java#L70
> > 2: https://github.com/apache/sqoop/blob/trunk/src/java/org/apache/sqoop/tool/ImportTool.java#L286
> > 3: https://github.com/apache/sqoop/blob/trunk/src/java/org/apache/sqoop/manager/OracleManager.java#L578
Hi Jarek,
checkColumnType is a private variable which gets assigned in method getMaxColumnId.
https://github.com/apache/sqoop/blob/trunk/src/java/org/apache/sqoop/tool/ImportTool.java#L165
Since, this method does not get called in case incremental mode is lastmodified. My changes preserve existing behavior for the most part - changing it only for Oracle. Hence, I have added a new method to the OracleManager.
I have added a test as per your comment.
Thanks,
Raghav.
- Raghav
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12032/#review22413
-----------------------------------------------------------
On June 30, 2013, 2:18 p.m., Raghav Gautam wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/12032/
> -----------------------------------------------------------
>
> (Updated June 30, 2013, 2:18 p.m.)
>
>
> Review request for Sqoop.
>
>
> Bugs: SQOOP-906
> https://issues.apache.org/jira/browse/SQOOP-906
>
>
> Repository: sqoop-trunk
>
>
> Description
> -------
>
> Incremental import using lastmodified mode always assumes column type to be TIMESTAMP which is causing issues with Oracle Connector. This patch fixes that.
>
>
> Diffs
> -----
>
> src/java/org/apache/sqoop/manager/ConnManager.java c9e05da
> src/java/org/apache/sqoop/manager/OracleManager.java edc888e
> src/java/org/apache/sqoop/tool/ImportTool.java cb800b6
> src/test/com/cloudera/sqoop/manager/OracleIncrementalImportTest.java PRE-CREATION
> src/test/com/cloudera/sqoop/testutil/BaseSqoopTestCase.java 877d7f8
>
> Diff: https://reviews.apache.org/r/12032/diff/
>
>
> Testing
> -------
>
> Unit tests & oracle third party tests are passing. Also tested manually.
>
>
> Thanks,
>
> Raghav Gautam
>
>
Re: Review Request 12032: Incremental import using lastmodified mode always
assumes column type to be timestamp, this patch fixes that
Posted by Jarek Cecho <ja...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12032/#review22413
-----------------------------------------------------------
Hi Raghav,
thank you very much for taking up this JIRA, appreciated! I'm wondering what is the reasoning for introducing a new query in the Oracle manager to retrive the check column type when the type has been already retrieved and seems to be stored in checkColumnType variable [1]? It seems to me that the real problem here is that the code in the ImportTool is simply overriding this type to Timestamp on [2] before passing it to the datetimeToQueryString [3]. Did you by any chance experiment with removing this line?
It would be also great if you could include a new test for this, so that we can be sure that we won't break this in the future again.
Jarcec
Links:
1: https://github.com/apache/sqoop/blob/trunk/src/java/org/apache/sqoop/tool/ImportTool.java#L70
2: https://github.com/apache/sqoop/blob/trunk/src/java/org/apache/sqoop/tool/ImportTool.java#L286
3: https://github.com/apache/sqoop/blob/trunk/src/java/org/apache/sqoop/manager/OracleManager.java#L578
- Jarek Cecho
On June 26, 2013, 1:21 p.m., Raghav Gautam wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/12032/
> -----------------------------------------------------------
>
> (Updated June 26, 2013, 1:21 p.m.)
>
>
> Review request for Sqoop.
>
>
> Bugs: SQOOP-906
> https://issues.apache.org/jira/browse/SQOOP-906
>
>
> Repository: sqoop-trunk
>
>
> Description
> -------
>
> Incremental import using lastmodified mode always assumes column type to be TIMESTAMP which is causing issues with Oracle Connector. This patch fixes that.
>
>
> Diffs
> -----
>
> src/java/org/apache/sqoop/manager/ConnManager.java c9e05da
> src/java/org/apache/sqoop/manager/OracleManager.java edc888e
> src/java/org/apache/sqoop/tool/ImportTool.java cb800b6
>
> Diff: https://reviews.apache.org/r/12032/diff/
>
>
> Testing
> -------
>
> Unit tests & oracle third party tests are passing. Also tested manually.
>
>
> Thanks,
>
> Raghav Gautam
>
>
Re: Review Request 12032: Incremental import using lastmodified mode always
assumes column type to be timestamp, this patch fixes that
Posted by Raghav Gautam <ra...@gmail.com>.
> On July 11, 2013, 3:24 p.m., Jarek Cecho wrote:
> > src/java/org/apache/sqoop/tool/ImportTool.java, line 286
> > <https://reviews.apache.org/r/12032/diff/5/?file=319942#file319942line286>
> >
> > This can be further simplified to manager.getColumnTypes(options.getTableName(), options.getSqlQuery()).get(options.getIncrementalTestColumn()) and we do not even need to touch the ConnManager, right?
The purpose of keeping manager.getLastModifiedCheckColumnType() is to let the existing connection managers be able to go back to the old behavior easily if they decide to do so.
- Raghav
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12032/#review23031
-----------------------------------------------------------
On July 10, 2013, 11:33 a.m., Raghav Gautam wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/12032/
> -----------------------------------------------------------
>
> (Updated July 10, 2013, 11:33 a.m.)
>
>
> Review request for Sqoop.
>
>
> Bugs: SQOOP-906
> https://issues.apache.org/jira/browse/SQOOP-906
>
>
> Repository: sqoop-trunk
>
>
> Description
> -------
>
> Incremental import using lastmodified mode always assumes column type to be TIMESTAMP which is causing issues with Oracle Connector. This patch fixes that.
>
>
> Diffs
> -----
>
> src/java/org/apache/sqoop/manager/ConnManager.java c84c859
> src/java/org/apache/sqoop/manager/OracleManager.java 686bc19
> src/java/org/apache/sqoop/tool/ImportTool.java cb800b6
> src/test/com/cloudera/sqoop/TestIncrementalImport.java 02080df
> src/test/com/cloudera/sqoop/TestMerge.java 5010cf2
> src/test/com/cloudera/sqoop/ThirdPartyTests.java ada5c72
> src/test/com/cloudera/sqoop/testutil/BaseSqoopTestCase.java 877d7f8
> src/test/org/apache/sqoop/manager/oracle/OracleIncrementalImportTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/12032/diff/
>
>
> Testing
> -------
>
> Unit tests & oracle third party tests are passing. Also tested manually.
>
>
> Thanks,
>
> Raghav Gautam
>
>
Re: Review Request 12032: Incremental import using lastmodified mode always
assumes column type to be timestamp, this patch fixes that
Posted by Jarek Cecho <ja...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12032/#review23031
-----------------------------------------------------------
Hi Raghav, thank you very much for incorporating my suggestion, greatly appreciated! I think that we can further simplify the change by the following:
src/java/org/apache/sqoop/tool/ImportTool.java
<https://reviews.apache.org/r/12032/#comment46846>
This can be further simplified to manager.getColumnTypes(options.getTableName(), options.getSqlQuery()).get(options.getIncrementalTestColumn()) and we do not even need to touch the ConnManager, right?
Jarcec
- Jarek Cecho
On July 10, 2013, 6:33 p.m., Raghav Gautam wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/12032/
> -----------------------------------------------------------
>
> (Updated July 10, 2013, 6:33 p.m.)
>
>
> Review request for Sqoop.
>
>
> Bugs: SQOOP-906
> https://issues.apache.org/jira/browse/SQOOP-906
>
>
> Repository: sqoop-trunk
>
>
> Description
> -------
>
> Incremental import using lastmodified mode always assumes column type to be TIMESTAMP which is causing issues with Oracle Connector. This patch fixes that.
>
>
> Diffs
> -----
>
> src/java/org/apache/sqoop/manager/ConnManager.java c84c859
> src/java/org/apache/sqoop/manager/OracleManager.java 686bc19
> src/java/org/apache/sqoop/tool/ImportTool.java cb800b6
> src/test/com/cloudera/sqoop/TestIncrementalImport.java 02080df
> src/test/com/cloudera/sqoop/TestMerge.java 5010cf2
> src/test/com/cloudera/sqoop/ThirdPartyTests.java ada5c72
> src/test/com/cloudera/sqoop/testutil/BaseSqoopTestCase.java 877d7f8
> src/test/org/apache/sqoop/manager/oracle/OracleIncrementalImportTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/12032/diff/
>
>
> Testing
> -------
>
> Unit tests & oracle third party tests are passing. Also tested manually.
>
>
> Thanks,
>
> Raghav Gautam
>
>
Re: Review Request 12032: Incremental import using lastmodified mode always
assumes column type to be timestamp, this patch fixes that
Posted by Raghav Gautam <ra...@gmail.com>.
> On July 15, 2013, 10:21 a.m., Jarek Cecho wrote:
> > Hi Raghav, thank you very much for incorporating all my comments and suggestions. Please accept my apologies for being so nitpicky about that. The patch looks good to me, please upload the last version to the JIRA and I'll commit it.
> >
> > Jarcec
I have uploaded the patch to the JIRA. Thanks for the reviewing.
- Raghav.
- Raghav
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12032/#review23162
-----------------------------------------------------------
On July 11, 2013, 7:13 p.m., Raghav Gautam wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/12032/
> -----------------------------------------------------------
>
> (Updated July 11, 2013, 7:13 p.m.)
>
>
> Review request for Sqoop.
>
>
> Bugs: SQOOP-906
> https://issues.apache.org/jira/browse/SQOOP-906
>
>
> Repository: sqoop-trunk
>
>
> Description
> -------
>
> Incremental import using lastmodified mode always assumes column type to be TIMESTAMP which is causing issues with Oracle Connector. This patch fixes that.
>
>
> Diffs
> -----
>
> src/java/org/apache/sqoop/manager/ConnManager.java c84c859
> src/java/org/apache/sqoop/manager/OracleManager.java 686bc19
> src/java/org/apache/sqoop/tool/ImportTool.java cb800b6
> src/test/com/cloudera/sqoop/TestIncrementalImport.java 02080df
> src/test/com/cloudera/sqoop/TestMerge.java 5010cf2
> src/test/com/cloudera/sqoop/ThirdPartyTests.java ada5c72
> src/test/com/cloudera/sqoop/testutil/BaseSqoopTestCase.java 877d7f8
> src/test/org/apache/sqoop/manager/oracle/OracleIncrementalImportTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/12032/diff/
>
>
> Testing
> -------
>
> Unit tests & oracle third party tests are passing. Also tested manually.
>
>
> Thanks,
>
> Raghav Gautam
>
>
Re: Review Request 12032: Incremental import using lastmodified mode always
assumes column type to be timestamp, this patch fixes that
Posted by Jarek Cecho <ja...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12032/#review23162
-----------------------------------------------------------
Ship it!
Hi Raghav, thank you very much for incorporating all my comments and suggestions. Please accept my apologies for being so nitpicky about that. The patch looks good to me, please upload the last version to the JIRA and I'll commit it.
Jarcec
- Jarek Cecho
On July 12, 2013, 2:13 a.m., Raghav Gautam wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/12032/
> -----------------------------------------------------------
>
> (Updated July 12, 2013, 2:13 a.m.)
>
>
> Review request for Sqoop.
>
>
> Bugs: SQOOP-906
> https://issues.apache.org/jira/browse/SQOOP-906
>
>
> Repository: sqoop-trunk
>
>
> Description
> -------
>
> Incremental import using lastmodified mode always assumes column type to be TIMESTAMP which is causing issues with Oracle Connector. This patch fixes that.
>
>
> Diffs
> -----
>
> src/java/org/apache/sqoop/manager/ConnManager.java c84c859
> src/java/org/apache/sqoop/manager/OracleManager.java 686bc19
> src/java/org/apache/sqoop/tool/ImportTool.java cb800b6
> src/test/com/cloudera/sqoop/TestIncrementalImport.java 02080df
> src/test/com/cloudera/sqoop/TestMerge.java 5010cf2
> src/test/com/cloudera/sqoop/ThirdPartyTests.java ada5c72
> src/test/com/cloudera/sqoop/testutil/BaseSqoopTestCase.java 877d7f8
> src/test/org/apache/sqoop/manager/oracle/OracleIncrementalImportTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/12032/diff/
>
>
> Testing
> -------
>
> Unit tests & oracle third party tests are passing. Also tested manually.
>
>
> Thanks,
>
> Raghav Gautam
>
>
Re: Review Request 12032: Incremental import using lastmodified mode always
assumes column type to be timestamp, this patch fixes that
Posted by Raghav Gautam <ra...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12032/
-----------------------------------------------------------
(Updated July 11, 2013, 7:13 p.m.)
Review request for Sqoop.
Changes
-------
Removed manager.getLastModifiedCheckColumnType() as per review comment.
Bugs: SQOOP-906
https://issues.apache.org/jira/browse/SQOOP-906
Repository: sqoop-trunk
Description
-------
Incremental import using lastmodified mode always assumes column type to be TIMESTAMP which is causing issues with Oracle Connector. This patch fixes that.
Diffs (updated)
-----
src/java/org/apache/sqoop/manager/ConnManager.java c84c859
src/java/org/apache/sqoop/manager/OracleManager.java 686bc19
src/java/org/apache/sqoop/tool/ImportTool.java cb800b6
src/test/com/cloudera/sqoop/TestIncrementalImport.java 02080df
src/test/com/cloudera/sqoop/TestMerge.java 5010cf2
src/test/com/cloudera/sqoop/ThirdPartyTests.java ada5c72
src/test/com/cloudera/sqoop/testutil/BaseSqoopTestCase.java 877d7f8
src/test/org/apache/sqoop/manager/oracle/OracleIncrementalImportTest.java PRE-CREATION
Diff: https://reviews.apache.org/r/12032/diff/
Testing
-------
Unit tests & oracle third party tests are passing. Also tested manually.
Thanks,
Raghav Gautam
Re: Review Request 12032: Incremental import using lastmodified mode always
assumes column type to be timestamp, this patch fixes that
Posted by Raghav Gautam <ra...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12032/
-----------------------------------------------------------
(Updated July 10, 2013, 6:33 p.m.)
Review request for Sqoop.
Changes
-------
Here is a short summary of the changes:
1. Changes in ConnManager deal with getting the type of checkcolumn and making sure that its type is either timestamp or date
2. Changes in OracleManager truncates the millisecond part of the time string, without this oracle gives error
3. ImportTool changes: remove the hardcoding and use connection manager to get the type of the checkcolumn
4. changes to TestIncrementalImport, TestMerge just uppercase the name of the checkcolumn - because implementation of getLastModifiedCheckColumn in ConnManager.java is sensitive to case, this is required
5. OracleIncrementalImportTest is new test added
6. Changes to BaseSqoopTest.java generalizes the function createTableWithColTypesAndNames - earlier it could take only single data row for populating the table; with my modifications it take take multiple data rows.
Bugs: SQOOP-906
https://issues.apache.org/jira/browse/SQOOP-906
Repository: sqoop-trunk
Description
-------
Incremental import using lastmodified mode always assumes column type to be TIMESTAMP which is causing issues with Oracle Connector. This patch fixes that.
Diffs (updated)
-----
src/java/org/apache/sqoop/manager/ConnManager.java c84c859
src/java/org/apache/sqoop/manager/OracleManager.java 686bc19
src/java/org/apache/sqoop/tool/ImportTool.java cb800b6
src/test/com/cloudera/sqoop/TestIncrementalImport.java 02080df
src/test/com/cloudera/sqoop/TestMerge.java 5010cf2
src/test/com/cloudera/sqoop/ThirdPartyTests.java ada5c72
src/test/com/cloudera/sqoop/testutil/BaseSqoopTestCase.java 877d7f8
src/test/org/apache/sqoop/manager/oracle/OracleIncrementalImportTest.java PRE-CREATION
Diff: https://reviews.apache.org/r/12032/diff/
Testing
-------
Unit tests & oracle third party tests are passing. Also tested manually.
Thanks,
Raghav Gautam
Re: Review Request 12032: Incremental import using lastmodified mode always
assumes column type to be timestamp, this patch fixes that
Posted by Jarek Cecho <ja...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12032/#review22925
-----------------------------------------------------------
Hi Raghav,
thank you very much for the explanation. I do understand the desire to keep backward compatibility. Overriding column type to a TIMESTAMP every time seems to me to as clear bug that just happen to surface when the Oracle Connector is in use. Therefore I would suggest to fix that bug rather then delegating fix to the connector.
Jarcec
- Jarek Cecho
On July 1, 2013, 8:02 p.m., Raghav Gautam wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/12032/
> -----------------------------------------------------------
>
> (Updated July 1, 2013, 8:02 p.m.)
>
>
> Review request for Sqoop.
>
>
> Bugs: SQOOP-906
> https://issues.apache.org/jira/browse/SQOOP-906
>
>
> Repository: sqoop-trunk
>
>
> Description
> -------
>
> Incremental import using lastmodified mode always assumes column type to be TIMESTAMP which is causing issues with Oracle Connector. This patch fixes that.
>
>
> Diffs
> -----
>
> src/java/org/apache/sqoop/manager/ConnManager.java c9e05da
> src/java/org/apache/sqoop/manager/OracleManager.java edc888e
> src/java/org/apache/sqoop/tool/ImportTool.java cb800b6
> src/test/com/cloudera/sqoop/manager/OracleIncrementalImportTest.java PRE-CREATION
> src/test/com/cloudera/sqoop/testutil/BaseSqoopTestCase.java 877d7f8
>
> Diff: https://reviews.apache.org/r/12032/diff/
>
>
> Testing
> -------
>
> Unit tests & oracle third party tests are passing. Also tested manually.
>
>
> Thanks,
>
> Raghav Gautam
>
>
Re: Review Request 12032: Incremental import using lastmodified mode always
assumes column type to be timestamp, this patch fixes that
Posted by Raghav Gautam <ra...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12032/
-----------------------------------------------------------
(Updated July 1, 2013, 8:02 p.m.)
Review request for Sqoop.
Changes
-------
Hi Jarek,
I had introduced new method to retain old behavior for all connectors and overridden it in OracleManager to make sure that the change happens only for Oracle. I have reimplemented the method to use getColumnTypes method. Thanks for suggesting manager.getColumnTypes, I was able to use it to greatly simplify the code changes. I have updated the patch.
Bugs: SQOOP-906
https://issues.apache.org/jira/browse/SQOOP-906
Repository: sqoop-trunk
Description
-------
Incremental import using lastmodified mode always assumes column type to be TIMESTAMP which is causing issues with Oracle Connector. This patch fixes that.
Diffs (updated)
-----
src/java/org/apache/sqoop/manager/ConnManager.java c9e05da
src/java/org/apache/sqoop/manager/OracleManager.java edc888e
src/java/org/apache/sqoop/tool/ImportTool.java cb800b6
src/test/com/cloudera/sqoop/manager/OracleIncrementalImportTest.java PRE-CREATION
src/test/com/cloudera/sqoop/testutil/BaseSqoopTestCase.java 877d7f8
Diff: https://reviews.apache.org/r/12032/diff/
Testing
-------
Unit tests & oracle third party tests are passing. Also tested manually.
Thanks,
Raghav Gautam
Re: Review Request 12032: Incremental import using lastmodified mode always
assumes column type to be timestamp, this patch fixes that
Posted by Raghav Gautam <ra...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12032/
-----------------------------------------------------------
(Updated June 30, 2013, 2:18 p.m.)
Review request for Sqoop.
Changes
-------
Adding OracleIncrementalImportTest.
Bugs: SQOOP-906
https://issues.apache.org/jira/browse/SQOOP-906
Repository: sqoop-trunk
Description
-------
Incremental import using lastmodified mode always assumes column type to be TIMESTAMP which is causing issues with Oracle Connector. This patch fixes that.
Diffs (updated)
-----
src/java/org/apache/sqoop/manager/ConnManager.java c9e05da
src/java/org/apache/sqoop/manager/OracleManager.java edc888e
src/java/org/apache/sqoop/tool/ImportTool.java cb800b6
src/test/com/cloudera/sqoop/manager/OracleIncrementalImportTest.java PRE-CREATION
src/test/com/cloudera/sqoop/testutil/BaseSqoopTestCase.java 877d7f8
Diff: https://reviews.apache.org/r/12032/diff/
Testing
-------
Unit tests & oracle third party tests are passing. Also tested manually.
Thanks,
Raghav Gautam