Dan Hecht 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 193: resolve_by_name_(state->query_options().resolve_parquet_by_name), let's just use the query option directly rather than having more state to keep track of. Line 1994: if (resolve_by_name_ && (next_idx == 0 || col_type->type == TYPE_STRUCT)) { why is it that arrays and maps don't also take this path? a comment explaining this might help. also, this logic and the one at L1945 now look kinda similar. i wonder if there's a way to refactor the code? http://gerrit.cloudera.org:8080/#/c/2384/1/be/src/exec/hdfs-parquet-scanner.h File be/src/exec/hdfs-parquet-scanner.h: Line 425: resolve_by_name query option http://gerrit.cloudera.org:8080/#/c/2384/1/be/src/service/query-options.h File be/src/service/query-options.h: Line 77: QUERY_OPT_FN(resolve_parquet_by_name, RESOLVE_PARQUET_BY_NAME); how about making this an enum (i.e. position, name, and eventually field-id). -- 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: Silvius Rus <[email protected]> Gerrit-HasComments: Yes
