Tim Armstrong 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: Code-Review+2 (3 comments) 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:: > It turns out that there is an llvm::make_unique and this file uses llvm nam Got it, thanks. This seems like a reason to not use "using namespace llvm;" but we don't need to fix that now. Filed IMPALA-6084 http://gerrit.cloudera.org:8080/#/c/8170/7/tests/custom_cluster/test_always_false_filter.py File tests/custom_cluster/test_always_false_filter.py: http://gerrit.cloudera.org:8080/#/c/8170/7/tests/custom_cluster/test_always_false_filter.py@37 PS7, Line 37: query = "select STRAIGHT_JOIN * from alltypes inner join " \ Would be better to use the """ multi-line literals http://gerrit.cloudera.org:8080/#/c/8170/7/tests/query_test/test_runtime_filters.py File tests/query_test/test_runtime_filters.py: http://gerrit.cloudera.org:8080/#/c/8170/7/tests/query_test/test_runtime_filters.py@55 PS7, Line 55: nit: extra space. Maybe also use the """ multi-line literals -- 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 <tw...@cloudera.com> Gerrit-Reviewer: Sailesh Mukil <sail...@cloudera.com> Gerrit-Reviewer: Thomas Tauber-Marshall <tmarsh...@cloudera.com> Gerrit-Reviewer: Tianyi Wang <tw...@cloudera.com> Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-Comment-Date: Thu, 19 Oct 2017 22:47:17 +0000 Gerrit-HasComments: Yes
