impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Dan Hecht (Code Review)" <>
Subject [Impala-ASF-CR] IMPALA-5844: use a MemPool for expr local allocations
Date Mon, 25 Sep 2017 22:33:10 GMT
Dan Hecht has posted comments on this change. ( )

Change subject: IMPALA-5844: use a MemPool for expr local allocations

Patch Set 7:

File be/src/exprs/agg-fn-evaluator.h:

Generally, I think it's hard to understand the two different pools when it comes to aggregates
since we don't document which pool different allocations come from. Maybe there is a simple
high level rule that can be described?
PS7, Line 156:  static void Serialize(const std::vector<AggFnEvaluator*>& evals,
Tuple* dst);
             :   static void GetValue(const std::vector<AggFnEvaluator*>& evals,
Tuple* src, Tuple* dst);
             :   static void Finalize(const std::vector<AggFnEvaluator*>& evals,
Tuple* src, Tuple* dst);
which mem pool can dst varlen data live in? i think we need to document this all more clearly
to make sense of it.
PS7, Line 162: Serialize(), Finalize() and GetValue()
I guess that implies the answer to my previous question, but I think those should be documented
PS7, Line 220: Tuple* dst
which mem pool?
PS7, Line 225: Note that StringVal result is
             :   /// from local allocation (which will be freed in the next QueryMaintenance())
so it
             :   /// needs to be copied out if it needs to survive beyond QueryMaintenance()
(e.g. if
             :   /// 'dst' lives in a row batch).
maybe this should be reworded now that you're introducing a mechanism that allows the "local
allocation" to be against the right mem-pool.

To view, visit
To unsubscribe, visit

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4ba5a7542ed90a49a4b5586c040b5985a7d45b61
Gerrit-Change-Number: 8025
Gerrit-PatchSet: 7
Gerrit-Owner: Tim Armstrong <>
Gerrit-Reviewer: Dan Hecht <>
Gerrit-Reviewer: Tim Armstrong <>
Gerrit-Comment-Date: Mon, 25 Sep 2017 22:33:10 +0000
Gerrit-HasComments: Yes

  • Unnamed multipart/alternative (inline, 8-Bit, 0 bytes)
View raw message