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 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 switch. Using paired_stats_value's nullness seems enough to express whether the paired stat exists. 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, and the type is a TINYINT/SMALLINT, then we can simply return false without parsing the stats. The same applies to the case when the stat exists, but we couldn't parse it (around line 106) - we should immediately return and in the rest of the switch case we should assume that the paired stats exists and was read successfully. 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 lines. 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. -- 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 <wz...@cloudera.com> Gerrit-Reviewer: Bikramjeet Vig <bikramjeet....@cloudera.com> Gerrit-Reviewer: Csaba Ringhofer <csringho...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com> Gerrit-Comment-Date: Thu, 30 Jan 2020 11:34:26 +0000 Gerrit-HasComments: Yes
