----------------------------------------------------------- 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 > >
