hive-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Vihang Karajgaonkar <vih...@cloudera.com>
Subject Re: Review Request 50982: HIVE-14345:Beeline result table has erroneous characters
Date Fri, 12 Aug 2016 16:54:02 GMT

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


Fix it, then Ship it!




Overall, the fix looks good. Some comments below.


beeline/src/java/org/apache/hive/beeline/TableOutputFormat.java (line 63)
<https://reviews.apache.org/r/50982/#comment211946>

    nit, use the formatter to correct the spaces



beeline/src/test/org/apache/hive/beeline/TestTableOutputFormat.java (line 35)
<https://reviews.apache.org/r/50982/#comment211945>

    I don't think the author comment is required. If you use Eclipse, I found importing the
coding-style "eclipse-styles.xml" in dev-support directory helpful to format the code using
the coding conventions. This will also remove all the trailing spaces (seen in red below)



beeline/src/test/org/apache/hive/beeline/TestTableOutputFormat.java (line 100)
<https://reviews.apache.org/r/50982/#comment211950>

    Is it possible to avoid hardcoding this string? If in the future beelineOpts.maxColoumnWidth
default values are changed, this test might fail. I think it will be more robust to determine
this string programatically using the values in mockResultData and value of maxColumnWidth.


- Vihang Karajgaonkar


On Aug. 12, 2016, 3:16 p.m., Miklos Csanady wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50982/
> -----------------------------------------------------------
> 
> (Updated Aug. 12, 2016, 3:16 p.m.)
> 
> 
> Review request for hive, Peter Vary and Sergio Pena.
> 
> 
> Bugs: HIVE-14345
>     https://issues.apache.org/jira/browse/HIVE-14345
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> Fixed output table formatting header and footer lines.
> 
> 
> Diffs
> -----
> 
>   beeline/src/java/org/apache/hive/beeline/TableOutputFormat.java 2753568 
>   beeline/src/test/org/apache/hive/beeline/TestTableOutputFormat.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/50982/diff/
> 
> 
> Testing
> -------
> 
> See attached Unit testClass.
> After building with patch, the bug eliminated.
> 
> 
> File Attachments
> ----------------
> 
> corrected version
>   https://reviews.apache.org/media/uploaded/files/2016/08/12/aed736a0-95f1-4f5b-9bda-4db767911065__HIVE-14345.patch
> 
> 
> Thanks,
> 
> Miklos Csanady
> 
>


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