Zoltan Borok-Nagy 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 26: Code-Review+2

(2 comments)

Great work, Daniel!

http://gerrit.cloudera.org:8080/#/c/17026/24//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/17026/24//COMMIT_MSG@7
PS24, Line 7: IMPALA-10640: Support reading Parquet Bloom filters - most common 
types
> No, I don't think we are planning to add filtering for complex types.
Yeah, I was thinking about leaf values inside complex types. I'm OK with 
deferring it. Probably it worth to open a Jira for it.


http://gerrit.cloudera.org:8080/#/c/17026/23/be/src/util/parquet-bloom-filter.cc
File be/src/util/parquet-bloom-filter.cc:

http://gerrit.cloudera.org:8080/#/c/17026/23/be/src/util/parquet-bloom-filter.cc@18
PS23, Line 18: #include "parquet-bloom-filter.h"
             :
             : #include <cmath>
             : #include <cstdint>
             :
             : #include "kudu/util/slice.h"
             : #include "kudu/util/status.h"
             : #include "util/kudu-status-util.h"
             :
             : #include "thirdparty/xxhash/xxhash.h"
> Isn't the first include in a .cc file usually the corresponding .h file? Se
Yeah, I think the include order is fine.



--
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: 26
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: Mon, 19 Apr 2021 13:40:20 +0000
Gerrit-HasComments: Yes

Reply via email to