Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/17026 )
Change subject: IMPALA-10640: Support reading Parquet Bloom filters - most common types ...................................................................... Patch Set 30: Code-Review+1 (3 comments) I can give +2 once these comments are resolved. A note about AVX2: I am ok with committing it as it is and clean it up in a later commit, as lot of cleanup should be done in other parts related to AVX2. http://gerrit.cloudera.org:8080/#/c/17026/30/be/src/util/parquet-bloom-filter-avx2.cc File be/src/util/parquet-bloom-filter-avx2.cc: http://gerrit.cloudera.org:8080/#/c/17026/30/be/src/util/parquet-bloom-filter-avx2.cc@17 PS30, Line 17: Can you add a comment to the files that are mainly copied from Kudu code about their source? http://gerrit.cloudera.org:8080/#/c/17026/30/be/src/util/parquet-bloom-filter.cc File be/src/util/parquet-bloom-filter.cc: http://gerrit.cloudera.org:8080/#/c/17026/30/be/src/util/parquet-bloom-filter.cc@37 PS30, Line 37: DEFINE_bool(disable_parquetbloomfilter_avx2, false, Note that AVX2 support became required recently in x86_64: https://gerrit.cloudera.org/#/c/17406/ http://gerrit.cloudera.org:8080/#/c/17026/30/testdata/workloads/functional-query/queries/QueryTest/parquet-bloom-filter.test File testdata/workloads/functional-query/queries/QueryTest/parquet-bloom-filter.test: http://gerrit.cloudera.org:8080/#/c/17026/30/testdata/workloads/functional-query/queries/QueryTest/parquet-bloom-filter.test@9 PS30, Line 9: select int8_col from parquet_bloom_filter where int8_col = 1; Can you add some tests with more predicates? HdfsParquetScanner::CreateColIdx2EqConjunctMap() has quite complex logic and some issues may not come up if there is only a single eq predicate. Some examples: - other predicates on a column besides = - eq predicates on more than one column -- To view, visit http://gerrit.cloudera.org:8080/17026 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7119c7161fa3658e561fc1265430cb90079d8287 Gerrit-Change-Number: 17026 Gerrit-PatchSet: 30 Gerrit-Owner: Daniel Becker <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Daniel Becker <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Tamas Mate <[email protected]> Gerrit-Reviewer: Zoltan Borok-Nagy <[email protected]> Gerrit-Comment-Date: Tue, 18 May 2021 08:13:33 +0000 Gerrit-HasComments: Yes
