Marcel Kornacker has posted comments on this change.

Change subject: IMPALA-3845: Split up hdfs-parquet-scanner.cc into more 
files/components.
......................................................................


Patch Set 3:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/3596/3/be/src/exec/hdfs-parquet-scanner.cc
File be/src/exec/hdfs-parquet-scanner.cc:

Line 2402
what happened to this?


Line 1078: HdfsParquetScanner::FileVersion::FileVersion(const string& 
created_by) {
also move this out?


Line 1126: Status HdfsParquetScanner::ValidateFileMetadata() {
can all metadata handling be moved to schema-resolver?


http://gerrit.cloudera.org:8080/#/c/3596/3/be/src/exec/parquet-column-readers.h
File be/src/exec/parquet-column-readers.h:

Line 15: #ifndef IMPALA_PARQUET_COLUMN_READERS_H
why not call this parquet-column-reader.h?


http://gerrit.cloudera.org:8080/#/c/3596/3/be/src/exec/parquet-schema-resolver.cc
File be/src/exec/parquet-schema-resolver.cc:

Line 35: string PrintRepetitionType(const parquet::FieldRepetitionType::type& 
t) {
are these meant to be globally visible?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I4c5fd46f9c1a0ff2a4c30ea5a712fbae17c68f92
Gerrit-PatchSet: 3
Gerrit-Project: Impala
Gerrit-Branch: cdh5-trunk
Gerrit-Owner: Alex Behm <[email protected]>
Gerrit-Reviewer: Alex Behm <[email protected]>
Gerrit-Reviewer: Marcel Kornacker <[email protected]>
Gerrit-Reviewer: Tim Armstrong <[email protected]>
Gerrit-HasComments: Yes

Reply via email to