Michael Ho has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8814 )

Change subject: IMPALA-6290: limit ScannerContext to 1 buffer at a time
......................................................................


Patch Set 6:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/8814/6/be/src/exec/scanner-context.h
File be/src/exec/scanner-context.h:

http://gerrit.cloudera.org:8080/#/c/8814/6/be/src/exec/scanner-context.h@162
PS6, Line 162:     /// Equivalent to java.io.DataInput.readBoolean()
May help to document what returning false means for the Read*() functions.


http://gerrit.cloudera.org:8080/#/c/8814/6/be/src/exec/scanner-context.cc
File be/src/exec/scanner-context.cc:

http://gerrit.cloudera.org:8080/#/c/8814/6/be/src/exec/scanner-context.cc@94
PS6, Line 94:     boundary_buffer_pos_ = nullptr;
            :     boundary_buffer_bytes_left_ = 0;
            :     boundary_buffer_->Reset();
Why weren't we doing it before ?


http://gerrit.cloudera.org:8080/#/c/8814/6/be/src/exec/scanner-context.cc@99
PS6, Line 99: ReturnIoBuffer()
I wonder if it warrants poisoning the IOBuffer returned to catch any cases in 
which the buffer is referenced after return to IOMgr.



--
To view, visit http://gerrit.cloudera.org:8080/8814
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I74c5960a75f7d88b0e1de4199af731fb13e592f0
Gerrit-Change-Number: 8814
Gerrit-PatchSet: 6
Gerrit-Owner: Tim Armstrong <tarmstr...@cloudera.com>
Gerrit-Reviewer: Lars Volker <l...@cloudera.com>
Gerrit-Reviewer: Michael Ho <k...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com>
Gerrit-Comment-Date: Tue, 09 Jan 2018 20:30:51 +0000
Gerrit-HasComments: Yes

Reply via email to