Tianyi Wang has posted comments on this change. ( http://gerrit.cloudera.org:8080/8170 )
Change subject: IMPALA-5789: Add always_false flag in bloom filter ...................................................................... Patch Set 7: (5 comments) http://gerrit.cloudera.org:8080/#/c/8170/4//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/8170/4//COMMIT_MSG@15 PS4, Line 15: In single node perf tests, time spent on primitive_empty_build_join_1 > Did you do any perf runs? It would be good to verify that the extra flag ch Commit message is updated with perf run result and the complete report is posted on JIRA. primitive_empty_build_join_1 does get faster. http://gerrit.cloudera.org:8080/#/c/8170/4/be/src/exec/hdfs-parquet-scanner.cc File be/src/exec/hdfs-parquet-scanner.cc: http://gerrit.cloudera.org:8080/#/c/8170/4/be/src/exec/hdfs-parquet-scanner.cc@403 PS4, Line 403: std:: > Shouldn't need std:: - it's imported in common/names.h It turns out that there is an llvm::make_unique and this file uses llvm namespace so it cannot be removed. http://gerrit.cloudera.org:8080/#/c/8170/4/be/src/exec/hdfs-scanner.cc File be/src/exec/hdfs-scanner.cc: http://gerrit.cloudera.org:8080/#/c/8170/4/be/src/exec/hdfs-scanner.cc@107 PS4, Line 107: bool is_sequence_based = BaseSequenceScanner::FileFormatIsSequenceBased( > Might be more readable to factor subexpression into variable e.g. is_sequen Done http://gerrit.cloudera.org:8080/#/c/8170/4/tests/custom_cluster/test_always_false_filter.py File tests/custom_cluster/test_always_false_filter.py: http://gerrit.cloudera.org:8080/#/c/8170/4/tests/custom_cluster/test_always_false_filter.py@53 PS4, Line 53: assert re.search("Splits rejected: 8 \(8\)", profile) is not None > We'd normally create a test matrix based on these. Is the idea here to avoi Done http://gerrit.cloudera.org:8080/#/c/8170/4/tests/custom_cluster/test_always_false_filter.py@82 PS4, Line 82: > Does this need to be a custom cluster test? I.e. does it need a special min It is moved into tests/query_test/test_runtime_filters.py -- To view, visit http://gerrit.cloudera.org:8080/8170 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If680240a3cd4583fc97c3192177d86d9567c4f8d Gerrit-Change-Number: 8170 Gerrit-PatchSet: 7 Gerrit-Owner: Tianyi Wang <[email protected]> Gerrit-Reviewer: Sailesh Mukil <[email protected]> Gerrit-Reviewer: Thomas Tauber-Marshall <[email protected]> Gerrit-Reviewer: Tianyi Wang <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-Comment-Date: Thu, 19 Oct 2017 18:20:59 +0000 Gerrit-HasComments: Yes
