Re: Review Request 66353: Connection resource related issues in DBOutputFormat and OracleManager

2018-04-11 Thread Szabolcs Vasas 

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



Hi László,

Thank you for adding a test case, I have left one comment on it?
Do you plan to add another test case for the change in DBOutputFormat? I think 
it could be pretty easily added after applying an extract method refactoring.


src/test/org/apache/sqoop/manager/TestOracleManager.java
Lines 70 (patched)


This test can pass when SQLException is not thrown so it can give a false 
green sign. I suggest using ExpectedException rule here.


- Szabolcs Vasas


On April 6, 2018, 1:26 p.m., Laszlo Bodor wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66353/
> ---
> 
> (Updated April 6, 2018, 1:26 p.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Bugs: SQOOP-3306
> https://issues.apache.org/jira/browse/SQOOP-3306
> 
> 
> Repository: sqoop-trunk
> 
> 
> Description
> ---
> 
> A fortify scan showed 2 possible resource leaks.
> 
> 1: The function getRecordWriter() in DBOutputFormat.java sometimes fails to 
> release a database resource allocated by getConnection() on line 117.
> In the file DBOutputFormat.java similar issues were on line numbers 117
> 
> 2: The function makeConnection() in OracleManager.java sometimes fails to 
> release a database resource allocated by getConnection() on line 321.
> In the file OracleManager.java similar issues were on line numbers 321
> 
> 
> Diffs
> -
> 
>   src/java/org/apache/sqoop/manager/OracleManager.java 929b5061 
>   src/java/org/apache/sqoop/mapreduce/db/DBOutputFormat.java 730ff286 
>   src/test/org/apache/sqoop/manager/TestOracleManager.java PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/66353/diff/2/
> 
> 
> Testing
> ---
> 
> ant test
> 
> 
> Thanks,
> 
> Laszlo Bodor
> 
>

[jira] [Commented] (SQOOP-3311) Importing as ORC file to support full ACID Hive tables

2018-04-11 Thread Daniel Voros (JIRA) 

[ 
https://issues.apache.org/jira/browse/SQOOP-3311?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16433797#comment-16433797
 ] 

Daniel Voros commented on SQOOP-3311:
-

Attached review request.

> Importing as ORC file to support full ACID Hive tables
> --
>
> Key: SQOOP-3311
> URL: https://issues.apache.org/jira/browse/SQOOP-3311
> Project: Sqoop
>  Issue Type: New Feature
>  Components: hive-integration
>Reporter: Daniel Voros
>Assignee: Daniel Voros
>Priority: Major
>
> Hive 3 will introduce a switch (HIVE-18294) to create eligible tables as ACID 
> by default. This will probably result in increased usage of ACID tables and 
> the need to support importing into ACID tables with Sqoop.
> Currently the only table format supporting full ACID tables is ORC.
> The easiest and most effective way to support importing into these tables 
> would be to write out files as ORC and keep using LOAD DATA as we do for all 
> other Hive tables (supported since HIVE-17361).
> Workaround could be to create table as textfile (as before) and then CTAS 
> from that. This would push the responsibility of creating ORC format to Hive. 
> However it would result in writing every record twice; in text format and in 
> ORC.
> Note that ORC is only necessary for full ACID tables. Insert-only (aka. 
> micromanaged) ACID tables can use arbitrary file format.
> Supporting full ACID tables would also be the first step in making 
> "lastmodified" incremental imports work with Hive.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

Re: Review Request 66548: Importing as ORC file to support full ACID Hive tables

2018-04-11 Thread daniel voros 

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



Patch #1 is an initial patch that contains the most fundamental changes to 
support ORC importing. I'll add documentation and extend the tests with 
thridparty tests etc. but wanted to share to get feedback early on.

- daniel voros


On April 11, 2018, 12:02 p.m., daniel voros wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66548/
> ---
> 
> (Updated April 11, 2018, 12:02 p.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Bugs: SQOOP-3311
> https://issues.apache.org/jira/browse/SQOOP-3311
> 
> 
> Repository: sqoop-trunk
> 
> 
> Description
> ---
> 
> Hive 3 will introduce a switch (HIVE-18294) to create eligible tables as ACID 
> by default. This will probably result in increased usage of ACID tables and 
> the need to support importing into ACID tables with Sqoop.
> 
> Currently the only table format supporting full ACID tables is ORC.
> 
> The easiest and most effective way to support importing into these tables 
> would be to write out files as ORC and keep using LOAD DATA as we do for all 
> other Hive tables (supported since HIVE-17361).
> 
> Workaround could be to create table as textfile (as before) and then CTAS 
> from that. This would push the responsibility of creating ORC format to Hive. 
> However it would result in writing every record twice; in text format and in 
> ORC.
> 
> Note that ORC is only necessary for full ACID tables. Insert-only (aka. 
> micromanaged) ACID tables can use arbitrary file format.
> 
> Supporting full ACID tables would also be the first step in making 
> "lastmodified" incremental imports work with Hive.
> 
> 
> Diffs
> -
> 
>   ivy.xml 6be4fa2 
>   ivy/libraries.properties c44b50b 
>   src/java/org/apache/sqoop/SqoopOptions.java 651cebd 
>   src/java/org/apache/sqoop/hive/TableDefWriter.java b7a25b7 
>   src/java/org/apache/sqoop/mapreduce/DataDrivenImportJob.java a5962ba 
>   src/java/org/apache/sqoop/mapreduce/OrcImportMapper.java PRE-CREATION 
>   src/java/org/apache/sqoop/tool/BaseSqoopTool.java b02e4fe 
>   src/java/org/apache/sqoop/tool/ExportTool.java 060f2c0 
>   src/java/org/apache/sqoop/tool/ImportTool.java e992005 
>   src/java/org/apache/sqoop/util/OrcUtil.java PRE-CREATION 
>   src/test/org/apache/sqoop/TestOrcImport.java PRE-CREATION 
>   src/test/org/apache/sqoop/hive/TestTableDefWriter.java 8bdc3be 
>   src/test/org/apache/sqoop/orm/TestClassWriter.java 0cc07cf 
>   src/test/org/apache/sqoop/util/TestOrcUtil.java PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/66548/diff/1/
> 
> 
> Testing
> ---
> 
> - added some unit tests
> - tested basic Hive import scenarios on a cluster
> 
> 
> Thanks,
> 
> daniel voros
> 
>

Review Request 66548: Importing as ORC file to support full ACID Hive tables

2018-04-11 Thread daniel voros 

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

Review request for Sqoop.


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


Repository: sqoop-trunk


Description
---

Hive 3 will introduce a switch (HIVE-18294) to create eligible tables as ACID 
by default. This will probably result in increased usage of ACID tables and the 
need to support importing into ACID tables with Sqoop.

Currently the only table format supporting full ACID tables is ORC.

The easiest and most effective way to support importing into these tables would 
be to write out files as ORC and keep using LOAD DATA as we do for all other 
Hive tables (supported since HIVE-17361).

Workaround could be to create table as textfile (as before) and then CTAS from 
that. This would push the responsibility of creating ORC format to Hive. 
However it would result in writing every record twice; in text format and in 
ORC.

Note that ORC is only necessary for full ACID tables. Insert-only (aka. 
micromanaged) ACID tables can use arbitrary file format.

Supporting full ACID tables would also be the first step in making 
"lastmodified" incremental imports work with Hive.


Diffs
-

  ivy.xml 6be4fa2 
  ivy/libraries.properties c44b50b 
  src/java/org/apache/sqoop/SqoopOptions.java 651cebd 
  src/java/org/apache/sqoop/hive/TableDefWriter.java b7a25b7 
  src/java/org/apache/sqoop/mapreduce/DataDrivenImportJob.java a5962ba 
  src/java/org/apache/sqoop/mapreduce/OrcImportMapper.java PRE-CREATION 
  src/java/org/apache/sqoop/tool/BaseSqoopTool.java b02e4fe 
  src/java/org/apache/sqoop/tool/ExportTool.java 060f2c0 
  src/java/org/apache/sqoop/tool/ImportTool.java e992005 
  src/java/org/apache/sqoop/util/OrcUtil.java PRE-CREATION 
  src/test/org/apache/sqoop/TestOrcImport.java PRE-CREATION 
  src/test/org/apache/sqoop/hive/TestTableDefWriter.java 8bdc3be 
  src/test/org/apache/sqoop/orm/TestClassWriter.java 0cc07cf 
  src/test/org/apache/sqoop/util/TestOrcUtil.java PRE-CREATION 


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


Testing
---

- added some unit tests
- tested basic Hive import scenarios on a cluster


Thanks,

daniel voros