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 5: (4 comments) http://gerrit.cloudera.org:8080/#/c/15087/5/be/src/exec/parquet/parquet-column-stats.cc File be/src/exec/parquet/parquet-column-stats.cc: http://gerrit.cloudera.org:8080/#/c/15087/5/be/src/exec/parquet/parquet-column-stats.cc@51 PS5, Line 51: has_paired_stats > This variable seems redundant as it is set to true in all cases of the swi I was thinking we may add other stats field in the future which is defined without paired stats. Will remote it as suggested. http://gerrit.cloudera.org:8080/#/c/15087/5/be/src/exec/parquet/parquet-column-stats.cc@116 PS5, Line 116: has_paired_stats > I think that this could be simplified a lot - if has_paired_stats is false, Will change code to return true only when both the stats value and its paired stats value are in valid range. http://gerrit.cloudera.org:8080/#/c/15087/5/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/5/testdata/workloads/functional-query/queries/QueryTest/parquet-stats.test@691 PS5, Line 691: PARQUET_READ_STATISTICS > PARQUET_READ_STATISTICS=1 is the default, so it is possibe to omit these li PARQUET_READ_STATISTICS is set as 0 in previous test cases (line 538). To be safe, we should set PARQUET_READ_STATISTICS as 1 here. http://gerrit.cloudera.org:8080/#/c/15087/5/testdata/workloads/functional-query/queries/QueryTest/parquet-stats.test@692 PS5, Line 692: < > Please add a test with = and > to be sure that those work too. will add test cases 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: 5 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, 31 Jan 2020 19:45:39 +0000 Gerrit-HasComments: Yes
