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 4: (5 comments) http://gerrit.cloudera.org:8080/#/c/15087/3//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/15087/3//COMMIT_MSG@10 PS3, Line 10: add > nit: adds will fix it http://gerrit.cloudera.org:8080/#/c/15087/3//COMMIT_MSG@14 PS3, Line 14: ting: > Please create an automatic test for this. Checking the example in the jira will add automatic tests in parquet-stats.test. http://gerrit.cloudera.org:8080/#/c/15087/3//COMMIT_MSG@16 PS3, Line 16: some values, then alter table to change the column data type as > nit: please wrap the commit message at 72 will fix it. http://gerrit.cloudera.org:8080/#/c/15087/3/be/src/exec/parquet/parquet-column-stats.cc File be/src/exec/parquet/parquet-column-stats.cc: http://gerrit.cloudera.org:8080/#/c/15087/3/be/src/exec/parquet/parquet-column-stats.cc@54 PS3, Line 54: tatsField::MIN: > This check is needed with max_value before using stats.max_value for paired will fix it. http://gerrit.cloudera.org:8080/#/c/15087/3/be/src/exec/parquet/parquet-column-stats.cc@106 PS3, Line 106: (ColumnStats<int32 > I think that we should return false even if the paired stats do not exist - will fix it as suggested. -- 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: 4 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: Thu, 30 Jan 2020 00:37:37 +0000 Gerrit-HasComments: Yes
