Michael Ho has posted comments on this change. Change subject: PREVIEW do parquet schema resolution by name instead of ordinal ......................................................................
Patch Set 1: (4 comments) http://gerrit.cloudera.org:8080/#/c/2384/1/be/src/exec/hdfs-parquet-scanner.cc File be/src/exec/hdfs-parquet-scanner.cc: Line 2005: scan_node_->hdfs_table()->col_descs()[table_idx].name() : : col_type->field_names[table_idx]; nit: wrong indentation. Line 2010: "File $0 does not contain path '$1' May be we can factor this string out or if you take the recommendation below, then we will have one copy of this only. Line 2019: if (node->children.size() <= file_idx) { : VLOG_FILE << Substitute( : "File '$0' does not contain path '$1' (resolving by ordinal)", filename(), : PrintPath(path)); : *missing_field = true; : return NULL; : } It seems that this code block can be factored out to the common path. It appears to me that the difference between if-stmt body and else-stmt body is the resolution of the file_idx. So, if we fail to search for a matching children above, file_idx == children.size() so it should fail. Similar comments for 'return &node->children[file_idx];'. http://gerrit.cloudera.org:8080/#/c/2384/1/be/src/exec/hdfs-parquet-scanner.h File be/src/exec/hdfs-parquet-scanner.h: Line 598: its nit: it's -- To view, visit http://gerrit.cloudera.org:8080/2384 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id0c715ea23792b2a6872610839a40532aabbb5a6 Gerrit-PatchSet: 1 Gerrit-Project: Impala Gerrit-Branch: cdh5-trunk Gerrit-Owner: Skye Wanderman-Milne <[email protected]> Gerrit-Reviewer: Dan Hecht <[email protected]> Gerrit-Reviewer: Michael Ho <[email protected]> Gerrit-Reviewer: Silvius Rus <[email protected]> Gerrit-HasComments: Yes
