Wenzhe Zhou has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15087 )

Change subject: IMPALA-8110: Fix the Parquet stats filtering issue to correctly 
handle narrowed integer types
......................................................................


Patch Set 7:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/15087/7/be/src/exec/parquet/parquet-column-stats.cc
File be/src/exec/parquet/parquet-column-stats.cc:

http://gerrit.cloudera.org:8080/#/c/15087/7/be/src/exec/parquet/parquet-column-stats.cc@100
PS7, Line 100:         return false;
> here + for SMALLINT: Fits to a one line if without braces.
will remove braces and make the code to one line.


http://gerrit.cloudera.org:8080/#/c/15087/7/be/src/exec/parquet/parquet-column-stats.cc@102
PS7, Line 102:         ret = 
ColumnStats<int32_t>::DecodePlainValue(*paired_stats_value,
             :             &paired_stats_val, parquet::Type::INT32);
> here + for SMALLINT: doesn't need to be in an else block, as the "if" block
will remove else.


http://gerrit.cloudera.org:8080/#/c/15087/7/testdata/workloads/functional-query/queries/QueryTest/parquet-stats.test
File testdata/workloads/functional-query/queries/QueryTest/parquet-stats.test:

http://gerrit.cloudera.org:8080/#/c/15087/7/testdata/workloads/functional-query/queries/QueryTest/parquet-stats.test@690
PS7, Line 690: i
> An idea to improve readability: i / j / k could be renamed to contain their
Will change column names as suggested.


http://gerrit.cloudera.org:8080/#/c/15087/7/testdata/workloads/functional-query/queries/QueryTest/parquet-stats.test@717
PS7, Line 717: 200
> Here and in other = predicates: it seems more interesting to test for the o
will change code to check overlown value.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id8bdaf4c4b2d0c6ea26d6e9bf013afca647e53a1
Gerrit-Change-Number: 15087
Gerrit-PatchSet: 7
Gerrit-Owner: Wenzhe Zhou <[email protected]>
Gerrit-Reviewer: Bikramjeet Vig <[email protected]>
Gerrit-Reviewer: Csaba Ringhofer <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Wenzhe Zhou <[email protected]>
Gerrit-Comment-Date: Fri, 07 Feb 2020 18:44:35 +0000
Gerrit-HasComments: Yes

Reply via email to