Re: Review Request 67314: Oozie Sqoop action with command splits the select clause into multiple parts due to delimiter being space

2018-06-06 Thread Peter Cseh via Review Board

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




core/src/main/java/org/apache/oozie/action/hadoop/SqoopActionExecutor.java
Lines 95 (patched)


Is "contains" strong enough here?
I can think of two scenarios when it might screw up the parsin:
1) a table name contains a "-" character
2) a query like "select * where price > -3 and price < 0



core/src/main/java/org/apache/oozie/action/hadoop/SqoopActionExecutor.java
Lines 98 (patched)


This changes behavior (for the better I thnik :)) Please update the 
documentation as well.



sharelib/sqoop/src/test/java/org/apache/oozie/action/hadoop/TestSqoopActionExecutor.java
Lines 50 (patched)


Please add a test case where this select is quoted


- Peter Cseh


On May 25, 2018, 7:37 a.m., Denes Bodo wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67314/
> ---
> 
> (Updated May 25, 2018, 7:37 a.m.)
> 
> 
> Review request for oozie and András Piros.
> 
> 
> Bugs: OOZIE-3218
> https://issues.apache.org/jira/browse/OOZIE-3218
> 
> 
> Repository: oozie-git
> 
> 
> Description
> ---
> 
> When running a Oozie Sqoop action which has command with --query in place the 
> query is split into multiple parts causing "Unrecognized argument:" and 
> in-turn fails.
> 
> 
> Diffs
> -
> 
>   core/src/main/java/org/apache/oozie/action/hadoop/SqoopActionExecutor.java 
> 69d5e7e 
>   
> sharelib/sqoop/src/test/java/org/apache/oozie/action/hadoop/TestSqoopActionExecutor.java
>  edfe0c7 
> 
> 
> Diff: https://reviews.apache.org/r/67314/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Denes Bodo
> 
>



Re: Review Request 67314: Oozie Sqoop action with command splits the select clause into multiple parts due to delimiter being space

2018-05-25 Thread András Piros via Review Board

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




core/src/main/java/org/apache/oozie/action/hadoop/SqoopActionExecutor.java
Lines 97 (patched)


I'd at most log at `TRACE` level here. But I'd rather leave that and log at 
`DEBUG` level the resulting `argList` that's gonna be forwarded to 
`setSqoopCommand()`.



core/src/main/java/org/apache/oozie/action/hadoop/SqoopActionExecutor.java
Lines 100 (patched)


I'd at most log at `TRACE` level here. But I'd rather leave that and log at 
`DEBUG` level the resulting `argList` that's gonna be forwarded to 
`setSqoopCommand()`.



core/src/main/java/org/apache/oozie/action/hadoop/SqoopActionExecutor.java
Lines 105 (patched)


Don't we want to replace all `\n\n`s?



core/src/main/java/org/apache/oozie/action/hadoop/SqoopActionExecutor.java
Lines 108 (patched)


`arrayLength` isn't a nice name for a variable that's incremented in a loop.



sharelib/sqoop/src/test/java/org/apache/oozie/action/hadoop/TestSqoopActionExecutor.java
Line 46 (original), 46 (patched)


Please cover old functionality that uses `"\n"` as word separator inside 
the query, and new functionality that user `" "` as word separator inside the 
query, as well.

Please also test a mixed case that uses both separators inside the same 
query.



sharelib/sqoop/src/test/java/org/apache/oozie/action/hadoop/TestSqoopActionExecutor.java
Lines 50 (patched)


`SQOOP_QUERY_COMMAND`


- András Piros


On May 25, 2018, 7:37 a.m., Denes Bodo wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67314/
> ---
> 
> (Updated May 25, 2018, 7:37 a.m.)
> 
> 
> Review request for oozie and András Piros.
> 
> 
> Bugs: OOZIE-3218
> https://issues.apache.org/jira/browse/OOZIE-3218
> 
> 
> Repository: oozie-git
> 
> 
> Description
> ---
> 
> When running a Oozie Sqoop action which has command with --query in place the 
> query is split into multiple parts causing "Unrecognized argument:" and 
> in-turn fails.
> 
> 
> Diffs
> -
> 
>   core/src/main/java/org/apache/oozie/action/hadoop/SqoopActionExecutor.java 
> 69d5e7e 
>   
> sharelib/sqoop/src/test/java/org/apache/oozie/action/hadoop/TestSqoopActionExecutor.java
>  edfe0c7 
> 
> 
> Diff: https://reviews.apache.org/r/67314/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Denes Bodo
> 
>



Review Request 67314: Oozie Sqoop action with command splits the select clause into multiple parts due to delimiter being space

2018-05-25 Thread Denes Bodo

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

Review request for oozie and András Piros.


Bugs: OOZIE-3218
https://issues.apache.org/jira/browse/OOZIE-3218


Repository: oozie-git


Description
---

When running a Oozie Sqoop action which has command with --query in place the 
query is split into multiple parts causing "Unrecognized argument:" and in-turn 
fails.


Diffs
-

  core/src/main/java/org/apache/oozie/action/hadoop/SqoopActionExecutor.java 
69d5e7e 
  
sharelib/sqoop/src/test/java/org/apache/oozie/action/hadoop/TestSqoopActionExecutor.java
 edfe0c7 


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


Testing
---


Thanks,

Denes Bodo