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 12:

(9 comments)

Did a first round, found a couple of nits

http://gerrit.cloudera.org:8080/#/c/16720/12//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/16720/12//COMMIT_MSG@20
PS12, Line 20: togther
nit: together


http://gerrit.cloudera.org:8080/#/c/16720/12//COMMIT_MSG@21
PS12, Line 21: evaualted
nit: evaluated


http://gerrit.cloudera.org:8080/#/c/16720/12/be/src/exec/hdfs-scan-node-base.cc
File be/src/exec/hdfs-scan-node-base.cc:

http://gerrit.cloudera.org:8080/#/c/16720/12/be/src/exec/hdfs-scan-node-base.cc@231
PS12, Line 231:   for (const auto& entry : 
tnode.hdfs_scan_node.overlap_predicate_filter_ids) {
              :     overlap_predicate_filter_ids_.push_back(entry);
              :   }
if 'overlap_predicate_filter_ids' is also a vector then probably we should just 
use

 overlap_predicate_filter_ids_ = 
tnode.hdfs_scan_node.overlap_predicate_filter_ids;


http://gerrit.cloudera.org:8080/#/c/16720/12/be/src/exec/parquet/hdfs-parquet-scanner.cc
File be/src/exec/parquet/hdfs-parquet-scanner.cc:

http://gerrit.cloudera.org:8080/#/c/16720/12/be/src/exec/parquet/hdfs-parquet-scanner.cc@553
PS12, Line 553:
nit: extra space


http://gerrit.cloudera.org:8080/#/c/16720/12/be/src/exec/parquet/hdfs-parquet-scanner.cc@595
PS12, Line 595: 4
nit: magic constant


http://gerrit.cloudera.org:8080/#/c/16720/12/be/src/exec/parquet/hdfs-parquet-scanner.cc@811
PS12, Line 811: return pnode.overlap_predicate_start_index_;
nit: seems like it's also a member of the scan node.


http://gerrit.cloudera.org:8080/#/c/16720/12/be/src/exec/parquet/hdfs-parquet-scanner.cc@817
PS12, Line 817:   return pnode.overlap_predicate_filter_ids_;
nit: seems like it's also a member of the scan node.


http://gerrit.cloudera.org:8080/#/c/16720/12/be/src/exec/parquet/hdfs-parquet-scanner.cc@950
PS12, Line 950: 2
nit: magic constant


http://gerrit.cloudera.org:8080/#/c/16720/12/be/src/exec/parquet/hdfs-parquet-scanner.cc@976
PS12, Line 976: 4
nit: magic constant



--
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: 12
Gerrit-Owner: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <csringho...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com>
Gerrit-Reviewer: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <borokna...@cloudera.com>
Gerrit-Comment-Date: Mon, 23 Nov 2020 15:54:04 +0000
Gerrit-HasComments: Yes

Reply via email to