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

Change subject: IMPALA-8656: Re-factor PlanRootSink into blocking and buffered 
implementations
......................................................................


Patch Set 6: Code-Review+2

(2 comments)

http://gerrit.cloudera.org:8080/#/c/13873/6/be/src/exec/buffered-plan-root-sink.h
File be/src/exec/buffered-plan-root-sink.h:

http://gerrit.cloudera.org:8080/#/c/13873/6/be/src/exec/buffered-plan-root-sink.h@24
PS6, Line 24: /// PlanRootSink that buffers RowBatches from the 'sender' 
(fragment) thread. RowBatches
            : /// are buffered in memory until a memory limit is hit.
This is just for the initial implementation only, right ? We kind need to 
support spilling to disk eventually so we can produce all the possible result 
rows from the underlying plan tree.

As it's described now, it's not completely solving the problem of a client not 
fetching all result rows.


http://gerrit.cloudera.org:8080/#/c/13873/6/be/src/exec/plan-root-sink.h
File be/src/exec/plan-root-sink.h:

http://gerrit.cloudera.org:8080/#/c/13873/6/be/src/exec/plan-root-sink.h@a67
PS6, Line 67:
            :
            :
            :
            :
            :
            :
            :
            :
nit: these are part of the interfaces of DataSink. It seems clearer to just 
leave them here as pure virtual functions.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8786b1a9af68ab0a8a094970d8f955eb20d04bca
Gerrit-Change-Number: 13873
Gerrit-PatchSet: 6
Gerrit-Owner: Sahil Takiar <stak...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com>
Gerrit-Reviewer: Michael Ho <k...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <stak...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com>
Gerrit-Comment-Date: Thu, 18 Jul 2019 04:58:46 +0000
Gerrit-HasComments: Yes

Reply via email to