Skye Wanderman-Milne has posted comments on this change.

Change subject: IMPALA-2835: introduce PARQUET_FALLBACK_SCHEMA_RESOLUTION query 
option
......................................................................


Patch Set 7:

(3 comments)

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

Line 2031:         TParquetFallbackSchemaResolution::POSITION);
> move this DCHECK to L2065. no reason to have a separate if-stmt for it when
Done. FWIW I put this extra if statement so you don't have to read through all 
of the below to figure out there's only two options, but I'm fine with moving 
it. I'm gonna keep 'resolve_by_name' since I use it on L2078.


http://gerrit.cloudera.org:8080/#/c/2384/7/be/src/service/query-options.cc
File be/src/service/query-options.cc:

Line 371: "0"
> Yeah these are good suggestions, and probably COMPRESSION_CODEC could be br
I added the comment and compare directly against the enum values.


Line 379: position
> in other statuses above, we use CAPS for the option name, no quotes, and al
I used caps, but left out the numerical values since they're not really for 
users.


-- 
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: 7
Gerrit-Project: Impala
Gerrit-Branch: cdh5-trunk
Gerrit-Owner: Skye Wanderman-Milne <[email protected]>
Gerrit-Reviewer: Dan Hecht <[email protected]>
Gerrit-Reviewer: Juan Yu <[email protected]>
Gerrit-Reviewer: Matthew Jacobs <[email protected]>
Gerrit-Reviewer: Michael Ho <[email protected]>
Gerrit-Reviewer: Silvius Rus <[email protected]>
Gerrit-Reviewer: Skye Wanderman-Milne <[email protected]>
Gerrit-HasComments: Yes

Reply via email to