Mailing-List: contact dev-help@impala.incubator.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@impala.incubator.apache.org
Message-Id: <201608150123.u7F1NKgt019335@ip-10-146-233-104.ec2.internal>
Date: Mon, 15 Aug 2016 01:23:20 +0000
From: "Alex Behm (Code Review)" <gerrit@cloudera.org>
To: impala-cr@cloudera.com, dev@impala.incubator.apache.org
CC: Matthew Jacobs <mj@cloudera.com>
Reply-To: alex.behm@cloudera.com
Subject: =?UTF-8?Q?=5BImpala-ASF-CR=5D_IMPALA-3952=3A_Clear_scratch_batch_mem_pool_if_Open=28=29_failed=2E=0A?=
In-Reply-To: <gerrit.1470976704000.I55f32ed698a5b6fed8c28af1391aa07e1560e782@gerrit.cloudera.org>
References: <gerrit.1470976704000.I55f32ed698a5b6fed8c28af1391aa07e1560e782@gerrit.cloudera.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Content-Disposition: inline
User-Agent: Gerrit/2.12.2
archived-at: Mon, 15 Aug 2016 01:23:26 -0000

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