[Impala-CR](cdh5-trunk) IMPALA-1740: Add support for skip.header.line.count.

["Marcel Kornacker (Code Review)" <ge...@cloudera.org>]

Marcel Kornacker has posted comments on this change.

Change subject: IMPALA-1740: Add support for skip.header.line.count.
......................................................................


Patch Set 17:

(6 comments)

http://gerrit.cloudera.org:8080/#/c/2110/17/be/src/exec/hdfs-scanner.cc
File be/src/exec/hdfs-scanner.cc:

Line 646:     if (scan_node_->skip_header_line_count() > 1) {
would be good to have a test case somewhere against a multi-format table (w/ text and parquet) where skip.header.linecount is set


http://gerrit.cloudera.org:8080/#/c/2110/17/fe/src/main/java/com/cloudera/impala/analysis/BaseTableRef.java
File fe/src/main/java/com/cloudera/impala/analysis/BaseTableRef.java:

Line 88:       if (error.length() > 0) {
single line?


http://gerrit.cloudera.org:8080/#/c/2110/17/fe/src/main/java/com/cloudera/impala/analysis/CreateTableStmt.java
File fe/src/main/java/com/cloudera/impala/analysis/CreateTableStmt.java:

Line 245:     HdfsFileFormat fileFormat = HdfsFileFormat.fromThrift(fileFormat_);
make this block a static function this class or the AlterTable.. class and then call that from both places.


http://gerrit.cloudera.org:8080/#/c/2110/17/fe/src/main/java/com/cloudera/impala/catalog/HdfsTable.java
File fe/src/main/java/com/cloudera/impala/catalog/HdfsTable.java:

Line 1232:             "Could not parse table property %s: %s (expecting integer value >= 0)",
i think you can return the same error message as in line 1237 here


http://gerrit.cloudera.org:8080/#/c/2110/17/fe/src/main/java/com/cloudera/impala/planner/HdfsScanNode.java
File fe/src/main/java/com/cloudera/impala/planner/HdfsScanNode.java:

Line 120:   private int skipHeaderLineCount_;
then init to 0?


http://gerrit.cloudera.org:8080/#/c/2110/17/fe/src/main/java/com/cloudera/impala/planner/SingleNodePlanner.java
File fe/src/main/java/com/cloudera/impala/planner/SingleNodePlanner.java:

Line 1290:       if (hdfsTable.hasSkipHeaderLineCount()) {
the planner shouldn't have to deal with this, do it in the scan node c'tor (and pass in the tbl ref)


-- 
To view, visit http://gerrit.cloudera.org:8080/2110
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I595f01a165d41499ca1956fe748ba3840a6eb543
Gerrit-PatchSet: 17
Gerrit-Project: Impala
Gerrit-Branch: cdh5-trunk
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Marcel Kornacker <ma...@cloudera.com>
Gerrit-Reviewer: Skye Wanderman-Milne <sk...@cloudera.com>
Gerrit-HasComments: Yes