Thomas Marshall has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10394 )

Change subject: IMPALA-110 (part 2): Refactor PartitionedAggregationNode
......................................................................


Patch Set 4:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/10394/3/be/src/exec/aggregator.h
File be/src/exec/aggregator.h:

http://gerrit.cloudera.org:8080/#/c/10394/3/be/src/exec/aggregator.h@54
PS3, Line 54: n the Aggregati
> Used for other exec nodes, no? (e.g. streaming)
Done


http://gerrit.cloudera.org:8080/#/c/10394/3/be/src/exec/aggregator.h@63
PS3, Line 63:
            :   /// Aggregators follow the same lifecycle as ExecNodes, except 
that after Open() and
            :   /// before GetNext() rows should be added with AddBatch(), 
followed by InputDone()[
            :   virtual Status Init(const TPlanNode& tnode, RuntimeState* 
state) WARN_UNUSED_RESULT;
            :   virtual Status Prepare(RuntimeState* state) WARN_UNUSED_RESULT;
            :   virtual void Codegen(RuntimeState* state) = 0;
            :   virtual Status Open(RuntimeState* state) WARN_UNUSED_RESULT;
            :   virtual Status GetNext(
> I guess these are used one-to-one to implement the ExecNode interface? Or a
Done


http://gerrit.cloudera.org:8080/#/c/10394/3/be/src/exec/aggregator.h@80
PS3, Line 80: um_grouping_exp
> why does the aggregator need to copy the debug_options? where does it get u
It's used for the 'deny reservation' debug action, since each Aggregator 
initializes its own BufferPool::ClientHandle via the ReservationManager.


http://gerrit.cloudera.org:8080/#/c/10394/3/be/src/exec/streaming-aggregation-node.h
File be/src/exec/streaming-aggregation-node.h:

http://gerrit.cloudera.org:8080/#/c/10394/3/be/src/exec/streaming-aggregation-node.h@31
PS3, Line 31: Node for doing streaming partitioned hash aggregation.
            : ///
            : /// This node consumes the input from child(0) during GetNext() 
and then passes it to the
            : /// Aggregator, which does the actual work of aggr
> somewhere in here we should explain how this differs from AggregationNode,
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9e7bb583f54aa4add3738bde7f57cf3511ac567e
Gerrit-Change-Number: 10394
Gerrit-PatchSet: 4
Gerrit-Owner: Thomas Marshall <[email protected]>
Gerrit-Reviewer: Alex Behm <[email protected]>
Gerrit-Reviewer: Dan Hecht <[email protected]>
Gerrit-Reviewer: Thomas Marshall <[email protected]>
Gerrit-Reviewer: Tim Armstrong <[email protected]>
Gerrit-Reviewer: Vuk Ercegovac <[email protected]>
Gerrit-Comment-Date: Thu, 21 Jun 2018 18:02:33 +0000
Gerrit-HasComments: Yes

Reply via email to