sqoop-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Attila Szabo <mau...@apache.org>
Subject Re: Review Request 54251: SQOOP-3061 - Fix a bug when parsing query in sqoop options file
Date Tue, 17 Jan 2017 15:04:27 GMT

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


Fix it, then Ship it!




Hey Eric,

We'd arrived into a "Fix it then ship it" phase. I've only found a very minor problem.
Please fix that one (in one of the suggested ways), and then I will be able to submit this
patch.

Cheers,
Attila


src/test/com/cloudera/sqoop/util/TestOptionsFileExpansion.java (lines 235 - 247)
<https://reviews.apache.org/r/54251/#comment233150>

    This statement can't be valied, as the ' is an allowed identifier/column name if it is
properly quoted (e.g. with back quotes), but not in this form.
    
    You should either tune your regex check to validate this as a failure (this would be the
better solution), or at least delete this test case, not giving the false impression to the
users, that these kind of queries could be handled on DB side.


- Attila Szabo


On Jan. 17, 2017, 7:23 a.m., Eric Lin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54251/
> -----------------------------------------------------------
> 
> (Updated Jan. 17, 2017, 7:23 a.m.)
> 
> 
> Review request for Sqoop and Szabolcs Vasas.
> 
> 
> Bugs: SQOOP-3061
>     https://issues.apache.org/jira/browse/SQOOP-3061
> 
> 
> Repository: sqoop-trunk
> 
> 
> Description
> -------
> 
> SQOOP-3061 - Sqoop --options-file failed with error "Malformed option in options file"
even though the query is correct
> 
> 
> Diffs
> -----
> 
>   src/java/org/apache/sqoop/util/OptionsFileUtil.java c476e00 
>   src/test/com/cloudera/sqoop/util/TestOptionsFileExpansion.java 3f0bfb9 
> 
> Diff: https://reviews.apache.org/r/54251/diff/
> 
> 
> Testing
> -------
> 
> Test case updated and have made sure that existing test cases and new test cases passed.
> 
> 
> Thanks,
> 
> Eric Lin
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message