Re: Review Request 61777: sqoop tries to re execute select query during import in case of a connection reset error and this is causing lots of duplicate records from source

2017-08-21 Thread Zoltán Tóth 

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

(Updated Aug. 21, 2017, 1:15 p.m.)


Review request for Sqoop.


Changes
---

Based on codereview function is separated into different methods with 
meaningful names


Bugs: SQOOP-3139
https://issues.apache.org/jira/browse/SQOOP-3139


Repository: sqoop-trunk


Description
---

In the case if the database table name and split-by parameter was different 
(eg.: Mycol vs. mycol) Sqoop couldn't continue the query from the last value if 
the connection was broken.


Diffs (updated)
-

  src/java/org/apache/sqoop/mapreduce/db/DBRecordReader.java a78eb061 
  src/java/org/apache/sqoop/mapreduce/db/SQLServerDBRecordReader.java 9a3621b0 
  src/test/org/apache/sqoop/mapreduce/db/TestSQLServerDBRecordReader.java 
PRE-CREATION 


Diff: https://reviews.apache.org/r/61777/diff/2/

Changes: https://reviews.apache.org/r/61777/diff/1-2/


Testing
---


Thanks,

Zoltán Tóth

Re: Review Request 61777: sqoop tries to re execute select query during import in case of a connection reset error and this is causing lots of duplicate records from source

2017-08-21 Thread Anna Szonyi 

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



Hi Zoltan,

Thanks for the contribution, and thanks for adding quite a few test cases, a 
few nitpicks above Szabolcs' ones.

Please take a look and let me know your thoughts.

Thanks,
Anna


src/java/org/apache/sqoop/mapreduce/db/SQLServerDBRecordReader.java
Line 76 (original), 89-91 (patched)


This comment is a little hard to understand, could you give a bit more 
specific example?



src/java/org/apache/sqoop/mapreduce/db/SQLServerDBRecordReader.java
Lines 93-99 (patched)


It might make sense to split the code into two separate methods for the 
"case sensitive match" and the "case insensitive match" cases.


- Anna Szonyi


On Aug. 21, 2017, 9:16 a.m., Zoltán Tóth wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61777/
> ---
> 
> (Updated Aug. 21, 2017, 9:16 a.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Bugs: SQOOP-3139
> https://issues.apache.org/jira/browse/SQOOP-3139
> 
> 
> Repository: sqoop-trunk
> 
> 
> Description
> ---
> 
> In the case if the database table name and split-by parameter was different 
> (eg.: Mycol vs. mycol) Sqoop couldn't continue the query from the last value 
> if the connection was broken.
> 
> 
> Diffs
> -
> 
>   src/java/org/apache/sqoop/mapreduce/db/DBRecordReader.java a78eb061 
>   src/java/org/apache/sqoop/mapreduce/db/SQLServerDBRecordReader.java 
> 9a3621b0 
>   src/test/org/apache/sqoop/mapreduce/db/TestSQLServerDBRecordReader.java 
> PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/61777/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zoltán Tóth
> 
>

Re: Review Request 61777: sqoop tries to re execute select query during import in case of a connection reset error and this is causing lots of duplicate records from source

2017-08-21 Thread Szabolcs Vasas 

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




src/test/org/apache/sqoop/mapreduce/db/TestSQLServerDBRecordReader.java
Lines 33 (patched)


We try to avoid star imports, can you please organize this import 
accordingly?



src/test/org/apache/sqoop/mapreduce/db/TestSQLServerDBRecordReader.java
Lines 39 (patched)


Nit: can we use SPLIT_BY_COLUMN.toUpperCase() here?


- Szabolcs Vasas


On Aug. 21, 2017, 9:16 a.m., Zoltán Tóth wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61777/
> ---
> 
> (Updated Aug. 21, 2017, 9:16 a.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Bugs: SQOOP-3139
> https://issues.apache.org/jira/browse/SQOOP-3139
> 
> 
> Repository: sqoop-trunk
> 
> 
> Description
> ---
> 
> In the case if the database table name and split-by parameter was different 
> (eg.: Mycol vs. mycol) Sqoop couldn't continue the query from the last value 
> if the connection was broken.
> 
> 
> Diffs
> -
> 
>   src/java/org/apache/sqoop/mapreduce/db/DBRecordReader.java a78eb061 
>   src/java/org/apache/sqoop/mapreduce/db/SQLServerDBRecordReader.java 
> 9a3621b0 
>   src/test/org/apache/sqoop/mapreduce/db/TestSQLServerDBRecordReader.java 
> PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/61777/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zoltán Tóth
> 
>

Review Request 61777: sqoop tries to re execute select query during import in case of a connection reset error and this is causing lots of duplicate records from source

2017-08-21 Thread Zoltán Tóth 

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

Review request for Sqoop.


Bugs: SQOOP-3139
https://issues.apache.org/jira/browse/SQOOP-3139


Repository: sqoop-trunk


Description
---

In the case if the database table name and split-by parameter was different 
(eg.: Mycol vs. mycol) Sqoop couldn't continue the query from the last value if 
the connection was broken.


Diffs
-

  src/java/org/apache/sqoop/mapreduce/db/DBRecordReader.java a78eb061 
  src/java/org/apache/sqoop/mapreduce/db/SQLServerDBRecordReader.java 9a3621b0 
  src/test/org/apache/sqoop/mapreduce/db/TestSQLServerDBRecordReader.java 
PRE-CREATION 


Diff: https://reviews.apache.org/r/61777/diff/1/


Testing
---


Thanks,

Zoltán Tóth