Thomas Tauber-Marshall has posted comments on this change.

Change subject: IMPALA-5611: KuduPartitionExpr holds onto memory unnecessarily
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/7346/1/be/src/exprs/kudu-partition-expr.cc
File be/src/exprs/kudu-partition-expr.cc:

Line 87:   eval->FreeLocalAllocations();
> Can you free the allocations outside of the expr? i.e. in DataStreamSender:
Two problems I see with that:
1. I'm not sure how we get the ScalarExprEvaluator object to call it on, since 
its a parameter to KuduPartitionExpr::GetIntVal. Is it safe to store a pointer 
to that in KuduPartitionExpr? Are we always guaranteed that it will be passed 
the same KuduPartiitonExpr?
2. This also needs to work for the Sort case, where we're just passing the 
KuduPartitionExpr as a regular Expr to sort on.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia661eb8bed114070728a1497ccf7ed6893237e5e
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Thomas Tauber-Marshall <[email protected]>
Gerrit-Reviewer: Matthew Jacobs <[email protected]>
Gerrit-Reviewer: Thomas Tauber-Marshall <[email protected]>
Gerrit-Reviewer: Tim Armstrong <[email protected]>
Gerrit-HasComments: Yes

Reply via email to