Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/16720 )
Change subject: IMPALA-10325: Parquet scan should use min/max statistics to skip pages based on equi-join predicate ...................................................................... Patch Set 63: (4 comments) Left a few comments, mostly focusing on the BE. But looks good overall. http://gerrit.cloudera.org:8080/#/c/16720/63//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/16720/63//COMMIT_MSG@16 PS63, Line 16: followign nit: following http://gerrit.cloudera.org:8080/#/c/16720/63/be/src/exec/parquet/hdfs-parquet-scanner.cc File be/src/exec/parquet/hdfs-parquet-scanner.cc: http://gerrit.cloudera.org:8080/#/c/16720/63/be/src/exec/parquet/hdfs-parquet-scanner.cc@722 PS63, Line 722: if (FindMinMaxFilter(desc.filter_id)) { : return true; : } shouldn't we check FilterStats.enabled_for_page here? http://gerrit.cloudera.org:8080/#/c/16720/63/be/src/exec/parquet/hdfs-parquet-scanner.cc@797 PS63, Line 797: continue nit: Since we have a 'continue' here there's no need to put the EvaluateOverlapForRowGroup() into an else branch. http://gerrit.cloudera.org:8080/#/c/16720/63/be/src/util/min-max-filter.cc File be/src/util/min-max-filter.cc: http://gerrit.cloudera.org:8080/#/c/16720/63/be/src/util/min-max-filter.cc@541 PS63, Line 541: // TODO: implement the overlap computation for TimestampValue. : return 0.0; This will cause the row groups to be rejected by the HdfsParquetScanner if we have a min/max filter on timestamp. -- To view, visit http://gerrit.cloudera.org:8080/16720 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I379405ee75b14929df7d6b5d20dabc6f51375691 Gerrit-Change-Number: 16720 Gerrit-PatchSet: 63 Gerrit-Owner: Qifan Chen <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Qifan Chen <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-Reviewer: Zoltan Borok-Nagy <[email protected]> Gerrit-Comment-Date: Wed, 10 Feb 2021 14:18:20 +0000 Gerrit-HasComments: Yes
