impala-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Alex Behm (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-3952: Clear scratch batch mem pool if Open() failed.
Date Mon, 15 Aug 2016 01:23:20 GMT
Alex Behm has posted comments on this change.

Change subject: IMPALA-3952: Clear scratch batch mem pool if Open() failed.
......................................................................


Patch Set 1:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/3953/1//COMMIT_MSG
Commit Message:

PS1, Line 11: pts
> ptrs
Done


http://gerrit.cloudera.org:8080/#/c/3953/1/be/src/exec/hdfs-parquet-scanner.cc
File be/src/exec/hdfs-parquet-scanner.cc:

PS1, Line 210: else if (!FLAGS_enable_partitioned_hash_join ||
             :       !FLAGS_enable_partitioned_aggregation) {
> Not necessarily an issue, but it might be better to just
I thought about executing the branch in any case, but I felt it's better to be explicit about
the conditions in which this code is required. Once we finally remove the legacy join/agg
flags, then this code becomes dead and it should be obvious that we can delete it.

Also note that we need this extra branch even if the plan does not contain any legacy joins
or aggs. The main difference in that if we enable a legacy joins or aggs, then the tuple ptrs
of a row batch are allocated from the row batcht's mem pool (see c'tor of row-batch.cc). That's
the main reason why need special handling here because if Open() fails the scratch batch will
still have the tuple ptrs allocated.


PS1, Line 212: pts
> ptrs
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I55f32ed698a5b6fed8c28af1391aa07e1560e782
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Alex Behm <alex.behm@cloudera.com>
Gerrit-Reviewer: Alex Behm <alex.behm@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message