Zach Amsden has posted comments on this change. Change subject: IMPALA-4864 Speed up single slot predicates with dictionaries ......................................................................
Patch Set 9: (13 comments) http://gerrit.cloudera.org:8080/#/c/6726/9/be/src/exec/exec-node.cc File be/src/exec/exec-node.cc: PS9, Line 567: skippable_map > skippable_map != nullptr Done PS9, Line 586: 0 > huh ? Was diverting for testing PS9, Line 586: (*skippable_map)[i] == true > (*skippable_map)[i] Done PS9, Line 590: bool > skip_val Done http://gerrit.cloudera.org:8080/#/c/6726/9/be/src/exec/hdfs-parquet-scanner-ir.cc File be/src/exec/hdfs-parquet-scanner-ir.cc: PS9, Line 88: LOG(INFO) << "tuple idx: " << tuple_index; > debugging output ? Yeah, removed this http://gerrit.cloudera.org:8080/#/c/6726/9/be/src/exec/hdfs-parquet-scanner.cc File be/src/exec/hdfs-parquet-scanner.cc: PS9, Line 1050: std::map<SlotId, std::vector<int>>& > DictionaryFilterPositionMap I think it will need to be HdfsScanNodeBase::DictionaryFilterPositionMap - not sure even a using declaration will help have the syntax, but I'll try. http://gerrit.cloudera.org:8080/#/c/6726/9/be/src/exec/hdfs-scan-node-base.h File be/src/exec/hdfs-scan-node-base.h: PS9, Line 167: typedef std::map<SlotId, std::vector<int>> DictFilterConjunctsPositionMap; > Comment what this map stands for ? Done PS9, Line 366: Mapping to original position in conjuncts > Mapping from SlotId to ... Done http://gerrit.cloudera.org:8080/#/c/6726/9/be/src/exec/parquet-column-readers.cc File be/src/exec/parquet-column-readers.cc: PS9, Line 206: IS_FILTERED > Comment what IS_FILTERED stands for. Done PS9, Line 240: (filters == nullptr) ^ IS_FILTERED > Is there a reason to not use DCHECK((filters == nullptr) != IS_FILTERED) ? 1 fewer character? I can change this though. http://gerrit.cloudera.org:8080/#/c/6726/9/be/src/exec/parquet-column-readers.h File be/src/exec/parquet-column-readers.h: PS9, Line 390: return false; > Is this hard coded for testing ? If not, please document why it's always fa No, it gets overridden in the derived class. http://gerrit.cloudera.org:8080/#/c/6726/9/be/src/util/bitmap.h File be/src/util/bitmap.h: PS9, Line 86: (mask & buffer_[word_index]) > (mask & buffer[word_index]) == 0 Done PS9, Line 99: buffer_.size() > num_words_ Done -- To view, visit http://gerrit.cloudera.org:8080/6726 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I65981c89e5292086809ec1268f5a273f4c1fe054 Gerrit-PatchSet: 9 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Zach Amsden <[email protected]> Gerrit-Reviewer: Joe McDonnell <[email protected]> Gerrit-Reviewer: Michael Ho <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-Reviewer: Zach Amsden <[email protected]> Gerrit-HasComments: Yes
