hive-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Eric Hanson" <eh...@microsoft.com>
Subject Re: Review Request 15663: Hive should be able to skip header and footer rows when reading data file for a table
Date Tue, 19 Nov 2013 02:04:31 GMT

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



common/src/java/org/apache/hadoop/hive/conf/HiveConf.java
<https://reviews.apache.org/r/15663/#comment56220>

    What does this mean exactly? Is this lines of footer or actual total number of footers?
    
    If it is number of footers, should say 
    "max number of footers ..."



itests/qtest/pom.xml
<https://reviews.apache.org/r/15663/#comment56221>

    is this really supposed to be in the patch?



ql/src/java/org/apache/hadoop/hive/ql/exec/FetchOperator.java
<https://reviews.apache.org/r/15663/#comment56228>

    Please put a paragraph of explanation of the header/footer skipping feature right in the
code. Including what it is and how to use it. 
    
    Also, please create web documentation for the new feature. Check with Lefty L. about where
to put it. You could start by putting a first draft under https://cwiki.apache.org/confluence/display/Hive/DesignDocs.
You could delete the design doc from there once the design becomes part of the Hive documentation.



ql/src/java/org/apache/hadoop/hive/ql/exec/FetchOperator.java
<https://reviews.apache.org/r/15663/#comment56222>

    Hive coding style guidelines say to put a blank line before all comments. Please check
all your comments for this.



ql/src/java/org/apache/hadoop/hive/ql/exec/FetchOperator.java
<https://reviews.apache.org/r/15663/#comment56227>

    I recommend using "skip.header.line.count" instead of "skip.header.number" to make it
explicit that you are skipping lines.
    
    Also, use "skip.footer.line.count" instead of skip.footer.number.



ql/src/java/org/apache/hadoop/hive/ql/exec/FetchOperator.java
<https://reviews.apache.org/r/15663/#comment56223>

    put blank after // before first word



ql/src/java/org/apache/hadoop/hive/ql/io/HiveContextAwareRecordReader.java
<https://reviews.apache.org/r/15663/#comment56225>

    Please put a comment before the is class explaining what it is for.



ql/src/java/org/apache/hadoop/hive/ql/io/HiveContextAwareRecordReader.java
<https://reviews.apache.org/r/15663/#comment56224>

    use camel case (footerCur)



ql/src/java/org/apache/hadoop/hive/ql/io/HiveContextAwareRecordReader.java
<https://reviews.apache.org/r/15663/#comment56226>

    Please run checkstyle. E.g. there should be a blank between ){


- Eric Hanson


On Nov. 19, 2013, 1:31 a.m., Eric Hanson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15663/
> -----------------------------------------------------------
> 
> (Updated Nov. 19, 2013, 1:31 a.m.)
> 
> 
> Review request for hive and Thejas Nair.
> 
> 
> Bugs: HIVE-5795
>     https://issues.apache.org/jira/browse/HIVE-5795
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> Hive should be able to skip header and footer rows when reading data file for a table
> 
> (I am uploading this on behalf of Shuaishuai Nie since he's not in the office)
> 
> 
> Diffs
> -----
> 
>   common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 32ab3d8 
>   data/files/header_footer_table_1/0001.txt PRE-CREATION 
>   data/files/header_footer_table_1/0002.txt PRE-CREATION 
>   data/files/header_footer_table_1/0003.txt PRE-CREATION 
>   data/files/header_footer_table_2/2012/01/01/0001.txt PRE-CREATION 
>   data/files/header_footer_table_2/2012/01/02/0002.txt PRE-CREATION 
>   data/files/header_footer_table_2/2012/01/03/0003.txt PRE-CREATION 
>   itests/qtest/pom.xml a453d8a 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/FetchOperator.java 5abcfc1 
>   ql/src/java/org/apache/hadoop/hive/ql/io/HiveContextAwareRecordReader.java dd5cb6b

>   ql/src/java/org/apache/hadoop/hive/ql/io/HiveInputFormat.java 0ec6e63 
>   ql/src/test/org/apache/hadoop/hive/ql/io/TestHiveBinarySearchRecordReader.java 85dd975

>   ql/src/test/org/apache/hadoop/hive/ql/io/TestSymlinkTextInputFormat.java 0686d9b 
>   ql/src/test/queries/clientpositive/file_with_header_footer.q PRE-CREATION 
>   ql/src/test/results/clientpositive/file_with_header_footer.q.out PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/15663/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Eric Hanson
> 
>


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