impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Vuk Ercegovac (Code Review)" <>
Subject [Impala-ASF-CR] IMPALA-4993: extend dictionary filtering to collections
Date Tue, 09 Jan 2018 23:58:02 GMT
Vuk Ercegovac has posted comments on this change. ( )

Change subject: IMPALA-4993: extend dictionary filtering to collections

Patch Set 6:

File be/src/exec/hdfs-parquet-scanner.h:
PS6, Line 462:   std::unordered_map<const TupleDescriptor*, std::unique_ptr<ScopedBuffer>>
> It would make more sense at this point to use a MemPool instead of a prolif
Replaced with MemPool.

It was unclear to me whether to use MemPool or ScopedBuffer for this case (I didn't see anything
in ScopedBuffer to point me in this direction). Let me know if you'd like:
1) a blurb in ScopedBuffer that refers to MemPool for the right use-cases
2) min_max_tuple_buffer also replaced to use MemPool.
PS6, Line 649:   /// Gets the TupleDescriptor of slot_desc.
> Mention that 'slot_desc' can belong to the top-level tuple or a tuple neste
File be/src/exec/
PS6, Line 771:   if (!col_reader->IsCollectionReader()) {
> nit: I think this would be easier to follow if we reversed the branches and
PS6, Line 815:   for (auto* col_reader : column_readers_) {
> nit: could fit on one line now
PS6, Line 1657:   if (column_readers.empty()) return Status::OK();
> Is the early exit necessary for correctness? Might be worth mentioning if i

To view, visit
To unsubscribe, visit

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If3a2abcfc3d0f7d18756816659fed77ce12668dd
Gerrit-Change-Number: 8775
Gerrit-PatchSet: 6
Gerrit-Owner: Vuk Ercegovac <>
Gerrit-Reviewer: Alex Behm <>
Gerrit-Reviewer: Lars Volker <>
Gerrit-Reviewer: Tim Armstrong <>
Gerrit-Reviewer: Vuk Ercegovac <>
Gerrit-Comment-Date: Tue, 09 Jan 2018 23:58:02 +0000
Gerrit-HasComments: Yes

  • Unnamed multipart/alternative (inline, 8-Bit, 0 bytes)
View raw message