Repository: incubator-impala Updated Branches: refs/heads/master 523130108 -> b14ca6d09
IMPALA-3645: Free probe expressions' local allocations in ConstructBuildSide() With the prefetching changes, the probe expressions' local allocations are no longer freed via QueryMaintenance() in PHJ. Instead, they are freed explicitly in GetNext() after an entire probe batch has been processed. Due to this change in how we handle local allocations of probe expressions, a DCHECK was added to verify that there is no local allocation from the probe expression in ProcessBuildInput(). Turns out that Expr::Open() called in ConstructBuildSide() on the probe expressions may have caused local allocations to occur for certain UDFs (e.g. extract()). This change handles the situation above by freeing local allocations of the probe expressions once before calling ProcessBuildInput() in ConstructBuildSide(). A new regression test is also added for this specific case. Change-Id: I2096ca3e2093c5ab0ecc0e7ca4cd1b5f3c1ed1ed Reviewed-on: http://gerrit.cloudera.org:8080/3253 Reviewed-by: Michael Ho <[email protected]> Tested-by: Internal Jenkins Project: http://git-wip-us.apache.org/repos/asf/incubator-impala/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-impala/commit/b14ca6d0 Tree: http://git-wip-us.apache.org/repos/asf/incubator-impala/tree/b14ca6d0 Diff: http://git-wip-us.apache.org/repos/asf/incubator-impala/diff/b14ca6d0 Branch: refs/heads/master Commit: b14ca6d09fcc431ea585810620c940ee2f537da9 Parents: 710fa06 Author: Michael Ho <[email protected]> Authored: Tue May 31 15:16:30 2016 -0700 Committer: Tim Armstrong <[email protected]> Committed: Thu Jun 2 09:32:54 2016 -0700 ---------------------------------------------------------------------- be/src/exec/partitioned-hash-join-node.cc | 9 ++++++++- .../functional-query/queries/QueryTest/joins.test | 10 ++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/b14ca6d0/be/src/exec/partitioned-hash-join-node.cc ---------------------------------------------------------------------- diff --git a/be/src/exec/partitioned-hash-join-node.cc b/be/src/exec/partitioned-hash-join-node.cc index 640c674..1d45597 100644 --- a/be/src/exec/partitioned-hash-join-node.cc +++ b/be/src/exec/partitioned-hash-join-node.cc @@ -623,6 +623,12 @@ Status PartitionedHashJoinNode::ConstructBuildSide(RuntimeState* state) { } AllocateRuntimeFilters(state); + // The prepare functions of probe expressions may have done local allocations implicitly + // (e.g. calling UdfBuiltins::Lower()). The probe expressions' local allocations need to + // be freed now as they don't get freed again till probing. Other exprs' local allocations + // are freed in ExecNode::FreeLocalAllocations() in ProcessBuildInput(). + ExprContext::FreeLocalAllocations(probe_expr_ctxs_); + // Do a full scan of child(1) and partition the rows. { SCOPED_STOP_WATCH(&built_probe_overlap_stop_watch_); @@ -678,7 +684,8 @@ Status PartitionedHashJoinNode::ProcessBuildInput(RuntimeState* state, int level RETURN_IF_CANCELLED(state); RETURN_IF_ERROR(QueryMaintenance(state)); // 'probe_expr_ctxs_' should have made no local allocations in this function. - DCHECK(!ExprContext::HasLocalAllocations(probe_expr_ctxs_)); + DCHECK(!ExprContext::HasLocalAllocations(probe_expr_ctxs_)) + << Expr::DebugString(probe_expr_ctxs_); if (input_partition_ == NULL) { // If we are still consuming batches from the build side. { http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/b14ca6d0/testdata/workloads/functional-query/queries/QueryTest/joins.test ---------------------------------------------------------------------- diff --git a/testdata/workloads/functional-query/queries/QueryTest/joins.test b/testdata/workloads/functional-query/queries/QueryTest/joins.test index e570d35..ebb6287 100644 --- a/testdata/workloads/functional-query/queries/QueryTest/joins.test +++ b/testdata/workloads/functional-query/queries/QueryTest/joins.test @@ -710,3 +710,13 @@ NULL,1,NULL,1 ---- TYPES TINYINT,TINYINT,TINYINT,TINYINT ==== +---- QUERY +# Regression test for IMPALA-3645. Certain UDFs may do local allocations in Expr::Open(). +# Make sure the DCHECK in PartitionedHashJoinNode::ProcessBuildInput() doesn't fire. +select count(*) from functional.alltypesagg t1 join functional.alltypesagg t2 +on extract(minute from t1.timestamp_col) = extract(hour from t2.timestamp_col); +---- RESULTS +2042200 +---- TYPES +BIGINT +====
