Michael Ho has posted comments on this change. Change subject: IMPALA-3838, IMPALA-4495: Codegen EvalRuntimeFilters() and fixes filter stats updates ......................................................................
Patch Set 6: (5 comments) http://gerrit.cloudera.org:8080/#/c/4833/6/be/src/exec/hdfs-parquet-scanner.cc File be/src/exec/hdfs-parquet-scanner.cc: Line 186 > what about this? We have to keep all the filters in filter_ctxs_ because when we codegen, no filters have arrived yet so we are forced to codegen for all filters. Therefore, we need to fill in the entire vector here. At runtime, filters which always return true or have low rejection rate would be disabled. Line 61: constexpr int BATCHES_PER_FILTER_SELECTIVITY_CHECK = 16; > just curious, but does this really need constexpr? Using constexpr to make sure it's a compile time constant so it gets constant folded. It's used in GetNext(). Line 63: "ROWS_PER_FILTER_SELECTIVITY_CHECK must be a power of two"); > update Done Line 677: RETURN_IF_ERROR(CodegenEvalRuntimeFilters(codegen, > single line? or move all args to new line Done http://gerrit.cloudera.org:8080/#/c/4833/6/be/src/util/bloom-filter.h File be/src/util/bloom-filter.h: Line 55: /// machines. > a brief sentence on why noexcept here is important. Done -- To view, visit http://gerrit.cloudera.org:8080/4833 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I27114869840e268d17e91d6e587ef811628e3837 Gerrit-PatchSet: 6 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Michael Ho <k...@cloudera.com> Gerrit-Reviewer: Dan Hecht <dhe...@cloudera.com> Gerrit-Reviewer: Marcel Kornacker <mar...@cloudera.com> Gerrit-Reviewer: Michael Ho <k...@cloudera.com> Gerrit-Reviewer: Mostafa Mokhtar <mmokh...@cloudera.com> Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-HasComments: Yes
