Alex Behm has posted comments on this change.

Change subject: MT: Planner for multi-threaded execution
......................................................................


Patch Set 3:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/2846/3/fe/src/main/java/com/cloudera/impala/planner/ParallelPlanner.java
File fe/src/main/java/com/cloudera/impala/planner/ParallelPlanner.java:

Line 181:     PlanId parentPlanId = join.getFragment().getPlanId();
> i don't think that's really easier. you need to pass that id as an argument
Why would it start out as null? If we call this function then we certainly need 
a cohort id. We can generate one in createBuildPlans() like this:

if (!joins.isEmpty() {
  CohortId cohortId = cohortIdGenerator_.getNextId();
  for (JoinNode join: joins) createBuildPlan(join, cohortId);
}

The flow seems easier to follow.


http://gerrit.cloudera.org:8080/#/c/2846/3/testdata/workloads/functional-planner/queries/PlannerTest/tpcds-all.test
File testdata/workloads/functional-planner/queries/PlannerTest/tpcds-all.test:

Line 129: |  hash table id: 00
> in this case, we know it's a hash table, so i thought we might as well call
Works for me, but let's use a consistency format, i.e. hash-table-id=00


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic3c34dd3f9190a131e6f03d901b4bfcd164a5174
Gerrit-PatchSet: 3
Gerrit-Project: Impala
Gerrit-Branch: cdh5-trunk
Gerrit-Owner: Marcel Kornacker <[email protected]>
Gerrit-Reviewer: Alex Behm <[email protected]>
Gerrit-Reviewer: Henry Robinson <[email protected]>
Gerrit-Reviewer: Marcel Kornacker <[email protected]>
Gerrit-HasComments: Yes

Reply via email to