You are viewing a plain text version of this content. The canonical link for it is here.

Posted to reviews@impala.apache.org by "Alex Behm (Code Review)" <ge...@cloudera.org> on 2017/01/23 23:11:00 UTC

[Impala-ASF-CR] IMPALA-4263: Fix wrong ommission of agg/analytic hash exchanges.

Alex Behm has uploaded a new change for review.

  http://gerrit.cloudera.org:8080/5774

Change subject: IMPALA-4263: Fix wrong ommission of agg/analytic hash exchanges.
......................................................................

IMPALA-4263: Fix wrong ommission of agg/analytic hash exchanges.

The bug: Our detection of partition compatibility for
grouping aggregations and analytic functions did not take into
account the effect of outer joins within the same fragment.
As a result, we used to incorrectly omit a required hash exchange.
For example, a hash exchange + merge phase is required if the
grouping expressions of an aggregation reference tuples
that are made nullable within the same fragment. The exchange is
needed to bring together NULLs produced by outer-join non-matches.

The fix: Check that the grouping/partition exprs do not reference
tuples that are made nullable within the same fragment.

Testing: Planner tests pass locally.

Change-Id: I121222179378e56836422a69451d840a012c9e54
---
M fe/src/main/java/org/apache/impala/planner/DistributedPlanner.java
M fe/src/main/java/org/apache/impala/planner/PlanFragment.java
M testdata/workloads/functional-planner/queries/PlannerTest/aggregation.test
M testdata/workloads/functional-planner/queries/PlannerTest/analytic-fns.test
4 files changed, 192 insertions(+), 25 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/74/5774/1
-- 
To view, visit http://gerrit.cloudera.org:8080/5774
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I121222179378e56836422a69451d840a012c9e54
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Alex Behm <al...@cloudera.com>

[Impala-ASF-CR] IMPALA-4263: Fix wrong ommission of agg/analytic hash exchanges.

Posted by "Marcel Kornacker (Code Review)" <ge...@cloudera.org>.

Marcel Kornacker has posted comments on this change.

Change subject: IMPALA-4263: Fix wrong ommission of agg/analytic hash exchanges.
......................................................................


Patch Set 1: Code-Review+2

(3 comments)

http://gerrit.cloudera.org:8080/#/c/5774/1/fe/src/main/java/org/apache/impala/planner/DistributedPlanner.java
File fe/src/main/java/org/apache/impala/planner/DistributedPlanner.java:

Line 799:         if (!childFragment.refsNullableTupleId(partitionExprs)) {
combine with preceding if


http://gerrit.cloudera.org:8080/#/c/5774/1/fe/src/main/java/org/apache/impala/planner/PlanFragment.java
File fe/src/main/java/org/apache/impala/planner/PlanFragment.java:

Line 386:    * Returns true if 'exprs' reference a tuple that is made nullable in this fragment.
"in this fragment but not in any of its input fragments"


Line 390:     List<TupleId> groupingExprTids = Lists.newArrayList();
remove the reference to grouping, it's overly specific.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I121222179378e56836422a69451d840a012c9e54
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Marcel Kornacker <ma...@cloudera.com>
Gerrit-HasComments: Yes