impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Alex Behm (Code Review)" <>
Subject [Impala-ASF-CR] IMPALA-4725: Query option to control Parquet array resolution.
Date Tue, 07 Mar 2017 05:59:41 GMT
Alex Behm has posted comments on this change.

Change subject: IMPALA-4725: Query option to control Parquet array resolution.

Patch Set 1:


All good suggestions, thanks! Reworked the code accordingly.
Commit Message:

PS1, Line 11: resoution
> resolution

PS1, Line 29: resoution
> resolution
File be/src/exec/

PS1, Line 409: 3)
> what's this? the number of enum elements? consider defining NUM_ARRAY_ENCOD
Removed this. Doesn't seem necessary with the new static table.

Line 416:   Status one_level_status = Status::OK();
> could be:
Reworked as suggested.
File be/src/exec/parquet-metadata-utils.h:

Line 144:     GetOrderedArrayEncodings(array_resolution_, &ordered_array_encodings_);
> why not do this in the constructor and don't bother saving array_resolution
Good point. Using a static array now.

Line 190:   void GetOrderedArrayEncodings(TParquetArrayResolution::type array_resolution,
> could be static, then there's no question that array_resolution is the only
I removed this function.
File common/thrift/ImpalaService.thrift:

Line 264:   // it non-default in a compatibility breaking release.
> why want a jira for that, or else we'll likely forget to do it.
Filed IMPALA-5037.

To view, visit
To unsubscribe, visit

Gerrit-MessageType: comment
Gerrit-Change-Id: I4f32e19ec542d4d485154c9d65d0f5e3f9f0a907
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Alex Behm <>
Gerrit-Reviewer: Alex Behm <>
Gerrit-Reviewer: Dan Hecht <>
Gerrit-HasComments: Yes

View raw message