You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hive.apache.org by Shuaishuai Nie <sh...@microsoft.com> on 2013/12/11 22:19:23 UTC
Review Request 16184: Hive should be able to skip header and footer rows
when reading data file for a table (HIVE-5795)
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16184/
-----------------------------------------------------------
Review request for hive, Eric Hanson 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
(follow up with review https://reviews.apache.org/r/15663/diff/#index_header)
Diffs
-----
common/src/java/org/apache/hadoop/hive/conf/HiveConf.java fa3e048
conf/hive-default.xml.template c61a0bb
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 c3cbb89
ql/src/java/org/apache/hadoop/hive/ql/exec/FetchOperator.java d2b2526
ql/src/java/org/apache/hadoop/hive/ql/io/HiveContextAwareRecordReader.java dd5cb6b
ql/src/java/org/apache/hadoop/hive/ql/io/HiveInputFormat.java 974a5d6
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/clientnegative/file_with_header_footer_negative.q PRE-CREATION
ql/src/test/queries/clientpositive/file_with_header_footer.q PRE-CREATION
ql/src/test/results/clientnegative/file_with_header_footer_negative.q.out PRE-CREATION
ql/src/test/results/clientpositive/file_with_header_footer.q.out PRE-CREATION
serde/if/serde.thrift 2ceb572
serde/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/serde/serdeConstants.java 22a6168
Diff: https://reviews.apache.org/r/16184/diff/
Testing
-------
Thanks,
Shuaishuai Nie
Re: Review Request 16184: Hive should be able to skip header and footer rows
when reading data file for a table (HIVE-5795)
Posted by Thejas Nair <th...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16184/#review30523
-----------------------------------------------------------
ql/src/java/org/apache/hadoop/hive/ql/exec/FetchOperator.java
<https://reviews.apache.org/r/16184/#comment58465>
skipHeader and initiaizeFooterBuf can be moved to a common util class and shared. We just need to pass the member variables as additional params
ql/src/java/org/apache/hadoop/hive/ql/exec/FetchOperator.java
<https://reviews.apache.org/r/16184/#comment58463>
code such as this block for parsing header count can be moved to a util class and shared between the two places
ql/src/java/org/apache/hadoop/hive/ql/exec/FetchOperator.java
<https://reviews.apache.org/r/16184/#comment58464>
code such as this block for parsing header count can be moved to a util class and shared between the two places
ql/src/java/org/apache/hadoop/hive/ql/exec/FetchOperator.java
<https://reviews.apache.org/r/16184/#comment58466>
logic of this block also looks same in 2 places, can we move it to a common util function ?
- Thejas Nair
On Dec. 11, 2013, 9:19 p.m., Shuaishuai Nie wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/16184/
> -----------------------------------------------------------
>
> (Updated Dec. 11, 2013, 9:19 p.m.)
>
>
> Review request for hive, Eric Hanson 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
> (follow up with review https://reviews.apache.org/r/15663/diff/#index_header)
>
>
> Diffs
> -----
>
> common/src/java/org/apache/hadoop/hive/conf/HiveConf.java fa3e048
> conf/hive-default.xml.template c61a0bb
> 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 c3cbb89
> ql/src/java/org/apache/hadoop/hive/ql/exec/FetchOperator.java d2b2526
> ql/src/java/org/apache/hadoop/hive/ql/io/HiveContextAwareRecordReader.java dd5cb6b
> ql/src/java/org/apache/hadoop/hive/ql/io/HiveInputFormat.java 974a5d6
> 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/clientnegative/file_with_header_footer_negative.q PRE-CREATION
> ql/src/test/queries/clientpositive/file_with_header_footer.q PRE-CREATION
> ql/src/test/results/clientnegative/file_with_header_footer_negative.q.out PRE-CREATION
> ql/src/test/results/clientpositive/file_with_header_footer.q.out PRE-CREATION
> serde/if/serde.thrift 2ceb572
> serde/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/serde/serdeConstants.java 22a6168
>
> Diff: https://reviews.apache.org/r/16184/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Shuaishuai Nie
>
>
Re: Review Request 16184: Hive should be able to skip header and footer rows
when reading data file for a table (HIVE-5795)
Posted by Thejas Nair <th...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16184/#review30876
-----------------------------------------------------------
ql/src/java/org/apache/hadoop/hive/ql/io/HiveInputFormat.java
<https://reviews.apache.org/r/16184/#comment59110>
how about using the new utilitiy functions from here as well?
- Thejas Nair
On Dec. 19, 2013, 12:40 a.m., Shuaishuai Nie wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/16184/
> -----------------------------------------------------------
>
> (Updated Dec. 19, 2013, 12:40 a.m.)
>
>
> Review request for hive, Eric Hanson 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
> (follow up with review https://reviews.apache.org/r/15663/diff/#index_header)
>
>
> Diffs
> -----
>
> common/src/java/org/apache/hadoop/hive/conf/HiveConf.java fa3e048
> conf/hive-default.xml.template 1b30d19
> 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 971c5d3
> ql/src/java/org/apache/hadoop/hive/ql/exec/FetchOperator.java fc9b7e4
> ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java 9afc80b
> ql/src/java/org/apache/hadoop/hive/ql/io/HiveContextAwareRecordReader.java dd5cb6b
> ql/src/java/org/apache/hadoop/hive/ql/io/HiveInputFormat.java 974a5d6
> 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/clientnegative/file_with_header_footer_negative.q PRE-CREATION
> ql/src/test/queries/clientpositive/file_with_header_footer.q PRE-CREATION
> ql/src/test/results/clientnegative/file_with_header_footer_negative.q.out PRE-CREATION
> ql/src/test/results/clientpositive/file_with_header_footer.q.out PRE-CREATION
> serde/if/serde.thrift 2ceb572
> serde/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/serde/serdeConstants.java 22a6168
>
> Diff: https://reviews.apache.org/r/16184/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Shuaishuai Nie
>
>
Re: Review Request 16184: Hive should be able to skip header and footer rows
when reading data file for a table (HIVE-5795)
Posted by Thejas Nair <th...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16184/#review30872
-----------------------------------------------------------
ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java
<https://reviews.apache.org/r/16184/#comment59093>
It would be better to have FooterBuffer as a top level class in this package (...ql.exec)
ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java
<https://reviews.apache.org/r/16184/#comment59099>
indentation needs fixing here
ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java
<https://reviews.apache.org/r/16184/#comment59103>
can you please change the variable name from "ret" to something more descriptive like "notEOF"
ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java
<https://reviews.apache.org/r/16184/#comment59104>
return false, is little more easy to read
ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java
<https://reviews.apache.org/r/16184/#comment59100>
indentation needs fixing here
ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java
<https://reviews.apache.org/r/16184/#comment59106>
can you please change the variable name from "ret" to something more descriptive like "notEOF"
ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java
<https://reviews.apache.org/r/16184/#comment59107>
this else block looks unnecessary (as mentioned in my previous review)
ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java
<https://reviews.apache.org/r/16184/#comment59105>
indentation needs fixing here
- Thejas Nair
On Dec. 19, 2013, 12:40 a.m., Shuaishuai Nie wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/16184/
> -----------------------------------------------------------
>
> (Updated Dec. 19, 2013, 12:40 a.m.)
>
>
> Review request for hive, Eric Hanson 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
> (follow up with review https://reviews.apache.org/r/15663/diff/#index_header)
>
>
> Diffs
> -----
>
> common/src/java/org/apache/hadoop/hive/conf/HiveConf.java fa3e048
> conf/hive-default.xml.template 1b30d19
> 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 971c5d3
> ql/src/java/org/apache/hadoop/hive/ql/exec/FetchOperator.java fc9b7e4
> ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java 9afc80b
> ql/src/java/org/apache/hadoop/hive/ql/io/HiveContextAwareRecordReader.java dd5cb6b
> ql/src/java/org/apache/hadoop/hive/ql/io/HiveInputFormat.java 974a5d6
> 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/clientnegative/file_with_header_footer_negative.q PRE-CREATION
> ql/src/test/queries/clientpositive/file_with_header_footer.q PRE-CREATION
> ql/src/test/results/clientnegative/file_with_header_footer_negative.q.out PRE-CREATION
> ql/src/test/results/clientpositive/file_with_header_footer.q.out PRE-CREATION
> serde/if/serde.thrift 2ceb572
> serde/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/serde/serdeConstants.java 22a6168
>
> Diff: https://reviews.apache.org/r/16184/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Shuaishuai Nie
>
>
Re: Review Request 16184: Hive should be able to skip header and footer rows
when reading data file for a table (HIVE-5795)
Posted by Shuaishuai Nie <sh...@microsoft.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16184/
-----------------------------------------------------------
(Updated Dec. 31, 2013, 11:04 p.m.)
Review request for hive, Eric Hanson and Thejas Nair.
Changes
-------
fixed unit test failures in TestCliDriver
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
(follow up with review https://reviews.apache.org/r/15663/diff/#index_header)
Diffs (updated)
-----
common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 2ddb08f
conf/hive-default.xml.template b94013a
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 88e0890
ql/src/java/org/apache/hadoop/hive/ql/exec/FetchOperator.java fc9b7e4
ql/src/java/org/apache/hadoop/hive/ql/exec/FooterBuffer.java PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java daf4e4a
ql/src/java/org/apache/hadoop/hive/ql/io/HiveContextAwareRecordReader.java dd5cb6b
ql/src/java/org/apache/hadoop/hive/ql/io/HiveInputFormat.java 974a5d6
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/clientnegative/file_with_header_footer_negative.q PRE-CREATION
ql/src/test/queries/clientpositive/file_with_header_footer.q PRE-CREATION
ql/src/test/results/clientnegative/file_with_header_footer_negative.q.out PRE-CREATION
ql/src/test/results/clientpositive/file_with_header_footer.q.out PRE-CREATION
serde/if/serde.thrift 2ceb572
serde/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/serde/serdeConstants.java 22a6168
Diff: https://reviews.apache.org/r/16184/diff/
Testing
-------
Thanks,
Shuaishuai Nie
Re: Review Request 16184: Hive should be able to skip header and footer rows
when reading data file for a table (HIVE-5795)
Posted by Shuaishuai Nie <sh...@microsoft.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16184/
-----------------------------------------------------------
(Updated Dec. 30, 2013, 10:41 p.m.)
Review request for hive, Eric Hanson and Thejas Nair.
Changes
-------
Fixed the patch based on Thejas's comment
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
(follow up with review https://reviews.apache.org/r/15663/diff/#index_header)
Diffs (updated)
-----
common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 2ddb08f
conf/hive-default.xml.template b94013a
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 88e0890
itests/util/src/main/java/org/apache/hadoop/hive/ql/QTestUtil.java 03fd30a
ql/src/java/org/apache/hadoop/hive/ql/exec/FetchOperator.java fc9b7e4
ql/src/java/org/apache/hadoop/hive/ql/exec/FooterBuffer.java PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java daf4e4a
ql/src/java/org/apache/hadoop/hive/ql/io/HiveContextAwareRecordReader.java dd5cb6b
ql/src/java/org/apache/hadoop/hive/ql/io/HiveInputFormat.java 974a5d6
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/clientnegative/file_with_header_footer_negative.q PRE-CREATION
ql/src/test/queries/clientpositive/file_with_header_footer.q PRE-CREATION
ql/src/test/results/clientnegative/file_with_header_footer_negative.q.out PRE-CREATION
ql/src/test/results/clientpositive/file_with_header_footer.q.out PRE-CREATION
serde/if/serde.thrift 2ceb572
serde/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/serde/serdeConstants.java 22a6168
Diff: https://reviews.apache.org/r/16184/diff/
Testing
-------
Thanks,
Shuaishuai Nie
Re: Review Request 16184: Hive should be able to skip header and footer rows
when reading data file for a table (HIVE-5795)
Posted by Shuaishuai Nie <sh...@microsoft.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16184/
-----------------------------------------------------------
(Updated Dec. 19, 2013, 12:40 a.m.)
Review request for hive, Eric Hanson 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
(follow up with review https://reviews.apache.org/r/15663/diff/#index_header)
Diffs (updated)
-----
common/src/java/org/apache/hadoop/hive/conf/HiveConf.java fa3e048
conf/hive-default.xml.template 1b30d19
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 971c5d3
ql/src/java/org/apache/hadoop/hive/ql/exec/FetchOperator.java fc9b7e4
ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java 9afc80b
ql/src/java/org/apache/hadoop/hive/ql/io/HiveContextAwareRecordReader.java dd5cb6b
ql/src/java/org/apache/hadoop/hive/ql/io/HiveInputFormat.java 974a5d6
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/clientnegative/file_with_header_footer_negative.q PRE-CREATION
ql/src/test/queries/clientpositive/file_with_header_footer.q PRE-CREATION
ql/src/test/results/clientnegative/file_with_header_footer_negative.q.out PRE-CREATION
ql/src/test/results/clientpositive/file_with_header_footer.q.out PRE-CREATION
serde/if/serde.thrift 2ceb572
serde/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/serde/serdeConstants.java 22a6168
Diff: https://reviews.apache.org/r/16184/diff/
Testing
-------
Thanks,
Shuaishuai Nie
Re: Review Request 16184: Hive should be able to skip header and footer rows
when reading data file for a table (HIVE-5795)
Posted by Thejas Nair <th...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16184/#review30516
-----------------------------------------------------------
ql/src/java/org/apache/hadoop/hive/ql/exec/FetchOperator.java
<https://reviews.apache.org/r/16184/#comment58439>
can you add a javadoc comment describing the what the return value is
ql/src/java/org/apache/hadoop/hive/ql/exec/FetchOperator.java
<https://reviews.apache.org/r/16184/#comment58440>
can you add a javadoc comment describing the what the return value is
ql/src/java/org/apache/hadoop/hive/ql/exec/FetchOperator.java
<https://reviews.apache.org/r/16184/#comment58441>
This variable name was used before as well. But can you rename this variable to something more descriptive like opNotEOF, while you are making these changes?
ql/src/java/org/apache/hadoop/hive/ql/exec/FetchOperator.java
<https://reviews.apache.org/r/16184/#comment58442>
please use braces for the if condition - "if (ret) {.."
ql/src/java/org/apache/hadoop/hive/ql/exec/FetchOperator.java
<https://reviews.apache.org/r/16184/#comment58443>
duplicate if block.
ql/src/java/org/apache/hadoop/hive/ql/io/HiveContextAwareRecordReader.java
<https://reviews.apache.org/r/16184/#comment58438>
In your comment in previous reviewboard link, you say that ObjectPair can't be used because "I need deep copy of the key and value field through ReflectionUtils".
But the use of ReflectionUtils is not happening from within KVPair. I haven't understood why ObjectPair can't be used instead.
ql/src/java/org/apache/hadoop/hive/ql/io/HiveContextAwareRecordReader.java
<https://reviews.apache.org/r/16184/#comment58445>
can you add this to the comment (unfortunately ReflectionsUtils.copy is poorly/incorrectly documented, created HADOOP-10168)
// copy value from footerBuf to key,value
ql/src/java/org/apache/hadoop/hive/ql/io/HiveContextAwareRecordReader.java
<https://reviews.apache.org/r/16184/#comment58446>
Please don't use same variable name as member variable, it gets very confusing! :)
maybe, call it inputEOF ?
pls add comment -
//read new value into the buffer
ql/src/java/org/apache/hadoop/hive/ql/io/HiveContextAwareRecordReader.java
<https://reviews.apache.org/r/16184/#comment58447>
looks like this else statement is not needed. the copy has already been done.
Comments applicable to the FetchOperator also
ql/src/test/queries/clientnegative/file_with_header_footer_negative.q
<https://reviews.apache.org/r/16184/#comment58444>
can you also add a test case with a table with empty file ?
- Thejas Nair
On Dec. 11, 2013, 9:19 p.m., Shuaishuai Nie wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/16184/
> -----------------------------------------------------------
>
> (Updated Dec. 11, 2013, 9:19 p.m.)
>
>
> Review request for hive, Eric Hanson 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
> (follow up with review https://reviews.apache.org/r/15663/diff/#index_header)
>
>
> Diffs
> -----
>
> common/src/java/org/apache/hadoop/hive/conf/HiveConf.java fa3e048
> conf/hive-default.xml.template c61a0bb
> 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 c3cbb89
> ql/src/java/org/apache/hadoop/hive/ql/exec/FetchOperator.java d2b2526
> ql/src/java/org/apache/hadoop/hive/ql/io/HiveContextAwareRecordReader.java dd5cb6b
> ql/src/java/org/apache/hadoop/hive/ql/io/HiveInputFormat.java 974a5d6
> 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/clientnegative/file_with_header_footer_negative.q PRE-CREATION
> ql/src/test/queries/clientpositive/file_with_header_footer.q PRE-CREATION
> ql/src/test/results/clientnegative/file_with_header_footer_negative.q.out PRE-CREATION
> ql/src/test/results/clientpositive/file_with_header_footer.q.out PRE-CREATION
> serde/if/serde.thrift 2ceb572
> serde/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/serde/serdeConstants.java 22a6168
>
> Diff: https://reviews.apache.org/r/16184/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Shuaishuai Nie
>
>
Re: Review Request 16184: Hive should be able to skip header and footer rows
when reading data file for a table (HIVE-5795)
Posted by Eric Hanson <eh...@microsoft.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16184/#review30489
-----------------------------------------------------------
common/src/java/org/apache/hadoop/hive/conf/HiveConf.java
<https://reviews.apache.org/r/16184/#comment58323>
fix comment to say "max number of lines of footer user can set for a table file"
ql/src/java/org/apache/hadoop/hive/ql/exec/FetchOperator.java
<https://reviews.apache.org/r/16184/#comment58324>
Please use block comment style for multi-line comments. Please use full sentences and proper punctuation (start with capital letter, end with period).
- Eric Hanson
On Dec. 11, 2013, 9:19 p.m., Shuaishuai Nie wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/16184/
> -----------------------------------------------------------
>
> (Updated Dec. 11, 2013, 9:19 p.m.)
>
>
> Review request for hive, Eric Hanson 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
> (follow up with review https://reviews.apache.org/r/15663/diff/#index_header)
>
>
> Diffs
> -----
>
> common/src/java/org/apache/hadoop/hive/conf/HiveConf.java fa3e048
> conf/hive-default.xml.template c61a0bb
> 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 c3cbb89
> ql/src/java/org/apache/hadoop/hive/ql/exec/FetchOperator.java d2b2526
> ql/src/java/org/apache/hadoop/hive/ql/io/HiveContextAwareRecordReader.java dd5cb6b
> ql/src/java/org/apache/hadoop/hive/ql/io/HiveInputFormat.java 974a5d6
> 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/clientnegative/file_with_header_footer_negative.q PRE-CREATION
> ql/src/test/queries/clientpositive/file_with_header_footer.q PRE-CREATION
> ql/src/test/results/clientnegative/file_with_header_footer_negative.q.out PRE-CREATION
> ql/src/test/results/clientpositive/file_with_header_footer.q.out PRE-CREATION
> serde/if/serde.thrift 2ceb572
> serde/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/serde/serdeConstants.java 22a6168
>
> Diff: https://reviews.apache.org/r/16184/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Shuaishuai Nie
>
>