Lars Volker has posted comments on this change. Change subject: IMPALA-1740: Add support for skip.header.line.count. ......................................................................
Patch Set 17: (6 comments) Thanks for the review, please see PS 18. 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 Done 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? Done 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 Done 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 Done 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? Done 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 Done -- 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 <[email protected]> Gerrit-Reviewer: Lars Volker <[email protected]> Gerrit-Reviewer: Marcel Kornacker <[email protected]> Gerrit-Reviewer: Skye Wanderman-Milne <[email protected]> Gerrit-HasComments: Yes
