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