Csaba Ringhofer 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 18: (2 comments) http://gerrit.cloudera.org:8080/#/c/16720/16/be/src/exec/parquet/hdfs-parquet-scanner.cc File be/src/exec/parquet/hdfs-parquet-scanner.cc: http://gerrit.cloudera.org:8080/#/c/16720/16/be/src/exec/parquet/hdfs-parquet-scanner.cc@715 PS16, Line 715: } nit: +2 indent http://gerrit.cloudera.org:8080/#/c/16720/18/be/src/exec/parquet/hdfs-parquet-scanner.cc File be/src/exec/parquet/hdfs-parquet-scanner.cc: http://gerrit.cloudera.org:8080/#/c/16720/18/be/src/exec/parquet/hdfs-parquet-scanner.cc@497 PS18, Line 497: DCHECK(all_nulls); : *all_nulls = false; : static char dummy[sizeof(ColumnStatsReader)]; : SchemaNode* node = nullptr; : Status status = HandlePosAndMissingField(slot_desc, missing_field, &node); : : if (!status.ok()) { : return std::pair<Status, ColumnStatsReader>( : status, *reinterpret_cast<ColumnStatsReader*>(dummy)); : } : : if (*missing_field) { : return std::pair<Status, ColumnStatsReader>( : Status::OK(), *reinterpret_cast<ColumnStatsReader*>(dummy)); : } : : int col_idx = node->col_idx; : DCHECK_LT(col_idx, row_group.columns.size()); Can't we move this outside of CreateStatsReader to the callsites? CreateStatsReader could get 'node' as an input parameter. I think that this would simplify code by no longer needing std::pair<Status, ColumnStatsReader> return values -- 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: 18 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, 25 Nov 2020 22:06:25 +0000 Gerrit-HasComments: Yes
