Csaba Ringhofer 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 8:

(2 comments)

Thanks for the changes! The code looks good to me, I have comments only about 
the commit message. Sorry for not pointing this out earlier.

http://gerrit.cloudera.org:8080/#/c/15087/8//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/15087/8//COMMIT_MSG@7
PS8, Line 7: IMPALA-8110: Fix the Parquet stats filtering issue to correctly
           : handle narrowed integer types
nit: can we use a bit shorter title? e.g. Fix Parquet min/max filters for 
narrowed integers types


http://gerrit.cloudera.org:8080/#/c/15087/8//COMMIT_MSG@15
PS8, Line 15:  - Did Manual tests: create table with column as int type, intert
            :    some values, then alter table to change the column data type as
            :    tinyint (int8), insert more values, verify that the query 
return
            :    correct number of rows when PARQUET_READ_STATISTICS is set as 
1.
            :    Did similar tests to change column data type from int to 
smallint,
            :    and from smallint to tinyint.
I think that this comment about manual testing is no longer, informative, as 
the .test file covers all these cases.



--
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: 8
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: Sat, 08 Feb 2020 09:50:56 +0000
Gerrit-HasComments: Yes

Reply via email to