Zach Amsden has posted comments on this change. Change subject: IMPALA-4864 Speed up single slot predicates with dictionaries ......................................................................
Patch Set 18: (4 comments) I could remove the conjunct cost propagation to the backend, but there doesn't seem to be much else to try here. I fought hard for this one but was unable to get a clear win. Special purpose benchmarks (filtered out rows which do timestamp / string conversion) might show a benefit, but is it worth the cost in terms of complexity? http://gerrit.cloudera.org:8080/#/c/6726/18/be/src/exec/hdfs-parquet-scanner.cc File be/src/exec/hdfs-parquet-scanner.cc: Line 738: runtime_filter_position_map_.find(scalar_reader->slot_desc_->id()); use slot_desc->id() instead of scalar_reader->slot_desc Line 913: if (conjuncts == nullptr && runtime_filters.size() == 0) continue; bug (rare but possible if runtime filters get disabled): deferred_dict_init_list.push_back(scalar_reader) http://gerrit.cloudera.org:8080/#/c/6726/18/be/src/exec/parquet-scratch-tuple-batch.h File be/src/exec/parquet-scratch-tuple-batch.h: Line 90: &tuple_mem)); All diffs here could be reverted as using a spare byte in scratch tuple was discontinued. Should not make a difference overall to perf however. http://gerrit.cloudera.org:8080/#/c/6726/18/common/thrift/PlanNodes.thrift File common/thrift/PlanNodes.thrift: Line 516: 25: optional list<i32> conjunct_costs This was a desperate attempt to catch a win which didn't seem to pay off. -- 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: 18 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Zach Amsden <[email protected]> Gerrit-Reviewer: Joe McDonnell <[email protected]> Gerrit-Reviewer: Marcel Kornacker <[email protected]> Gerrit-Reviewer: Michael Ho <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-Reviewer: Zach Amsden <[email protected]> Gerrit-HasComments: Yes
